| Message ID | 20241125101017.2398904-1-xu.yang_2@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc3: imx8mp: fix software node kernel dump | expand |
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. 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 >
On Mon, Nov 25, 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 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. > > 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); You need to check all the fail cases after this to properly remove the software node. BR, Thinh > 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 >
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 > >
On Mon, Nov 25, 2024 at 06:27:23PM +0000, Thinh Nguyen wrote: > On Mon, Nov 25, 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 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. > > > > 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); > > You need to check all the fail cases after this to properly remove the > software node. Indeed. I will improve it in v2. Thanks, Xu Yang > > BR, > Thinh > > > 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 > >
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);
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. 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(-)