[U-Boot] [PATCH v2 1/9] usb: r8a66597: return -ENOTSUPP from unimplemented submit_int_msg

[U-Boot] [PATCH v2 1/9] usb: r8a66597: return -ENOTSUPP from unimplemented submit_int_msg

[Michal Suchanek]

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 drivers/usb/host/r8a66597-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
index 3c263e51c160..7b699d3f4788 100644
--- a/drivers/usb/host/r8a66597-hcd.c
+++ b/drivers/usb/host/r8a66597-hcd.c
@@ -826,7 +826,7 @@ int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 {
 	/* no implement */
 	R8A66597_DPRINT("%s\n", __func__);
-	return 0;
+	return -ENOTSUPP;
 }
 
 int usb_lowlevel_init(int index, enum usb_init_type init, void **controller)
-- 
2.21.0

[U-Boot] [PATCH v2 2/9] usb: sl811-hcd: return -ENOTSUPP from unimplemented submit_int_msg

[Michal Suchanek]

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 drivers/usb/host/sl811-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c
index daba0dcd1aee..4fd2ad464312 100644
--- a/drivers/usb/host/sl811-hcd.c
+++ b/drivers/usb/host/sl811-hcd.c
@@ -388,7 +388,7 @@ int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
 {
 	PDEBUG(0, "dev = %p pipe = %#lx buf = %p size = %d int = %d\n", dev, pipe,
 	       buffer, len, interval);
-	return -1;
+	return -ENOTSUPP;
 }
 
 /*
-- 
2.21.0

[U-Boot] [PATCH v2 2/9] usb: sl811-hcd: return -ENOTSUPP from unimplemented submit_int_msg

[Marek Vasut]

On 7/2/19 7:55 PM, Michal Suchanek wrote:

Commit message is still missing ...

> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
[...]

[U-Boot] [PATCH v2 2/9] usb: sl811-hcd: return -ENOTSUPP from unimplemented submit_int_msg

[Michal Suchánek]

On Tue, 2 Jul 2019 20:41:04 +0200
Marek Vasut <marex@denx.de> wrote:

> On 7/2/19 7:55 PM, Michal Suchanek wrote:
> 
> Commit message is still missing ...
> 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>  
> [...]

It says "usb: sl811-hcd: return -ENOTSUPP from unimplemented
submit_int_msg"

Thanks

Michal

[U-Boot] [PATCH v2 2/9] usb: sl811-hcd: return -ENOTSUPP from unimplemented submit_int_msg

[Marek Vasut]

On 7/2/19 9:35 PM, Michal Suchánek wrote:
> On Tue, 2 Jul 2019 20:41:04 +0200
> Marek Vasut <marex@denx.de> wrote:
> 
>> On 7/2/19 7:55 PM, Michal Suchanek wrote:
>>
>> Commit message is still missing ...
>>
>>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>  
>> [...]
> 
> It says "usb: sl811-hcd: return -ENOTSUPP from unimplemented
> submit_int_msg"

That's subject, not commit message.

[U-Boot] [PATCH v2 2/9] usb: sl811-hcd: return -ENOTSUPP from unimplemented submit_int_msg

[Michal Suchánek]

On Tue, 2 Jul 2019 23:20:52 +0200
Marek Vasut <marex@denx.de> wrote:

> On 7/2/19 9:35 PM, Michal Suchánek wrote:
> > On Tue, 2 Jul 2019 20:41:04 +0200
> > Marek Vasut <marex@denx.de> wrote:
> >   
> >> On 7/2/19 7:55 PM, Michal Suchanek wrote:
> >>
> >> Commit message is still missing ...
> >>  
> >>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>    
> >> [...]  
> > 
> > It says "usb: sl811-hcd: return -ENOTSUPP from unimplemented
> > submit_int_msg"  
> 
> That's subject, not commit message.

Should I add a dot like in bugzilla when it insists "you must add a
comment for this change"?

Thanks

Michal

[U-Boot] [PATCH v2 2/9] usb: sl811-hcd: return -ENOTSUPP from unimplemented submit_int_msg

[Marek Vasut]

On 7/4/19 6:00 PM, Michal Suchánek wrote:
> On Tue, 2 Jul 2019 23:20:52 +0200
> Marek Vasut <marex@denx.de> wrote:
> 
>> On 7/2/19 9:35 PM, Michal Suchánek wrote:
>>> On Tue, 2 Jul 2019 20:41:04 +0200
>>> Marek Vasut <marex@denx.de> wrote:
>>>   
>>>> On 7/2/19 7:55 PM, Michal Suchanek wrote:
>>>>
>>>> Commit message is still missing ...
>>>>  
>>>>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>    
>>>> [...]  
>>>
>>> It says "usb: sl811-hcd: return -ENOTSUPP from unimplemented
>>> submit_int_msg"  
>>
>> That's subject, not commit message.
> 
> Should I add a dot like in bugzilla when it insists "you must add a
> comment for this change"?

Please add a commit message which explains the change.

[...]

[U-Boot] [PATCH v2 2/9] usb: sl811-hcd: return -ENOTSUPP from unimplemented submit_int_msg

[Michal Suchánek]

On Thu, 4 Jul 2019 18:19:20 +0200
Marek Vasut <marex@denx.de> wrote:

> On 7/4/19 6:00 PM, Michal Suchánek wrote:
> > On Tue, 2 Jul 2019 23:20:52 +0200
> > Marek Vasut <marex@denx.de> wrote:
> >   
> >> On 7/2/19 9:35 PM, Michal Suchánek wrote:  
> >>> On Tue, 2 Jul 2019 20:41:04 +0200
> >>> Marek Vasut <marex@denx.de> wrote:
> >>>     
> >>>> On 7/2/19 7:55 PM, Michal Suchanek wrote:
> >>>>
> >>>> Commit message is still missing ...
> >>>>    
> >>>>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>      
> >>>> [...]    
> >>>
> >>> It says "usb: sl811-hcd: return -ENOTSUPP from unimplemented
> >>> submit_int_msg"    
> >>
> >> That's subject, not commit message.  
> > 
> > Should I add a dot like in bugzilla when it insists "you must add a
> > comment for this change"?  
> 
> Please add a commit message which explains the change.
> 
> [...]

Which is "usb: sl811-hcd: return -ENOTSUPP from unimplemented
submit_int_msg" 

Thanks

Michal

[U-Boot] [PATCH v2 2/9] usb: sl811-hcd: return -ENOTSUPP from unimplemented submit_int_msg

[Marek Vasut]

On 7/4/19 8:54 PM, Michal Suchánek wrote:
> On Thu, 4 Jul 2019 18:19:20 +0200
> Marek Vasut <marex@denx.de> wrote:
> 
>> On 7/4/19 6:00 PM, Michal Suchánek wrote:
>>> On Tue, 2 Jul 2019 23:20:52 +0200
>>> Marek Vasut <marex@denx.de> wrote:
>>>   
>>>> On 7/2/19 9:35 PM, Michal Suchánek wrote:  
>>>>> On Tue, 2 Jul 2019 20:41:04 +0200
>>>>> Marek Vasut <marex@denx.de> wrote:
>>>>>     
>>>>>> On 7/2/19 7:55 PM, Michal Suchanek wrote:
>>>>>>
>>>>>> Commit message is still missing ...
>>>>>>    
>>>>>>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>      
>>>>>> [...]    
>>>>>
>>>>> It says "usb: sl811-hcd: return -ENOTSUPP from unimplemented
>>>>> submit_int_msg"    
>>>>
>>>> That's subject, not commit message.  
>>>
>>> Should I add a dot like in bugzilla when it insists "you must add a
>>> comment for this change"?  
>>
>> Please add a commit message which explains the change.
>>
>> [...]
> 
> Which is "usb: sl811-hcd: return -ENOTSUPP from unimplemented
> submit_int_msg" 

That is subject. Until this is fixed, consider this patch NAKed.

https://www.denx.de/wiki/U-Boot/Patches#General_Patch_Submission_Rules
clearly states that meaningful commit message is mandatory.

[U-Boot] [PATCH v2 2/9] usb: sl811-hcd: return -ENOTSUPP from unimplemented submit_int_msg

[Michal Suchánek]

On Thu, 4 Jul 2019 21:05:26 +0200
Marek Vasut <marex@denx.de> wrote:

> On 7/4/19 8:54 PM, Michal Suchánek wrote:
> > On Thu, 4 Jul 2019 18:19:20 +0200
> > Marek Vasut <marex@denx.de> wrote:
> >   
> >> On 7/4/19 6:00 PM, Michal Suchánek wrote:  
> >>> On Tue, 2 Jul 2019 23:20:52 +0200
> >>> Marek Vasut <marex@denx.de> wrote:
> >>>     
> >>>> On 7/2/19 9:35 PM, Michal Suchánek wrote:    
> >>>>> On Tue, 2 Jul 2019 20:41:04 +0200
> >>>>> Marek Vasut <marex@denx.de> wrote:
> >>>>>       
> >>>>>> On 7/2/19 7:55 PM, Michal Suchanek wrote:
> >>>>>>
> >>>>>> Commit message is still missing ...
> >>>>>>      
> >>>>>>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>        
> >>>>>> [...]      
> >>>>>
> >>>>> It says "usb: sl811-hcd: return -ENOTSUPP from unimplemented
> >>>>> submit_int_msg"      
> >>>>
> >>>> That's subject, not commit message.    
> >>>
> >>> Should I add a dot like in bugzilla when it insists "you must add a
> >>> comment for this change"?    
> >>
> >> Please add a commit message which explains the change.
> >>
> >> [...]  
> > 
> > Which is "usb: sl811-hcd: return -ENOTSUPP from unimplemented
> > submit_int_msg"   
> 
> That is subject. Until this is fixed, consider this patch NAKed.
> 
> https://www.denx.de/wiki/U-Boot/Patches#General_Patch_Submission_Rules
> clearly states that meaningful commit message is mandatory.

That's completely fine. I do not need this change for anything. Due to
touching all implementations of submit_int_msg I noticed this one is
obviously broken. I don't use it so I do not care either way.

Thanks

Michal

[U-Boot] [PATCH v2 3/9] usb_kdb: only process events succesfully received

[Michal Suchanek]

On error the data buffer does not contain valid data so do not submit it
for processing. Usually it will contain the last data recieved so the
last pressed key will be repeated indefinitely on device failure.

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 020f0d4117f7..74206d2de74f 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -317,10 +317,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)
-- 
2.21.0

[U-Boot] [PATCH v2 3/9] usb_kdb: only process events succesfully received

[Bin Meng]

Hi Michal,

On Wed, Jul 3, 2019 at 1:57 AM Michal Suchanek <msuchanek@suse.de> wrote:
>
> On error the data buffer does not contain valid data so do not submit it
> for processing. Usually it will contain the last data recieved so the
> last pressed key will be repeated indefinitely on device failure.
>
> 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 020f0d4117f7..74206d2de74f 100644
> --- a/common/usb_kbd.c
> +++ b/common/usb_kbd.c
> @@ -317,10 +317,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))

The indentation still looks incorrect. It should align to one
character after the open parenthesis.

> +               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)

Regards,
Bin

[U-Boot] [PATCH v2 3/9] usb_kdb: only process events succesfully received

[Michal Suchánek]

On Wed, 3 Jul 2019 09:37:42 +0800
Bin Meng <bmeng.cn@gmail.com> wrote:

> Hi Michal,
> 
> On Wed, Jul 3, 2019 at 1:57 AM Michal Suchanek <msuchanek@suse.de> wrote:
> >
> > On error the data buffer does not contain valid data so do not submit it
> > for processing. Usually it will contain the last data recieved so the
> > last pressed key will be repeated indefinitely on device failure.
> >
> > 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 020f0d4117f7..74206d2de74f 100644
> > --- a/common/usb_kbd.c
> > +++ b/common/usb_kbd.c
> > @@ -317,10 +317,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))  
> 
> The indentation still looks incorrect. It should align to one
> character after the open parenthesis.
It looks correct after the function is renamed twice in following
patches.
> 
> > +               usb_kbd_irq_worker(dev);
Anyway, the problem I was referring to was excessive indentation of
this line in previous version of the patch.

Thanks

Michal

[U-Boot] [PATCH v2 4/9] usb: usb_submit_int_msg -> usb_int_msg

[Michal Suchanek]

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 74206d2de74f..71da890b762a 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -317,7 +317,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) || \
@@ -481,8 +481,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);
-- 
2.21.0

[U-Boot] [PATCH v2 5/9] usb: storage: submit_int_msg -> usb_int_msg

[Michal Suchanek]

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
---
 common/usb_storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/usb_storage.c b/common/usb_storage.c
index 8c889bb1a648..7be965964f09 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -650,7 +650,7 @@ 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,
+	usb_int_msg(us->pusb_dev, us->irqpipe,
 			(void *) &us->ip_data, us->irqmaxp, us->irqinterval);
 	timeout = 1000;
 	while (timeout--) {
-- 
2.21.0

[U-Boot] [PATCH v2 5/9] usb: storage: submit_int_msg -> usb_int_msg

[Bin Meng]

On Wed, Jul 3, 2019 at 1:57 AM 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
> ---
>  common/usb_storage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index 8c889bb1a648..7be965964f09 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -650,7 +650,7 @@ 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,
> +       usb_int_msg(us->pusb_dev, us->irqpipe,
>                         (void *) &us->ip_data, us->irqmaxp, us->irqinterval);

Please also change the indentation here.

>         timeout = 1000;
>         while (timeout--) {
> --

Regards,
Bin

[U-Boot] [PATCH v2 5/9] usb: storage: submit_int_msg -> usb_int_msg

[Michal Suchánek]

On Wed, 3 Jul 2019 09:39:10 +0800
Bin Meng <bmeng.cn@gmail.com> wrote:

> On Wed, Jul 3, 2019 at 1:57 AM 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
> > ---
> >  common/usb_storage.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/usb_storage.c b/common/usb_storage.c
> > index 8c889bb1a648..7be965964f09 100644
> > --- a/common/usb_storage.c
> > +++ b/common/usb_storage.c
> > @@ -650,7 +650,7 @@ 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,
> > +       usb_int_msg(us->pusb_dev, us->irqpipe,
> >                         (void *) &us->ip_data, us->irqmaxp, us->irqinterval);  
> 
> Please also change the indentation here.

I prefer to not reindent lines I would not need to touch otherwise.

Thanks

Michal

[U-Boot] [PATCH v2 5/9] usb: storage: submit_int_msg -> usb_int_msg

[Bin Meng]

On Fri, Jul 5, 2019 at 12:01 AM Michal Suchánek <msuchanek@suse.de> wrote:
>
> On Wed, 3 Jul 2019 09:39:10 +0800
> Bin Meng <bmeng.cn@gmail.com> wrote:
>
> > On Wed, Jul 3, 2019 at 1:57 AM 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
> > > ---
> > >  common/usb_storage.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/common/usb_storage.c b/common/usb_storage.c
> > > index 8c889bb1a648..7be965964f09 100644
> > > --- a/common/usb_storage.c
> > > +++ b/common/usb_storage.c
> > > @@ -650,7 +650,7 @@ 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,
> > > +       usb_int_msg(us->pusb_dev, us->irqpipe,
> > >                         (void *) &us->ip_data, us->irqmaxp, us->irqinterval);
> >
> > Please also change the indentation here.
>
> I prefer to not reindent lines I would not need to touch otherwise.

We need keep code conform with our coding standard. So please change that.

Regards,
Bin

[U-Boot] [PATCH v2 6/9] usb: Add nonblock argument to submit_int_msg

[Michal Suchanek]

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 3b6f889f7b7a..2645b64b0eba 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1702,7 +1702,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,
@@ -2151,7 +2151,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 7b699d3f4788..fe23a88733be 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 4fd2ad464312..21872285feeb 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 611ea97a7249..72ce2a2fca75 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
-- 
2.21.0

[U-Boot] [PATCH v2 7/9] usb: add usb_int_msg_nonblock

[Michal Suchanek]

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,
-- 
2.21.0

[U-Boot] [PATCH v2 8/9] usb: kbd: use usb_int_msg_nonblock for polling

[Michal Suchanek]

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 71da890b762a..c7be125f1811 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -317,7 +317,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) || \
-- 
2.21.0

[U-Boot] [PATCH v2 9/9] dwc2: use the nonblock argument in submit_int_msg

[Michal Suchanek]

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;
 	}
 }
-- 
2.21.0