| Message ID | 20211106104401.10846-1-Meng.Li@windriver.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | driver: ethernet: stmmac: remove the redundant clock disable action | expand |
On Sat, Nov 06, 2021 at 06:44:01PM +0800, Meng Li wrote: > When run below command to remove ethernet driver on > stratix10 platform, there will be warning trace as below: > > $ cd /sys/class/net/etha01/device/driver > $ echo ff800000.ethernet > unbind > > WARNING: CPU: 3 PID: 386 at drivers/clk/clk.c:810 clk_core_unprepare+0x114/0x274 > Modules linked in: sch_fq_codel > CPU: 3 PID: 386 Comm: sh Tainted: G W 5.10.74-yocto-standard #1 > Hardware name: SoCFPGA Stratix 10 SoCDK (DT) > pstate: 00000005 (nzcv daif -PAN -UAO -TCO BTYPE=--) > pc : clk_core_unprepare+0x114/0x274 > lr : clk_core_unprepare+0x114/0x274 > sp : ffff800011bdbb10 > clk_core_unprepare+0x114/0x274 > clk_unprepare+0x38/0x50 > stmmac_remove_config_dt+0x40/0x80 > stmmac_pltfr_remove+0x64/0x80 > platform_drv_remove+0x38/0x60 > ... .. > el0_sync_handler+0x1a4/0x1b0 > el0_sync+0x180/0x1c0 > This issue is introduced by introducing upstream commit 8f269102baf7 > ("net: stmmac: disable clocks in stmmac_remove_config_dt()") > Because clock has been disabled in function stmmac_dvr_remove() > It not reasonable the remove clock disable action from function > stmmac_remove_config_dt(), because it is mainly used in probe failed, > and other platform drivers also use this common function. So, remove > stmmac_remove_config_dt() from stmmac_pltfr_remove(), only other > necessary code. > > Fixes: 1af3a8e91f1a ("net: stmmac: disable clocks in stmmac_remove_config_dt()") > Signed-off-by: Meng Li <Meng.Li@windriver.com> > > --- > > Some extra comments as below: > > 1. This patch is only for linux-stable kernel v5.10, so the fixed commit ID is the one > in linux-stable kernel, not the one in mainline upsteam kernel. Ick, why? > 2. I created a patch only to fix the linux-stable kernel v5.10, not submit it to upstream kernel. > The reason as below: > In fact, upstream kernel doesn't have this issue any more. Because it has a patch to improve > the clock management and other 4 patches to fix the 1st patch. Detial patches as below: > 5ec55823438e("net: stmmac: add clocks management for gmac driver") > 30f347ae7cc1("net: stmmac: fix missing unlock on error in stmmac_suspend()") > b3dcb3127786("net: stmmac: correct clocks enabled in stmmac_vlan_rx_kill_vid()") > 4691ffb18ac9("net: stmmac: fix system hang if change mac address after interface ifdown") > ab00f3e051e8("net: stmmac: fix issue where clk is being unprepared twice") > > But I think it is a little complex to backport all the 5 patches. Moreover, it may be related > with other patches and code context mofification. > Therefore, I create a simple and clear patch to only this issue on linux-stable kernel, v 5.10 We almost ALWAYS want the original patches instead. When we try to do stable-only patches, 95% of the time it gets wrong and it makes backporting future fixes for the same code area impossible. So please submit the above patches as a series and I will be glad to consider them. thanks, greg k-h
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c index 53be8fc1d125..0fb702ce2408 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -706,7 +706,8 @@ int stmmac_pltfr_remove(struct platform_device *pdev) if (plat->exit) plat->exit(pdev, plat->bsp_priv); - stmmac_remove_config_dt(pdev, plat); + of_node_put(plat->phy_node); + of_node_put(plat->mdio_node); return ret; }
When run below command to remove ethernet driver on stratix10 platform, there will be warning trace as below: $ cd /sys/class/net/etha01/device/driver $ echo ff800000.ethernet > unbind WARNING: CPU: 3 PID: 386 at drivers/clk/clk.c:810 clk_core_unprepare+0x114/0x274 Modules linked in: sch_fq_codel CPU: 3 PID: 386 Comm: sh Tainted: G W 5.10.74-yocto-standard #1 Hardware name: SoCFPGA Stratix 10 SoCDK (DT) pstate: 00000005 (nzcv daif -PAN -UAO -TCO BTYPE=--) pc : clk_core_unprepare+0x114/0x274 lr : clk_core_unprepare+0x114/0x274 sp : ffff800011bdbb10 clk_core_unprepare+0x114/0x274 clk_unprepare+0x38/0x50 stmmac_remove_config_dt+0x40/0x80 stmmac_pltfr_remove+0x64/0x80 platform_drv_remove+0x38/0x60 ... .. el0_sync_handler+0x1a4/0x1b0 el0_sync+0x180/0x1c0 This issue is introduced by introducing upstream commit 8f269102baf7 ("net: stmmac: disable clocks in stmmac_remove_config_dt()") Because clock has been disabled in function stmmac_dvr_remove() It not reasonable the remove clock disable action from function stmmac_remove_config_dt(), because it is mainly used in probe failed, and other platform drivers also use this common function. So, remove stmmac_remove_config_dt() from stmmac_pltfr_remove(), only other necessary code. Fixes: 1af3a8e91f1a ("net: stmmac: disable clocks in stmmac_remove_config_dt()") Signed-off-by: Meng Li <Meng.Li@windriver.com> --- Some extra comments as below: 1. This patch is only for linux-stable kernel v5.10, so the fixed commit ID is the one in linux-stable kernel, not the one in mainline upsteam kernel. 2. I created a patch only to fix the linux-stable kernel v5.10, not submit it to upstream kernel. The reason as below: In fact, upstream kernel doesn't have this issue any more. Because it has a patch to improve the clock management and other 4 patches to fix the 1st patch. Detial patches as below: 5ec55823438e("net: stmmac: add clocks management for gmac driver") 30f347ae7cc1("net: stmmac: fix missing unlock on error in stmmac_suspend()") b3dcb3127786("net: stmmac: correct clocks enabled in stmmac_vlan_rx_kill_vid()") 4691ffb18ac9("net: stmmac: fix system hang if change mac address after interface ifdown") ab00f3e051e8("net: stmmac: fix issue where clk is being unprepared twice") But I think it is a little complex to backport all the 5 patches. Moreover, it may be related with other patches and code context mofification. Therefore, I create a simple and clear patch to only this issue on linux-stable kernel, v 5.10 --- drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)