On 7/2/19 7:50 PM, Michal Suchánek wrote:
> On Tue, 2 Jul 2019 18:58:54 +0200
> Marek Vasut <ma...@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 <ma...@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 <ma...@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 <msucha...@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).

I can see a patch which checks usb_kbd_poll_for_event() return value.
Can you add one ?

And then propagate the usb_submit_int_msg() return value too.

-- 
Best regards,
Marek Vasut

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-56 Email: ma...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to