[U-Boot] [PATCH 1/8] dm: usb: Copy over usb_device values from usb_scan_device() to final usb_device

[Simon Glass]

Hi Hans,

On 30 April 2015 at 08:35, Hans de Goede <hdegoede at redhat.com> wrote:
> Currently we copy over a number of usb_device values stored in the on stack
> struct usb_device probed in usb_scan_device() to the final driver-model managed
> struct usb_device in usb_child_pre_probe() through usb_device_platdata, and
> then call usb_select_config() to fill in the rest.
>
> There are 2 problems with this approach:
>
> 1) It does not fill in enough fields before calling usb_select_config(),
> specifically it does not fill in ep0's maxpacketsize causing a div by zero
> exception in the ehci driver.

Didn't you have another patch that fixes that?

>
> 2) It unnecessarily redoes a number of usb requests making usb probing slower,
> and potentially upsetting some devices.

Does it actually upset anything?

The extra requests are in the second call to usb_select_config().

Do you think we could put the things we want to copy in a struct, and
copy just those?

>
> This commit fixes both issues by removing the usb_select_config() call from
> usb_child_pre_probe(), and instead of copying over things field by field
> through usb_device_platdata, store a pointer to the in stack usb_device
> (which is still valid when usb_child_pre_probe() gets called) and copy
> over the entire struct.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  drivers/usb/host/usb-uclass.c | 27 ++++++---------------------
>  include/usb.h                 |  5 +----
>  2 files changed, 7 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index 714bc0e..95e371d 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -535,12 +535,7 @@ int usb_scan_device(struct udevice *parent, int port,
>         }
>         plat = dev_get_parent_platdata(dev);
>         debug("%s: Probing '%s', plat=%p\n", __func__, dev->name, plat);
> -       plat->devnum = udev->devnum;
> -       plat->speed = udev->speed;
> -       plat->slot_id = udev->slot_id;
> -       plat->portnr = port;
> -       debug("** device '%s': stashing slot_id=%d\n", dev->name,
> -             plat->slot_id);
> +       plat->udev = udev;
>         priv->next_addr++;
>         ret = device_probe(dev);
>         if (ret) {
> @@ -599,25 +594,15 @@ int usb_get_bus(struct udevice *dev, struct udevice **busp)
>
>  int usb_child_pre_probe(struct udevice *dev)
>  {
> -       struct udevice *bus;
>         struct usb_device *udev = dev_get_parentdata(dev);
>         struct usb_dev_platdata *plat = dev_get_parent_platdata(dev);
> -       int ret;
>
> -       ret = usb_get_bus(dev, &bus);
> -       if (ret)
> -               return ret;
> -       udev->controller_dev = bus;
> +       /*
> +        * Copy over all the values set in the on stack struct usb_device in
> +        * usb_scan_device() to our final struct usb_device for this dev.
> +        */
> +       *udev = *(plat->udev);
>         udev->dev = dev;
> -       udev->devnum = plat->devnum;
> -       udev->slot_id = plat->slot_id;
> -       udev->portnr = plat->portnr;
> -       udev->speed = plat->speed;
> -       debug("** device '%s': getting slot_id=%d\n", dev->name, plat->slot_id);
> -
> -       ret = usb_select_config(udev);
> -       if (ret)
> -               return ret;
>
>         return 0;
>  }
> diff --git a/include/usb.h b/include/usb.h
> index 1984e8f..1b667ff 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -581,10 +581,7 @@ struct usb_platdata {
>   */
>  struct usb_dev_platdata {
>         struct usb_device_id id;
> -       enum usb_device_speed speed;
> -       int devnum;
> -       int slot_id;
> -       int portnr;     /* Hub port number, 1..n */
> +       struct usb_device *udev;
>  #ifdef CONFIG_SANDBOX
>         struct usb_string *strings;
>         /* NULL-terminated list of descriptor pointers */
> --
> 2.3.6
>

Regards,
Simon