Re: [PATCH] HID: i2c-hid: Block a rogue device on ASUS TUF A16

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

 



Hi,

sorry for taking so long to reply.

On 02.06.23 20:43, Limonciello, Mario wrote:
+ some AMD guys

On 5/30/2023 10:40 AM, Friedrich Vock wrote:
On these laptops, there seems to be a device that, when probed by
i2c-hid, constantly sends bogus interrupts and interferes with the
keyboard controller. When the device is enabled, it takes the keyboard
around 8 seconds to register that keys are being pressed or released.

Do you know what interrupt is firing constantly?
Presumably it is the GPIO controller master interrupt, right?
And it's for GPIO 7 (guessed from acpidump on one of the bug
reports).

To confirm check /proc/interrupts.
Seems likely that you guessed correctly. The corresponsing line in
/proc/interrupts (with the interrupts counts omitted):
71:   amd_gpio    7  ITE5570:00

If it's not obvious which GPIO is firing there is also a dynamic
debug statement in pinctrl-amd.c that may be helpful to figure
this out.

I would also suspect in Windows this doesn't happen.  If possible
can you confirm that? Check in Device Manager what driver is bound
to this device. Is it "inbox" from Microsoft or is it an ASUS
specific driver?

I wonder if the GPIO controller got programmed differently in
Windows for some reason. We may want to confirm the values for
GPIO registers from /sys/kernel/debug/gpio in Linux against those
that are programmed in Windows.

This can be accomplished using R/W everything in Windows.

Unfortunately I don't have Windows installed on this system. I know some
people with the Ryzen 9 7940HS model which might, I'll ask them if they
can give me the configuration on Windows and Linux.



Nothing I tried seemed to make the device work, and preventing the
device from being probed doesn't seem to break any functionality of
the laptop.

Signed-off-by: Friedrich Vock <friedrich.vock@xxxxxx>

There are a few bug reports that popped up around this issue that should
probably also be tagged.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217493

---
  drivers/hid/i2c-hid/i2c-hid-core.c       |  5 +++
  drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 48 ++++++++++++++++++++++++
  drivers/hid/i2c-hid/i2c-hid.h            |  3 ++
  3 files changed, 56 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
b/drivers/hid/i2c-hid/i2c-hid-core.c
index efbba0465eef..5f0686d058df 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1035,6 +1035,11 @@ int i2c_hid_core_probe(struct i2c_client
*client, struct i2chid_ops *ops,

      ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);

+    if (i2c_hid_device_blocked(hid->vendor, hid->product)) {
+        ret = -ENODEV;
+        goto err_irq;
+    }
+
The thing I worry about here is that an unserviced interrupt can
prevent the
hardware from going into proper low power states; particularly at
runtime.

I think we should better understand what's going on before going down
this
path of just ignoring it.
Yeah, I guess I should've searched more for a proper explanation/fix
before submitting hacks like this. Let's see if this can be fixed in a
cleaner manner than preemptively disabling parts of the system.
      ret = hid_add_device(hid);
      if (ret) {
          if (ret != -ENODEV)
diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
index 210f17c3a0be..d2c2806b64b4 100644
--- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
+++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
@@ -440,6 +440,38 @@ static const struct dmi_system_id
i2c_hid_dmi_quirk_table[] = {
      { }    /* Terminate list */
  };

+static const struct hid_device_id i2c_hid_blocked_ite_device = {
+    HID_DEVICE(BUS_I2C, HID_GROUP_GENERIC, USB_VENDOR_ID_ITE, 0x8051)
+};
+
+/*
+ * This list contains devices that can't be activated at all, for
example
+ * because activating them breaks other important parts of the system.
+ */
+static const struct dmi_system_id i2c_hid_dmi_block_table[] = {
+    /*
+     * On ASUS TUF Gaming A16 laptops, there is a device that will
make the
+     * keyboard controller stop working correctly and flood the CPU
with bogus
+     * interrupts.
+     */
+    {
+        .ident = "ASUS TUF Gaming A16 (Ryzen 7 7735HS)",
+        .matches = {
+            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+            DMI_MATCH(DMI_PRODUCT_NAME, "FA617NS"),
+        },
+        .driver_data = (void *)&i2c_hid_blocked_ite_device,
+    },
+    {
+        .ident = "ASUS TUF Gaming A16 (Ryzen 9 7940HS)",
+        .matches = {
+            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+            DMI_MATCH(DMI_PRODUCT_NAME, "FA617XS"),
+        },
+        .driver_data = (void *)&i2c_hid_blocked_ite_device,
+    },
+    { }    /* Terminate list */
If this *does* end up being the best solution, I think it's better
to patch in the DMI to gpiolib-acpi.c similar to other quirks for
floating
GPIOs.  Example:

https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib-acpi.c#L1654


+};

  struct i2c_hid_desc *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t
*i2c_name)
  {
@@ -492,3 +524,19 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor,
const u16 product)

      return quirks;
  }
+
+bool i2c_hid_device_blocked(const u16 vendor, const u16 product)
+{
+    const struct dmi_system_id *system_id =
+            dmi_first_match(i2c_hid_dmi_block_table);
+
+    if (system_id) {
+        const struct hid_device_id *device_id =
+                (struct hid_device_id *)(system_id->driver_data);
+
+        if (device_id && device_id->vendor == vendor &&
+            device_id->product == product)
+            return true;
+    }
+    return false;
+}
diff --git a/drivers/hid/i2c-hid/i2c-hid.h
b/drivers/hid/i2c-hid/i2c-hid.h
index 2c7b66d5caa0..e17bdd758f39 100644
--- a/drivers/hid/i2c-hid/i2c-hid.h
+++ b/drivers/hid/i2c-hid/i2c-hid.h
@@ -10,6 +10,7 @@ struct i2c_hid_desc
*i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name);
  char *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
                             unsigned int *size);
  u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product);
+bool i2c_hid_device_blocked(const u16 vendor, const u16 product);
  #else
  static inline struct i2c_hid_desc
             *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name)
@@ -19,6 +20,8 @@ static inline char
*i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
  { return NULL; }
  static inline u32 i2c_hid_get_dmi_quirks(const u16 vendor, const
u16 product)
  { return 0; }
+static inline bool i2c_hid_device_blocked(const u16 vendor, const
u16 product)
+{ return false; }
  #endif

  /**
--
2.40.1






[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux