Re: [PATCH] Add driver for mouse logitech M560

[Goffredo Baroncelli <kreijack@libero.it>]

Hi Jiri,
On 2015-05-29 16:44, Jiri Kosina wrote:
> 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.

I will update the patch on the basis of your request



> 
>>> [ ... 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,
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5