net: axienet: Set mac_managed_pm

[PATCH] net: axienet: Set mac_managed_pm

Posted by Nick Hu 10 months ago

The external PHY will undergo a soft reset twice during the resume process
when it wake up from suspend. The first reset occurs when the axienet
driver calls phylink_of_phy_connect(), and the second occurs when
mdio_bus_phy_resume() invokes phy_init_hw(). The second soft reset of the
external PHY does not reinitialize the internal PHY, which causes issues
with the internal PHY, resulting in the PHY link being down. To prevent
this, setting the mac_managed_pm flag skips the mdio_bus_phy_resume()
function.

Fixes: a129b41fe0a8 ("Revert "net: phy: dp83867: perform soft reset and retain established link"")
Signed-off-by: Nick Hu <nick.hu@sifive.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 2ffaad0b0477..2deeb982bf6b 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -3078,6 +3078,7 @@ static int axienet_probe(struct platform_device *pdev)
 
 	lp->phylink_config.dev = &ndev->dev;
 	lp->phylink_config.type = PHYLINK_NETDEV;
+	lp->phylink_config.mac_managed_pm = true;
 	lp->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
 		MAC_10FD | MAC_100FD | MAC_1000FD;
 
-- 
2.17.1

Re: [PATCH] net: axienet: Set mac_managed_pm

Posted by Jakub Kicinski 10 months ago

On Mon, 17 Feb 2025 13:58:42 +0800 Nick Hu wrote:
> The external PHY will undergo a soft reset twice during the resume process
> when it wake up from suspend. The first reset occurs when the axienet
> driver calls phylink_of_phy_connect(), and the second occurs when
> mdio_bus_phy_resume() invokes phy_init_hw(). The second soft reset of the
> external PHY does not reinitialize the internal PHY, which causes issues
> with the internal PHY, resulting in the PHY link being down. To prevent
> this, setting the mac_managed_pm flag skips the mdio_bus_phy_resume()
> function.
> 
> Fixes: a129b41fe0a8 ("Revert "net: phy: dp83867: perform soft reset and retain established link"")
> Signed-off-by: Nick Hu <nick.hu@sifive.com>

Applied, thanks!

Re: [PATCH] net: axienet: Set mac_managed_pm

Posted by Jacob Keller 10 months ago


On 2/16/2025 9:58 PM, Nick Hu wrote:
Nit: subject should include the "net" prefix since this is clearly a bug
fix.

> The external PHY will undergo a soft reset twice during the resume process
> when it wake up from suspend. The first reset occurs when the axienet
> driver calls phylink_of_phy_connect(), and the second occurs when
> mdio_bus_phy_resume() invokes phy_init_hw(). The second soft reset of the
> external PHY does not reinitialize the internal PHY, which causes issues
> with the internal PHY, resulting in the PHY link being down. To prevent
> this, setting the mac_managed_pm flag skips the mdio_bus_phy_resume()
> function.
> 
> Fixes: a129b41fe0a8 ("Revert "net: phy: dp83867: perform soft reset and retain established link"")
> Signed-off-by: Nick Hu <nick.hu@sifive.com>
> ---

Otherwise, the fix seems correct to me.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 2ffaad0b0477..2deeb982bf6b 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -3078,6 +3078,7 @@ static int axienet_probe(struct platform_device *pdev)
>  
>  	lp->phylink_config.dev = &ndev->dev;
>  	lp->phylink_config.type = PHYLINK_NETDEV;
> +	lp->phylink_config.mac_managed_pm = true;
>  	lp->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
>  		MAC_10FD | MAC_100FD | MAC_1000FD;
>

Re: [PATCH] net: axienet: Set mac_managed_pm

Posted by Nick Hu 10 months ago

Hi Jacob

On Thu, Feb 20, 2025 at 7:29 AM Jacob Keller <jacob.e.keller@intel.com> wrote:
>
>
>
> On 2/16/2025 9:58 PM, Nick Hu wrote:
> Nit: subject should include the "net" prefix since this is clearly a bug
> fix.
>
I've added the 'net' prefix to the subject 'net: axienet: Set
mac_managed_pm'. Is there something I'm missing?

> > The external PHY will undergo a soft reset twice during the resume process
> > when it wake up from suspend. The first reset occurs when the axienet
> > driver calls phylink_of_phy_connect(), and the second occurs when
> > mdio_bus_phy_resume() invokes phy_init_hw(). The second soft reset of the
> > external PHY does not reinitialize the internal PHY, which causes issues
> > with the internal PHY, resulting in the PHY link being down. To prevent
> > this, setting the mac_managed_pm flag skips the mdio_bus_phy_resume()
> > function.
> >
> > Fixes: a129b41fe0a8 ("Revert "net: phy: dp83867: perform soft reset and retain established link"")
> > Signed-off-by: Nick Hu <nick.hu@sifive.com>
> > ---
>
> Otherwise, the fix seems correct to me.
>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
>
> >  drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > index 2ffaad0b0477..2deeb982bf6b 100644
> > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > @@ -3078,6 +3078,7 @@ static int axienet_probe(struct platform_device *pdev)
> >
> >       lp->phylink_config.dev = &ndev->dev;
> >       lp->phylink_config.type = PHYLINK_NETDEV;
> > +     lp->phylink_config.mac_managed_pm = true;
> >       lp->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
> >               MAC_10FD | MAC_100FD | MAC_1000FD;
> >
>

Regards,
Nick

Re: [PATCH] net: axienet: Set mac_managed_pm

Posted by Michal Swiatkowski 10 months ago

On Thu, Feb 20, 2025 at 10:47:40AM +0800, Nick Hu wrote:
> Hi Jacob
> 
> On Thu, Feb 20, 2025 at 7:29 AM Jacob Keller <jacob.e.keller@intel.com> wrote:
> >
> >
> >
> > On 2/16/2025 9:58 PM, Nick Hu wrote:
> > Nit: subject should include the "net" prefix since this is clearly a bug
> > fix.
> >
> I've added the 'net' prefix to the subject 'net: axienet: Set
> mac_managed_pm'. Is there something I'm missing?
> 

It should be [PATCH net] net: axienet: Set mac_managed_pm
Like here for example [1]. You can look at netdev FAQ [2]. It is
described there how to specify the subject.

Probably you don't need to resend it only because of that.

[1] https://lore.kernel.org/netdev/CAL+tcoC3TuZPTwnHTDvXC+JPoJbgW2UywZ2=xv=E=utokb3pCQ@mail.gmail.com/T/#m2b5603fbf355216ab035aa0f69c10c5f4ba98772
[2] https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt

Thanks,
Michal

> > > The external PHY will undergo a soft reset twice during the resume process
> > > when it wake up from suspend. The first reset occurs when the axienet
> > > driver calls phylink_of_phy_connect(), and the second occurs when
> > > mdio_bus_phy_resume() invokes phy_init_hw(). The second soft reset of the
> > > external PHY does not reinitialize the internal PHY, which causes issues
> > > with the internal PHY, resulting in the PHY link being down. To prevent
> > > this, setting the mac_managed_pm flag skips the mdio_bus_phy_resume()
> > > function.
> > >
> > > Fixes: a129b41fe0a8 ("Revert "net: phy: dp83867: perform soft reset and retain established link"")
> > > Signed-off-by: Nick Hu <nick.hu@sifive.com>
> > > ---
> >
> > Otherwise, the fix seems correct to me.
> >
> > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> >
> > >  drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > > index 2ffaad0b0477..2deeb982bf6b 100644
> > > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > > @@ -3078,6 +3078,7 @@ static int axienet_probe(struct platform_device *pdev)
> > >
> > >       lp->phylink_config.dev = &ndev->dev;
> > >       lp->phylink_config.type = PHYLINK_NETDEV;
> > > +     lp->phylink_config.mac_managed_pm = true;
> > >       lp->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
> > >               MAC_10FD | MAC_100FD | MAC_1000FD;
> > >
> >
> 
> Regards,
> Nick

Re: [PATCH] net: axienet: Set mac_managed_pm

Posted by Nick Hu 10 months ago

Hi Michal

Thanks for the information! I'll pay attention next time.

On Thu, Feb 20, 2025 at 3:00 PM Michal Swiatkowski
<michal.swiatkowski@linux.intel.com> wrote:
>
> On Thu, Feb 20, 2025 at 10:47:40AM +0800, Nick Hu wrote:
> > Hi Jacob
> >
> > On Thu, Feb 20, 2025 at 7:29 AM Jacob Keller <jacob.e.keller@intel.com> wrote:
> > >
> > >
> > >
> > > On 2/16/2025 9:58 PM, Nick Hu wrote:
> > > Nit: subject should include the "net" prefix since this is clearly a bug
> > > fix.
> > >
> > I've added the 'net' prefix to the subject 'net: axienet: Set
> > mac_managed_pm'. Is there something I'm missing?
> >
>
> It should be [PATCH net] net: axienet: Set mac_managed_pm
> Like here for example [1]. You can look at netdev FAQ [2]. It is
> described there how to specify the subject.
>
> Probably you don't need to resend it only because of that.
>
> [1] https://lore.kernel.org/netdev/CAL+tcoC3TuZPTwnHTDvXC+JPoJbgW2UywZ2=xv=E=utokb3pCQ@mail.gmail.com/T/#m2b5603fbf355216ab035aa0f69c10c5f4ba98772
> [2] https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
>
> Thanks,
> Michal
>
> > > > The external PHY will undergo a soft reset twice during the resume process
> > > > when it wake up from suspend. The first reset occurs when the axienet
> > > > driver calls phylink_of_phy_connect(), and the second occurs when
> > > > mdio_bus_phy_resume() invokes phy_init_hw(). The second soft reset of the
> > > > external PHY does not reinitialize the internal PHY, which causes issues
> > > > with the internal PHY, resulting in the PHY link being down. To prevent
> > > > this, setting the mac_managed_pm flag skips the mdio_bus_phy_resume()
> > > > function.
> > > >
> > > > Fixes: a129b41fe0a8 ("Revert "net: phy: dp83867: perform soft reset and retain established link"")
> > > > Signed-off-by: Nick Hu <nick.hu@sifive.com>
> > > > ---
> > >
> > > Otherwise, the fix seems correct to me.
> > >
> > > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> > >
> > > >  drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > > > index 2ffaad0b0477..2deeb982bf6b 100644
> > > > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > > > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> > > > @@ -3078,6 +3078,7 @@ static int axienet_probe(struct platform_device *pdev)
> > > >
> > > >       lp->phylink_config.dev = &ndev->dev;
> > > >       lp->phylink_config.type = PHYLINK_NETDEV;
> > > > +     lp->phylink_config.mac_managed_pm = true;
> > > >       lp->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
> > > >               MAC_10FD | MAC_100FD | MAC_1000FD;
> > > >
> > >
> >
> > Regards,
> > Nick

Regards,
Nick