Re: [PATCH net-next 01/11] net: dsa: make EEE ops optional

From: Andrew Lunn
Date: Tue Aug 01 2017 - 10:07:29 EST


Hi Vivien

> @@ -646,38 +646,42 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
> {
> struct dsa_slave_priv *p = netdev_priv(dev);
> struct dsa_switch *ds = p->dp->ds;
> - int ret;
> + int err = -ENODEV;
>
> - if (!ds->ops->set_eee)
> - return -EOPNOTSUPP;
> + if (ds->ops->set_eee) {
> + err = ds->ops->set_eee(ds, p->dp->index, p->phy, e);
> + if (err)
> + return err;
> + }
>
> - ret = ds->ops->set_eee(ds, p->dp->index, p->phy, e);
> - if (ret)
> - return ret;
> + if (p->phy) {
> + err = phy_ethtool_set_eee(p->phy, e);
> + if (err)
> + return err;

I don't think you need this if (err). You unconditionally return err
as you exit the function.

> + }
>
> - if (p->phy)
> - ret = phy_ethtool_set_eee(p->phy, e);
> -
> - return ret;
> + return err;




> }
>
> static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
> {
> struct dsa_slave_priv *p = netdev_priv(dev);
> struct dsa_switch *ds = p->dp->ds;
> - int ret;
> + int err = -ENODEV;
>
> - if (!ds->ops->get_eee)
> - return -EOPNOTSUPP;
> + if (ds->ops->get_eee) {
> + err = ds->ops->get_eee(ds, p->dp->index, e);
> + if (err)
> + return err;
> + }
>
> - ret = ds->ops->get_eee(ds, p->dp->index, e);
> - if (ret)
> - return ret;
> + if (p->phy) {
> + err = phy_ethtool_get_eee(p->phy, e);
> + if (err)
> + return err;

Same here.

> + }
>
> - if (p->phy)
> - ret = phy_ethtool_get_eee(p->phy, e);
> -
> - return ret;
> + return err;
> }
>
> #ifdef CONFIG_NET_POLL_CONTROLLER
> --
> 2.13.3
>