[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEXTbpeKUMJKz-PV-ft5WCg5TYo22knBaMX2WwCWn=ix4OgX+g@mail.gmail.com>
Date: Mon, 24 Nov 2025 22:22:18 +0800
From: Pin-yen Lin <treapking@...omium.org>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: jerry xzq <jerry.xzq@...il.com>, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] USB: of: filter disabled device node
Hi Greg,
On Mon, Nov 24, 2025 at 10:01 PM Greg KH <gregkh@...uxfoundation.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@...il.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@...il.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", ®))
> > > 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
Powered by blists - more mailing lists