On Thu, 10 Sep 2015 09:38:15 +0200
Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> wrote:
> Excellent! This makes the framebuffer console support complete :)
I'm sorry to dissapoint you, but this implementation of the usb
keyboard driver seems to be wrong. The driver employs the get_report
request to poll the keyboard state via the control endpoint, however
the "Device Class Definition for Human Interface Devices (HID)" states
the following:
"This request is not intended to be used for polling the device state
on a regular basis. [...] The Interrupt In pipe should be used for
recurring Input reports."
I guess this is the source of the problems with some keyboards: 2 of 4
keyboards I've tested changed their state only on key down, but not
on key up.
So, according the standard, we need to poll a keyboard via interrupt
endpoint. However, current implementation of EHCI driver doesn't support
interrupt transactions: submit_int_msg() in ehci-hcd.c is a stub. I'll
try to implement it.
Peter
>
> I have not much to say, the driver looks ok to me. I'll test it once I
> find time. Some smally comments inline.
>
> On Wed, Sep 09, 2015 at 07:36:52PM +0300, Peter Mamonov wrote:
> > diff --git a/drivers/input/usb_kbd.c b/drivers/input/usb_kbd.c
>
> Please add some copyright header and don't forget to put references in
> it where you got the code from.
>
> > + if (pressed == 1) {
> > + if (scancode == NUM_LOCK) {
> > + data->flags ^= USB_KBD_NUMLOCK;
> > + return 1;
> > + }
> > +
> > + if (scancode == CAPS_LOCK) {
> > + data->flags ^= USB_KBD_CAPSLOCK;
> > + return 1;
> > + }
> > + if (scancode == SCROLL_LOCK) {
> > + data->flags ^= USB_KBD_SCROLLLOCK;
> > + return 1;
> > + }
> > + }
> > +
> > + /* Report keycode if any */
> > + if (keycode) {
> > + pr_debug("%s: key pressed: '%c'\n", __FUNCTION__,
> > keycode);
>
> Please use __func__ rather than __FUNCTION__
>
> Sascha
>
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
