[PATCH] USB: of: filter disabled device node

[PATCH] USB: of: filter disabled device node

[Zhengqiao Xia]

We should not point the of_node of a USB device to a disabled devicetree
node. Otherwise, the interface under this USB device will not be able
to register.

Signed-off-by: Zhengqiao Xia <jerry.xzq@gmail.com>
---
 drivers/usb/core/of.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
index 763e4122ed5b3..6bb577e711811 100644
--- a/drivers/usb/core/of.c
+++ b/drivers/usb/core/of.c
@@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct usb_device *hub, int port1)
 		if (of_property_read_u32(node, "reg", &reg))
 			continue;
 
+		if (!of_device_is_available(node))
+			continue;
+
 		if (reg == port1)
 			return node;
 	}
-- 
2.34.1

Re: [PATCH] USB: of: filter disabled device node

[Greg KH]

On Sat, Nov 22, 2025 at 07:25:39PM +0800, Zhengqiao Xia wrote:
> We should not point the of_node of a USB device to a disabled devicetree
> node. Otherwise, the interface under this USB device will not be able
> to register.
> 
> Signed-off-by: Zhengqiao Xia <jerry.xzq@gmail.com>
> ---
>  drivers/usb/core/of.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
> index 763e4122ed5b3..6bb577e711811 100644
> --- a/drivers/usb/core/of.c
> +++ b/drivers/usb/core/of.c
> @@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct usb_device *hub, int port1)
>  		if (of_property_read_u32(node, "reg", &reg))
>  			continue;
>  
> +		if (!of_device_is_available(node))
> +			continue;
> +
>  		if (reg == port1)
>  			return node;
>  	}
> -- 
> 2.34.1
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH] USB: of: filter disabled device node

[Greg KH]

On Sat, Nov 22, 2025 at 07:25:39PM +0800, Zhengqiao Xia wrote:
> We should not point the of_node of a USB device to a disabled devicetree
> node. Otherwise, the interface under this USB device will not be able
> to register.

Why would a USB device point to a disabled device tree node?  Shouldn't
the device tree be fixed instead?

thanks,

greg k-h

Re: [PATCH] USB: of: filter disabled device node

[jerry xzq]

On Sat, Nov 22, 2025 at 7:32 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Nov 22, 2025 at 07:25:39PM +0800, Zhengqiao Xia wrote:
> > We should not point the of_node of a USB device to a disabled devicetree
> > node. Otherwise, the interface under this USB device will not be able
> > to register.
>
> Why would a USB device point to a disabled device tree node?  Shouldn't
> the device tree be fixed instead?

According to your suggestion, when a USB device is directly connected
to a USB port,
we should delete the unused USB hub node instead of disabling it?

>
> thanks,
>
> greg k-h

thanks

Re: [PATCH] USB: of: filter disabled device node

[Greg KH]

On Sat, Nov 22, 2025 at 07:49:02PM +0800, jerry xzq wrote:
> On Sat, Nov 22, 2025 at 7:32 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, Nov 22, 2025 at 07:25:39PM +0800, Zhengqiao Xia wrote:
> > > We should not point the of_node of a USB device to a disabled devicetree
> > > node. Otherwise, the interface under this USB device will not be able
> > > to register.
> >
> > Why would a USB device point to a disabled device tree node?  Shouldn't
> > the device tree be fixed instead?
> 
> According to your suggestion, when a USB device is directly connected
> to a USB port,
> we should delete the unused USB hub node instead of disabling it?

Why are you using device tree to describe USB devices at all?  What is
the root problem here that you are trying to solve?

The use of USB in DT should be _VERY_ limited, if at all.  You should
only do so for very good reasons in very limited situations.  If this
starts to get abused, we'll just have to rip it all out :(

thanks,

greg k-h

Re: [PATCH] USB: of: filter disabled device node

[jerry xzq]

On Sat, Nov 22, 2025 at 8:32 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Nov 22, 2025 at 07:49:02PM +0800, jerry xzq wrote:
> > On Sat, Nov 22, 2025 at 7:32 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Sat, Nov 22, 2025 at 07:25:39PM +0800, Zhengqiao Xia wrote:
> > > > We should not point the of_node of a USB device to a disabled devicetree
> > > > node. Otherwise, the interface under this USB device will not be able
> > > > to register.
> > >
> > > Why would a USB device point to a disabled device tree node?  Shouldn't
> > > the device tree be fixed instead?
> >
> > According to your suggestion, when a USB device is directly connected
> > to a USB port,
> > we should delete the unused USB hub node instead of disabling it?
>
> Why are you using device tree to describe USB devices at all?  What is
> the root problem here that you are trying to solve?

Our device follows a public board that has a USB hub, which is
described in dtsi as a USB hub node.
However, our device only has a USB device, not a USB hub, so I
disabled the USB hub node in our device's DTS.
&xhci3 {
        status = "okay";

        /* 2.x hub on port 1 */
        usb_hub_2_x: hub@1 {
                compatible = "usbbda,5411";
                reg = <1>;
                vdd-supply = <&pp3300_s3>;
                peer-hub = <&usb_hub_3_x>;

----->         status = "disabled";

                ports {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        port@1 {
                                reg = <1>;
                                usb_hub_dsp1_hs: endpoint { };
                        };
                        port@2 {
                                reg = <2>;
                                usb_hub_dsp2_hs: endpoint { };
                        };
                        port@3 {
                                reg = <3>;
                                usb_hub_dsp3_hs: endpoint { };
                        };
                        port@4 {
                                reg = <4>;

                                /* On-board WWAN card */
                                usb_hub_dsp4_hs: endpoint { };
                        };
                };
        };
Then, during use, it was discovered that the LTE device's of_node
pointed to the hub(hub@1).
However, due to the existence of this piece of code,  USB interface
cannot be registered.

if (intf->dev.of_node &&
!of_device_is_available(intf->dev.of_node)) {
dev_info(&dev->dev, "skipping disabled interface %d\n",
intf->cur_altsetting->desc.bInterfaceNumber);
continue;
}

When the upgrade program for an LTE device calls the USB character device,
because this LTE interface was not registered, a null pointer
exception was encountered, causing a restart.

please refer to
https://lore.kernel.org/all/20251114141821.416835-1-xiazhengqiao@huaqin.corp-partner.google.com/

>
> The use of USB in DT should be _VERY_ limited, if at all.  You should
> only do so for very good reasons in very limited situations.  If this
> starts to get abused, we'll just have to rip it all out :(
>
> thanks,
>
> greg k-h

thanks.

Re: [PATCH] USB: of: filter disabled device node

[Greg KH]

On Sat, Nov 22, 2025 at 07:25:39PM +0800, Zhengqiao Xia wrote:
> We should not point the of_node of a USB device to a disabled devicetree
> node. Otherwise, the interface under this USB device will not be able
> to register.
> 
> Signed-off-by: Zhengqiao Xia <jerry.xzq@gmail.com>

Also, gmail.com is not a company email :)

Re: [PATCH] USB: of: filter disabled device node

[Greg KH]

On Sat, Nov 22, 2025 at 07:31:47PM +0800, jerry xzq wrote:
> On Sat, Nov 22, 2025 at 7:26 PM Zhengqiao Xia <jerry.xzq@gmail.com> wrote:
> 
> > We should not point the of_node of a USB device to a disabled devicetree
> > node. Otherwise, the interface under this USB device will not be able
> > to register.
> >
> > Signed-off-by: Zhengqiao Xia <jerry.xzq@gmail.com>
> > ---
> >  drivers/usb/core/of.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
> > index 763e4122ed5b3..6bb577e711811 100644
> > --- a/drivers/usb/core/of.c
> > +++ b/drivers/usb/core/of.c
> > @@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct
> > usb_device *hub, int port1)
> >                 if (of_property_read_u32(node, "reg", &reg))
> >                         continue;
> >
> > +               if (!of_device_is_available(node))
> > +                       continue;
> > +
> >                 if (reg == port1)
> >                         return node;
> >         }
> > --
> > 2.34.1
> >
> >  Supplementing questions from the previous email:
> 
> > What changed to require this?  What commit id does this fix?
> > And what devices have a disabled devicetree node?
> 
> fixes: 01fdf179f4b064d4c9d30(usb: core: skip interfaces disabled in
> devicetree )
> 
> Connect a USB device directly to the USB port, for me, LTE RW101.

Why?  Why not just us the normal USB device topology?  Why is this in DT
at all?

> However, a disabled node is attached to the DTS node of this port.

Why?

> &xhci3 {
>         status = "okay";
> 
>         /* 2.x hub on port 1 */
>         usb_hub_2_x: hub@1 {
>                 compatible = "usbbda,5411";
>                 reg = <1>;
>                 vdd-supply = <&pp3300_s3>;
>                 peer-hub = <&usb_hub_3_x>;
>                 status = "disabled";
> 
>                 ports {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         port@1 {
>                                 reg = <1>;
>                                 usb_hub_dsp1_hs: endpoint { };
>                         };
>                         port@2 {
>                                 reg = <2>;
>                                 usb_hub_dsp2_hs: endpoint { };
>                         };
>                         port@3 {
>                                 reg = <3>;
>                                 usb_hub_dsp3_hs: endpoint { };
>                         };
>                         port@4 {
>                                 reg = <4>;
> 
>                                 /* On-board WWAN card */
>                                 usb_hub_dsp4_hs: endpoint { };

That's the thing I don't want to see, why is that WWAN card described
here?  Why can't the normal USB device discovery find it and use it
properly?

>                         };
>                 };
>         };
> 
> Based on the current code, the of_node of this directly connected LTE
> device is hub.

But why is that needed?

> If there is only one LTE interface, then the of_node of this interface
> is also the hub.

Again, why?

> With the following code, the interface will be unable to create a device.
> 
> if (intf->dev.of_node &&
> !of_device_is_available(intf->dev.of_node)) {
> dev_info(&dev->dev, "skipping disabled interface %d\n",
> intf->cur_altsetting->desc.bInterfaceNumber);
> continue;
> }
> Then this LTE will be unable to create a device.
> this is not the result I wanted.

Then try removing it from dt entirely, it should not be necessary to
describe USB devices in dt.

thanks,

greg k-h

Re: [PATCH] USB: of: filter disabled device node

[jerry xzq]

On Sat, Nov 22, 2025 at 8:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Nov 22, 2025 at 07:31:47PM +0800, jerry xzq wrote:
> > On Sat, Nov 22, 2025 at 7:26 PM Zhengqiao Xia <jerry.xzq@gmail.com> wrote:
> >
> > > We should not point the of_node of a USB device to a disabled devicetree
> > > node. Otherwise, the interface under this USB device will not be able
> > > to register.
> > >
> > > Signed-off-by: Zhengqiao Xia <jerry.xzq@gmail.com>
> > > ---
> > >  drivers/usb/core/of.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
> > > index 763e4122ed5b3..6bb577e711811 100644
> > > --- a/drivers/usb/core/of.c
> > > +++ b/drivers/usb/core/of.c
> > > @@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct
> > > usb_device *hub, int port1)
> > >                 if (of_property_read_u32(node, "reg", &reg))
> > >                         continue;
> > >
> > > +               if (!of_device_is_available(node))
> > > +                       continue;
> > > +
> > >                 if (reg == port1)
> > >                         return node;
> > >         }
> > > --
> > > 2.34.1
> > >
> > >  Supplementing questions from the previous email:
> >
> > > What changed to require this?  What commit id does this fix?
> > > And what devices have a disabled devicetree node?
> >
> > fixes: 01fdf179f4b064d4c9d30(usb: core: skip interfaces disabled in
> > devicetree )
> >
> > Connect a USB device directly to the USB port, for me, LTE RW101.
>
> Why?  Why not just us the normal USB device topology?  Why is this in DT
> at all?
This is why I need to disable this USB hub node.  I didn't want this
node to work properly, so I disabled it.

>
> > However, a disabled node is attached to the DTS node of this port.
>
> Why?
please see:
https://github.com/torvalds/linux/blob/master/drivers/usb/core/usb.c#L731
https://github.com/torvalds/linux/blob/master/drivers/usb/core/of.c#L25

>
> > &xhci3 {
> >         status = "okay";
> >
> >         /* 2.x hub on port 1 */
> >         usb_hub_2_x: hub@1 {
> >                 compatible = "usbbda,5411";
> >                 reg = <1>;
> >                 vdd-supply = <&pp3300_s3>;
> >                 peer-hub = <&usb_hub_3_x>;
> >                 status = "disabled";
> >
> >                 ports {
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >                         port@1 {
> >                                 reg = <1>;
> >                                 usb_hub_dsp1_hs: endpoint { };
> >                         };
> >                         port@2 {
> >                                 reg = <2>;
> >                                 usb_hub_dsp2_hs: endpoint { };
> >                         };
> >                         port@3 {
> >                                 reg = <3>;
> >                                 usb_hub_dsp3_hs: endpoint { };
> >                         };
> >                         port@4 {
> >                                 reg = <4>;
> >
> >                                 /* On-board WWAN card */
> >                                 usb_hub_dsp4_hs: endpoint { };
>
> That's the thing I don't want to see, why is that WWAN card described
> here?  Why can't the normal USB device discovery find it and use it
> properly?

I want to disable the whole hub.

>
> >                         };
> >                 };
> >         };
> >
> > Based on the current code, the of_node of this directly connected LTE
> > device is hub.
>
> But why is that needed?
this is the behavior of code:
please see:
https://github.com/torvalds/linux/blob/master/drivers/usb/core/usb.c#L731
https://github.com/torvalds/linux/blob/master/drivers/usb/core/of.c#L25

>
> > If there is only one LTE interface, then the of_node of this interface
> > is also the hub.
>
> Again, why?
please see: https://github.com/torvalds/linux/blob/master/drivers/usb/core/of.c#L57
>
> > With the following code, the interface will be unable to create a device.
> >
> > if (intf->dev.of_node &&
> > !of_device_is_available(intf->dev.of_node)) {
> > dev_info(&dev->dev, "skipping disabled interface %d\n",
> > intf->cur_altsetting->desc.bInterfaceNumber);
> > continue;
> > }
> > Then this LTE will be unable to create a device.
> > this is not the result I wanted.
>
> Then try removing it from dt entirely, it should not be necessary to
> describe USB devices in dt.
>
> thanks,
>
> greg k-h

Re: [PATCH] USB: of: filter disabled device node

[Pin-yen Lin]

Hi Greg,

On Mon, Nov 24, 2025 at 10:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Nov 22, 2025 at 07:31:47PM +0800, jerry xzq wrote:
> > On Sat, Nov 22, 2025 at 7:26 PM Zhengqiao Xia <jerry.xzq@gmail.com> wrote:
> >
> > > We should not point the of_node of a USB device to a disabled devicetree
> > > node. Otherwise, the interface under this USB device will not be able
> > > to register.
> > >
> > > Signed-off-by: Zhengqiao Xia <jerry.xzq@gmail.com>
> > > ---
> > >  drivers/usb/core/of.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
> > > index 763e4122ed5b3..6bb577e711811 100644
> > > --- a/drivers/usb/core/of.c
> > > +++ b/drivers/usb/core/of.c
> > > @@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct
> > > usb_device *hub, int port1)
> > >                 if (of_property_read_u32(node, "reg", &reg))
> > >                         continue;
> > >
> > > +               if (!of_device_is_available(node))
> > > +                       continue;
> > > +
> > >                 if (reg == port1)
> > >                         return node;
> > >         }
> > > --
> > > 2.34.1
> > >
> > >  Supplementing questions from the previous email:
> >
> > > What changed to require this?  What commit id does this fix?
> > > And what devices have a disabled devicetree node?
> >
> > fixes: 01fdf179f4b064d4c9d30(usb: core: skip interfaces disabled in
> > devicetree )
> >
> > Connect a USB device directly to the USB port, for me, LTE RW101.
>
> Why?  Why not just us the normal USB device topology?  Why is this in DT
> at all?

In our use case, the USB hub and the USB devices (e.g., modem card,
USB camera) are fixed on the board, and describing them allows us to:
(1) Describe the extra resources for the USB devices, like the usages
in drivers/misc/onboard_usb_dev.c. They are mostly USB hubs that
require extra power or reset pin, but there are also USB device
usages.
(2) Let the userspace know which devices are fixed on the board, which
makes it trustable.
>
> > However, a disabled node is attached to the DTS node of this port.
>
> Why?

This is the usage from a downstream DTS that hasn't been upstreamed.

The USB hub and devices are defined in a DTSI file, and another DTS
inherits it but wants to disable those USB devices. We expected that
disabling them should be the same as removing them.
>
> > &xhci3 {
> >         status = "okay";
> >
> >         /* 2.x hub on port 1 */
> >         usb_hub_2_x: hub@1 {
> >                 compatible = "usbbda,5411";
> >                 reg = <1>;
> >                 vdd-supply = <&pp3300_s3>;
> >                 peer-hub = <&usb_hub_3_x>;
> >                 status = "disabled";
> >
> >                 ports {
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >                         port@1 {
> >                                 reg = <1>;
> >                                 usb_hub_dsp1_hs: endpoint { };
> >                         };
> >                         port@2 {
> >                                 reg = <2>;
> >                                 usb_hub_dsp2_hs: endpoint { };
> >                         };
> >                         port@3 {
> >                                 reg = <3>;
> >                                 usb_hub_dsp3_hs: endpoint { };
> >                         };
> >                         port@4 {
> >                                 reg = <4>;
> >
> >                                 /* On-board WWAN card */
> >                                 usb_hub_dsp4_hs: endpoint { };
>
> That's the thing I don't want to see, why is that WWAN card described
> here?  Why can't the normal USB device discovery find it and use it
> properly?
>
> >                         };
> >                 };
> >         };
> >
> > Based on the current code, the of_node of this directly connected LTE
> > device is hub.
>
> But why is that needed?
>
> > If there is only one LTE interface, then the of_node of this interface
> > is also the hub.
>
> Again, why?

We haven't had a driver for the LTE card on the linux mainline. But,
it is using M.2 USB interface and requires reset and enable pins, so I
believe we want to describe it as a USB device in DT, and implement
the resource control in onboard_usb_dev.c.
>
> > With the following code, the interface will be unable to create a device.
> >
> > if (intf->dev.of_node &&
> > !of_device_is_available(intf->dev.of_node)) {
> > dev_info(&dev->dev, "skipping disabled interface %d\n",
> > intf->cur_altsetting->desc.bInterfaceNumber);
> > continue;
> > }
> > Then this LTE will be unable to create a device.
> > this is not the result I wanted.
>
> Then try removing it from dt entirely, it should not be necessary to
> describe USB devices in dt.

Hope the above reply answers your questions.
>
> thanks,
>
> greg k-h

Thanks and regards,
Pin-yen

Re: [PATCH] USB: of: filter disabled device node

[Greg KH]

On Mon, Nov 24, 2025 at 10:22:18PM +0800, Pin-yen Lin wrote:
> Hi Greg,
> 
> On Mon, Nov 24, 2025 at 10:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, Nov 22, 2025 at 07:31:47PM +0800, jerry xzq wrote:
> > > On Sat, Nov 22, 2025 at 7:26 PM Zhengqiao Xia <jerry.xzq@gmail.com> wrote:
> > >
> > > > We should not point the of_node of a USB device to a disabled devicetree
> > > > node. Otherwise, the interface under this USB device will not be able
> > > > to register.
> > > >
> > > > Signed-off-by: Zhengqiao Xia <jerry.xzq@gmail.com>
> > > > ---
> > > >  drivers/usb/core/of.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
> > > > index 763e4122ed5b3..6bb577e711811 100644
> > > > --- a/drivers/usb/core/of.c
> > > > +++ b/drivers/usb/core/of.c
> > > > @@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct
> > > > usb_device *hub, int port1)
> > > >                 if (of_property_read_u32(node, "reg", &reg))
> > > >                         continue;
> > > >
> > > > +               if (!of_device_is_available(node))
> > > > +                       continue;
> > > > +
> > > >                 if (reg == port1)
> > > >                         return node;
> > > >         }
> > > > --
> > > > 2.34.1
> > > >
> > > >  Supplementing questions from the previous email:
> > >
> > > > What changed to require this?  What commit id does this fix?
> > > > And what devices have a disabled devicetree node?
> > >
> > > fixes: 01fdf179f4b064d4c9d30(usb: core: skip interfaces disabled in
> > > devicetree )
> > >
> > > Connect a USB device directly to the USB port, for me, LTE RW101.
> >
> > Why?  Why not just us the normal USB device topology?  Why is this in DT
> > at all?
> 
> In our use case, the USB hub and the USB devices (e.g., modem card,
> USB camera) are fixed on the board, and describing them allows us to:
> (1) Describe the extra resources for the USB devices, like the usages
> in drivers/misc/onboard_usb_dev.c. They are mostly USB hubs that
> require extra power or reset pin, but there are also USB device
> usages.

The USB devices should NOT be in DT at all, only for hub controls that
you need the extra pin controls please.

> (2) Let the userspace know which devices are fixed on the board, which
> makes it trustable.

There is different ways to do this, NOT in dt.

> > > However, a disabled node is attached to the DTS node of this port.
> >
> > Why?
> 
> This is the usage from a downstream DTS that hasn't been upstreamed.

There's nothing we can do about that.  Please work to get it upstream.

> The USB hub and devices are defined in a DTSI file, and another DTS
> inherits it but wants to disable those USB devices. We expected that
> disabling them should be the same as removing them.

No, just disable them from userspace properly.

> > > &xhci3 {
> > >         status = "okay";
> > >
> > >         /* 2.x hub on port 1 */
> > >         usb_hub_2_x: hub@1 {
> > >                 compatible = "usbbda,5411";
> > >                 reg = <1>;
> > >                 vdd-supply = <&pp3300_s3>;
> > >                 peer-hub = <&usb_hub_3_x>;
> > >                 status = "disabled";
> > >
> > >                 ports {
> > >                         #address-cells = <1>;
> > >                         #size-cells = <0>;
> > >                         port@1 {
> > >                                 reg = <1>;
> > >                                 usb_hub_dsp1_hs: endpoint { };
> > >                         };
> > >                         port@2 {
> > >                                 reg = <2>;
> > >                                 usb_hub_dsp2_hs: endpoint { };
> > >                         };
> > >                         port@3 {
> > >                                 reg = <3>;
> > >                                 usb_hub_dsp3_hs: endpoint { };
> > >                         };
> > >                         port@4 {
> > >                                 reg = <4>;
> > >
> > >                                 /* On-board WWAN card */
> > >                                 usb_hub_dsp4_hs: endpoint { };
> >
> > That's the thing I don't want to see, why is that WWAN card described
> > here?  Why can't the normal USB device discovery find it and use it
> > properly?
> >
> > >                         };
> > >                 };
> > >         };
> > >
> > > Based on the current code, the of_node of this directly connected LTE
> > > device is hub.
> >
> > But why is that needed?
> >
> > > If there is only one LTE interface, then the of_node of this interface
> > > is also the hub.
> >
> > Again, why?
> 
> We haven't had a driver for the LTE card on the linux mainline.

Why is it not merged upstream?  That should be a very simple thing to
get accepted.

> But,
> it is using M.2 USB interface and requires reset and enable pins, so I
> believe we want to describe it as a USB device in DT, and implement
> the resource control in onboard_usb_dev.c.

No, that is not how USB devices work, they should control themselves.

thanks,

greg k-h

Re: [PATCH] USB: of: filter disabled device node

[Pin-yen Lin]

Hi Greg,

On Mon, Nov 24, 2025 at 11:51 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Nov 24, 2025 at 10:22:18PM +0800, Pin-yen Lin wrote:
> > Hi Greg,
> >
> > On Mon, Nov 24, 2025 at 10:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Sat, Nov 22, 2025 at 07:31:47PM +0800, jerry xzq wrote:
> > > > On Sat, Nov 22, 2025 at 7:26 PM Zhengqiao Xia <jerry.xzq@gmail.com> wrote:
> > > >
> > > > > We should not point the of_node of a USB device to a disabled devicetree
> > > > > node. Otherwise, the interface under this USB device will not be able
> > > > > to register.
> > > > >
> > > > > Signed-off-by: Zhengqiao Xia <jerry.xzq@gmail.com>
> > > > > ---
> > > > >  drivers/usb/core/of.c | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
> > > > > index 763e4122ed5b3..6bb577e711811 100644
> > > > > --- a/drivers/usb/core/of.c
> > > > > +++ b/drivers/usb/core/of.c
> > > > > @@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct
> > > > > usb_device *hub, int port1)
> > > > >                 if (of_property_read_u32(node, "reg", &reg))
> > > > >                         continue;
> > > > >
> > > > > +               if (!of_device_is_available(node))
> > > > > +                       continue;
> > > > > +
> > > > >                 if (reg == port1)
> > > > >                         return node;
> > > > >         }
> > > > > --
> > > > > 2.34.1
> > > > >
> > > > >  Supplementing questions from the previous email:
> > > >
> > > > > What changed to require this?  What commit id does this fix?
> > > > > And what devices have a disabled devicetree node?
> > > >
> > > > fixes: 01fdf179f4b064d4c9d30(usb: core: skip interfaces disabled in
> > > > devicetree )
> > > >
> > > > Connect a USB device directly to the USB port, for me, LTE RW101.
> > >
> > > Why?  Why not just us the normal USB device topology?  Why is this in DT
> > > at all?
> >
> > In our use case, the USB hub and the USB devices (e.g., modem card,
> > USB camera) are fixed on the board, and describing them allows us to:
> > (1) Describe the extra resources for the USB devices, like the usages
> > in drivers/misc/onboard_usb_dev.c. They are mostly USB hubs that
> > require extra power or reset pin, but there are also USB device
> > usages.
>
> The USB devices should NOT be in DT at all, only for hub controls that
> you need the extra pin controls please.

I assumed we should describe USB devices because [1] introduced
bindings for downstream USB ports for on-board hubs. Or should we only
describe USB connectors but not USB devices?

[1]: https://lore.kernel.org/all/20240223005823.3074029-2-swboyd@chromium.org/
>
> > (2) Let the userspace know which devices are fixed on the board, which
> > makes it trustable.
>
> There is different ways to do this, NOT in dt.
>
> > > > However, a disabled node is attached to the DTS node of this port.
> > >
> > > Why?
> >
> > This is the usage from a downstream DTS that hasn't been upstreamed.
>
> There's nothing we can do about that.  Please work to get it upstream.
>
> > The USB hub and devices are defined in a DTSI file, and another DTS
> > inherits it but wants to disable those USB devices. We expected that
> > disabling them should be the same as removing them.
>
> No, just disable them from userspace properly.

I mean, it is disabled because of some DTS inheritance, and I believe
we usually disable the nodes instead of removing them. How do we
disable them from userspace in this case?
>
> > > > &xhci3 {
> > > >         status = "okay";
> > > >
> > > >         /* 2.x hub on port 1 */
> > > >         usb_hub_2_x: hub@1 {
> > > >                 compatible = "usbbda,5411";
> > > >                 reg = <1>;
> > > >                 vdd-supply = <&pp3300_s3>;
> > > >                 peer-hub = <&usb_hub_3_x>;
> > > >                 status = "disabled";
> > > >
> > > >                 ports {
> > > >                         #address-cells = <1>;
> > > >                         #size-cells = <0>;
> > > >                         port@1 {
> > > >                                 reg = <1>;
> > > >                                 usb_hub_dsp1_hs: endpoint { };
> > > >                         };
> > > >                         port@2 {
> > > >                                 reg = <2>;
> > > >                                 usb_hub_dsp2_hs: endpoint { };
> > > >                         };
> > > >                         port@3 {
> > > >                                 reg = <3>;
> > > >                                 usb_hub_dsp3_hs: endpoint { };
> > > >                         };
> > > >                         port@4 {
> > > >                                 reg = <4>;
> > > >
> > > >                                 /* On-board WWAN card */
> > > >                                 usb_hub_dsp4_hs: endpoint { };
> > >
> > > That's the thing I don't want to see, why is that WWAN card described
> > > here?  Why can't the normal USB device discovery find it and use it
> > > properly?
> > >
> > > >                         };
> > > >                 };
> > > >         };
> > > >
> > > > Based on the current code, the of_node of this directly connected LTE
> > > > device is hub.
> > >
> > > But why is that needed?
> > >
> > > > If there is only one LTE interface, then the of_node of this interface
> > > > is also the hub.
> > >
> > > Again, why?
> >
> > We haven't had a driver for the LTE card on the linux mainline.
>
> Why is it not merged upstream?  That should be a very simple thing to
> get accepted.

We would love to, but those work was deprioritized internally.
>
> > But,
> > it is using M.2 USB interface and requires reset and enable pins, so I
> > believe we want to describe it as a USB device in DT, and implement
> > the resource control in onboard_usb_dev.c.
>
> No, that is not how USB devices work, they should control themselves.

I see "RTL8188ETV 2.4GHz WiFi" is included in the onboard_usb_hub.c
driver, and it also seems to be a USB device that requires extra
resources. Shouldn't we describe them describe them in DT and include
it in onboard_usb_dev.c if there are hardwares designed like this?
>
> thanks,
>
> greg k-h

Regards,
Pin-yen

Re: [PATCH] USB: of: filter disabled device node

[Greg KH]

On Tue, Nov 25, 2025 at 05:44:04PM +0800, Pin-yen Lin wrote:
> > > In our use case, the USB hub and the USB devices (e.g., modem card,
> > > USB camera) are fixed on the board, and describing them allows us to:
> > > (1) Describe the extra resources for the USB devices, like the usages
> > > in drivers/misc/onboard_usb_dev.c. They are mostly USB hubs that
> > > require extra power or reset pin, but there are also USB device
> > > usages.
> >
> > The USB devices should NOT be in DT at all, only for hub controls that
> > you need the extra pin controls please.
> 
> I assumed we should describe USB devices because [1] introduced
> bindings for downstream USB ports for on-board hubs. Or should we only
> describe USB connectors but not USB devices?

Describe the USB connectors please, not USB devices.  USB devices
already properly describe themselves.

> > > This is the usage from a downstream DTS that hasn't been upstreamed.
> >
> > There's nothing we can do about that.  Please work to get it upstream.
> >
> > > The USB hub and devices are defined in a DTSI file, and another DTS
> > > inherits it but wants to disable those USB devices. We expected that
> > > disabling them should be the same as removing them.
> >
> > No, just disable them from userspace properly.
> 
> I mean, it is disabled because of some DTS inheritance, and I believe
> we usually disable the nodes instead of removing them. How do we
> disable them from userspace in this case?

You can disable USB devices in userspace through sysfs.

> > > We haven't had a driver for the LTE card on the linux mainline.
> >
> > Why is it not merged upstream?  That should be a very simple thing to
> > get accepted.
> 
> We would love to, but those work was deprioritized internally.

As you know, we can't do anything about external drivers, so there's
nothing we can do.  Please revisit that decision, it's one that is
already costing you time and money :(

> > > But,
> > > it is using M.2 USB interface and requires reset and enable pins, so I
> > > believe we want to describe it as a USB device in DT, and implement
> > > the resource control in onboard_usb_dev.c.
> >
> > No, that is not how USB devices work, they should control themselves.
> 
> I see "RTL8188ETV 2.4GHz WiFi" is included in the onboard_usb_hub.c
> driver, and it also seems to be a USB device that requires extra
> resources. Shouldn't we describe them describe them in DT and include
> it in onboard_usb_dev.c if there are hardwares designed like this?

The driver for the USB device itself should handle this, but really, DT
should never be used for this as that goes against what USB is supposed
to be (i.e. your devices are not passing the USB standard by relying on
external DT information.)

thanks,

greg k-h

Re: [PATCH] USB: of: filter disabled device node

[Pin-yen Lin]

Hi Greg,

On Tue, Nov 25, 2025 at 7:53 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Nov 25, 2025 at 05:44:04PM +0800, Pin-yen Lin wrote:
> > > > In our use case, the USB hub and the USB devices (e.g., modem card,
> > > > USB camera) are fixed on the board, and describing them allows us to:
> > > > (1) Describe the extra resources for the USB devices, like the usages
> > > > in drivers/misc/onboard_usb_dev.c. They are mostly USB hubs that
> > > > require extra power or reset pin, but there are also USB device
> > > > usages.
> > >
> > > The USB devices should NOT be in DT at all, only for hub controls that
> > > you need the extra pin controls please.
> >
> > I assumed we should describe USB devices because [1] introduced
> > bindings for downstream USB ports for on-board hubs. Or should we only
> > describe USB connectors but not USB devices?
>
> Describe the USB connectors please, not USB devices.  USB devices
> already properly describe themselves.

Okay. But in our case, we can reproduce the crash even if we only
describe the on-board USB hub and the USB connectors and set the
status as "disabled".
>
> > > > This is the usage from a downstream DTS that hasn't been upstreamed.
> > >
> > > There's nothing we can do about that.  Please work to get it upstream.
> > >
> > > > The USB hub and devices are defined in a DTSI file, and another DTS
> > > > inherits it but wants to disable those USB devices. We expected that
> > > > disabling them should be the same as removing them.
> > >
> > > No, just disable them from userspace properly.
> >
> > I mean, it is disabled because of some DTS inheritance, and I believe
> > we usually disable the nodes instead of removing them. How do we
> > disable them from userspace in this case?
>
> You can disable USB devices in userspace through sysfs.

I think you misunderstood our use case. We have device A that has an
on-board USB hub, so we describe it in the DTS. Then, another device B
is similar to device A but does not have the USB hub. So, we inherit
device A's DTS and disable the hub nodes, and we observed a NULL
pointer dereference after boot.
>
> > > > We haven't had a driver for the LTE card on the linux mainline.
> > >
> > > Why is it not merged upstream?  That should be a very simple thing to
> > > get accepted.
> >
> > We would love to, but those work was deprioritized internally.
>
> As you know, we can't do anything about external drivers, so there's
> nothing we can do.  Please revisit that decision, it's one that is
> already costing you time and money :(
>
> > > > But,
> > > > it is using M.2 USB interface and requires reset and enable pins, so I
> > > > believe we want to describe it as a USB device in DT, and implement
> > > > the resource control in onboard_usb_dev.c.
> > >
> > > No, that is not how USB devices work, they should control themselves.
> >
> > I see "RTL8188ETV 2.4GHz WiFi" is included in the onboard_usb_hub.c
> > driver, and it also seems to be a USB device that requires extra
> > resources. Shouldn't we describe them describe them in DT and include
> > it in onboard_usb_dev.c if there are hardwares designed like this?
>
> The driver for the USB device itself should handle this, but really, DT
> should never be used for this as that goes against what USB is supposed
> to be (i.e. your devices are not passing the USB standard by relying on
> external DT information.)

Maybe those devices violated the USB standard, but we do see WWAN
cards that use M.2 socket 2 and work on USB with additional reset
pins.

Though this is off-topic because we can reproduce even without
describing the WWAN card in our devicetree.
>
> thanks,
>
> greg k-h

Regards,
Pin-yen

[PATCH] USB: of: filter disabled device node

[Zhengqiao Xia]

We should not point the of_node of a USB device to a disabled devicetree
node. Otherwise, the interface under this USB device will not be able
to register.

Signed-off-by: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
---
 drivers/usb/core/of.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
index 763e4122ed5b3..6bb577e711811 100644
--- a/drivers/usb/core/of.c
+++ b/drivers/usb/core/of.c
@@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct usb_device *hub, int port1)
 		if (of_property_read_u32(node, "reg", &reg))
 			continue;
 
+		if (!of_device_is_available(node))
+			continue;
+
 		if (reg == port1)
 			return node;
 	}
-- 
2.34.1

Re: [PATCH] USB: of: filter disabled device node

[Greg KH]

On Fri, Nov 21, 2025 at 04:09:05PM +0800, Zhengqiao Xia wrote:
> We should not point the of_node of a USB device to a disabled devicetree
> node. Otherwise, the interface under this USB device will not be able
> to register.

What changed to require this?  What commit id does this fix?

And what devices have a disabled devicetree node?

> Signed-off-by: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>

Please use a real email address, "corp-partner.google.com" is not "real"
:)

thanks,

greg k-h

Re: [PATCH] USB: of: filter disabled device node

[Zhengqiao Xia]

Greg KH <gregkh@linuxfoundation.org> 于2025年11月21日周五 20:35写道：
>
> On Fri, Nov 21, 2025 at 08:24:02PM +0800, Zhengqiao Xia wrote:
>
> <snip>
>
> For some reason you sent this only to me, which is a bit rude to
> everyone else on the mailing list.  I'll be glad to respond if you
> resend it to everyone.

I am so sorry, I didn't notice this at the time.

I will send it again with my company email
>
> thanks,
>
> greg k-h