Hi Arnd, On Fri, Oct 13, 2017 at 9:37 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > On Thu, Oct 12, 2017 at 10:56 PM, Martin Blumenstingl > <martin.blumenstingl@xxxxxxxxxxxxxx> wrote: >> Hi Arnd, >> >> thank you for reviewing this patch! >> >> On Mon, Oct 9, 2017 at 12:24 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote: >>> On Sun, Oct 8, 2017 at 11:17 PM, Martin Blumenstingl >>> <martin.blumenstingl@xxxxxxxxxxxxxx> wrote: >>>> A USB root-hub may have several PHYs which need to be configured before >>>> the root-hub starts working. >>>> This adds the documentation for such a USB root-hub as well as a hint >>>> regarding the child-nodes on XHCI controllers which can include the >>>> roothub. >>>> >>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> >>>> Acked-by: Rob Herring <robh@xxxxxxxxxx> >>> >>> Have you checked that this still works with DT properties on a USB device >>> that is listed in the DT? A common use-case is to provide the MAC address >>> of a soldered-down USB-ethernet that lacks its own eeprom, and it seems >>> you are changing the way the parent devices of that get represented, >>> so the dev->of_node pointer in the USB device might no longer refer >>> to the correct device. >> I haven't tested the described use-case. however, this patch is not >> supposed to change the binding for actual devices. >> USB device numbering starts at 1, while 0 is reserved for the root-hub >> (at least from what I know). before this patch there was no way to >> describe the root-hub via .dts. this however is required for some >> platforms that need to set up a PHY for each port on the root-hub >> (Amlogic Meson GXL platform is one example: there are two ports >> enabled on dwc3's root-hub with 2x USB2 and 1x USB3 PHYs) - so this >> patch uses a similar binding as we already have (to describe the >> devices) to describe the root-hub > > My point is that the DT binding does depend on the hierarchy, there > is no way to find a particular device unless each parent up the > chain is described correctly in DT as well and has a valid of_node > pointer. > > It's possible that this has never worked on XHCI because of the lack > of the root-hub in DT. Either way, we should ensure that it does work > now and you didn't break it, so please at least test it with your patches. > > The patch below should be sufficient to see if any device has an > of_node pointer when you add it to your DT: I slightly modified your patch (see the attached version) and tried it: # lsusb -t /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/0p, 5000M /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/2p, 480M |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M |__ Port 3: Dev 3, If 0, Class=Mass Storage, Driver=usb-storage, 480M the roothub (bus 1 port 1 dev 1) and a soldered down hub (child of that root-hub, port 1 dev 2) have an entry in the .dts # dmesg | grep usb [ 0.174097] usbcore: registered new interface driver usbfs [ 0.174147] usbcore: registered new interface driver hub [ 0.174217] usbcore: registered new device driver usb [ 1.354280] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM. [ 1.373297] usbcore: registered new interface driver usb-storage [ 1.512840] usbcore: registered new interface driver dvb_usb_rtl28xxu [ 1.550506] usb 1-1: of_node /soc/usb@c9000000/hub@1 parent /soc/usb@c9000000 ^ this is the soldered down hub [ 1.552579] usbcore: registered new interface driver bcm203x [ 1.712033] usbcore: registered new interface driver usbhid [ 1.716994] usbhid: USB HID core driver [ 1.738827] usb 1-1: new high-speed USB device number 2 using xhci-hcd [ 2.142392] usb 1-1.3: of_node <no-node> parent /soc/usb@c9000000/hub@1 ^ this is the thumb drive plugged into the hub (not specified in the .dts) [ 2.220399] usb 1-1.3: new high-speed USB device number 3 using xhci-hcd [ 2.326144] usb-storage 1-1.3:1.0: USB Mass Storage device detected [ 2.328352] scsi host0: usb-storage 1-1.3:1.0 so for me it seems to be working fine is there anything else you want me to test? > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c > index 17681d5638ac..498d0aa0a5c0 100644 > --- a/drivers/usb/core/usb.c > +++ b/drivers/usb/core/usb.c > @@ -647,6 +647,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, > } > dev->dev.of_node = usb_of_get_child_node(parent->dev.of_node, > raw_port); > + dev_info(&dev->dev, "of_node %p parent %p\n", > dev->dev.of_node, parent->dev.of_node); > > /* hub driver sets up TT records */ > } > > > Arnd Regards, Martin
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi index cfbfbfeff85d..7677ce77d122 100644 --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi @@ -81,6 +81,11 @@ phy-names = "usb2-phy"; }; }; + + hub@1 { + compatible = "usb1a40,801"; + reg = <1>; + }; }; }; }; diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 17681d5638ac..893f4e8b0733 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -647,6 +647,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, } dev->dev.of_node = usb_of_get_child_node(parent->dev.of_node, raw_port); + dev_info(&dev->dev, "of_node %s parent %s\n", of_node_full_name(dev->dev.of_node), of_node_full_name(parent->dev.of_node)); /* hub driver sets up TT records */ }