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 */
}
