From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal =?UTF-8?B?U3VjaMOhbmVr?= Date: Tue, 2 Jul 2019 20:09:07 +0200 Subject: [U-Boot] [PATCH 3/8] usb_kdb: only process events succesfully received In-Reply-To: <20190702195005.46823451@kitsune.suse.cz> References: <5dfc00046742db978f1ae046d8855663ccf7c281.1561996556.git.msuchanek@suse.de> <4fd1fdcc59394509236814a45d7cd384794acaae.1561996556.git.msuchanek@suse.de> <00f68615-155e-d513-1bae-c03f2d6b8097@denx.de> <20190702150400.090f85ba@kitsune.suse.cz> <41ce804f-1a82-29d8-ffdf-f1b95032016a@denx.de> <20190702162252.153a725b@kitsune.suse.cz> <20190702195005.46823451@kitsune.suse.cz> Message-ID: <20190702200907.0ef284a4@kitsune.suse.cz> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Tue, 2 Jul 2019 19:50:05 +0200 Michal Suchánek wrote: > On Tue, 2 Jul 2019 18:58:54 +0200 > Marek Vasut wrote: > > > On 7/2/19 4:22 PM, Michal Suchánek wrote: > > > On Tue, 2 Jul 2019 15:11:07 +0200 > > > Marek Vasut wrote: > > > > > >> On 7/2/19 3:04 PM, Michal Suchánek wrote: > > >>> On Tue, 2 Jul 2019 13:58:30 +0200 > > >>> Marek Vasut wrote: > > >>> > > >>>> On 7/1/19 5:56 PM, Michal Suchanek wrote: > > >>>>> Causes unbound key repeat on error otherwise. > > >>>>> > > >>>>> Signed-off-by: Michal Suchanek > > >>>>> --- > > >>>>> 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