[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#671292: [PATCH/RFC] HID: logitech: read all 32 bits of report type bitfield



The patch looks perfect to me. Wonderful 'get_unaligned_le32' that I
looked for and did not find when trying to fix the original code.

On my side no issues for Jiri to apply it.

Thanks again Jonathan and Hugo.

Cheers,
Nestor.

On Fri, May 11, 2012 at 9:22 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> On big-endian systems (e.g., Apple PowerBook), trying to use a
> logitech wireless mouse with the Logitech Unifying Receiver does not
> work with v3.2 and later kernels.  The device doesn't show up in
> /dev/input.  Older kernels work fine.
>
> That is because the new hid-logitech-dj driver claims the device.  The
> device arrival notification appears:
>
>        20 00 41 02 00 00 00 00 00 00 00 00 00 00 00
>
> and we read the report_types bitfield (02 00 00 00) to find out what
> kind of device it is.  Unfortunately the driver only reads the first 8
> bits and treats that value as a 32-bit little-endian number, so on a
> powerpc the report type seems to be 0x02000000 and is not recognized.
>
> Even on little-endian machines, connecting a media center remote
> control (report type 00 01 00 00) with this driver loaded would
> presumably fail for the same reason.
>
> Fix both problems by using get_unaligned_le32() to read all four
> bytes, which is a little clearer anyway.  After this change, the
> wireless mouse works on Hugo's PowerBook again.
>
> Based on a patch by Nestor Lopez Casado.
> Addresses http://bugs.debian.org/671292
>
> Reported-by: Hugo Osvaldo Barrera <hugo@osvaldobarrera.com.ar>
> Inspired-by: Nestor Lopez Casado <nlopezcasad@logitech.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Cc: <stable@vger.kernel.org>
> ---
> Nestor Lopez Casado wrote:
>
>> Great news. Thanks Hugo for your help testing the patches.
>>
>> Jonathan, how do we proceed now, I mean to submit the fix ?
>
> If the patch looks good to you, we ask Jiri to pick it up.  What do
> you think?
>
> Thanks,
> Jonathan
>
>  drivers/hid/hid-logitech-dj.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 38b12e45780c..2eac8c566b17 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -26,6 +26,7 @@
>  #include <linux/hid.h>
>  #include <linux/module.h>
>  #include <linux/usb.h>
> +#include <asm/unaligned.h>
>  #include "usbhid/usbhid.h"
>  #include "hid-ids.h"
>  #include "hid-logitech-dj.h"
> @@ -265,8 +266,8 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
>                goto dj_device_allocate_fail;
>        }
>
> -       dj_dev->reports_supported = le32_to_cpu(
> -               dj_report->report_params[DEVICE_PAIRED_RF_REPORT_TYPE]);
> +       dj_dev->reports_supported = get_unaligned_le32(
> +               dj_report->report_params + DEVICE_PAIRED_RF_REPORT_TYPE);
>        dj_dev->hdev = dj_hiddev;
>        dj_dev->dj_receiver_dev = djrcv_dev;
>        dj_dev->device_index = dj_report->device_index;
> --
> 1.7.10.1
>



Reply to: