| Message ID | 20220419014439.2561835-1-lv.ruyi@zte.com.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: fix error check return value of phy_read() | expand |
On Tue, Apr 19, 2022 at 01:44:39AM +0000, cgel.zte@gmail.com wrote: > From: Lv Ruyi <lv.ruyi@zte.com.cn> > > phy_read() returns a negative number if there's an error, but the > error-checking code in the bcm87xx driver's config_intr function > triggers if phy_read() returns non-zero. Correct that. > > Reported-by: Zeal Robot <zealci@zte.com.cn> > Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn> > --- > drivers/net/phy/bcm87xx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c > index 313563482690..e62b53718010 100644 > --- a/drivers/net/phy/bcm87xx.c > +++ b/drivers/net/phy/bcm87xx.c > @@ -146,7 +146,7 @@ static int bcm87xx_config_intr(struct phy_device *phydev) > > if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { > err = phy_read(phydev, BCM87XX_LASI_STATUS); > - if (err) > + if (err < 0) > return err; This should probably have a Fixes: tag, and be for net, not next-next. Please read the netdev FAQ about the trees, and submittinng fixes for netdev. Andrew
On 4/19/2022 5:07 AM, Andrew Lunn wrote: > On Tue, Apr 19, 2022 at 01:44:39AM +0000, cgel.zte@gmail.com wrote: >> From: Lv Ruyi <lv.ruyi@zte.com.cn> >> >> phy_read() returns a negative number if there's an error, but the >> error-checking code in the bcm87xx driver's config_intr function >> triggers if phy_read() returns non-zero. Correct that. >> >> Reported-by: Zeal Robot <zealci@zte.com.cn> >> Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn> >> --- >> drivers/net/phy/bcm87xx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c >> index 313563482690..e62b53718010 100644 >> --- a/drivers/net/phy/bcm87xx.c >> +++ b/drivers/net/phy/bcm87xx.c >> @@ -146,7 +146,7 @@ static int bcm87xx_config_intr(struct phy_device *phydev) >> >> if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { >> err = phy_read(phydev, BCM87XX_LASI_STATUS); >> - if (err) >> + if (err < 0) >> return err; > > This should probably have a Fixes: tag, and be for net, not next-next. > Please read the netdev FAQ about the trees, and submittinng fixes for > netdev. Yes, it should be: Fixes: 15772e4ddf3f ("net: phy: broadcom: remove use of ack_interrupt()") Also, please subject this change properly with: net: phy: bcm87xx: Added missing error checking Thank you
diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c index 313563482690..e62b53718010 100644 --- a/drivers/net/phy/bcm87xx.c +++ b/drivers/net/phy/bcm87xx.c @@ -146,7 +146,7 @@ static int bcm87xx_config_intr(struct phy_device *phydev) if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { err = phy_read(phydev, BCM87XX_LASI_STATUS); - if (err) + if (err < 0) return err; reg |= 1;