| Message ID | 1430819679-1687-7-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: > 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(-) Applied to u-boot-dm, thanks!
Hi Hans, On 5 May 2015 at 11:42, Simon Glass <sjg@chromium.org> wrote: > On 5 May 2015 at 03:54, Hans de Goede <hdegoede@redhat.com> wrote: >> 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(-) > > Applied to u-boot-dm, thanks! Again I am getting ahead of myself. This one fails on some Samsung boards: 35: dm: usb: Add support for interrupt queues to the dm usb code arm: + arndale odroid snow odroid-xu3 +drivers/usb/host/ehci-hcd.o: In function `create_int_queue': +build/../drivers/usb/host/ehci-hcd.c:1258: multiple definition of `create_int_queue' +drivers/usb/host/usb-uclass.o:build/../drivers/usb/host/usb-uclass.c:71: first defined here +drivers/usb/host/ehci-hcd.o: In function `poll_int_queue': +build/../drivers/usb/host/ehci-hcd.c:1414: multiple definition of `poll_int_queue' +drivers/usb/host/usb-uclass.o:build/../drivers/usb/host/usb-uclass.c:83: first defined here +drivers/usb/host/ehci-hcd.o: In function `destroy_int_queue': +build/../drivers/usb/host/ehci-hcd.c:1450: multiple definition of `destroy_int_queue' +drivers/usb/host/usb-uclass.o:build/../drivers/usb/host/usb-uclass.c:94: first defined here +make[2]: *** [drivers/usb/host/built-in.o] Error 1 +make[1]: *** [drivers/usb/host] Error 2 I'll drop this patch until you respin it. Regards, Simon
Hi, On 06-05-15 16:41, Simon Glass wrote: > Hi Hans, > > On 5 May 2015 at 11:42, Simon Glass <sjg@chromium.org> wrote: >> On 5 May 2015 at 03:54, Hans de Goede <hdegoede@redhat.com> wrote: >>> 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(-) >> >> Applied to u-boot-dm, thanks! > > Again I am getting ahead of myself. This one fails on some Samsung boards: > > 35: dm: usb: Add support for interrupt queues to the dm usb code > arm: + arndale odroid snow odroid-xu3 > +drivers/usb/host/ehci-hcd.o: In function `create_int_queue': > +build/../drivers/usb/host/ehci-hcd.c:1258: multiple definition of > `create_int_queue' > +drivers/usb/host/usb-uclass.o:build/../drivers/usb/host/usb-uclass.c:71: > first defined here > +drivers/usb/host/ehci-hcd.o: In function `poll_int_queue': > +build/../drivers/usb/host/ehci-hcd.c:1414: multiple definition of > `poll_int_queue' > +drivers/usb/host/usb-uclass.o:build/../drivers/usb/host/usb-uclass.c:83: > first defined here > +drivers/usb/host/ehci-hcd.o: In function `destroy_int_queue': > +build/../drivers/usb/host/ehci-hcd.c:1450: multiple definition of > `destroy_int_queue' > +drivers/usb/host/usb-uclass.o:build/../drivers/usb/host/usb-uclass.c:94: > first defined here > +make[2]: *** [drivers/usb/host/built-in.o] Error 1 > +make[1]: *** [drivers/usb/host] Error 2 > > I'll drop this patch until you respin it. Weird, in my tree, after this patch, the definition of create_int_queue in ehci-hcd.c is in a "#ifndef CONFIG_DM_USB" block. So I think this maybe an issue elsewhere. Maybe related to "[PATCH v2 79/80] dm: usb: exynos: Drop legacy USB code" which judging from your mail timestamps you did not have applied yet when seeing this build failure. Anyways I see that you've already merged quite a bit of my patches, so I'll rebase on top of u-boot-dm/master, fix all the review comments and I'll also look into this one. Regards, Hans
Hi, On 06-05-15 16:41, Simon Glass wrote: > Hi Hans, > > On 5 May 2015 at 11:42, Simon Glass <sjg@chromium.org> wrote: >> On 5 May 2015 at 03:54, Hans de Goede <hdegoede@redhat.com> wrote: >>> 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(-) >> >> Applied to u-boot-dm, thanks! > > Again I am getting ahead of myself. This one fails on some Samsung boards: > > 35: dm: usb: Add support for interrupt queues to the dm usb code > arm: + arndale odroid snow odroid-xu3 > +drivers/usb/host/ehci-hcd.o: In function `create_int_queue': > +build/../drivers/usb/host/ehci-hcd.c:1258: multiple definition of > `create_int_queue' > +drivers/usb/host/usb-uclass.o:build/../drivers/usb/host/usb-uclass.c:71: > first defined here > +drivers/usb/host/ehci-hcd.o: In function `poll_int_queue': > +build/../drivers/usb/host/ehci-hcd.c:1414: multiple definition of > `poll_int_queue' > +drivers/usb/host/usb-uclass.o:build/../drivers/usb/host/usb-uclass.c:83: > first defined here > +drivers/usb/host/ehci-hcd.o: In function `destroy_int_queue': > +build/../drivers/usb/host/ehci-hcd.c:1450: multiple definition of > `destroy_int_queue' > +drivers/usb/host/usb-uclass.o:build/../drivers/usb/host/usb-uclass.c:94: > first defined here > +make[2]: *** [drivers/usb/host/built-in.o] Error 1 > +make[1]: *** [drivers/usb/host] Error 2 > > I'll drop this patch until you respin it. Thinking more about this, now I understand what is going on, this build failure goes away with the patch titled: "dm: usb: Add support for interrupt queues to the dm ehci code" I understand that having a build failure halfway through the series is not acceptable, so I'll squash the 2 together. Regards Hans
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index c5ece58..9ee25ed 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 4c21050..7b55844 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); @@ -660,6 +660,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) *