On Tue, Jun 11, 2019 at 2:13 PM Nicolas Saenz Julienne
<nsaenzjulienne@xxxxxxx> wrote:
>
> Some a4tech mice use the 'GenericDesktop.00b8' usage to inform whether
> the previous wheel report was horizontal or vertical. Before
> c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this
> usage was being mapped to 'Relative.Misc'. After the patch it's simply
> ignored (usage->type == 0 & usage->code == 0). Which ultimately makes
> hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no
> usage->type.
>
> We shouldn't rely on a mapping for that usage as it's nonstandard and
> doesn't really map to an input event. So we bypass the mapping and make
> sure the custom event handling properly handles both reports.
>
> Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key")
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
> ---
Yep, I like this one much better.
>
> NOTE: I CC'd Wolfgang as he's the one who can test this.
I'll wait for Wolfram to confirm that the patch works before pushing then.
>
> Changes since v1:
> - new approach, moved fix into hid-a4tech
>
> drivers/hid/hid-a4tech.c | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
> index 98bf694626f7..3a8c4a5971f7 100644
> --- a/drivers/hid/hid-a4tech.c
> +++ b/drivers/hid/hid-a4tech.c
> @@ -23,12 +23,36 @@
> #define A4_2WHEEL_MOUSE_HACK_7 0x01
> #define A4_2WHEEL_MOUSE_HACK_B8 0x02
>
> +#define A4_WHEEL_ORIENTATION (HID_UP_GENDESK | 0x000000b8)
> +
> struct a4tech_sc {
> unsigned long quirks;
> unsigned int hw_wheel;
> __s32 delayed_value;
> };
>
> +static int a4_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> + struct hid_field *field, struct hid_usage *usage,
> + unsigned long **bit, int *max)
> +{
> + struct a4tech_sc *a4 = hid_get_drvdata(hdev);
> +
> + if (a4->quirks & A4_2WHEEL_MOUSE_HACK_B8 &&
> + usage->hid == A4_WHEEL_ORIENTATION) {
> + /*
> + * We do not want to have this usage mapped to anything as it's
> + * nonstandard and doesn't really behave like an HID report.
> + * It's only selecting the orientation (vertical/horizontal) of
> + * the previous mouse wheel report. The input_events will be
> + * generated once both reports are recorded in a4_event().
> + */
> + return -1;
You seem to be extra precocious here. This is basically what the
current hid-input.c mapping does. But I think it is for the best,
given that this time you are locking the behavior of this reserved
usage.
Cheers,
Benjamin
> + }
> +
> + return 0;
> +
> +}
> +
> static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,
> struct hid_field *field, struct hid_usage *usage,
> unsigned long **bit, int *max)
> @@ -52,8 +76,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
> struct a4tech_sc *a4 = hid_get_drvdata(hdev);
> struct input_dev *input;
>
> - if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput ||
> - !usage->type)
> + if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput)
> return 0;
>
> input = field->hidinput->input;
> @@ -64,7 +87,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
> return 1;
> }
>
> - if (usage->hid == 0x000100b8) {
> + if (usage->hid == A4_WHEEL_ORIENTATION) {
> input_event(input, EV_REL, value ? REL_HWHEEL :
> REL_WHEEL, a4->delayed_value);
> input_event(input, EV_REL, value ? REL_HWHEEL_HI_RES :
> @@ -131,6 +154,7 @@ MODULE_DEVICE_TABLE(hid, a4_devices);
> static struct hid_driver a4_driver = {
> .name = "a4tech",
> .id_table = a4_devices,
> + .input_mapping = a4_input_mapping,
> .input_mapped = a4_input_mapped,
> .event = a4_event,
> .probe = a4_probe,
> --
> 2.21.0
>
