On 8/18/20 8:47 PM, Jason Wessel wrote: > > > On 8/18/20 12:20 PM, Marek Vasut wrote: >> On 8/18/20 6:54 PM, Jason Wessel wrote: >>> >>> >>> On 8/18/20 10:05 AM, Marek Vasut wrote: >>>> On 8/18/20 4:34 PM, Jason Wessel wrote: >>>>> After the initial configuration some USB keyboard+mouse devices never >>>>> return any kind of event on the interrupt line. In particular, the >>>>> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as >>>>> 3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0" >>>>> never returns a data packet until the first external input event. >>>>> >>>>> I found this was also true with some newer model Dell keyboards. >>>>> >>>>> When the device is plugged into a xhci controller there is also no >>>>> point in waiting 5 seconds for a device that is never going to present >>>>> data, so the call to the interrupt service was changed to a >>>>> nonblocking operation for the controllers that support this. >>>>> >>>>> With the patch applied, the rpi3 and rpi4 work well with the more >>>>> complex keyboard devices. >>>> >>>> Please extend the comment in the code, it is not clear from it or from >>>> the commit message what the problem really is that this patch tries to fix. >>>> >>>> Also, the printf() below the return 1 is never reached. >>>> >>> >>> >>> The printf() is only reached in the case of the #ifdef above it being true. >> >> That's pretty awful and confusing code then. The original code wasn't >> stellar either, but can this be somehow improved now ? >> >>> The compiler in theory should optimize it away, but for clarity it can be >>> moved >>> with in the #ifdef but that also requires fixing it in two places because >>> there are multiple levels to the #ifdef. >> >> I think it's better to make it more readable if possible. >> >>> Would the following be acceptable, which I can put in the next version. >>> >>> >>> commit 319c75ee3d1fce8a3389d1de857751504b5110cb (HEAD -> master) >>> Author: Jason Wessel <jason.wes...@windriver.com> >>> Date: Thu Jun 25 05:31:25 2020 -0700 >>> >>> usb_kbd: succeed even if no interrupt is pending >>> >>> After the initial configuration some USB keyboard+mouse devices never >>> return any kind of event on the interrupt line. In particular, the >>> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as >>> 3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0" >>> never returns a data packet until the first external input event. >>> >>> I found this was also true with some newer model Dell keyboards. >>> >>> Without the patch u-boot prints the following on one of keyboards and >>> leave it unable to accept input events. >>> >>> ===== >>> scanning bus xhci_pci for devices... >>> Failed to get keyboard state from device 14dd:1009 >> >> So I have to wonder, if the keyboard never returns a data packet until >> you press a key (that makes sense), how does Linux deal with this ? >> > > > As far as I can tell the usb_kbd_probe() probe function in the Linux kernel > sets up the configuration and exits right away. The keyboard drivers state > was zeroed out in the probe function and the kernel later processes callbacks > from the interrupt handler.
Why can't we return right away and why do we poll/wait then ? > Does that imply the other 2 code paths should also just return 1 and we get > rid of the printf() entirely? > > I don't have any boards that wanted either of these two paths through code, > so I wasn't inclined to change it. I'd say, change it, it'd go in ~October and the next release is in 3 months from October anyway.