| Message ID | 1430819679-1687-3-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
On 5 May 2015 at 03:54, Hans de Goede <hdegoede@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 3 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. > > 2) It unnecessarily redoes a number of usb requests making usb probing slower > > 3) Calling usb_select_config() a second time fails on some usb-1 devices > plugged into usb-2 hubs, causing u-boot to not recognize these devices. > > This commit fixes these 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. > > *) Except for devices which are explictly instantiated through device-tree > rather then discovered through usb_scan_device() such as emulated usb devices > in the sandbox. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v3: > -Add a big fat comment to warn that plat->udev is for usb-uclass.c internal > use only > Changes in v4: > -Fix sandbox emul usb device breakage > --- > drivers/usb/host/usb-uclass.c | 43 ++++++++++++++++++++++++++++--------------- > include/usb.h | 17 ++++++++++------- > 2 files changed, 38 insertions(+), 22 deletions(-) Acked-by: Simon Glass <sjg@chromium.org>
On 5 May 2015 at 09:31, Simon Glass <sjg@chromium.org> wrote: > On 5 May 2015 at 03:54, Hans de Goede <hdegoede@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 3 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. >> >> 2) It unnecessarily redoes a number of usb requests making usb probing slower >> >> 3) Calling usb_select_config() a second time fails on some usb-1 devices >> plugged into usb-2 hubs, causing u-boot to not recognize these devices. >> >> This commit fixes these 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. >> >> *) Except for devices which are explictly instantiated through device-tree >> rather then discovered through usb_scan_device() such as emulated usb devices >> in the sandbox. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v3: >> -Add a big fat comment to warn that plat->udev is for usb-uclass.c internal >> use only >> Changes in v4: >> -Fix sandbox emul usb device breakage >> --- >> drivers/usb/host/usb-uclass.c | 43 ++++++++++++++++++++++++++++--------------- >> include/usb.h | 17 ++++++++++------- >> 2 files changed, 38 insertions(+), 22 deletions(-) > > Acked-by: Simon Glass <sjg@chromium.org> Applied to u-boot-dm, thanks!
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index ba7d32a..c5ece58 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -533,11 +533,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) { @@ -597,17 +593,34 @@ int usb_child_pre_probe(struct udevice *dev) struct usb_dev_platdata *plat = dev_get_parent_platdata(dev); int ret; - udev->controller_dev = usb_get_bus(dev); - 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); + if (plat->udev) { + /* + * 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); + /* And clear plat->udev as it will not be valid for long */ + plat->udev = NULL; + udev->dev = dev; + } else { + /* + * This happens with devices which are explicitly bound + * instead of being discovered through usb_scan_device() + * such as sandbox emul devices. + */ + udev->dev = dev; + udev->controller_dev = usb_get_bus(dev); + udev->devnum = plat->devnum; - ret = usb_select_config(udev); - if (ret) - return ret; + /* + * udev did not go through usb_scan_device(), so we need to + * select the config and read the config descriptors. + */ + ret = usb_select_config(udev); + if (ret) + return ret; + } return 0; } diff --git a/include/usb.h b/include/usb.h index 6207d36..4c21050 100644 --- a/include/usb.h +++ b/include/usb.h @@ -571,20 +571,23 @@ struct usb_platdata { * This is used by sandbox to provide emulation data also. * * @id: ID used to match this device - * @speed: Stores the speed associated with a USB device * @devnum: Device address on the USB bus - * @slot_id: USB3 slot ID, which is separate from the device address - * @portnr: Port number of this device on its parent hub, numbered from 1 - * (0 mean this device is the root hub) + * @udev: usb-uclass internal use only do NOT use * @strings: List of descriptor strings (for sandbox emulation purposes) * @desc_list: List of descriptors (for sandbox emulation purposes) */ 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 */ + /* + * This pointer is used to pass the usb_device used in usb_scan_device, + * to get the usb descriptors before the driver is known, to the + * actual udevice once the driver is known and the udevice is created. + * This will be NULL except during probe, do NOT use. + * + * This should eventually go away. + */ + struct usb_device *udev; #ifdef CONFIG_SANDBOX struct usb_string *strings; /* NULL-terminated list of descriptor pointers */
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 3 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. 2) It unnecessarily redoes a number of usb requests making usb probing slower 3) Calling usb_select_config() a second time fails on some usb-1 devices plugged into usb-2 hubs, causing u-boot to not recognize these devices. This commit fixes these 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. *) Except for devices which are explictly instantiated through device-tree rather then discovered through usb_scan_device() such as emulated usb devices in the sandbox. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v3: -Add a big fat comment to warn that plat->udev is for usb-uclass.c internal use only Changes in v4: -Fix sandbox emul usb device breakage --- drivers/usb/host/usb-uclass.c | 43 ++++++++++++++++++++++++++++--------------- include/usb.h | 17 ++++++++++------- 2 files changed, 38 insertions(+), 22 deletions(-)