Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752082AbdHAOH0 (ORCPT ); Tue, 1 Aug 2017 10:07:26 -0400 Received: from vps0.lunn.ch ([178.209.37.122]:47866 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751339AbdHAOHY (ORCPT ); Tue, 1 Aug 2017 10:07:24 -0400 Date: Tue, 1 Aug 2017 16:07:20 +0200 From: Andrew Lunn To: Vivien Didelot Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com, "David S. Miller" , Florian Fainelli Subject: Re: [PATCH net-next 01/11] net: dsa: make EEE ops optional Message-ID: <20170801140720.GD23157@lunn.ch> References: <20170731221719.16695-1-vivien.didelot@savoirfairelinux.com> <20170731221719.16695-2-vivien.didelot@savoirfairelinux.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170731221719.16695-2-vivien.didelot@savoirfairelinux.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1557 Lines: 79 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 >