lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEXTbpeN0Mk+Y-UeV79JzE547UCa_Fhh7T75L+2mhoSq3ark8g@mail.gmail.com>
Date: Tue, 25 Nov 2025 17:44:04 +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 11:51 PM Greg KH <gregkh@...uxfoundation.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@...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", &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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ