On 7/2/19 9:31 PM, Michal Suchánek wrote: > On Tue, 2 Jul 2019 20:38:27 +0200 > Marek Vasut <ma...@denx.de> wrote: > >> 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 ? > > What for? Apparently the keypress is processed in usb_kbd_irq_worker. > So checking the return value is needed to decide if the worker should > run, and is not particularly useful outside usb_kbd_poll_for_event. We > could signal a getc() failure but do we have any code handling getc() > failures?
I presume getc() might signal EOF if the underlying hardware fails. But in general, it's a good practice to not ignore errors. -- 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
