From: Christian Marangi Date: Tue, 16 Apr 2024 14:44:41 +0000 (+0200) Subject: qca-nss-dp: add patch fixing rmmod and insmod X-Git-Url: http://git.lede-project.org./?a=commitdiff_plain;h=918e2fca1e818a346a5f04e11b4e9cb0d5ad6510;p=openwrt%2Fopenwrt.git qca-nss-dp: add patch fixing rmmod and insmod Add patch fixing rmmod and insmod. Lots of flawed logic fixed that permits the module to correctly rmmod and insmod later. Just to quote some change, use phy_detach instead of phy_disconnect, fix exclusive reset_control that could only be used once, fix kernel panic on second edma_cleanup, stop traffic before module exit... Signed-off-by: Christian Marangi --- diff --git a/package/kernel/qca-nss-dp/Makefile b/package/kernel/qca-nss-dp/Makefile index 504074a81e..e6c380aead 100644 --- a/package/kernel/qca-nss-dp/Makefile +++ b/package/kernel/qca-nss-dp/Makefile @@ -1,7 +1,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=qca-nss-dp -PKG_RELEASE:=2 +PKG_RELEASE:=3 PKG_SOURCE_URL:=https://git.codelinaro.org/clo/qsdk/oss/lklm/nss-dp.git PKG_SOURCE_PROTO:=git diff --git a/package/kernel/qca-nss-dp/patches/0011-01-edma_v1-rework-hw_reset-logic-to-permit-rmmod-and-in.patch b/package/kernel/qca-nss-dp/patches/0011-01-edma_v1-rework-hw_reset-logic-to-permit-rmmod-and-in.patch new file mode 100644 index 0000000000..7e2a593657 --- /dev/null +++ b/package/kernel/qca-nss-dp/patches/0011-01-edma_v1-rework-hw_reset-logic-to-permit-rmmod-and-in.patch @@ -0,0 +1,43 @@ +From c318c90b824c59539bf2e33618e381293398616c Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Tue, 16 Apr 2024 15:02:49 +0200 +Subject: [PATCH 1/6] edma_v1: rework hw_reset logic to permit rmmod and insmod + +Rework hw_reset logic for edma v1 to permit rmmod and insmod by using +get_exclusive_released variant (assuming the reset control was released) +and manually acquire and release it. + +This permits rmmod and insmod without triggering warning or receiving +-EBUSY errors. + +Signed-off-by: Christian Marangi +--- + hal/dp_ops/edma_dp/edma_v1/edma_cfg.c | 6 +++++- + 1 file changed, 5 insertions(+), 1 deletion(-) + +--- a/hal/dp_ops/edma_dp/edma_v1/edma_cfg.c ++++ b/hal/dp_ops/edma_dp/edma_v1/edma_cfg.c +@@ -719,18 +719,22 @@ int edma_hw_reset(struct edma_hw *ehw) + struct reset_control *rst; + struct platform_device *pdev = ehw->pdev; + +- rst = devm_reset_control_get(&pdev->dev, EDMA_HW_RESET_ID); ++ rst = devm_reset_control_get_exclusive_released(&pdev->dev, EDMA_HW_RESET_ID); + if (IS_ERR(rst)) { + pr_warn("DTS Node: %s does not exist\n", EDMA_HW_RESET_ID); + return -EINVAL; + } + ++ reset_control_acquire(rst); ++ + reset_control_assert(rst); + udelay(100); + + reset_control_deassert(rst); + udelay(100); + ++ reset_control_release(rst); ++ + pr_info("EDMA HW Reset completed succesfully\n"); + + return 0; diff --git a/package/kernel/qca-nss-dp/patches/0011-02-nss_dp_switchdev-correctly-unregister-notifier-on-dp.patch b/package/kernel/qca-nss-dp/patches/0011-02-nss_dp_switchdev-correctly-unregister-notifier-on-dp.patch new file mode 100644 index 0000000000..e0a47cfca0 --- /dev/null +++ b/package/kernel/qca-nss-dp/patches/0011-02-nss_dp_switchdev-correctly-unregister-notifier-on-dp.patch @@ -0,0 +1,59 @@ +From 079bfe441b274a8c06474be82e4ccc88599a5e0e Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Tue, 16 Apr 2024 16:08:46 +0200 +Subject: [PATCH 2/6] nss_dp_switchdev: correctly unregister notifier on + dp_remove + +Correctly unregister notifier on dp_remove to fix kernel panic on system +reboot. + +Signed-off-by: Christian Marangi +--- + include/nss_dp_dev.h | 1 + + nss_dp_main.c | 4 ++++ + nss_dp_switchdev.c | 13 +++++++++++++ + 3 files changed, 18 insertions(+) + +--- a/include/nss_dp_dev.h ++++ b/include/nss_dp_dev.h +@@ -312,6 +312,7 @@ void nss_dp_set_ethtool_ops(struct net_d + */ + #ifdef CONFIG_NET_SWITCHDEV + void nss_dp_switchdev_setup(struct net_device *dev); ++void nss_dp_switchdev_remove(struct net_device *dev); + bool nss_dp_is_phy_dev(struct net_device *dev); + #endif + +--- a/nss_dp_main.c ++++ b/nss_dp_main.c +@@ -913,6 +913,10 @@ static int nss_dp_remove(struct platform + if (!dp_priv) + continue; + ++ #ifdef CONFIG_NET_SWITCHDEV ++ nss_dp_switchdev_remove(dp_priv->netdev); ++ #endif ++ + dp_ops = dp_priv->data_plane_ops; + hal_ops = dp_priv->gmac_hal_ops; + +--- a/nss_dp_switchdev.c ++++ b/nss_dp_switchdev.c +@@ -635,4 +635,17 @@ void nss_dp_switchdev_setup(struct net_d + + switch_init_done = true; + } ++ ++void nss_dp_switchdev_remove(struct net_device *dev) ++{ ++ if (!switch_init_done) ++ return; ++ ++ if (nss_dp_sw_ev_nb) ++ unregister_switchdev_notifier(nss_dp_sw_ev_nb); ++ ++ unregister_switchdev_blocking_notifier(&nss_dp_switchdev_notifier); ++ ++ switch_init_done = false; ++} + #endif diff --git a/package/kernel/qca-nss-dp/patches/0011-03-nss_dp_main-swap-dp_exit-function-call.patch b/package/kernel/qca-nss-dp/patches/0011-03-nss_dp_main-swap-dp_exit-function-call.patch new file mode 100644 index 0000000000..7ffde3d286 --- /dev/null +++ b/package/kernel/qca-nss-dp/patches/0011-03-nss_dp_main-swap-dp_exit-function-call.patch @@ -0,0 +1,35 @@ +From ab7b1a361d51157118e1a61ce6530a59bcef4b61 Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Tue, 16 Apr 2024 16:10:09 +0200 +Subject: [PATCH 3/6] nss_dp_main: swap dp_exit function call + +First unregister nss_dp platform devices then cleanup the HAL. + +This is to fix kernel panic by cleaning data that needs to be used by +platform driver unregister functions. + +Signed-off-by: Christian Marangi +--- + nss_dp_main.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +--- a/nss_dp_main.c ++++ b/nss_dp_main.c +@@ -1082,6 +1082,8 @@ int __init nss_dp_init(void) + */ + void __exit nss_dp_exit(void) + { ++ platform_driver_unregister(&nss_dp_drv); ++ + /* + * TODO Move this to soc_ops + */ +@@ -1089,8 +1091,6 @@ void __exit nss_dp_exit(void) + nss_dp_hal_cleanup(); + dp_global_ctx.common_init_done = false; + } +- +- platform_driver_unregister(&nss_dp_drv); + } + + module_init(nss_dp_init); diff --git a/package/kernel/qca-nss-dp/patches/0011-04-nss_dp_main-call-unregister_netdev-first-in-dp_remov.patch b/package/kernel/qca-nss-dp/patches/0011-04-nss_dp_main-call-unregister_netdev-first-in-dp_remov.patch new file mode 100644 index 0000000000..20e87459f8 --- /dev/null +++ b/package/kernel/qca-nss-dp/patches/0011-04-nss_dp_main-call-unregister_netdev-first-in-dp_remov.patch @@ -0,0 +1,35 @@ +From 33dd3aa6d0f9cd240d63f53a49157ae44ebccf87 Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Tue, 16 Apr 2024 16:12:11 +0200 +Subject: [PATCH 4/6] nss_dp_main: call unregister_netdev first in dp_remove + and carrifer_off + +In dp_remove move unregister_netdev up before calling exit and deinit +and first call netif_carrier_off to stop any traffic from happening and +prevent kernel panics for napi in the middle of transfer. + +Signed-off-by: Christian Marangi +--- + nss_dp_main.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +--- a/nss_dp_main.c ++++ b/nss_dp_main.c +@@ -920,6 +920,9 @@ static int nss_dp_remove(struct platform + dp_ops = dp_priv->data_plane_ops; + hal_ops = dp_priv->gmac_hal_ops; + ++ netif_carrier_off(dp_priv->netdev); ++ unregister_netdev(dp_priv->netdev); ++ + if (dp_priv->phydev) + phy_disconnect(dp_priv->phydev); + +@@ -931,7 +934,6 @@ static int nss_dp_remove(struct platform + #endif + hal_ops->exit(dp_priv->gmac_hal_ctx); + dp_ops->deinit(dp_priv->dpc); +- unregister_netdev(dp_priv->netdev); + free_netdev(dp_priv->netdev); + dp_global_ctx.nss_dp[i] = NULL; + } diff --git a/package/kernel/qca-nss-dp/patches/0011-05-nss_dp_main-use-phy_detach-instead-of-disconnect-in-.patch b/package/kernel/qca-nss-dp/patches/0011-05-nss_dp_main-use-phy_detach-instead-of-disconnect-in-.patch new file mode 100644 index 0000000000..6e87e4e8c4 --- /dev/null +++ b/package/kernel/qca-nss-dp/patches/0011-05-nss_dp_main-use-phy_detach-instead-of-disconnect-in-.patch @@ -0,0 +1,26 @@ +From 655b07b701271bc00952fe64aeb14f993a48a50e Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Tue, 16 Apr 2024 16:17:36 +0200 +Subject: [PATCH 5/6] nss_dp_main: use phy_detach instead of disconnect in + dp_remove + +Use phy_detach instead of disconnect in dp_remove. On Module remove, phy +are already disconnected but they need to be detached to be correctly +reattached later with an insmod. + +Signed-off-by: Christian Marangi +--- + nss_dp_main.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/nss_dp_main.c ++++ b/nss_dp_main.c +@@ -924,7 +924,7 @@ static int nss_dp_remove(struct platform + unregister_netdev(dp_priv->netdev); + + if (dp_priv->phydev) +- phy_disconnect(dp_priv->phydev); ++ phy_detach(dp_priv->phydev); + + #if defined(NSS_DP_PPE_SUPPORT) + /* diff --git a/package/kernel/qca-nss-dp/patches/0011-06-edma_v1-skip-edma_disable_port-in-edma_cleanup-subse.patch b/package/kernel/qca-nss-dp/patches/0011-06-edma_v1-skip-edma_disable_port-in-edma_cleanup-subse.patch new file mode 100644 index 0000000000..ad784e557f --- /dev/null +++ b/package/kernel/qca-nss-dp/patches/0011-06-edma_v1-skip-edma_disable_port-in-edma_cleanup-subse.patch @@ -0,0 +1,37 @@ +From c7c59c6097d94dbab8fc68dae798017bdbc5b3b9 Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Tue, 16 Apr 2024 16:22:32 +0200 +Subject: [PATCH 6/6] edma_v1: skip edma_disable_port in edma_cleanup + subsequent run + +Skip edma_disable_port in edma_cleanup subsequent run as it will cause +the kernel panic as the regs are already freed by previous run of +edma_cleanup. It's use it's not clear but the call is already done in +the first run of edma_cleanup. Maybe an oversight never dropped? + +Signed-off-by: Christian Marangi +--- + hal/dp_ops/edma_dp/edma_v1/edma_data_plane.c | 12 +++++++++--- + 1 file changed, 9 insertions(+), 3 deletions(-) + +--- a/hal/dp_ops/edma_dp/edma_v1/edma_data_plane.c ++++ b/hal/dp_ops/edma_dp/edma_v1/edma_data_plane.c +@@ -326,9 +326,15 @@ void edma_cleanup(bool is_dp_override) + * Disable EDMA only at module exit time, since NSS firmware + * depends on this setting. + */ +- if (!is_dp_override) { +- edma_disable_port(); +- } ++ /* This call will make the kernel panic as reg used by ++ * edma_disable_port are already freed by previous call of ++ * edma_cleanup. Logic is not clear of WHY this is called. ++ * Keep this here for reference if someone EVER wants ++ * to investigate. ++ */ ++ // if (!is_dp_override) { ++ // edma_disable_port(); ++ // } + return; + } +