| Message ID | 20210609174353.298731-1-colin.king@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [next] net: dsa: sja1105: Fix assigned yet unused return code rc | expand |
On Wed, Jun 09, 2021 at 06:43:53PM +0100, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The return code variable rc is being set to return error values in two > places in sja1105_mdiobus_base_tx_register and yet it is not being > returned, the function always returns 0 instead. Fix this by replacing > the return 0 with the return code rc. > > Addresses-Coverity: ("Unused value") > Fixes: 5a8f09748ee7 ("net: dsa: sja1105: register the MDIO buses for 100base-T1 and 100base-TX") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/net/dsa/sja1105/sja1105_mdio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c > index 8dfd06318b23..08517c70cb48 100644 > --- a/drivers/net/dsa/sja1105/sja1105_mdio.c > +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c > @@ -171,7 +171,7 @@ static int sja1105_mdiobus_base_tx_register(struct sja1105_private *priv, > out_put_np: > of_node_put(np); > > - return 0; > + return rc; Should this function really return success if of_device_is_available()? regards, dan carpenter
On Thu, Jun 10, 2021 at 09:23:58AM +0300, Dan Carpenter wrote: > On Wed, Jun 09, 2021 at 06:43:53PM +0100, Colin King wrote: > > From: Colin Ian King <colin.king@canonical.com> > > > > The return code variable rc is being set to return error values in two > > places in sja1105_mdiobus_base_tx_register and yet it is not being > > returned, the function always returns 0 instead. Fix this by replacing > > the return 0 with the return code rc. > > > > Addresses-Coverity: ("Unused value") > > Fixes: 5a8f09748ee7 ("net: dsa: sja1105: register the MDIO buses for 100base-T1 and 100base-TX") > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > --- > > drivers/net/dsa/sja1105/sja1105_mdio.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c > > index 8dfd06318b23..08517c70cb48 100644 > > --- a/drivers/net/dsa/sja1105/sja1105_mdio.c > > +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c > > @@ -171,7 +171,7 @@ static int sja1105_mdiobus_base_tx_register(struct sja1105_private *priv, > > out_put_np: > > of_node_put(np); > > > > - return 0; > > + return rc; > > Should this function really return success if of_device_is_available()? If _not_ of_device_is_available you mean? Yup. Nothing wrong with not having an internal MDIO bus. This is a driver which supports switches that do and switches that don't, and even if the node exists, it may have status = "disabled", which is again fine. Or am I misunderstanding the question?
On Thu, Jun 10, 2021 at 08:51:24PM +0300, Vladimir Oltean wrote: > On Thu, Jun 10, 2021 at 09:23:58AM +0300, Dan Carpenter wrote: > > On Wed, Jun 09, 2021 at 06:43:53PM +0100, Colin King wrote: > > > From: Colin Ian King <colin.king@canonical.com> > > > > > > The return code variable rc is being set to return error values in two > > > places in sja1105_mdiobus_base_tx_register and yet it is not being > > > returned, the function always returns 0 instead. Fix this by replacing > > > the return 0 with the return code rc. > > > > > > Addresses-Coverity: ("Unused value") > > > Fixes: 5a8f09748ee7 ("net: dsa: sja1105: register the MDIO buses for 100base-T1 and 100base-TX") > > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > > --- > > > drivers/net/dsa/sja1105/sja1105_mdio.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c > > > index 8dfd06318b23..08517c70cb48 100644 > > > --- a/drivers/net/dsa/sja1105/sja1105_mdio.c > > > +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c > > > @@ -171,7 +171,7 @@ static int sja1105_mdiobus_base_tx_register(struct sja1105_private *priv, > > > out_put_np: > > > of_node_put(np); > > > > > > - return 0; > > > + return rc; > > > > Should this function really return success if of_device_is_available()? > > If _not_ of_device_is_available you mean? Yup. Nothing wrong with not > having an internal MDIO bus. This is a driver which supports switches > that do and switches that don't, and even if the node exists, it may > have status = "disabled", which is again fine. > Or am I misunderstanding the question? Yeah. I left out the "!". That answers the question. Thanks! regards, dan carpenter
diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c index 8dfd06318b23..08517c70cb48 100644 --- a/drivers/net/dsa/sja1105/sja1105_mdio.c +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c @@ -171,7 +171,7 @@ static int sja1105_mdiobus_base_tx_register(struct sja1105_private *priv, out_put_np: of_node_put(np); - return 0; + return rc; } static void sja1105_mdiobus_base_tx_unregister(struct sja1105_private *priv)