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.

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

Make usb_get_bus easier to use for callers, by directly returning the bus rather then returning it via a pass-by-ref argument.

This also removes the error checking from the single current caller, as we alreayd have an assert() for bus not being NULL in usb_get_bus().

Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/usb-uclass.c | 11 +++-------- include/usb.h | 7 +++---- 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 95e371d..9f690dc 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -477,9 +477,7 @@ int usb_scan_device(struct udevice *parent, int port,

*devp = NULL; memset(udev, '\0', sizeof(*udev)); - ret = usb_get_bus(parent, &udev->controller_dev); - if (ret) - return ret; + udev->controller_dev = usb_get_bus(parent); priv = dev_get_uclass_priv(udev->controller_dev);

/* @@ -574,22 +572,19 @@ int usb_child_post_bind(struct udevice *dev) return 0; }

-int usb_get_bus(struct udevice *dev, struct udevice **busp) +struct udevice *usb_get_bus(struct udevice *dev) { struct udevice *bus;

- *busp = NULL; for (bus = dev; bus && device_get_uclass_id(bus) != UCLASS_USB; ) bus = bus->parent; if (!bus) { /* By design this cannot happen */ assert(bus); debug("USB HUB '%s' does not have a controller\n", dev->name); - return -EXDEV; } - *busp = bus;

- return 0; + return bus; }

int usb_child_pre_probe(struct udevice *dev) diff --git a/include/usb.h b/include/usb.h index 1b667ff..d8bde1d 100644 --- a/include/usb.h +++ b/include/usb.h @@ -739,11 +739,10 @@ int usb_scan_device(struct udevice *parent, int port, * will be a device with uclass UCLASS_USB. * * @dev: Device to check - * @busp: Returns bus, or NULL if not found - * @return 0 if OK, -EXDEV is somehow this bus does not have a controller (this - * indicates a critical error in the USB stack + * @return The bus, or NULL if not found (this indicates a critical error in + * the USB stack */ -int usb_get_bus(struct udevice *dev, struct udevice **busp); +struct udevice *usb_get_bus(struct udevice *dev);

/** * usb_select_config() - Set up a device ready for use

Use usb_get_bus in dm ehci code rather then re-implementing it.

Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/ehci-hcd.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index bd9861d..85adbf4 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -125,14 +125,7 @@ static struct descriptor { static struct ehci_ctrl *ehci_get_ctrl(struct usb_device *udev) { #ifdef CONFIG_DM_USB - struct udevice *dev; - - /* Find the USB controller */ - for (dev = udev->dev; - device_get_uclass_id(dev) != UCLASS_USB; - dev = dev->parent) - ; - return dev_get_priv(dev); + return dev_get_priv(usb_get_bus(udev->dev)); #else return udev->controller; #endif

Interrupt endpoints typically are polled for a long time by the usb controller before they return anything, so calls to submit_int_msg() can take a long time to complete this.

To avoid this the u-boot code has the an interrupt queue mechanism / API, add support for this to the driver-model usb code.

See the added doc comments for more details.

Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Simon Glass sjg@chromium.org --- drivers/usb/host/usb-uclass.c | 36 ++++++++++++++++++++++++++++++++ include/usb.h | 48 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 9f690dc..a3c3080 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -65,6 +65,42 @@ int submit_bulk_msg(struct usb_device *udev, unsigned long pipe, void *buffer, return ops->bulk(bus, udev, pipe, buffer, length); }

+struct int_queue *create_int_queue(struct usb_device *udev, + unsigned long pipe, int queuesize, int elementsize, + void *buffer, int interval) +{ + struct udevice *bus = udev->controller_dev; + struct dm_usb_ops *ops = usb_get_ops(bus); + + if (!ops->create_int_queue) + return NULL; + + return ops->create_int_queue(bus, udev, pipe, queuesize, elementsize, + buffer, interval); +} + +void *poll_int_queue(struct usb_device *udev, struct int_queue *queue) +{ + struct udevice *bus = udev->controller_dev; + struct dm_usb_ops *ops = usb_get_ops(bus); + + if (!ops->poll_int_queue) + return NULL; + + return ops->poll_int_queue(bus, udev, queue); +} + +int destroy_int_queue(struct usb_device *udev, struct int_queue *queue) +{ + struct udevice *bus = udev->controller_dev; + struct dm_usb_ops *ops = usb_get_ops(bus); + + if (!ops->destroy_int_queue) + return -ENOSYS; + + return ops->destroy_int_queue(bus, udev, queue); +} + int usb_alloc_device(struct usb_device *udev) { struct udevice *bus = udev->controller_dev; diff --git a/include/usb.h b/include/usb.h index d8bde1d..92c5bbd 100644 --- a/include/usb.h +++ b/include/usb.h @@ -198,7 +198,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval);

-#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST +#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST || defined(CONFIG_DM_USB) struct int_queue *create_int_queue(struct usb_device *dev, unsigned long pipe, int queuesize, int elementsize, void *buffer, int interval); int destroy_int_queue(struct usb_device *dev, struct int_queue *queue); @@ -654,6 +654,52 @@ struct dm_usb_ops { int (*interrupt)(struct udevice *bus, struct usb_device *udev, unsigned long pipe, void *buffer, int length, int interval); + + /** + * create_int_queue() - Create and queue interrupt packets + * + * Create and queue @queuesize number of interrupt usb packets of + * @elementsize bytes each. @buffer must be atleast @queuesize * + * @elementsize bytes. + * + * Note some controllers only support a queuesize of 1. + * + * @interval: Interrupt interval + * + * @return A pointer to the created interrupt queue or NULL on error + */ + struct int_queue *(*create_int_queue)(struct udevice *bus, + struct usb_device *udev, unsigned long pipe, + int queuesize, int elementsize, void *buffer, + int interval); + + /** + * poll_int_queue() - Poll an interrupt queue for completed packets + * + * Poll an interrupt queue for completed packets. The return value + * points to the part of the buffer passed to create_int_queue() + * corresponding to the completed packet. + * + * @queue: queue to poll + * + * @return Pointer to the data of the first completed packet, or + * NULL if no packets are ready + */ + void *(*poll_int_queue)(struct udevice *bus, struct usb_device *udev, + struct int_queue *queue); + + /** + * destroy_int_queue() - Destroy an interrupt queue + * + * Destroy an interrupt queue created by create_int_queue(). + * + * @queue: queue to poll + * + * @return 0 if OK, -ve on error + */ + int (*destroy_int_queue)(struct udevice *bus, struct usb_device *udev, + struct int_queue *queue); + /** * alloc_device() - Allocate a new device context (XHCI) *

Add support for interrupt queues to the dm ehci code.

Signed-off-by: Hans de Goede hdegoede@redhat.com Acked-by: Simon Glass sjg@chromium.org --- drivers/usb/host/ehci-hcd.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 363ce38..0e84265 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1594,6 +1594,29 @@ static int ehci_submit_int_msg(struct udevice *dev, struct usb_device *udev, return _ehci_submit_int_msg(udev, pipe, buffer, length, interval); }

+static struct int_queue *ehci_create_int_queue(struct udevice *dev, + struct usb_device *udev, unsigned long pipe, int queuesize, + int elementsize, void *buffer, int interval) +{ + debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev); + return _ehci_create_int_queue(udev, pipe, queuesize, elementsize, + buffer, interval); +} + +static void *ehci_poll_int_queue(struct udevice *dev, struct usb_device *udev, + struct int_queue *queue) +{ + debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev); + return _ehci_poll_int_queue(udev, queue); +} + +static int ehci_destroy_int_queue(struct udevice *dev, struct usb_device *udev, + struct int_queue *queue) +{ + debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev); + return _ehci_destroy_int_queue(udev, queue); +} + int ehci_register(struct udevice *dev, struct ehci_hccr *hccr, struct ehci_hcor *hcor, const struct ehci_ops *ops, uint tweaks, enum usb_init_type init) @@ -1642,6 +1665,9 @@ struct dm_usb_ops ehci_usb_ops = { .control = ehci_submit_control_msg, .bulk = ehci_submit_bulk_msg, .interrupt = ehci_submit_int_msg, + .create_int_queue = ehci_create_int_queue, + .poll_int_queue = ehci_poll_int_queue, + .destroy_int_queue = ehci_destroy_int_queue, };

#endif