Hi Hans,
On 30 April 2015 at 08:35, 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 2 problems with this approach:
- 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?
- 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@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