[PATCH V3 0/2] CPSW: enable mac_managed_pm to fix mdio

[PATCH V3 0/2] CPSW: enable mac_managed_pm to fix mdio

[Sinthu Raja]

From: Sinthu Raja <sinthu.raja@ti.com>

Hi All,
This patch fix the resume/suspend issue on CPSW interface.

Reference from the foloowing patchwork: 
https://lore.kernel.org/netdev/20221014144729.1159257-2-shenwei.wang@nxp.com/T/

V1: https://patchwork.kernel.org/project/netdevbpf/patch/20240122083414.6246-1-sinthu.raja@ti.com/
V2: https://patchwork.kernel.org/project/netdevbpf/patch/20240122093326.7618-1-sinthu.raja@ti.com/

Changes in V3:
Address review comments:
	- Add the same fix to the drivers/net/ethernet/ti/cpsw.c file as a
      seperate patch.

Changes in V2:
Address review comment:
	- Add Fixes tag.

Sinthu Raja (2):
  net: ethernet: ti: cpsw_new: enable mac_managed_pm to fix mdio
  net: ethernet: ti: cpsw: enable mac_managed_pm to fix mdio

 drivers/net/ethernet/ti/cpsw.c     | 2 ++
 drivers/net/ethernet/ti/cpsw_new.c | 3 +++
 2 files changed, 5 insertions(+)

-- 
2.36.1

[PATCH V3 1/2] net: ethernet: ti: cpsw_new: enable mac_managed_pm to fix mdio

[Sinthu Raja]

From: Sinthu Raja <sinthu.raja@ti.com>

The below commit  introduced a WARN when phy state is not in the states:
PHY_HALTED, PHY_READY and PHY_UP.
commit 744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")

When cpsw_new resumes, there have port in PHY_NOLINK state, so the below
warning comes out. Set mac_managed_pm be true to tell mdio that the phy
resume/suspend is managed by the mac, to fix the following warning:

WARNING: CPU: 0 PID: 965 at drivers/net/phy/phy_device.c:326 mdio_bus_phy_resume+0x140/0x144
CPU: 0 PID: 965 Comm: sh Tainted: G           O       6.1.46-g247b2535b2 #1
Hardware name: Generic AM33XX (Flattened Device Tree)
 unwind_backtrace from show_stack+0x18/0x1c
 show_stack from dump_stack_lvl+0x24/0x2c
 dump_stack_lvl from __warn+0x84/0x15c
 __warn from warn_slowpath_fmt+0x1a8/0x1c8
 warn_slowpath_fmt from mdio_bus_phy_resume+0x140/0x144
 mdio_bus_phy_resume from dpm_run_callback+0x3c/0x140
 dpm_run_callback from device_resume+0xb8/0x2b8
 device_resume from dpm_resume+0x144/0x314
 dpm_resume from dpm_resume_end+0x14/0x20
 dpm_resume_end from suspend_devices_and_enter+0xd0/0x924
 suspend_devices_and_enter from pm_suspend+0x2e0/0x33c
 pm_suspend from state_store+0x74/0xd0
 state_store from kernfs_fop_write_iter+0x104/0x1ec
 kernfs_fop_write_iter from vfs_write+0x1b8/0x358
 vfs_write from ksys_write+0x78/0xf8
 ksys_write from ret_fast_syscall+0x0/0x54
Exception stack(0xe094dfa8 to 0xe094dff0)
dfa0:                   00000004 005c3fb8 00000001 005c3fb8 00000004 00000001
dfc0: 00000004 005c3fb8 b6f6bba0 00000004 00000004 0059edb8 00000000 00000000
dfe0: 00000004 bed918f0 b6f09bd3 b6e89a66

Cc: <stable@vger.kernel.org> # v6.0+
Fixes: 744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")
Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
---

Changes in V3:
	- No Change

Changes in V2:
	- Add fixes tag.

 drivers/net/ethernet/ti/cpsw_new.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
index 498c50c6d1a7..087dcb67505a 100644
--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c
@@ -773,6 +773,9 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 			slave->slave_num);
 		return;
 	}
+
+	phy->mac_managed_pm = true;
+
 	slave->phy = phy;
 
 	phy_attached_info(slave->phy);
-- 
2.36.1

[PATCH V3 2/2] net: ethernet: ti: cpsw: enable mac_managed_pm to fix mdio

[Sinthu Raja]

From: Sinthu Raja <sinthu.raja@ti.com>

The below commit  introduced a WARN when phy state is not in the states:
PHY_HALTED, PHY_READY and PHY_UP.
commit 744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")

When cpsw resumes, there have port in PHY_NOLINK state, so the below
warning comes out. Set mac_managed_pm be true to tell mdio that the phy
resume/suspend is managed by the mac, to fix the following warning:

WARNING: CPU: 0 PID: 965 at drivers/net/phy/phy_device.c:326 mdio_bus_phy_resume+0x140/0x144
CPU: 0 PID: 965 Comm: sh Tainted: G           O       6.1.46-g247b2535b2 #1
Hardware name: Generic AM33XX (Flattened Device Tree)
 unwind_backtrace from show_stack+0x18/0x1c
 show_stack from dump_stack_lvl+0x24/0x2c
 dump_stack_lvl from __warn+0x84/0x15c
 __warn from warn_slowpath_fmt+0x1a8/0x1c8
 warn_slowpath_fmt from mdio_bus_phy_resume+0x140/0x144
 mdio_bus_phy_resume from dpm_run_callback+0x3c/0x140
 dpm_run_callback from device_resume+0xb8/0x2b8
 device_resume from dpm_resume+0x144/0x314
 dpm_resume from dpm_resume_end+0x14/0x20
 dpm_resume_end from suspend_devices_and_enter+0xd0/0x924
 suspend_devices_and_enter from pm_suspend+0x2e0/0x33c
 pm_suspend from state_store+0x74/0xd0
 state_store from kernfs_fop_write_iter+0x104/0x1ec
 kernfs_fop_write_iter from vfs_write+0x1b8/0x358
 vfs_write from ksys_write+0x78/0xf8
 ksys_write from ret_fast_syscall+0x0/0x54
Exception stack(0xe094dfa8 to 0xe094dff0)
dfa0:                   00000004 005c3fb8 00000001 005c3fb8 00000004 00000001
dfc0: 00000004 005c3fb8 b6f6bba0 00000004 00000004 0059edb8 00000000 00000000
dfe0: 00000004 bed918f0 b6f09bd3 b6e89a66

Cc: <stable@vger.kernel.org> # v6.0+
Fixes: 744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")
Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index ea85c6dd5484..c0a5abd8d9a8 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -631,6 +631,8 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 		}
 	}
 
+	phy->mac_managed_pm = true;
+
 	slave->phy = phy;
 
 	phy_attached_info(slave->phy);
-- 
2.36.1

Re: [PATCH V3 1/2] net: ethernet: ti: cpsw_new: enable mac_managed_pm to fix mdio

[Ravi Gunasekaran]

On 2/6/24 6:29 AM, Sinthu Raja wrote:
> From: Sinthu Raja <sinthu.raja@ti.com>
> 
> The below commit  introduced a WARN when phy state is not in the states:
> PHY_HALTED, PHY_READY and PHY_UP.
> commit 744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")
> 
> When cpsw_new resumes, there have port in PHY_NOLINK state, so the below
> warning comes out. Set mac_managed_pm be true to tell mdio that the phy
> resume/suspend is managed by the mac, to fix the following warning:
> 
> WARNING: CPU: 0 PID: 965 at drivers/net/phy/phy_device.c:326 mdio_bus_phy_resume+0x140/0x144
> CPU: 0 PID: 965 Comm: sh Tainted: G           O       6.1.46-g247b2535b2 #1
> Hardware name: Generic AM33XX (Flattened Device Tree)
>  unwind_backtrace from show_stack+0x18/0x1c
>  show_stack from dump_stack_lvl+0x24/0x2c
>  dump_stack_lvl from __warn+0x84/0x15c
>  __warn from warn_slowpath_fmt+0x1a8/0x1c8
>  warn_slowpath_fmt from mdio_bus_phy_resume+0x140/0x144
>  mdio_bus_phy_resume from dpm_run_callback+0x3c/0x140
>  dpm_run_callback from device_resume+0xb8/0x2b8
>  device_resume from dpm_resume+0x144/0x314
>  dpm_resume from dpm_resume_end+0x14/0x20
>  dpm_resume_end from suspend_devices_and_enter+0xd0/0x924
>  suspend_devices_and_enter from pm_suspend+0x2e0/0x33c
>  pm_suspend from state_store+0x74/0xd0
>  state_store from kernfs_fop_write_iter+0x104/0x1ec
>  kernfs_fop_write_iter from vfs_write+0x1b8/0x358
>  vfs_write from ksys_write+0x78/0xf8
>  ksys_write from ret_fast_syscall+0x0/0x54
> Exception stack(0xe094dfa8 to 0xe094dff0)
> dfa0:                   00000004 005c3fb8 00000001 005c3fb8 00000004 00000001
> dfc0: 00000004 005c3fb8 b6f6bba0 00000004 00000004 0059edb8 00000000 00000000
> dfe0: 00000004 bed918f0 b6f09bd3 b6e89a66
> 
> Cc: <stable@vger.kernel.org> # v6.0+
> Fixes: 744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")

In v1, you received a comment to add the fixes tag. The reference stmmac patch also points
to this commit as Fixes tag. But as Paolo pointed out in v2, this is not the right
fixes tag for your patch series.

I did a git blame on few drivers where PHY is managed by MAC. These have 
Fixes: fba863b81604 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM")
which seems to be more appropriate, as this is the commit that introduced the
'mac_managed_pm' flag.

I have Cc'ed Paolo in this reply. But in future, please take care of adding the people
who provided review comments in To/Cc when sending reworked patch/series.

> Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
> ---
> 
> Changes in V3:
> 	- No Change
> 
> Changes in V2:
> 	- Add fixes tag.
> 
>  drivers/net/ethernet/ti/cpsw_new.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
> index 498c50c6d1a7..087dcb67505a 100644
> --- a/drivers/net/ethernet/ti/cpsw_new.c
> +++ b/drivers/net/ethernet/ti/cpsw_new.c
> @@ -773,6 +773,9 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
>  			slave->slave_num);
>  		return;
>  	}
> +
> +	phy->mac_managed_pm = true;
> +
>  	slave->phy = phy;
>  
>  	phy_attached_info(slave->phy);

-- 
Regards,
Ravi

Re: [PATCH V3 1/2] net: ethernet: ti: cpsw_new: enable mac_managed_pm to fix mdio

[Sinthu Raja M]

On Tue, Feb 6, 2024 at 11:31 AM Ravi Gunasekaran <r-gunasekaran@ti.com> wrote:
>
>
>
> On 2/6/24 6:29 AM, Sinthu Raja wrote:
> > From: Sinthu Raja <sinthu.raja@ti.com>
> >
> > The below commit  introduced a WARN when phy state is not in the states:
> > PHY_HALTED, PHY_READY and PHY_UP.
> > commit 744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")
> >
> > When cpsw_new resumes, there have port in PHY_NOLINK state, so the below
> > warning comes out. Set mac_managed_pm be true to tell mdio that the phy
> > resume/suspend is managed by the mac, to fix the following warning:
> >
> > WARNING: CPU: 0 PID: 965 at drivers/net/phy/phy_device.c:326 mdio_bus_phy_resume+0x140/0x144
> > CPU: 0 PID: 965 Comm: sh Tainted: G           O       6.1.46-g247b2535b2 #1
> > Hardware name: Generic AM33XX (Flattened Device Tree)
> >  unwind_backtrace from show_stack+0x18/0x1c
> >  show_stack from dump_stack_lvl+0x24/0x2c
> >  dump_stack_lvl from __warn+0x84/0x15c
> >  __warn from warn_slowpath_fmt+0x1a8/0x1c8
> >  warn_slowpath_fmt from mdio_bus_phy_resume+0x140/0x144
> >  mdio_bus_phy_resume from dpm_run_callback+0x3c/0x140
> >  dpm_run_callback from device_resume+0xb8/0x2b8
> >  device_resume from dpm_resume+0x144/0x314
> >  dpm_resume from dpm_resume_end+0x14/0x20
> >  dpm_resume_end from suspend_devices_and_enter+0xd0/0x924
> >  suspend_devices_and_enter from pm_suspend+0x2e0/0x33c
> >  pm_suspend from state_store+0x74/0xd0
> >  state_store from kernfs_fop_write_iter+0x104/0x1ec
> >  kernfs_fop_write_iter from vfs_write+0x1b8/0x358
> >  vfs_write from ksys_write+0x78/0xf8
> >  ksys_write from ret_fast_syscall+0x0/0x54
> > Exception stack(0xe094dfa8 to 0xe094dff0)
> > dfa0:                   00000004 005c3fb8 00000001 005c3fb8 00000004 00000001
> > dfc0: 00000004 005c3fb8 b6f6bba0 00000004 00000004 0059edb8 00000000 00000000
> > dfe0: 00000004 bed918f0 b6f09bd3 b6e89a66
> >
> > Cc: <stable@vger.kernel.org> # v6.0+
> > Fixes: 744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")
>
> In v1, you received a comment to add the fixes tag. The reference stmmac patch also points
> to this commit as Fixes tag. But as Paolo pointed out in v2, this is not the right
> fixes tag for your patch series.
>
> I did a git blame on few drivers where PHY is managed by MAC. These have
> Fixes: fba863b81604 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM")

Thanks, Ravi for pointing this out.
But the warning message was caused only after the below commit had been added.
744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")

With the below commit the warning didn't pop up.
fba863b81604 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM")
That is the reason I have not changed the Fixes tag.

Let's wait for Paolo's comment on this Fixes: fba863b81604 ("net: phy:
make PHY PM ops a no-op if MAC driver manages PHY PM")

> which seems to be more appropriate, as this is the commit that introduced the
> 'mac_managed_pm' flag.
>
> I have Cc'ed Paolo in this reply. But in future, please take care of adding the people
> who provided review comments in To/Cc when sending reworked patch/series.

Noted.

Regards,
Sinthu Raja

>
> > Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
> > ---
> >
> > Changes in V3:
> >       - No Change
> >
> > Changes in V2:
> >       - Add fixes tag.
> >
> >  drivers/net/ethernet/ti/cpsw_new.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
> > index 498c50c6d1a7..087dcb67505a 100644
> > --- a/drivers/net/ethernet/ti/cpsw_new.c
> > +++ b/drivers/net/ethernet/ti/cpsw_new.c
> > @@ -773,6 +773,9 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
> >                       slave->slave_num);
> >               return;
> >       }
> > +
> > +     phy->mac_managed_pm = true;
> > +
> >       slave->phy = phy;
> >
> >       phy_attached_info(slave->phy);
>
> --
> Regards,
> Ravi



--
With Regards
Sinthu Raja

Re: [PATCH V3 1/2] net: ethernet: ti: cpsw_new: enable mac_managed_pm to fix mdio

[Paolo Abeni]

On Tue, 2024-02-06 at 12:49 +0530, Sinthu Raja M wrote:
> On Tue, Feb 6, 2024 at 11:31 AM Ravi Gunasekaran <r-gunasekaran@ti.com> wrote:
> > In v1, you received a comment to add the fixes tag. The reference stmmac patch also points
> > to this commit as Fixes tag. But as Paolo pointed out in v2, this is not the right
> > fixes tag for your patch series.
> > 
> > I did a git blame on few drivers where PHY is managed by MAC. These have
> > Fixes: fba863b81604 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM")
> 
> Thanks, Ravi for pointing this out.
> But the warning message was caused only after the below commit had been added.
> 744d23c71af3 ("net: phy: Warn about incorrect mdio_bus_phy_resume() state")
> 
> With the below commit the warning didn't pop up.
> fba863b81604 ("net: phy: make PHY PM ops a no-op if MAC driver manages PHY PM")
> That is the reason I have not changed the Fixes tag.
> 
> Let's wait for Paolo's comment on this Fixes: fba863b81604 ("net: phy:
> make PHY PM ops a no-op if MAC driver manages PHY PM")

The main point of fixes tag is helping stable teams to backport the
fixes if and where relevant.

In this case the issue addressed is arguably an inconsistent state,
which may lead to later functional issue. The warning emitted by the
phy core is just an head-up, but it does not look like the 'real'
problem.

An older kernel including fba863b81604 but not including 744d23c71af3
will likely be prone to the issue.

TL;DR: I think it's better to include both fixes tag.

I'll add the 2nd one while applying the patch, no need to repost.

Cheers,

Paolo

Re: [PATCH V3 0/2] CPSW: enable mac_managed_pm to fix mdio

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue,  6 Feb 2024 06:29:26 +0530 you wrote:
> From: Sinthu Raja <sinthu.raja@ti.com>
> 
> Hi All,
> This patch fix the resume/suspend issue on CPSW interface.
> 
> Reference from the foloowing patchwork:
> https://lore.kernel.org/netdev/20221014144729.1159257-2-shenwei.wang@nxp.com/T/
> 
> [...]

Here is the summary with links:
  - [V3,1/2] net: ethernet: ti: cpsw_new: enable mac_managed_pm to fix mdio
    https://git.kernel.org/netdev/net/c/9def04e759ca
  - [V3,2/2] net: ethernet: ti: cpsw: enable mac_managed_pm to fix mdio
    https://git.kernel.org/netdev/net/c/bc4ce46b1e3d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html