| Message ID | 20210604032224.136268-1-dingsenjie@163.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: Simplify the return expression of dp83640_ack_interrupt | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 1 maintainers not CCed: linux@armlinux.org.uk |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 13 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On 04.06.2021 05:22, dingsenjie@163.com wrote: > From: dingsenjie <dingsenjie@yulong.com> > > Simplify the return expression. > > Signed-off-by: dingsenjie <dingsenjie@yulong.com> > --- > drivers/net/phy/dp83640.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c > index 0d79f68..bcd14ec 100644 > --- a/drivers/net/phy/dp83640.c > +++ b/drivers/net/phy/dp83640.c > @@ -1141,12 +1141,7 @@ static int dp83640_config_init(struct phy_device *phydev) > > static int dp83640_ack_interrupt(struct phy_device *phydev) > { > - int err = phy_read(phydev, MII_DP83640_MISR); > - > - if (err < 0) > - return err; > - > - return 0; > + return phy_read(phydev, MII_DP83640_MISR); > } > > static int dp83640_config_intr(struct phy_device *phydev) > This would be a functional change. You'd return a positive value instead of 0.
On Fri, Jun 04, 2021 at 09:49:17AM +0200, Heiner Kallweit wrote: > On 04.06.2021 05:22, dingsenjie@163.com wrote: > > From: dingsenjie <dingsenjie@yulong.com> > > > > Simplify the return expression. > > > > Signed-off-by: dingsenjie <dingsenjie@yulong.com> > > --- > > drivers/net/phy/dp83640.c | 7 +------ > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c > > index 0d79f68..bcd14ec 100644 > > --- a/drivers/net/phy/dp83640.c > > +++ b/drivers/net/phy/dp83640.c > > @@ -1141,12 +1141,7 @@ static int dp83640_config_init(struct phy_device *phydev) > > > > static int dp83640_ack_interrupt(struct phy_device *phydev) > > { > > - int err = phy_read(phydev, MII_DP83640_MISR); > > - > > - if (err < 0) > > - return err; > > - > > - return 0; > > + return phy_read(phydev, MII_DP83640_MISR); > > } > > > > static int dp83640_config_intr(struct phy_device *phydev) > > > This would be a functional change. You'd return a positive value > instead of 0. And looking a bit further down in the code: static int dp83640_ack_interrupt(struct phy_device *phydev) { int err = phy_read(phydev, MII_DP83640_MISR); if (err < 0) return err; return 0; } static int dp83640_config_intr(struct phy_device *phydev) { int micr; int misr; int err; if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { err = dp83640_ack_interrupt(phydev); if (err) return err; So a positive value is going to break the driver. NACK Andrew
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c index 0d79f68..bcd14ec 100644 --- a/drivers/net/phy/dp83640.c +++ b/drivers/net/phy/dp83640.c @@ -1141,12 +1141,7 @@ static int dp83640_config_init(struct phy_device *phydev) static int dp83640_ack_interrupt(struct phy_device *phydev) { - int err = phy_read(phydev, MII_DP83640_MISR); - - if (err < 0) - return err; - - return 0; + return phy_read(phydev, MII_DP83640_MISR); } static int dp83640_config_intr(struct phy_device *phydev)