[U-Boot] [PATCH 3/8] dm: usb: Store usb_device parent pointer in usb_device

[Hans de Goede]

Hi,

On 01-05-15 06:11, Simon Glass wrote:
> Hi Hans,
>
> On 30 April 2015 at 08:35, Hans de Goede <hdegoede at redhat.com> wrote:
>> When calling into the hcd code in usb_scan_device() we do not yet have
>> the actual udevice for the device we are scanning, so we temporarily set
>> usb_device.dev to the parent.
>>
>> This means that we cannot use usb_device.dev to accurately determine our
>> place in the usb topology when reading descriptors, and this is necessary
>> to correctly program the usb-2 hub tt settings when a usb-1 device is
>> connected to a usb-2 hub.
>>
>> This commit (re)adds a direct parent usb_device pointer to struct usb_device
>> so that the usb hcd code can get to the usb_device parent without needing to
>> go through the dm.
>>
>> This fixes usb-1 devices plugged into usb-2 hubs not working with the driver
>> model.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>   drivers/usb/host/ehci-hcd.c   | 24 +-----------------------
>>   drivers/usb/host/usb-uclass.c |  5 ++---
>>   include/usb.h                 |  2 +-
>>   3 files changed, 4 insertions(+), 27 deletions(-)
>>
>
> While I agree this is expedient, is there really no other way? How are
> we going to move to driver model if we add back in the
> non-driver-model pointers, etc.? We'll end up with a hybrid approach
> and a real mess.
>
> Is it possible to bug-fix the driver model code (presumably below) instead?
>

That is what I tried first, and failed todo. But after implementing the fix
as posted to the list I've been thinking about it some more and I think it should
be possible so I'll try again.

Regards,

Hans


>> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
>> index 19f1e29..00c038c 100644
>> --- a/drivers/usb/host/ehci-hcd.c
>> +++ b/drivers/usb/host/ehci-hcd.c
>> @@ -292,7 +292,6 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev,
>>                                            struct QH *qh)
>>   {
>>          struct usb_device *ttdev;
>> -       int parent_devnum;
>>
>>          if (udev->speed != USB_SPEED_LOW && udev->speed != USB_SPEED_FULL)
>>                  return;
>> @@ -302,35 +301,14 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device *udev,
>>           * the tt, so of the first upstream usb-2 hub, there may be usb-1 hubs
>>           * in the tree before that one!
>>           */
>> -#ifdef CONFIG_DM_USB
>> -       struct udevice *parent;
>> -
>> -       for (ttdev = udev; ; ) {
>> -               struct udevice *dev = ttdev->dev;
>> -
>> -               if (dev->parent &&
>> -                   device_get_uclass_id(dev->parent) == UCLASS_USB_HUB)
>> -                       parent = dev->parent;
>> -               else
>> -                       parent = NULL;
>> -               if (!parent)
>> -                       return;
>> -               ttdev = dev_get_parentdata(parent);
>> -               if (!ttdev->speed != USB_SPEED_HIGH)
>> -                       break;
>> -       }
>> -       parent_devnum = ttdev->devnum;
>> -#else
>>          ttdev = udev;
>>          while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH)
>>                  ttdev = ttdev->parent;
>>          if (!ttdev->parent)
>>                  return;
>> -       parent_devnum = ttdev->parent->devnum;
>> -#endif
>>
>>          qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(ttdev->portnr) |
>> -                                    QH_ENDPT2_HUBADDR(parent_devnum));
>> +                                    QH_ENDPT2_HUBADDR(ttdev->parent->devnum));
>>   }
>>
>>   static int
>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>> index 95e371d..0157bc6 100644
>> --- a/drivers/usb/host/usb-uclass.c
>> +++ b/drivers/usb/host/usb-uclass.c
>> @@ -470,7 +470,6 @@ int usb_scan_device(struct udevice *parent, int port,
>>          bool created = false;
>>          struct usb_dev_platdata *plat;
>>          struct usb_bus_priv *priv;
>> -       struct usb_device *parent_udev;
>>          int ret;
>>          ALLOC_CACHE_ALIGN_BUFFER(struct usb_device, udev, 1);
>>          struct usb_interface_descriptor *iface = &udev->config.if_desc[0].desc;
>> @@ -515,9 +514,9 @@ int usb_scan_device(struct udevice *parent, int port,
>>          udev->devnum = priv->next_addr + 1;
>>          udev->portnr = port;
>>          debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr);
>> -       parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
>> +       udev->parent = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
>>                  dev_get_parentdata(parent) : NULL;
>> -       ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev, port);
>> +       ret = usb_setup_device(udev, priv->desc_before_addr, udev->parent, port);
>>          debug("read_descriptor for '%s': ret=%d\n", parent->name, ret);
>>          if (ret)
>>                  return ret;
>> diff --git a/include/usb.h b/include/usb.h
>> index 1b667ff..7876df4 100644
>> --- a/include/usb.h
>> +++ b/include/usb.h
>> @@ -141,9 +141,9 @@ struct usb_device {
>>          int act_len;                    /* transfered bytes */
>>          int maxchild;                   /* Number of ports if hub */
>>          int portnr;                     /* Port number, 1=first */
>> -#ifndef CONFIG_DM_USB
>>          /* parent hub, or NULL if this is the root hub */
>>          struct usb_device *parent;
>> +#ifndef CONFIG_DM_USB
>>          struct usb_device *children[USB_MAXCHILDREN];
>>          void *controller;               /* hardware controller private data */
>>   #endif
>> --
>> 2.3.6
>>