Re: [PATCH] usb: dwc3: imx8mp: fix software node kernel dump

[Xu Yang <xu.yang_2@nxp.com>]

On Mon, Nov 25, 2024 at 12:40:23PM -0500, Frank Li wrote:
> On Mon, Nov 25, 2024 at 06:10:17PM +0800, 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 root cause is the '4c010010.usb' device is a platform device which is
> > created during initcall and it will never be removed. So the software node
> > will never be removed too even using device_create_managed_software_node().
> > This will use device_add/remove_software_node() to improve the logic.
> 
> 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.

Okay. Will improve it in v2.

Thanks,
Xu Yang

> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> 
> >
> > Fixes: a9400f1979a0 ("usb: dwc3: imx8mp: add 2 software managed quirk properties for host mode")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/dwc3/dwc3-imx8mp.c | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> > index 356812cbcd88..edd544c6e1c1 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,10 +220,10 @@ 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;
> >  	}
> >
> > @@ -268,6 +267,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
> >