[U-Boot] [PATCH v3 1/7] usb_kdb: only process events succesfully received
Causes unbound key repeat on error otherwise.
Signed-off-by: Michal Suchanek msuchanek@suse.de --- v2: fix indentation --- common/usb_kbd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index cc99c6be0720..fc9419e0238a 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,10 +339,9 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */ - usb_submit_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize, - data->intinterval); - - usb_kbd_irq_worker(dev); + if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0], + data->intpktsize, data->intinterval)) + usb_kbd_irq_worker(dev); #elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP) || \ defined(CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE) #if defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP)
This aligns naming with usb_bulk_msg and usb_control_msg.
Signed-off-by: Michal Suchanek msuchanek@suse.de --- v2: new patch --- common/usb.c | 2 +- common/usb_kbd.c | 6 +++--- include/usb.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/usb.c b/common/usb.c index b70f614d244f..704937dec8a8 100644 --- a/common/usb.c +++ b/common/usb.c @@ -194,7 +194,7 @@ int usb_disable_asynch(int disable) /* * submits an Interrupt Message */ -int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, +int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval) { return submit_int_msg(dev, pipe, buffer, transfer_len, interval); diff --git a/common/usb_kbd.c b/common/usb_kbd.c index fc9419e0238a..4c23023d4e59 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,7 +339,7 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */ - if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0], + if (!usb_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize, data->intinterval)) usb_kbd_irq_worker(dev); #elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP) || \ @@ -503,8 +503,8 @@ static int usb_kbd_probe_dev(struct usb_device *dev, unsigned int ifnum) if (usb_get_report(dev, iface->desc.bInterfaceNumber, 1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) { #else - if (usb_submit_int_msg(dev, data->intpipe, data->new, data->intpktsize, - data->intinterval) < 0) { + if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize, + data->intinterval) < 0) { #endif printf("Failed to get keyboard state from device %04x:%04x\n", dev->descriptor.idVendor, dev->descriptor.idProduct); diff --git a/include/usb.h b/include/usb.h index 420a30e49fa1..8cd73863b876 100644 --- a/include/usb.h +++ b/include/usb.h @@ -261,7 +261,7 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, void *data, unsigned short size, int timeout); int usb_bulk_msg(struct usb_device *dev, unsigned int pipe, void *data, int len, int *actual_length, int timeout); -int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, +int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval); int usb_disable_asynch(int disable); int usb_maxpacket(struct usb_device *dev, unsigned long pipe);
On Fri, Jul 5, 2019 at 6:44 PM Michal Suchanek msuchanek@suse.de wrote:
This aligns naming with usb_bulk_msg and usb_control_msg.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: new patch
common/usb.c | 2 +- common/usb_kbd.c | 6 +++--- include/usb.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/usb.c b/common/usb.c index b70f614d244f..704937dec8a8 100644 --- a/common/usb.c +++ b/common/usb.c @@ -194,7 +194,7 @@ int usb_disable_asynch(int disable) /*
- submits an Interrupt Message
*/ -int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, +int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval)
nits: indentation
{ return submit_int_msg(dev, pipe, buffer, transfer_len, interval); diff --git a/common/usb_kbd.c b/common/usb_kbd.c index fc9419e0238a..4c23023d4e59 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,7 +339,7 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */
if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0],
if (!usb_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize, data->intinterval))
ditto
usb_kbd_irq_worker(dev);#elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP) || \ @@ -503,8 +503,8 @@ static int usb_kbd_probe_dev(struct usb_device *dev, unsigned int ifnum) if (usb_get_report(dev, iface->desc.bInterfaceNumber, 1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) { #else
if (usb_submit_int_msg(dev, data->intpipe, data->new, data->intpktsize,data->intinterval) < 0) {
if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,data->intinterval) < 0) {#endif printf("Failed to get keyboard state from device %04x:%04x\n", dev->descriptor.idVendor, dev->descriptor.idProduct); diff --git a/include/usb.h b/include/usb.h index 420a30e49fa1..8cd73863b876 100644 --- a/include/usb.h +++ b/include/usb.h @@ -261,7 +261,7 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, void *data, unsigned short size, int timeout); int usb_bulk_msg(struct usb_device *dev, unsigned int pipe, void *data, int len, int *actual_length, int timeout); -int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, +int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval);
ditto
int usb_disable_asynch(int disable); int usb_maxpacket(struct usb_device *dev, unsigned long pipe); --
Regards, Bin
On Fri, 5 Jul 2019 21:51:43 +0800 Bin Meng bmeng.cn@gmail.com wrote:
On Fri, Jul 5, 2019 at 6:44 PM Michal Suchanek msuchanek@suse.de wrote:
This aligns naming with usb_bulk_msg and usb_control_msg.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: new patch
common/usb.c | 2 +- common/usb_kbd.c | 6 +++--- include/usb.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/usb.c b/common/usb.c index b70f614d244f..704937dec8a8 100644 --- a/common/usb.c +++ b/common/usb.c @@ -194,7 +194,7 @@ int usb_disable_asynch(int disable) /*
- submits an Interrupt Message
*/ -int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, +int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval)
nits: indentation
{ return submit_int_msg(dev, pipe, buffer, transfer_len, interval); diff --git a/common/usb_kbd.c b/common/usb_kbd.c index fc9419e0238a..4c23023d4e59 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,7 +339,7 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */
if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0],
if (!usb_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize, data->intinterval))ditto
This is the correct indentation
usb_kbd_irq_worker(dev);#elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP) || \ @@ -503,8 +503,8 @@ static int usb_kbd_probe_dev(struct usb_device *dev, unsigned int ifnum) if (usb_get_report(dev, iface->desc.bInterfaceNumber, 1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) { #else
if (usb_submit_int_msg(dev, data->intpipe, data->new, data->intpktsize,data->intinterval) < 0) {
if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,data->intinterval) < 0) {#endif printf("Failed to get keyboard state from device %04x:%04x\n", dev->descriptor.idVendor, dev->descriptor.idProduct); diff --git a/include/usb.h b/include/usb.h index 420a30e49fa1..8cd73863b876 100644 --- a/include/usb.h +++ b/include/usb.h @@ -261,7 +261,7 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, void *data, unsigned short size, int timeout); int usb_bulk_msg(struct usb_device *dev, unsigned int pipe, void *data, int len, int *actual_length, int timeout); -int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, +int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval);
ditto
Here the indentation matches the surrounding declarations.
Thanks
Michal
Use the wrapper because the unwrapped function prototype will be changed in the following patch.
Signed-off-by: Michal Suchanek msuchanek@suse.de --- v2: usb_submit_int_msg -> usb_int_msg v3: fix indentation --- common/usb_storage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index 8c889bb1a648..995a96baa57d 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -650,8 +650,8 @@ static int usb_stor_CBI_get_status(struct scsi_cmd *srb, struct us_data *us) int timeout;
us->ip_wanted = 1; - submit_int_msg(us->pusb_dev, us->irqpipe, - (void *) &us->ip_data, us->irqmaxp, us->irqinterval); + usb_int_msg(us->pusb_dev, us->irqpipe, + (void *) &us->ip_data, us->irqmaxp, us->irqinterval); timeout = 1000; while (timeout--) { if (us->ip_wanted == 0)
On Fri, Jul 5, 2019 at 6:44 PM Michal Suchanek msuchanek@suse.de wrote:
Use the wrapper because the unwrapped function prototype will be changed in the following patch.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: usb_submit_int_msg -> usb_int_msg v3: fix indentation
common/usb_storage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
This will be used to implement non-blocking keyboard polling in case of errors.
Signed-off-by: Michal Suchanek msuchanek@suse.de --- v2: ad missing hunk from last patch --- common/usb.c | 2 +- drivers/usb/host/dwc2.c | 12 +++++++----- drivers/usb/host/ehci-hcd.c | 13 ++++++++----- drivers/usb/host/ohci-hcd.c | 4 ++-- drivers/usb/host/r8a66597-hcd.c | 2 +- drivers/usb/host/sl811-hcd.c | 2 +- drivers/usb/host/usb-uclass.c | 4 ++-- drivers/usb/host/xhci.c | 13 ++++++++----- drivers/usb/musb-new/musb_uboot.c | 12 +++++++----- include/usb.h | 4 ++-- 10 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 704937dec8a8..f57c0e8cdf57 100644 --- a/common/usb.c +++ b/common/usb.c @@ -197,7 +197,7 @@ int usb_disable_asynch(int disable) int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval) { - return submit_int_msg(dev, pipe, buffer, transfer_len, interval); + return submit_int_msg(dev, pipe, buffer, transfer_len, interval, false); }
/* diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index a62a2f8a951d..b4121a49b805 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -1108,7 +1108,8 @@ static int _submit_control_msg(struct dwc2_priv *priv, struct usb_device *dev, }
int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, - unsigned long pipe, void *buffer, int len, int interval) + unsigned long pipe, void *buffer, int len, int interval, + bool nonblock) { unsigned long timeout; int ret; @@ -1236,9 +1237,10 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, }
int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, - int len, int interval) + int len, int interval, bool nonblock) { - return _submit_int_msg(&local, dev, pipe, buffer, len, interval); + return _submit_int_msg(&local, dev, pipe, buffer, len, interval, + nonblock); }
/* U-Boot USB control interface */ @@ -1292,13 +1294,13 @@ static int dwc2_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
static int dwc2_submit_int_msg(struct udevice *dev, struct usb_device *udev, unsigned long pipe, void *buffer, int length, - int interval) + int interval, bool nonblock) { struct dwc2_priv *priv = dev_get_priv(dev);
debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev);
- return _submit_int_msg(priv, udev, pipe, buffer, length, interval); + return _submit_int_msg(priv, udev, pipe, buffer, length, interval, nonblock); }
static int dwc2_usb_ofdata_to_platdata(struct udevice *dev) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 4b28db70a566..61a61abb2112 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1482,7 +1482,8 @@ out: }
static int _ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe, - void *buffer, int length, int interval) + void *buffer, int length, int interval, + bool nonblock) { void *backbuffer; struct int_queue *queue; @@ -1532,9 +1533,10 @@ 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 length, int interval) + void *buffer, int length, int interval, bool nonblock) { - return _ehci_submit_int_msg(dev, pipe, buffer, length, interval); + return _ehci_submit_int_msg(dev, pipe, buffer, length, interval, + nonblock); }
struct int_queue *create_int_queue(struct usb_device *dev, @@ -1576,10 +1578,11 @@ static int ehci_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
static int ehci_submit_int_msg(struct udevice *dev, struct usb_device *udev, unsigned long pipe, void *buffer, int length, - int interval) + int interval, bool nonblock) { debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev); - return _ehci_submit_int_msg(udev, pipe, buffer, length, interval); + return _ehci_submit_int_msg(udev, pipe, buffer, length, interval, + nonblock); }
static struct int_queue *ehci_create_int_queue(struct udevice *dev, diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 2b0df88f49ec..f1cc6547fbcb 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1700,7 +1700,7 @@ int submit_bulk_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) + int transfer_len, int interval, bool nonblock) { info("submit_int_msg"); return submit_common_msg(&gohci, dev, pipe, buffer, transfer_len, NULL, @@ -2149,7 +2149,7 @@ static int ohci_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
static int ohci_submit_int_msg(struct udevice *dev, struct usb_device *udev, unsigned long pipe, void *buffer, int length, - int interval) + int interval, bool nonblock) { ohci_t *ohci = dev_get_priv(usb_get_bus(dev));
diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c index 3c263e51c160..83056feeae22 100644 --- a/drivers/usb/host/r8a66597-hcd.c +++ b/drivers/usb/host/r8a66597-hcd.c @@ -822,7 +822,7 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, }
int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, - int transfer_len, int interval) + int transfer_len, int interval, bool nonblock) { /* no implement */ R8A66597_DPRINT("%s\n", __func__); diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c index daba0dcd1aee..e08da6130bd5 100644 --- a/drivers/usb/host/sl811-hcd.c +++ b/drivers/usb/host/sl811-hcd.c @@ -384,7 +384,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 len, int interval) + int len, int interval, bool nonblock) { PDEBUG(0, "dev = %p pipe = %#lx buf = %p size = %d int = %d\n", dev, pipe, buffer, len, interval); diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 6e118b5a8ffa..35934fab0e45 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -31,7 +31,7 @@ int usb_disable_asynch(int disable) }
int submit_int_msg(struct usb_device *udev, unsigned long pipe, void *buffer, - int length, int interval) + int length, int interval, bool nonblock) { struct udevice *bus = udev->controller_dev; struct dm_usb_ops *ops = usb_get_ops(bus); @@ -39,7 +39,7 @@ int submit_int_msg(struct usb_device *udev, unsigned long pipe, void *buffer, if (!ops->interrupt) return -ENOSYS;
- return ops->interrupt(bus, udev, pipe, buffer, length, interval); + return ops->interrupt(bus, udev, pipe, buffer, length, interval, nonblock); }
int submit_control_msg(struct usb_device *udev, unsigned long pipe, diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 44c5f2d264c1..b3e4dcd66fa1 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1109,7 +1109,8 @@ unknown: * @return 0 */ static int _xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe, - void *buffer, int length, int interval) + void *buffer, int length, int interval, + bool nonblock) { if (usb_pipetype(pipe) != PIPE_INTERRUPT) { printf("non-interrupt pipe (type=%lu)", usb_pipetype(pipe)); @@ -1277,9 +1278,10 @@ int submit_bulk_msg(struct usb_device *udev, unsigned long pipe, void *buffer, }
int submit_int_msg(struct usb_device *udev, unsigned long pipe, void *buffer, - int length, int interval) + int length, int interval, bool nonblock) { - return _xhci_submit_int_msg(udev, pipe, buffer, length, interval); + return _xhci_submit_int_msg(udev, pipe, buffer, length, interval, + nonblock); }
/** @@ -1386,10 +1388,11 @@ static int xhci_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
static int xhci_submit_int_msg(struct udevice *dev, struct usb_device *udev, unsigned long pipe, void *buffer, int length, - int interval) + int interval, bool nonblock) { debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev); - return _xhci_submit_int_msg(udev, pipe, buffer, length, interval); + return _xhci_submit_int_msg(udev, pipe, buffer, length, interval, + nonblock); }
static int xhci_alloc_device(struct udevice *dev, struct usb_device *udev) diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c index 9c8cc6e58443..9eb593402ea0 100644 --- a/drivers/usb/musb-new/musb_uboot.c +++ b/drivers/usb/musb-new/musb_uboot.c @@ -110,7 +110,7 @@ static int _musb_submit_bulk_msg(struct musb_host_data *host,
static int _musb_submit_int_msg(struct musb_host_data *host, struct usb_device *dev, unsigned long pipe, - void *buffer, int len, int interval) + void *buffer, int len, int interval, bool nonblock) { construct_urb(&host->urb, &host->hep, dev, USB_ENDPOINT_XFER_INT, pipe, buffer, len, NULL, interval); @@ -268,9 +268,10 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, }
int submit_int_msg(struct usb_device *dev, unsigned long pipe, - void *buffer, int length, int interval) + void *buffer, int length, int interval, bool nonblock) { - return _musb_submit_int_msg(&musb_host, dev, pipe, buffer, length, interval); + return _musb_submit_int_msg(&musb_host, dev, pipe, buffer, length, + interval, nonblock); }
struct int_queue *create_int_queue(struct usb_device *dev, @@ -320,10 +321,11 @@ static int musb_submit_bulk_msg(struct udevice *dev, struct usb_device *udev,
static int musb_submit_int_msg(struct udevice *dev, struct usb_device *udev, unsigned long pipe, void *buffer, int length, - int interval) + int interval, bool nonblock) { struct musb_host_data *host = dev_get_priv(dev); - return _musb_submit_int_msg(host, udev, pipe, buffer, length, interval); + return _musb_submit_int_msg(host, udev, pipe, buffer, length, interval, + nonblock); }
static struct int_queue *musb_create_int_queue(struct udevice *dev, diff --git a/include/usb.h b/include/usb.h index 8cd73863b876..6ca2eb30d08a 100644 --- a/include/usb.h +++ b/include/usb.h @@ -184,7 +184,7 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, struct devrequest *setup); int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, - int transfer_len, int interval); + int transfer_len, int interval, bool nonblock);
#if defined CONFIG_USB_EHCI_HCD || defined CONFIG_USB_MUSB_HOST \ || CONFIG_IS_ENABLED(DM_USB) @@ -708,7 +708,7 @@ struct dm_usb_ops { */ int (*interrupt)(struct udevice *bus, struct usb_device *udev, unsigned long pipe, void *buffer, int length, - int interval); + int interval, bool nonblock);
/** * create_int_queue() - Create and queue interrupt packets
Variant of the int_msg wrapper that does not introduce excessive retry delay on error.
Signed-off-by: Michal Suchanek msuchanek@suse.de --- v2: usb_submit_int_msg -> usb_int_msg --- common/usb.c | 9 +++++++++ include/usb.h | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/common/usb.c b/common/usb.c index f57c0e8cdf57..1bd60b24a555 100644 --- a/common/usb.c +++ b/common/usb.c @@ -200,6 +200,15 @@ int usb_int_msg(struct usb_device *dev, unsigned long pipe, return submit_int_msg(dev, pipe, buffer, transfer_len, interval, false); }
+/* + * submits an Interrupt Message without retry + */ +int usb_int_msg_nonblock(struct usb_device *dev, unsigned long pipe, + void *buffer, int transfer_len, int interval) +{ + return submit_int_msg(dev, pipe, buffer, transfer_len, interval, true); +} + /* * submits a control message and waits for comletion (at least timeout * 1ms) * If timeout is 0, we don't wait for completion (used as example to set and diff --git a/include/usb.h b/include/usb.h index 6ca2eb30d08a..b0a079766652 100644 --- a/include/usb.h +++ b/include/usb.h @@ -263,6 +263,8 @@ int usb_bulk_msg(struct usb_device *dev, unsigned int pipe, void *data, int len, int *actual_length, int timeout); int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval); +int usb_int_msg_nonblock(struct usb_device *dev, unsigned long pipe, + void *buffer, int transfer_len, int interval); int usb_disable_asynch(int disable); int usb_maxpacket(struct usb_device *dev, unsigned long pipe); int usb_get_configuration_no(struct usb_device *dev, int cfgno,
On 7/5/19 12:44 PM, Michal Suchanek wrote:
Variant of the int_msg wrapper that does not introduce excessive retry delay on error.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: usb_submit_int_msg -> usb_int_msg
common/usb.c | 9 +++++++++ include/usb.h | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/common/usb.c b/common/usb.c index f57c0e8cdf57..1bd60b24a555 100644 --- a/common/usb.c +++ b/common/usb.c @@ -200,6 +200,15 @@ int usb_int_msg(struct usb_device *dev, unsigned long pipe, return submit_int_msg(dev, pipe, buffer, transfer_len, interval, false); }
+/*
- submits an Interrupt Message without retry
- */
+int usb_int_msg_nonblock(struct usb_device *dev, unsigned long pipe,
void *buffer, int transfer_len, int interval)+{
- return submit_int_msg(dev, pipe, buffer, transfer_len, interval, true);
+}
Wouldn't it be shorter to just call submit_int_msg() directly, with the extra parameter ?
On Fri, 5 Jul 2019 14:13:50 +0200 Marek Vasut marex@denx.de wrote:
On 7/5/19 12:44 PM, Michal Suchanek wrote:
Variant of the int_msg wrapper that does not introduce excessive retry delay on error.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: usb_submit_int_msg -> usb_int_msg
common/usb.c | 9 +++++++++ include/usb.h | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/common/usb.c b/common/usb.c index f57c0e8cdf57..1bd60b24a555 100644 --- a/common/usb.c +++ b/common/usb.c @@ -200,6 +200,15 @@ int usb_int_msg(struct usb_device *dev, unsigned long pipe, return submit_int_msg(dev, pipe, buffer, transfer_len, interval, false); }
+/*
- submits an Interrupt Message without retry
- */
+int usb_int_msg_nonblock(struct usb_device *dev, unsigned long pipe,
void *buffer, int transfer_len, int interval)+{
- return submit_int_msg(dev, pipe, buffer, transfer_len, interval, true);
+}
Wouldn't it be shorter to just call submit_int_msg() directly, with the extra parameter ?
It is easier to understand the change this way. The idea is that only some very specific callers need to know about the difference.
Thanks
Michal
On 7/10/19 5:52 PM, Michal Suchánek wrote:
On Fri, 5 Jul 2019 14:13:50 +0200 Marek Vasut marex@denx.de wrote:
On 7/5/19 12:44 PM, Michal Suchanek wrote:
Variant of the int_msg wrapper that does not introduce excessive retry delay on error.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: usb_submit_int_msg -> usb_int_msg
common/usb.c | 9 +++++++++ include/usb.h | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/common/usb.c b/common/usb.c index f57c0e8cdf57..1bd60b24a555 100644 --- a/common/usb.c +++ b/common/usb.c @@ -200,6 +200,15 @@ int usb_int_msg(struct usb_device *dev, unsigned long pipe, return submit_int_msg(dev, pipe, buffer, transfer_len, interval, false); }
+/*
- submits an Interrupt Message without retry
- */
+int usb_int_msg_nonblock(struct usb_device *dev, unsigned long pipe,
void *buffer, int transfer_len, int interval)+{
- return submit_int_msg(dev, pipe, buffer, transfer_len, interval, true);
+}
Wouldn't it be shorter to just call submit_int_msg() directly, with the extra parameter ?
It is easier to understand the change this way. The idea is that only some very specific callers need to know about the difference.
It's just another layer of meaningless obfuscation. Explain what the parameter does in a comment and be done with it.
With the following patch it avoids excessive delays with USB 1.1 keyboard connected to high-speed USB hub conncted to dwc2.
Signed-off-by: Michal Suchanek msuchanek@suse.de --- v2: usb_submit_int_msg -> usb_int_msg --- common/usb_kbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 4c23023d4e59..57a3cba496fe 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,7 +339,7 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */ - if (!usb_int_msg(dev, data->intpipe, &data->new[0], + if (!usb_int_msg_nonblock(dev, data->intpipe, &data->new[0], data->intpktsize, data->intinterval)) usb_kbd_irq_worker(dev); #elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP) || \
An USB 1.1 keyboard connected to dwc2 through a high-speed hub does not report status until it changes. With this patch you can enable keyboard by pressing a key while USB devices are probed. Without a keypress no state is reported and the probe times out. We don't want to wait for a keypress or timeout while polling for keypresses so implement an int_msg nonblock variant that exits early on error.
Signed-off-by: Michal Suchanek msuchanek@suse.de --- v2: move superfluous hunk to earlier patch --- drivers/usb/host/dwc2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index b4121a49b805..78829d56199c 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -1123,7 +1123,7 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, return -ETIMEDOUT; } ret = _submit_bulk_msg(priv, dev, pipe, buffer, len); - if (ret != -EAGAIN) + if ((ret != -EAGAIN) || nonblock) return ret; } }
On 7/5/19 12:44 PM, Michal Suchanek wrote:
Causes unbound key repeat on error otherwise.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: fix indentation
What changed in V3 ?
common/usb_kbd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index cc99c6be0720..fc9419e0238a 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,10 +339,9 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */
- usb_submit_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize,
data->intinterval);- usb_kbd_irq_worker(dev);
- if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0],
data->intpktsize, data->intinterval))
This still doesn't propagate errors.
On Fri, 5 Jul 2019 14:12:36 +0200 Marek Vasut marex@denx.de wrote:
On 7/5/19 12:44 PM, Michal Suchanek wrote:
Causes unbound key repeat on error otherwise.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: fix indentation
What changed in V3 ?
common/usb_kbd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index cc99c6be0720..fc9419e0238a 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,10 +339,9 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */
- usb_submit_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize,
data->intinterval);- usb_kbd_irq_worker(dev);
- if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0],
data->intpktsize, data->intinterval))This still doesn't propagate errors.
Yes, it does not. I don't have a grand design that would make use of these propagated errors so I don't know what errors to propagate.
You could, say, want to propagate errors from kbd_irq_worker as well.
Until there is a way to make use of the errors you really don't know.
Might be good idea to document that getting EAGAIN from usb_int_msg_nonblock is expected.
Thanks
Michal
On 7/10/19 5:47 PM, Michal Suchánek wrote:
On Fri, 5 Jul 2019 14:12:36 +0200 Marek Vasut marex@denx.de wrote:
On 7/5/19 12:44 PM, Michal Suchanek wrote:
Causes unbound key repeat on error otherwise.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: fix indentation
What changed in V3 ?
common/usb_kbd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index cc99c6be0720..fc9419e0238a 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,10 +339,9 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */
- usb_submit_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize,
data->intinterval);- usb_kbd_irq_worker(dev);
- if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0],
data->intpktsize, data->intinterval))This still doesn't propagate errors.
Yes, it does not. I don't have a grand design that would make use of these propagated errors so I don't know what errors to propagate.
Each and every called of usb_kbd_poll_for_event() returns some return value, so these functions should at least be aware of the new errors which might come from usb_kbd_poll_for_event().
You could, say, want to propagate errors from kbd_irq_worker as well.
Until there is a way to make use of the errors you really don't know.
Might be good idea to document that getting EAGAIN from usb_int_msg_nonblock is expected.
[...]
On Fri, 12 Jul 2019 06:05:45 +0200 Marek Vasut marex@denx.de wrote:
On 7/10/19 5:47 PM, Michal Suchánek wrote:
On Fri, 5 Jul 2019 14:12:36 +0200 Marek Vasut marex@denx.de wrote:
On 7/5/19 12:44 PM, Michal Suchanek wrote:
Causes unbound key repeat on error otherwise.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: fix indentation
What changed in V3 ?
common/usb_kbd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index cc99c6be0720..fc9419e0238a 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,10 +339,9 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */
- usb_submit_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize,
data->intinterval);- usb_kbd_irq_worker(dev);
- if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0],
data->intpktsize, data->intinterval))This still doesn't propagate errors.
Yes, it does not. I don't have a grand design that would make use of these propagated errors so I don't know what errors to propagate.
Each and every called of usb_kbd_poll_for_event() returns some return value,
A void
so these functions should at least be aware of the new errors which might come from usb_kbd_poll_for_event().
No errors come from the function. The u-boot input flow is designed for usb_kbd_poll_for_event handling errors internally.
The function is only used by the usb_kbd specific testc and getc. The input users are supposed to use testc to see if there is input available and getc to get input (and it should not block if testc indicated that there is input). The result of testc is treated as boolean so there is no room for error handling. The result of getc is treated as character input and I found no user that implements any kind of error handling. Also at the getc time the testc error is no longer available.
If you want to redesign it to handle errors in the caller then please do share the design. Without that it is not clear which errors make sense to propagate.
Thanks
Michal
On 7/12/19 4:24 PM, Michal Suchánek wrote:
On Fri, 12 Jul 2019 06:05:45 +0200 Marek Vasut marex@denx.de wrote:
On 7/10/19 5:47 PM, Michal Suchánek wrote:
On Fri, 5 Jul 2019 14:12:36 +0200 Marek Vasut marex@denx.de wrote:
On 7/5/19 12:44 PM, Michal Suchanek wrote:
Causes unbound key repeat on error otherwise.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: fix indentation
What changed in V3 ?
common/usb_kbd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index cc99c6be0720..fc9419e0238a 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,10 +339,9 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */
- usb_submit_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize,
data->intinterval);- usb_kbd_irq_worker(dev);
- if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0],
data->intpktsize, data->intinterval))This still doesn't propagate errors.
Yes, it does not. I don't have a grand design that would make use of these propagated errors so I don't know what errors to propagate.
Each and every called of usb_kbd_poll_for_event() returns some return value,
A void
so these functions should at least be aware of the new errors which might come from usb_kbd_poll_for_event().
No errors come from the function. The u-boot input flow is designed for usb_kbd_poll_for_event handling errors internally.
The function is only used by the usb_kbd specific testc and getc. The input users are supposed to use testc to see if there is input available and getc to get input (and it should not block if testc indicated that there is input). The result of testc is treated as boolean so there is no room for error handling. The result of getc is treated as character input and I found no user that implements any kind of error handling. Also at the getc time the testc error is no longer available.
If you want to redesign it to handle errors in the caller then please do share the design. Without that it is not clear which errors make sense to propagate.
Presumably, usb_kbd_tstc() should return 0 if usb_kbd_poll_for_event() returns an error code or somesuch ?
On Mon, 15 Jul 2019 06:28:01 +0200 Marek Vasut marex@denx.de wrote:
On 7/12/19 4:24 PM, Michal Suchánek wrote:
On Fri, 12 Jul 2019 06:05:45 +0200 Marek Vasut marex@denx.de wrote:
On 7/10/19 5:47 PM, Michal Suchánek wrote:
On Fri, 5 Jul 2019 14:12:36 +0200 Marek Vasut marex@denx.de wrote:
On 7/5/19 12:44 PM, Michal Suchanek wrote:
Causes unbound key repeat on error otherwise.
Signed-off-by: Michal Suchanek msuchanek@suse.de
v2: fix indentation
What changed in V3 ?
common/usb_kbd.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index cc99c6be0720..fc9419e0238a 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,10 +339,9 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) struct usb_kbd_pdata *data = dev->privptr;
/* Submit a interrupt transfer request */
- usb_submit_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize,
data->intinterval);- usb_kbd_irq_worker(dev);
- if (!usb_submit_int_msg(dev, data->intpipe, &data->new[0],
data->intpktsize, data->intinterval))This still doesn't propagate errors.
Yes, it does not. I don't have a grand design that would make use of these propagated errors so I don't know what errors to propagate.
Each and every called of usb_kbd_poll_for_event() returns some return value,
A void
so these functions should at least be aware of the new errors which might come from usb_kbd_poll_for_event().
No errors come from the function. The u-boot input flow is designed for usb_kbd_poll_for_event handling errors internally.
The function is only used by the usb_kbd specific testc and getc. The input users are supposed to use testc to see if there is input available and getc to get input (and it should not block if testc indicated that there is input). The result of testc is treated as boolean so there is no room for error handling. The result of getc is treated as character input and I found no user that implements any kind of error handling. Also at the getc time the testc error is no longer available.
If you want to redesign it to handle errors in the caller then please do share the design. Without that it is not clear which errors make sense to propagate.
Presumably, usb_kbd_tstc() should return 0 if usb_kbd_poll_for_event() returns an error code or somesuch ?
The return value only reflects if there are characters in the buffer. Technically there can be pre-existing characters in the buffer even if error was returned, and there can be no characters even if keyboard state was successfully received. Handling the error here with the existing interface does not make sense.
Thanks
Michal
participants (4)
-
Bin Meng -
Marek Vasut -
Michal Suchanek -
Michal Suchánek