Re: [PATCH] Driver for Primax keyboards violating USB HID spec

[Terry Lambert]

Hi Jiri; thanks for looking so promptly!

The routine right now just caches the hi->input for later look up
during the raw input report, which I suppose could be done inline on
each report with a couple function calls.

The reasoning was that there's some  hardware that's going to fall
into this class that needs some key remapping done.  Maybe this
hardware, too, depending on what is decided about the num lock not
being on by default for a newly plugged keyboard (this specific
keyboard has no num lock key).

I don't have one of the folding keyboards people are complaining about
not being able to use for Android tablets on hand, or the unhappy Dell
Inspiron laptop, both of which use the controller   That's to explain
why the remapping code and the product ID for them are missing from
this patch.

If it would make things easier, I could inline the input lookup and
just ditch the routine altogether, until I or someone else gets their
hands on the other hardware.

Thanks,
-- Terry

On Thu, Oct 13, 2011 at 4:35 PM, Jiri Kosina <jkosina@xxxxxxx> wrote:
>
> On Thu, 13 Oct 2011, Terry Lambert wrote:
>
> > Primax keyboards with the issue this driver addresses report modifier
> > keys as in band key events instead of as out of band modifier bits,
> > resulting in the modifier keys generating key up events immediately
> > before the keys they are intended to modify.  This driver rewrites
> > the raw report data from such keyboards into USB HID 1.11 compliant
> > report data.  It only matches the USB vendor and product IDs for the
> > keyboard it has been tested on. Since there are several keyboards,
> > notably a number of laptops and folding USB keyboards known to have
> > similar unresolved problem reports, the list is expected to grow.
> >
> > Signed-off-by: Terry Lambert <tlambert@xxxxxxxxxxxx>
>
> Terry,
>
> thanks for the patch.
>
> [ ... snip ... ]
> > +static int px_input_mapping(struct hid_device *hid, struct hid_input *hi,
> > +     struct hid_field *field, struct hid_usage *usage,
> > +     unsigned long **bit, int *max)
> > +{
> > +     struct px_device *px = hid_get_drvdata(hid);
> > +
> > +     px->input_primax = hi->input;
> > +
> > +     /* Remap keys, if necesssary, here */
> > +
> > +     return 0;
> > +}
>
> I fail to see the point of this routine ... ?
>
> Otherwise, the patch seems fine.
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/