[RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Jason Wessel]

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.wessel at 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.  

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.

Jason.