Linux USB
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Pin-yen Lin <treapking@chromium.org>
Cc: jerry xzq <jerry.xzq@gmail.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] USB: of: filter disabled device node
Date: Mon, 24 Nov 2025 16:51:21 +0100	[thread overview]
Message-ID: <2025112426-seventeen-duvet-d9c4@gregkh> (raw)
In-Reply-To: <CAEXTbpeKUMJKz-PV-ft5WCg5TYo22knBaMX2WwCWn=ix4OgX+g@mail.gmail.com>

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

  reply	other threads:[~2025-11-24 15:51 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-22 11:25 [PATCH] USB: of: filter disabled device node Zhengqiao Xia
2025-11-22 11:31 ` Greg KH
2025-11-22 11:32 ` Greg KH
2025-11-22 11:49   ` jerry xzq
2025-11-22 12:32     ` Greg KH
2025-11-22 12:54       ` jerry xzq
2025-11-22 11:32 ` Greg KH
     [not found] ` <CAD48c-UjbPK4GewGpVVdEY30fhnhdAGQqrXQ3r0RgHc6suMo7Q@mail.gmail.com>
2025-11-22 12:34   ` Greg KH
2025-11-22 13:15     ` jerry xzq
2025-11-24 14:22     ` Pin-yen Lin
2025-11-24 15:51       ` Greg KH [this message]
2025-11-25  9:44         ` Pin-yen Lin
2025-11-25 11:53           ` Greg KH
2025-11-26 10:02             ` Pin-yen Lin
  -- strict thread matches above, loose matches on Subject: below --
2025-11-21  8:09 Zhengqiao Xia
2025-11-21  8:28 ` Greg KH
     [not found]   ` <CADYyEwQqghzWedZYmZtwk1+R5DxMYg0aMJhVAutrW1w+CTB-7w@mail.gmail.com>
     [not found]     ` <2025112126-leverage-wrongful-85c4@gregkh>
2025-11-22 11:04       ` Zhengqiao Xia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2025112426-seventeen-duvet-d9c4@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=jerry.xzq@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=treapking@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox