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

[Simon Glass]

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?

> 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
>