netdev - Re: [PATCH V3 1/2] net: ethernet: ti: cpsw_new: enable mac_managed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2ffdca7f-f865-b719-b701-9ed4716da71a@ti.com>
Date: Tue, 6 Feb 2024 11:30:49 +0530
From: Ravi Gunasekaran <r-gunasekaran@...com>
To: Sinthu Raja <sinthu.raja@...tralsolutions.com>,
        Denis Kirjanov
	<dkirjanov@...e.de>,
        Siddharth Vadapalli <s-vadapalli@...com>,
        Roger Quadros
	<rogerq@...nel.org>
CC: <linux-omap@...r.kernel.org>, <netdev@...r.kernel.org>,
        Sinthu Raja
	<sinthu.raja@...com>, <stable@...r.kernel.org>,
        Ravi Gunasekaran
	<r-gunasekaran@...com>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH V3 1/2] net: ethernet: ti: cpsw_new: enable mac_managed_pm
 to fix mdio



On 2/6/24 6:29 AM, Sinthu Raja wrote:
> From: Sinthu Raja <sinthu.raja@...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@...r.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@...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