Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH v2] usb: dwc3: imx8mp: fix software node kernel dump
@ 2024-11-26  3:28 Xu Yang
  2024-11-26 22:14 ` Thinh Nguyen
  0 siblings, 1 reply; 2+ messages in thread
From: Xu Yang @ 2024-11-26  3:28 UTC (permalink / raw)
  To: Thinh.Nguyen, gregkh, shawnguo, s.hauer, kernel, festevam,
	Frank.Li
  Cc: linux-usb, imx, jun.li

When unbind and bind the device again, kernel will dump below warning:

[  173.972130] sysfs: cannot create duplicate filename '/devices/platform/soc/4c010010.usb/software_node'
[  173.981564] CPU: 2 UID: 0 PID: 536 Comm: sh Not tainted 6.12.0-rc6-06344-g2aed7c4a5c56 #144
[  173.989923] Hardware name: NXP i.MX95 15X15 board (DT)
[  173.995062] Call trace:
[  173.997509]  dump_backtrace+0x90/0xe8
[  174.001196]  show_stack+0x18/0x24
[  174.004524]  dump_stack_lvl+0x74/0x8c
[  174.008198]  dump_stack+0x18/0x24
[  174.011526]  sysfs_warn_dup+0x64/0x80
[  174.015201]  sysfs_do_create_link_sd+0xf0/0xf8
[  174.019656]  sysfs_create_link+0x20/0x40
[  174.023590]  software_node_notify+0x90/0x100
[  174.027872]  device_create_managed_software_node+0xec/0x108
...

The '4c010010.usb' device is a platform device created during the initcall
and is never removed, which causes its associated software node to persist
indefinitely.

The existing device_create_managed_software_node() does not provide a
corresponding removal function.

Replace device_create_managed_software_node() with the
device_add_software_node() and device_remove_software_node() pair to ensure
proper addition and removal of software nodes, addressing this issue.

Fixes: a9400f1979a0 ("usb: dwc3: imx8mp: add 2 software managed quirk properties for host mode")
Cc: stable@vger.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - add error handling as suggested by Thinh Nguyen
 - modify commit message as suggested Frank
 - add Rb-tag
---
 drivers/usb/dwc3/dwc3-imx8mp.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
index 356812cbcd88..3edc5aca76f9 100644
--- a/drivers/usb/dwc3/dwc3-imx8mp.c
+++ b/drivers/usb/dwc3/dwc3-imx8mp.c
@@ -129,6 +129,16 @@ static void dwc3_imx8mp_wakeup_disable(struct dwc3_imx8mp *dwc3_imx)
 	writel(val, dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL);
 }
 
+static const struct property_entry dwc3_imx8mp_properties[] = {
+	PROPERTY_ENTRY_BOOL("xhci-missing-cas-quirk"),
+	PROPERTY_ENTRY_BOOL("xhci-skip-phy-init-quirk"),
+	{},
+};
+
+static const struct software_node dwc3_imx8mp_swnode = {
+	.properties = dwc3_imx8mp_properties,
+};
+
 static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
 {
 	struct dwc3_imx8mp	*dwc3_imx = _dwc3_imx;
@@ -148,17 +158,6 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
 	return IRQ_HANDLED;
 }
 
-static int dwc3_imx8mp_set_software_node(struct device *dev)
-{
-	struct property_entry props[3] = { 0 };
-	int prop_idx = 0;
-
-	props[prop_idx++] = PROPERTY_ENTRY_BOOL("xhci-missing-cas-quirk");
-	props[prop_idx++] = PROPERTY_ENTRY_BOOL("xhci-skip-phy-init-quirk");
-
-	return device_create_managed_software_node(dev, props, NULL);
-}
-
 static int dwc3_imx8mp_probe(struct platform_device *pdev)
 {
 	struct device		*dev = &pdev->dev;
@@ -221,17 +220,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto disable_rpm;
 
-	err = dwc3_imx8mp_set_software_node(dev);
+	err = device_add_software_node(dev, &dwc3_imx8mp_swnode);
 	if (err) {
 		err = -ENODEV;
-		dev_err(dev, "failed to create software node\n");
+		dev_err(dev, "failed to add software node\n");
 		goto disable_rpm;
 	}
 
 	err = of_platform_populate(node, NULL, NULL, dev);
 	if (err) {
 		dev_err(&pdev->dev, "failed to create dwc3 core\n");
-		goto disable_rpm;
+		goto remove_swnode;
 	}
 
 	dwc3_imx->dwc3 = of_find_device_by_node(dwc3_np);
@@ -255,6 +254,8 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 
 depopulate:
 	of_platform_depopulate(dev);
+remove_swnode:
+	device_remove_software_node(dev);
 disable_rpm:
 	pm_runtime_disable(dev);
 	pm_runtime_put_noidle(dev);
@@ -268,6 +269,7 @@ static void dwc3_imx8mp_remove(struct platform_device *pdev)
 
 	pm_runtime_get_sync(dev);
 	of_platform_depopulate(dev);
+	device_remove_software_node(dev);
 
 	pm_runtime_disable(dev);
 	pm_runtime_put_noidle(dev);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] usb: dwc3: imx8mp: fix software node kernel dump
  2024-11-26  3:28 [PATCH v2] usb: dwc3: imx8mp: fix software node kernel dump Xu Yang
@ 2024-11-26 22:14 ` Thinh Nguyen
  0 siblings, 0 replies; 2+ messages in thread
From: Thinh Nguyen @ 2024-11-26 22:14 UTC (permalink / raw)
  To: Xu Yang
  Cc: Thinh Nguyen, gregkh@linuxfoundation.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
	Frank.Li@nxp.com, linux-usb@vger.kernel.org, imx@lists.linux.dev,
	jun.li@nxp.com

On Tue, Nov 26, 2024, Xu Yang wrote:
> When unbind and bind the device again, kernel will dump below warning:
> 
> [  173.972130] sysfs: cannot create duplicate filename '/devices/platform/soc/4c010010.usb/software_node'
> [  173.981564] CPU: 2 UID: 0 PID: 536 Comm: sh Not tainted 6.12.0-rc6-06344-g2aed7c4a5c56 #144
> [  173.989923] Hardware name: NXP i.MX95 15X15 board (DT)
> [  173.995062] Call trace:
> [  173.997509]  dump_backtrace+0x90/0xe8
> [  174.001196]  show_stack+0x18/0x24
> [  174.004524]  dump_stack_lvl+0x74/0x8c
> [  174.008198]  dump_stack+0x18/0x24
> [  174.011526]  sysfs_warn_dup+0x64/0x80
> [  174.015201]  sysfs_do_create_link_sd+0xf0/0xf8
> [  174.019656]  sysfs_create_link+0x20/0x40
> [  174.023590]  software_node_notify+0x90/0x100
> [  174.027872]  device_create_managed_software_node+0xec/0x108
> ...
> 
> The '4c010010.usb' device is a platform device created during the initcall
> and is never removed, which causes its associated software node to persist
> indefinitely.
> 
> The existing device_create_managed_software_node() does not provide a
> corresponding removal function.
> 
> Replace device_create_managed_software_node() with the
> device_add_software_node() and device_remove_software_node() pair to ensure
> proper addition and removal of software nodes, addressing this issue.
> 
> Fixes: a9400f1979a0 ("usb: dwc3: imx8mp: add 2 software managed quirk properties for host mode")
> Cc: stable@vger.kernel.org
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - add error handling as suggested by Thinh Nguyen
>  - modify commit message as suggested Frank
>  - add Rb-tag
> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> index 356812cbcd88..3edc5aca76f9 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -129,6 +129,16 @@ static void dwc3_imx8mp_wakeup_disable(struct dwc3_imx8mp *dwc3_imx)
>  	writel(val, dwc3_imx->hsio_blk_base + USB_WAKEUP_CTRL);
>  }
>  
> +static const struct property_entry dwc3_imx8mp_properties[] = {
> +	PROPERTY_ENTRY_BOOL("xhci-missing-cas-quirk"),
> +	PROPERTY_ENTRY_BOOL("xhci-skip-phy-init-quirk"),
> +	{},
> +};
> +
> +static const struct software_node dwc3_imx8mp_swnode = {
> +	.properties = dwc3_imx8mp_properties,
> +};
> +
>  static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
>  {
>  	struct dwc3_imx8mp	*dwc3_imx = _dwc3_imx;
> @@ -148,17 +158,6 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
>  	return IRQ_HANDLED;
>  }
>  
> -static int dwc3_imx8mp_set_software_node(struct device *dev)
> -{
> -	struct property_entry props[3] = { 0 };
> -	int prop_idx = 0;
> -
> -	props[prop_idx++] = PROPERTY_ENTRY_BOOL("xhci-missing-cas-quirk");
> -	props[prop_idx++] = PROPERTY_ENTRY_BOOL("xhci-skip-phy-init-quirk");
> -
> -	return device_create_managed_software_node(dev, props, NULL);
> -}
> -
>  static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  {
>  	struct device		*dev = &pdev->dev;
> @@ -221,17 +220,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto disable_rpm;
>  
> -	err = dwc3_imx8mp_set_software_node(dev);
> +	err = device_add_software_node(dev, &dwc3_imx8mp_swnode);
>  	if (err) {
>  		err = -ENODEV;
> -		dev_err(dev, "failed to create software node\n");
> +		dev_err(dev, "failed to add software node\n");
>  		goto disable_rpm;
>  	}
>  
>  	err = of_platform_populate(node, NULL, NULL, dev);
>  	if (err) {
>  		dev_err(&pdev->dev, "failed to create dwc3 core\n");
> -		goto disable_rpm;
> +		goto remove_swnode;
>  	}
>  
>  	dwc3_imx->dwc3 = of_find_device_by_node(dwc3_np);
> @@ -255,6 +254,8 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  
>  depopulate:
>  	of_platform_depopulate(dev);
> +remove_swnode:
> +	device_remove_software_node(dev);
>  disable_rpm:
>  	pm_runtime_disable(dev);
>  	pm_runtime_put_noidle(dev);
> @@ -268,6 +269,7 @@ static void dwc3_imx8mp_remove(struct platform_device *pdev)
>  
>  	pm_runtime_get_sync(dev);
>  	of_platform_depopulate(dev);
> +	device_remove_software_node(dev);
>  
>  	pm_runtime_disable(dev);
>  	pm_runtime_put_noidle(dev);
> -- 
> 2.34.1
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-11-26 22:15 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-26  3:28 [PATCH v2] usb: dwc3: imx8mp: fix software node kernel dump Xu Yang
2024-11-26 22:14 ` Thinh Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox