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);
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);
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
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
linux-stable-mirror@lists.linaro.org
-
Paolo Abeni -
Ravi Gunasekaran -
Sinthu Raja -
Sinthu Raja M