[U-Boot] [PATCH v3 1/7] usb_kdb: only process events succesfully received

[Michal Suchánek]

On Mon, 15 Jul 2019 06:28:01 +0200
Marek Vasut <marex at 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 at 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 at 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 at 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