[PATCH v5 1/2] usb: provide a device tree node to USB devices

[Simon Glass]

Hi Michael,

On Thu, 21 May 2020 at 17:28, Michael Walle <michael at walle.cc> wrote:
>
> Am 2020-05-21 16:13, schrieb Bin Meng:
> > On Thu, May 21, 2020 at 12:40 AM Michael Walle <michael at walle.cc>
> > wrote:
> >>
> >> It is possible to specify a device tree node for an USB device. This
> >> is
> >> useful if you have a static USB setup and want to use aliases which
> >> point to these nodes, like on the Raspberry Pi.
> >> The nodes are matched against their hub port number, the compatible
> >> strings are not matched for now.
> >>
> >> Signed-off-by: Michael Walle <michael at walle.cc>
> >> ---
> >> This is a new patch in v5:
> >>   Fixes the ethernet0 alias on Raspberry Pis. This has never been
> >>   working, but wasn't a problem until recently. Patch 2/2 changes
> >>   the allocation of the numbers and reserves possible aliases.
> >>
> >>  drivers/usb/host/usb-uclass.c | 41
> >> ++++++++++++++++++++++++++++++-----
> >>  1 file changed, 36 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/usb/host/usb-uclass.c
> >> b/drivers/usb/host/usb-uclass.c
> >> index cb79dfbbd5..f42c0625cb 100644
> >> --- a/drivers/usb/host/usb-uclass.c
> >> +++ b/drivers/usb/host/usb-uclass.c
> >> @@ -494,6 +494,35 @@ static int usb_match_one_id(struct
> >> usb_device_descriptor *desc,
> >>         return usb_match_one_id_intf(desc, int_desc, id);
> >>  }
> >>
> >> +static ofnode usb_get_ofnode(struct udevice *hub, int port)
> >> +{
> >> +       ofnode node;
> >> +       u32 reg;
> >> +
> >> +       if (!dev_has_of_node(hub))
> >> +               return ofnode_null();
> >> +
> >> +       /*
> >> +        * The USB controller and its USB hub are two different
> >> udevices,
> >> +        * but the device tree has only one node for both. Thus we are
> >> +        * assigning this node to both udevices.
> >> +        * If port is zero, the controller scans its root hub, thus we
> >> +        * are using the same ofnode as the controller here.
> >> +        */
> >> +       if (!port)
> >> +               return dev_ofnode(hub);
> >> +
> >> +       ofnode_for_each_subnode(node, dev_ofnode(hub)) {
> >> +               if (ofnode_read_u32(node, "reg", &reg))
> >> +                       continue;
> >> +
> >> +               if (reg == port)
> >> +                       return node;
> >> +       }
> >> +
> >> +       return ofnode_null();
> >> +}
> >> +
> >>  /**
> >>   * usb_find_and_bind_driver() - Find and bind the right USB driver
> >>   *
> >> @@ -502,13 +531,14 @@ static int usb_match_one_id(struct
> >> usb_device_descriptor *desc,
> >>  static int usb_find_and_bind_driver(struct udevice *parent,
> >>                                     struct usb_device_descriptor
> >> *desc,
> >>                                     struct usb_interface_descriptor
> >> *iface,
> >> -                                   int bus_seq, int devnum,
> >> +                                   int bus_seq, int devnum, int port,
> >>                                     struct udevice **devp)
> >>  {
> >>         struct usb_driver_entry *start, *entry;
> >>         int n_ents;
> >>         int ret;
> >>         char name[30], *str;
> >> +       ofnode node = usb_get_ofnode(parent, port);
> >>
> >>         *devp = NULL;
> >>         debug("%s: Searching for driver\n", __func__);
> >> @@ -533,8 +563,8 @@ static int usb_find_and_bind_driver(struct udevice
> >> *parent,
> >>                          * find another driver. For now this doesn't
> >> seem
> >>                          * necesssary, so just bind the first match.
> >>                          */
> >> -                       ret = device_bind(parent, drv, drv->name,
> >> NULL, -1,
> >> -                                         &dev);
> >> +                       ret = device_bind_ofnode(parent, drv,
> >> drv->name, NULL,
> >> +                                                node, &dev);
> >>                         if (ret)
> >>                                 goto error;
> >>                         debug("%s: Match found: %s\n", __func__,
> >> drv->name);
> >> @@ -651,9 +681,10 @@ int usb_scan_device(struct udevice *parent, int
> >> port,
> >>         if (ret) {
> >>                 if (ret != -ENOENT)
> >>                         return ret;
> >> -               ret = usb_find_and_bind_driver(parent,
> >> &udev->descriptor, iface,
> >> +               ret = usb_find_and_bind_driver(parent,
> >> &udev->descriptor,
> >> +                                              iface,
> >>
> >> udev->controller_dev->seq,
> >> -                                              udev->devnum, &dev);
> >> +                                              udev->devnum, port,
> >> &dev);
> >>                 if (ret)
> >>                         return ret;
> >>                 created = true;
> >> --
> >
> > Do we have tests added ?
>
> Adding tests for this isn't straight forward. Mostly because the device
> tree
> is used to add the emulated USB devices. OTOH we try to match the device
> tree
> to the scanned devices. To make things worse, the hierarchy of the USB
> hubs
> and usb devices doesn't seem to fit a "normal" device tree.

This is described in usb-info.rst

> Eg. in sandbox/dts/test.dts it is:
> usb at 1 {
>    /* this is the controller */
>    hub {
>      /* I don't know what this is */

The root hub

>      hub-emul {
>        /* this is the root hub */

No, this is the emulation driver. I is added so that the root hub works.

>        flash-stick at 0 {
>          reg = <0>;
>          /* this is an usb device on port _1_ of the root hub */

No, this is an emulation device. So when we probe the hub we will see
this emulation driver and create a device.


>        };
>      };
>    };
> };
>
> On a real device tree (eg. the raspberry pi one) it is:
> usb {
>    /* this is the controller & root hub */
>    usb1 at 1 {
>      /* this is another _external_ hub on port 1 of the root hub */
>      reg = <1>;
>      usbether at 1 {
>        /* this is an usb device on port 1 of the external hub */
>        reg = <1>;
>      };
>    };
> };
>
> They don't match at all. Thus the new code won't match any device
> of the emulated usb hierarchy.

If you ignore the emulation nodes, they match. The emulation nodes are
added so that there is actually something on the bus for your test
code to see.

REgards,
SImon