On Fri, 29 May 2015, Benjamin Tissoires wrote:
> >> +static u32 hidpp_extract(u8 *report, unsigned offset, unsigned n)
> >> +{
> >> + u64 x;
> >> +
> >> + report += offset >> 3; /* adjust byte index */
> >> + offset &= 7; /* now only need bit offset into one byte */
> >> + x = get_unaligned_le64(report);
> >> + x = (x >> offset) & ((1ULL << n) - 1); /* extract bit field */
> >> + return (u32)x;
> >> +}
> >
> > I hate such code duplication. How about we rename it to
> > hid_field_extract() and make its linkage external?
>
> works for me
Thanks. So I'd prefer this patch to be resent as a two-patch series, first
one bringing hid_field_extract(), and the second being the driver making
use of it.
> > [ ... snip ... ]
> >> @@ -1301,6 +1532,10 @@ static const struct hid_device_id hidpp_devices[] =
> >> {
> >> USB_VENDOR_ID_LOGITECH, 0x4102),
> >> .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_MULTI_INPUT |
> >> HIDPP_QUIRK_CLASS_WTP },
> >> + { /* Mouse logitech M560 */
> >> + HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
> >> + USB_VENDOR_ID_LOGITECH, 0x402d),
> >> + .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
> >
> > Seems like you forgot to add the device id to hid_have_special_driver[]?
>
> nope, the device is tagged with HID_GROUP_LOGITECH_DJ_DEVICE, so
> hid-generic ignores it by default.
Gah, I keep forgetting about HID_GROUP_LOGITECH_DJ_DEVICE again and again.
Thanks,
--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
