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

[Michal Suchánek]

On Tue, 2 Jul 2019 19:50:05 +0200
Michal Suchánek <msuchanek at suse.de> wrote:

> On Tue, 2 Jul 2019 18:58:54 +0200
> Marek Vasut <marex at denx.de> wrote:
> 
> > On 7/2/19 4:22 PM, Michal Suchánek wrote:  
> > > On Tue, 2 Jul 2019 15:11:07 +0200
> > > Marek Vasut <marex at denx.de> wrote:
> > >     
> > >> On 7/2/19 3:04 PM, Michal Suchánek wrote:    
> > >>> On Tue, 2 Jul 2019 13:58:30 +0200
> > >>> Marek Vasut <marex at denx.de> wrote:
> > >>>       
> > >>>> On 7/1/19 5:56 PM, Michal Suchanek wrote:      
> > >>>>> Causes unbound key repeat on error otherwise.
> > >>>>>
> > >>>>> Signed-off-by: Michal Suchanek <msuchanek at suse.de>
> > >>>>> ---
> > >>>>>  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..948f9fd68490 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],        
> > >>>>
> > >>>> Shouldn't you propagate return value from this function ? It can return
> > >>>> ENOTSUPP.
> > >>>>      
> > >>>
> > >>> If it did then probing keyboard would fail and we would not get here.      
> > >>
> > >> So there is no chance this function could return an error here, ever ?
> > >> E.g. what if it's implemented and someone yanks the keyboard cable out
> > >> just at the right time ?    
> > > 
> > > It returns errors all the time with dwc2. That's why we need to check
> > > for the error condition. We should not get here if probing the keyboard
> > > failed, though. So if the function is not supported we will not get
> > > here. Anyway, if it's not supported or the keyboard is missing it by
> > > definition cannot provide useful result so we should not process it.    
> > 
> > Except you start ignoring the error value from e.g. malfunctioning
> > keyboard here, instead of propagating it, correct ?  
> 
> It was never propagated to start with. The return value was not checked
> at all. What I do here is check the return value and not process the
> data on error whatever it contains (like the keypress returned last
> time valid data was received).

Maybe the error here is using usb_kbd_irq_worker rather than
usb_kbd_irq which checks some status before calling usb_kbd_irq_worker.

Nonetheless, checking the return value from usb_submit_int_msg works
for me.

Thanks

Michal