mbox series

[net-next,0/3] net: make PHY PM ops a no-op if MAC driver manages PHY PM

Message ID 9e695411-ab1d-34fe-8b90-3e8192ab84f6@gmail.com (mailing list archive)
Headers show
Series net: make PHY PM ops a no-op if MAC driver manages PHY PM | expand

Message

Heiner Kallweit April 7, 2021, 3:50 p.m. UTC
Resume callback of the PHY driver is called after the one for the MAC
driver. The PHY driver resume callback calls phy_init_hw(), and this is
potentially problematic if the MAC driver calls phy_start() in its resume
callback. One issue was reported with the fec driver and a KSZ8081 PHY
which seems to become unstable if a soft reset is triggered during aneg.

The new flag allows MAC drivers to indicate that they take care of
suspending/resuming the PHY. Then the MAC PM callbacks can handle
any dependency between MAC and PHY PM.

Heiner Kallweit (3):
  net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM
  net: fec: use mac-managed PHY PM
  r8169: use mac-managed PHY PM

 drivers/net/ethernet/freescale/fec_main.c | 3 +++
 drivers/net/ethernet/realtek/r8169_main.c | 3 +++
 drivers/net/phy/phy_device.c              | 6 ++++++
 include/linux/phy.h                       | 2 ++
 4 files changed, 14 insertions(+)

Comments

Joakim Zhang April 8, 2021, 5:42 a.m. UTC | #1
Hi Heiner,

Why not target this patch set to net repo as a bug fixes? Others may also suffer from this.

Best Regards,
Joakim Zhang

> -----Original Message-----
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Sent: 2021年4月7日 23:51
> To: Andrew Lunn <andrew@lunn.ch>; Russell King - ARM Linux
> <linux@armlinux.org.uk>; Jakub Kicinski <kuba@kernel.org>; David Miller
> <davem@davemloft.net>; Fugang Duan <fugang.duan@nxp.com>
> Cc: netdev@vger.kernel.org; Joakim Zhang <qiangqing.zhang@nxp.com>
> Subject: [PATCH net-next 0/3] net: make PHY PM ops a no-op if MAC driver
> manages PHY PM
> 
> Resume callback of the PHY driver is called after the one for the MAC driver.
> The PHY driver resume callback calls phy_init_hw(), and this is potentially
> problematic if the MAC driver calls phy_start() in its resume callback. One issue
> was reported with the fec driver and a KSZ8081 PHY which seems to become
> unstable if a soft reset is triggered during aneg.
> 
> The new flag allows MAC drivers to indicate that they take care of
> suspending/resuming the PHY. Then the MAC PM callbacks can handle any
> dependency between MAC and PHY PM.
> 
> Heiner Kallweit (3):
>   net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM
>   net: fec: use mac-managed PHY PM
>   r8169: use mac-managed PHY PM
> 
>  drivers/net/ethernet/freescale/fec_main.c | 3 +++
> drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>  drivers/net/phy/phy_device.c              | 6 ++++++
>  include/linux/phy.h                       | 2 ++
>  4 files changed, 14 insertions(+)
> 
> --
> 2.31.1
Heiner Kallweit April 8, 2021, 9:02 a.m. UTC | #2
On 08.04.2021 07:42, Joakim Zhang wrote:
> 
> Hi Heiner,
> 
> Why not target this patch set to net repo as a bug fixes? Others may also suffer from this.
> 
Reason is that the patch includes new functionality that usually
doesn't get backported. I'd like to see whether your case remains
the only one or whether there will be more similar reports.
In this case we had to think about a fix that doesn't need new
functionality or check whether backporting the new functionality
would be acceptable.

> Best Regards,
> Joakim Zhang
> 
Heiner

>> -----Original Message-----
>> From: Heiner Kallweit <hkallweit1@gmail.com>
>> Sent: 2021年4月7日 23:51
>> To: Andrew Lunn <andrew@lunn.ch>; Russell King - ARM Linux
>> <linux@armlinux.org.uk>; Jakub Kicinski <kuba@kernel.org>; David Miller
>> <davem@davemloft.net>; Fugang Duan <fugang.duan@nxp.com>
>> Cc: netdev@vger.kernel.org; Joakim Zhang <qiangqing.zhang@nxp.com>
>> Subject: [PATCH net-next 0/3] net: make PHY PM ops a no-op if MAC driver
>> manages PHY PM
>>
>> Resume callback of the PHY driver is called after the one for the MAC driver.
>> The PHY driver resume callback calls phy_init_hw(), and this is potentially
>> problematic if the MAC driver calls phy_start() in its resume callback. One issue
>> was reported with the fec driver and a KSZ8081 PHY which seems to become
>> unstable if a soft reset is triggered during aneg.
>>
>> The new flag allows MAC drivers to indicate that they take care of
>> suspending/resuming the PHY. Then the MAC PM callbacks can handle any
>> dependency between MAC and PHY PM.
>>
>> Heiner Kallweit (3):
>>   net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM
>>   net: fec: use mac-managed PHY PM
>>   r8169: use mac-managed PHY PM
>>
>>  drivers/net/ethernet/freescale/fec_main.c | 3 +++
>> drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>>  drivers/net/phy/phy_device.c              | 6 ++++++
>>  include/linux/phy.h                       | 2 ++
>>  4 files changed, 14 insertions(+)
>>
>> --
>> 2.31.1
>
Heiner Kallweit April 9, 2021, 9:23 a.m. UTC | #3
On 07.04.2021 17:50, Heiner Kallweit wrote:
> Resume callback of the PHY driver is called after the one for the MAC
> driver. The PHY driver resume callback calls phy_init_hw(), and this is
> potentially problematic if the MAC driver calls phy_start() in its resume
> callback. One issue was reported with the fec driver and a KSZ8081 PHY
> which seems to become unstable if a soft reset is triggered during aneg.
> 
> The new flag allows MAC drivers to indicate that they take care of
> suspending/resuming the PHY. Then the MAC PM callbacks can handle
> any dependency between MAC and PHY PM.
> 
> Heiner Kallweit (3):
>   net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM
>   net: fec: use mac-managed PHY PM
>   r8169: use mac-managed PHY PM
> 
>  drivers/net/ethernet/freescale/fec_main.c | 3 +++
>  drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>  drivers/net/phy/phy_device.c              | 6 ++++++
>  include/linux/phy.h                       | 2 ++
>  4 files changed, 14 insertions(+)
> 

This series has status "Needs ACK". For which part an ACK is needed?

Regarding the fec driver:
The mail to Fugang, the current fec maintainer, bounced and Joakim
confirmed that he left NXP. Joakim will take over, see
https://patchwork.kernel.org/project/netdevbpf/patch/20210409091145.27488-1-qiangqing.zhang@nxp.com/
Joakim also tested the patch.

Heiner
Jakub Kicinski April 10, 2021, 1:13 a.m. UTC | #4
On Wed, 7 Apr 2021 17:50:46 +0200 Heiner Kallweit wrote:
> Resume callback of the PHY driver is called after the one for the MAC
> driver. The PHY driver resume callback calls phy_init_hw(), and this is
> potentially problematic if the MAC driver calls phy_start() in its resume
> callback. One issue was reported with the fec driver and a KSZ8081 PHY
> which seems to become unstable if a soft reset is triggered during aneg.
> 
> The new flag allows MAC drivers to indicate that they take care of
> suspending/resuming the PHY. Then the MAC PM callbacks can handle
> any dependency between MAC and PHY PM.

Applied, thanks!