linux-kernel - Re: [PATCH] HID: retain initial quirks set up when creating HID devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEs41JAFTGs8LErq1e7esANbHGnfyyP=xZzeax7KfPwoazVK8Q@mail.gmail.com>
Date:   Tue, 7 Feb 2023 14:11:24 -0800
From:   Allen Ballway <ballway@...omium.org>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Alistair Francis <alistair@...stair23.me>,
        Guenter Roeck <groeck@...omium.org>,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HID: retain initial quirks set up when creating HID devices

On Tue, Feb 7, 2023 at 1:16 PM Dmitry Torokhov
<dmitry.torokhov@...il.com> wrote:
>
> In certain circumstances, such as when creating I2C-connected HID
> devices, we want to pass and retain some querks (axis inversion, etc).
> The source of such quirks may be device tree, or DMI data, or something
> else not readily available to the HID core itself and therefore cannot
> be reconstructed easily. To allow this introduce "initial_quirks" field
> in hid_device structure and use it when determining the final set of
> quirks.
>
> This fixes problem with i2c-hid setting up device-tree sourced quirks
> too late and losing them on device rebind, and also allows to sever the
> tie between hid-code and i2c-hid when applying DMI-based quirks.
>
> Fixes: b60d3c803d76 ("HID: i2c-hid-of: Expose the touchscreen-inverted properties")
> Fixes: a2f416bf062a ("HID: multitouch: Add quirks for flipped axes")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> ---
>
> Compiled only, not tested on hardware.
>

Tested on Dynabook K50 with Elan9034 touchscreen and confirmed quirks are
applied as expected.

>  drivers/hid/hid-quirks.c                 | 8 +-------
>  drivers/hid/i2c-hid/i2c-hid-core.c       | 6 ++++--
>  drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 1 -
>  include/linux/hid.h                      | 1 +
>  4 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 30e35f79def4..66e64350f138 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -19,7 +19,6 @@
>  #include <linux/input/elan-i2c-ids.h>
>
>  #include "hid-ids.h"
> -#include "i2c-hid/i2c-hid.h"
>
>  /*
>   * Alphabetically sorted by vendor then product.
> @@ -1238,7 +1237,7 @@ EXPORT_SYMBOL_GPL(hid_quirks_exit);
>  static unsigned long hid_gets_squirk(const struct hid_device *hdev)
>  {
>         const struct hid_device_id *bl_entry;
> -       unsigned long quirks = 0;
> +       unsigned long quirks = hdev->initial_quirks;
>
>         if (hid_match_id(hdev, hid_ignore_list))
>                 quirks |= HID_QUIRK_IGNORE;
> @@ -1299,11 +1298,6 @@ unsigned long hid_lookup_quirk(const struct hid_device *hdev)
>                 quirks = hid_gets_squirk(hdev);
>         mutex_unlock(&dquirks_lock);
>
> -       /* Get quirks specific to I2C devices */
> -       if (IS_ENABLED(CONFIG_I2C_DMI_CORE) && IS_ENABLED(CONFIG_DMI) &&
> -           hdev->bus == BUS_I2C)
> -               quirks |= i2c_hid_get_dmi_quirks(hdev->vendor, hdev->product);
> -
>         return quirks;
>  }
>  EXPORT_SYMBOL_GPL(hid_lookup_quirk);
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 0ab8f47a84e9..efbba0465eef 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1025,6 +1025,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
>         hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
>         hid->product = le16_to_cpu(ihid->hdesc.wProductID);
>
> +       hid->initial_quirks = quirks;
> +       hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
> +                                                     hid->product);
> +
>         snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
>                  client->name, (u16)hid->vendor, (u16)hid->product);
>         strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
> @@ -1038,8 +1042,6 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
>                 goto err_mem_free;
>         }
>
> -       hid->quirks |= quirks;
> -
>         return 0;
>
>  err_mem_free:
> diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> index 554a7dc28536..210f17c3a0be 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> @@ -492,4 +492,3 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product)
>
>         return quirks;
>  }
> -EXPORT_SYMBOL_GPL(i2c_hid_get_dmi_quirks);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index daaac4d7f370..56dac09c99d9 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -622,6 +622,7 @@ struct hid_device {                                                 /* device report descriptor */
>         unsigned long status;                                           /* see STAT flags above */
>         unsigned claimed;                                               /* Claimed by hidinput, hiddev? */
>         unsigned quirks;                                                /* Various quirks the device can pull on us */
> +       unsigned initial_quirks;                                        /* Initial set of quirks supplied when creating device */
>         bool io_started;                                                /* If IO has started */
>
>         struct list_head inputs;                                        /* The list of inputs */
> --
> 2.39.1.519.gcb327c4b5f-goog
>
>
> --
> Dmitry