| Message ID | 8ec796f47620980fdd0403e21bd8b7200b4fa1d4.1663598796.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4924c0cdce75575295f8fa682851fb8e5d619dd2 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ravb: Fix PHY state warning splat during system resume | expand |
On 9/19/22 07:48, Geert Uytterhoeven wrote: > Since commit 744d23c71af39c7d ("net: phy: Warn about incorrect > mdio_bus_phy_resume() state"), a warning splat is printed during system > resume with Wake-on-LAN disabled: > > WARNING: CPU: 0 PID: 1197 at drivers/net/phy/phy_device.c:323 mdio_bus_phy_resume+0xbc/0xc8 > > As the Renesas Ethernet AVB driver already calls phy_{stop,start}() in > its suspend/resume callbacks, it is sufficient to just mark the MAC > responsible for managing the power state of the PHY. > > Fixes: fba863b816049b03 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM") > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
On 9/19/22 5:48 PM, Geert Uytterhoeven wrote: > Since commit 744d23c71af39c7d ("net: phy: Warn about incorrect > mdio_bus_phy_resume() state"), a warning splat is printed during system > resume with Wake-on-LAN disabled: > > WARNING: CPU: 0 PID: 1197 at drivers/net/phy/phy_device.c:323 mdio_bus_phy_resume+0xbc/0xc8 > > As the Renesas Ethernet AVB driver already calls phy_{stop,start}() in > its suspend/resume callbacks, it is sufficient to just mark the MAC > responsible for managing the power state of the PHY. > > Fixes: fba863b816049b03 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM") > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index d013cc1c8a0ad007..abe6f570fe102636 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -1449,6 +1449,8 @@ static int ravb_phy_init(struct net_device *ndev) > phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT); > } > > + /* Indicate that the MAC is responsible for managing PHY PM */ > + phydev->mac_managed_pm = true; Hm, this field is declared as *unsigned*... > phy_attached_info(phydev); [...] MBR, Sergey
On 9/19/22 5:48 PM, Geert Uytterhoeven wrote: > Since commit 744d23c71af39c7d ("net: phy: Warn about incorrect > mdio_bus_phy_resume() state"), a warning splat is printed during system > resume with Wake-on-LAN disabled: > > WARNING: CPU: 0 PID: 1197 at drivers/net/phy/phy_device.c:323 mdio_bus_phy_resume+0xbc/0xc8 > > As the Renesas Ethernet AVB driver already calls phy_{stop,start}() in > its suspend/resume callbacks, it is sufficient to just mark the MAC > responsible for managing the power state of the PHY. > > Fixes: fba863b816049b03 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM") > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index d013cc1c8a0ad007..abe6f570fe102636 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -1449,6 +1449,8 @@ static int ravb_phy_init(struct net_device *ndev) > phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT); > } > > + /* Indicate that the MAC is responsible for managing PHY PM */ > + phydev->mac_managed_pm = true; Hm, this field is declared as *unsigned*... > phy_attached_info(phydev); [...] MBR, Sergey
Hi Sergey, On Mon, Sep 19, 2022 at 8:40 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > On 9/19/22 5:48 PM, Geert Uytterhoeven wrote: > > Since commit 744d23c71af39c7d ("net: phy: Warn about incorrect > > mdio_bus_phy_resume() state"), a warning splat is printed during system > > resume with Wake-on-LAN disabled: > > > > WARNING: CPU: 0 PID: 1197 at drivers/net/phy/phy_device.c:323 mdio_bus_phy_resume+0xbc/0xc8 > > > > As the Renesas Ethernet AVB driver already calls phy_{stop,start}() in > > its suspend/resume callbacks, it is sufficient to just mark the MAC > > responsible for managing the power state of the PHY. > > > > Fixes: fba863b816049b03 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM") > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> Thanks for your review! > > --- a/drivers/net/ethernet/renesas/ravb_main.c > > +++ b/drivers/net/ethernet/renesas/ravb_main.c > > @@ -1449,6 +1449,8 @@ static int ravb_phy_init(struct net_device *ndev) > > phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT); > > } > > > > + /* Indicate that the MAC is responsible for managing PHY PM */ > > + phydev->mac_managed_pm = true; > > Hm, this field is declared as *unsigned*... True, I copied this from drivers/net/ethernet/broadcom/genet/bcmmii.c. But true/false are fully compatible with single-bit values. The linuxdoc suggests to use true, like for all other single-bit fields used as booleans: include/linux/phy.h: * @mac_managed_pm: Set true if MAC driver takes of suspending/resuming PHY Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 9/20/22 10:07 AM, Geert Uytterhoeven wrote: [...] >>> Since commit 744d23c71af39c7d ("net: phy: Warn about incorrect >>> mdio_bus_phy_resume() state"), a warning splat is printed during system >>> resume with Wake-on-LAN disabled: >>> >>> WARNING: CPU: 0 PID: 1197 at drivers/net/phy/phy_device.c:323 mdio_bus_phy_resume+0xbc/0xc8 >>> >>> As the Renesas Ethernet AVB driver already calls phy_{stop,start}() in >>> its suspend/resume callbacks, it is sufficient to just mark the MAC >>> responsible for managing the power state of the PHY. >>> >>> Fixes: fba863b816049b03 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM") >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >> >> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> > > Thanks for your review! My duty! :-) >>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c >>> @@ -1449,6 +1449,8 @@ static int ravb_phy_init(struct net_device *ndev) >>> phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT); >>> } >>> >>> + /* Indicate that the MAC is responsible for managing PHY PM */ >>> + phydev->mac_managed_pm = true; >> >> Hm, this field is declared as *unsigned*... > > True, I copied this from drivers/net/ethernet/broadcom/genet/bcmmii.c. > But true/false are fully compatible with single-bit values. Yes, however these 2 drivers do use 1 for setting the single bit fields... > The linuxdoc suggests to use true, like for all other single-bit fields used > as booleans: > > include/linux/phy.h: * @mac_managed_pm: Set true if MAC driver takes Hah, here's the error I noticed: missing "care " here! :-) > of suspending/resuming PHY > > Gr{oetje,eeting}s, > > Geert [...] MBR, Sergey
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Mon, 19 Sep 2022 16:48:00 +0200 you wrote: > Since commit 744d23c71af39c7d ("net: phy: Warn about incorrect > mdio_bus_phy_resume() state"), a warning splat is printed during system > resume with Wake-on-LAN disabled: > > WARNING: CPU: 0 PID: 1197 at drivers/net/phy/phy_device.c:323 mdio_bus_phy_resume+0xbc/0xc8 > > As the Renesas Ethernet AVB driver already calls phy_{stop,start}() in > its suspend/resume callbacks, it is sufficient to just mark the MAC > responsible for managing the power state of the PHY. > > [...] Here is the summary with links: - net: ravb: Fix PHY state warning splat during system resume https://git.kernel.org/netdev/net/c/4924c0cdce75 You are awesome, thank you!
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index d013cc1c8a0ad007..abe6f570fe102636 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -1449,6 +1449,8 @@ static int ravb_phy_init(struct net_device *ndev) phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT); } + /* Indicate that the MAC is responsible for managing PHY PM */ + phydev->mac_managed_pm = true; phy_attached_info(phydev); return 0;
Since commit 744d23c71af39c7d ("net: phy: Warn about incorrect mdio_bus_phy_resume() state"), a warning splat is printed during system resume with Wake-on-LAN disabled: WARNING: CPU: 0 PID: 1197 at drivers/net/phy/phy_device.c:323 mdio_bus_phy_resume+0xbc/0xc8 As the Renesas Ethernet AVB driver already calls phy_{stop,start}() in its suspend/resume callbacks, it is sufficient to just mark the MAC responsible for managing the power state of the PHY. Fixes: fba863b816049b03 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM") Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- drivers/net/ethernet/renesas/ravb_main.c | 2 ++ 1 file changed, 2 insertions(+)