Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias
@ 2026-05-27 15:17 谢致邦 (XIE Zhibang)
  2026-05-27 15:44 ` Hans de Goede
  2026-05-27 16:00 ` [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias sashiko-bot
  0 siblings, 2 replies; 6+ messages in thread
From: 谢致邦 (XIE Zhibang) @ 2026-05-27 15:17 UTC (permalink / raw)
  To: linux-input
  Cc: 谢致邦 (XIE Zhibang), Jiri Kosina,
	Benjamin Tissoires, Mario Limonciello (AMD), Hans de Goede,
	Douglas Anderson, linux-kernel

Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
declare their I2C HID ACPI touchpad in the DSDT as _HID "PRP0001" with
_DSD compatible "hid-over-i2c" instead of the standard "PNP0C50". This
worked before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and
OF are separate modules"), but after the split, PRP0001 devices on the
ACPI bus are no longer probed by either driver.

Fix this by adding PRP0001 to i2c_hid_acpi_match so the driver probes
these devices. The existing _DSM HID descriptor call in probe()
naturally rejects any PRP0001 device that does not implement the
protocol.

A MODULE_ALIAS is also needed for autoloading: when an ACPI device has a
_DSD "compatible" property, the uevent modalias uses the OF format
(of:N<name>T<compatible>) instead of the ACPI format (acpi:<HID>), so
udev would otherwise load only i2c-hid-of, which fails to probe because
these devices lack the "hid-descr-addr" property.

Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
 drivers/hid/i2c-hid/i2c-hid-acpi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index abd700a101f4..515ced22c978 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -119,10 +119,30 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
 static const struct acpi_device_id i2c_hid_acpi_match[] = {
 	{ "ACPI0C50" },
 	{ "PNP0C50" },
+	/*
+	 * Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
+	 * declare their I2C HID ACPI touchpad in the DSDT as _HID "PRP0001"
+	 * with _DSD compatible "hid-over-i2c" instead of the standard
+	 * "PNP0C50". This worked before i2c-hid was split into i2c-hid-acpi
+	 * and i2c-hid-of, but PRP0001 devices on the ACPI bus are no longer
+	 * probed after the split. The _DSM call in probe() naturally rejects
+	 * PRP0001 devices that are not actually I2C HID, so matching PRP0001
+	 * here is safe.
+	 */
+	{ "PRP0001" },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, i2c_hid_acpi_match);
 
+	/*
+	 * When an ACPI device has a _DSD "compatible" property, the uevent
+	 * modalias uses the OF format (of:N<name>T<compatible>) instead of
+	 * the ACPI format (acpi:<HID>). Add an OF alias so udev can autoload
+	 * this module for such devices. probe() will reject pure DT devices
+	 * via the _DSM HID descriptor call.
+	 */
+MODULE_ALIAS("of:N*TChid-over-i2c");
+
 static struct i2c_driver i2c_hid_acpi_driver = {
 	.driver = {
 		.name	= "i2c_hid_acpi",
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias
  2026-05-27 15:17 [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias 谢致邦 (XIE Zhibang)
@ 2026-05-27 15:44 ` Hans de Goede
  2026-05-29 12:16   ` [PATCH] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing 谢致邦 (XIE Zhibang)
  2026-05-27 16:00 ` [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2026-05-27 15:44 UTC (permalink / raw)
  To: 谢致邦 (XIE Zhibang), linux-input
  Cc: Jiri Kosina, Benjamin Tissoires, Mario Limonciello (AMD),
	Douglas Anderson, linux-kernel

Hi,

On 27-May-26 17:17, 谢致邦 (XIE Zhibang) wrote:
> Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
> declare their I2C HID ACPI touchpad in the DSDT as _HID "PRP0001" with
> _DSD compatible "hid-over-i2c" instead of the standard "PNP0C50". This
> worked before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and
> OF are separate modules"), but after the split, PRP0001 devices on the
> ACPI bus are no longer probed by either driver.
> 
> Fix this by adding PRP0001 to i2c_hid_acpi_match so the driver probes
> these devices. The existing _DSM HID descriptor call in probe()
> naturally rejects any PRP0001 device that does not implement the
> protocol.
> 
> A MODULE_ALIAS is also needed for autoloading: when an ACPI device has a
> _DSD "compatible" property, the uevent modalias uses the OF format
> (of:N<name>T<compatible>) instead of the ACPI format (acpi:<HID>), so
> udev would otherwise load only i2c-hid-of, which fails to probe because
> these devices lack the "hid-descr-addr" property.

Ok, so first of all please contact the vendors of these devices to fix
their firmware.

Either a _HID "PRP0001" value should be used with a *full* of description
matching the binding from Documentation/devicetree/bindings/input/hid-over-i2c.yaml
including hid-descr-addr. Or the firmware should use PNP0C50 + the _DSM
method to get the hid-desc-addr. Mixing and matching these 2 is bad,
very very bad.

IMHO the fix here as is is not acceptable this will make the i2c_hid_acpi
module load and worse *probe* every ACPI device with a "PRP0001" HID.

You claim the existing _DSM HID descriptor call will save the driver from
actually doing much of anything but IMHO that should not be relied on.

Currently the i2c_hid_of driver will get automatically loaded + try
to probe the device, but as you say this will fail due to lacking
hid-desc-addr. Have you tried adding DMI quirks to i2c_hid_of to
provide the hid-desc-addr through a quirk?  (I wonder if the IRQ will
get picked up ok)

That seems a better solution than making 2 drivers probe the same
"hid-over-i2c" compatible and let one fail (with an ugly error msg
in the logs), while also making i2c_hid_acpi probe all PRP0001 devices
and make that fail (with more err logging) on all other devices.

I see the 2 laptops here are both using a Loongson architecture,
so any fix for this should IMHO also be wrapped in
#ifdef CONFIG_LOONGARCH ... #endif

Regards,

Hans





> 
> Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> ---
>  drivers/hid/i2c-hid/i2c-hid-acpi.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> index abd700a101f4..515ced22c978 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> @@ -119,10 +119,30 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
>  static const struct acpi_device_id i2c_hid_acpi_match[] = {
>  	{ "ACPI0C50" },
>  	{ "PNP0C50" },
> +	/*
> +	 * Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
> +	 * declare their I2C HID ACPI touchpad in the DSDT as _HID "PRP0001"
> +	 * with _DSD compatible "hid-over-i2c" instead of the standard
> +	 * "PNP0C50". This worked before i2c-hid was split into i2c-hid-acpi
> +	 * and i2c-hid-of, but PRP0001 devices on the ACPI bus are no longer
> +	 * probed after the split. The _DSM call in probe() naturally rejects
> +	 * PRP0001 devices that are not actually I2C HID, so matching PRP0001
> +	 * here is safe.
> +	 */
> +	{ "PRP0001" },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(acpi, i2c_hid_acpi_match);
>  
> +	/*
> +	 * When an ACPI device has a _DSD "compatible" property, the uevent
> +	 * modalias uses the OF format (of:N<name>T<compatible>) instead of
> +	 * the ACPI format (acpi:<HID>). Add an OF alias so udev can autoload
> +	 * this module for such devices. probe() will reject pure DT devices
> +	 * via the _DSM HID descriptor call.
> +	 */
> +MODULE_ALIAS("of:N*TChid-over-i2c");
> +
>  static struct i2c_driver i2c_hid_acpi_driver = {
>  	.driver = {
>  		.name	= "i2c_hid_acpi",


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias
  2026-05-27 15:17 [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias 谢致邦 (XIE Zhibang)
  2026-05-27 15:44 ` Hans de Goede
@ 2026-05-27 16:00 ` sashiko-bot
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-05-27 16:00 UTC (permalink / raw)
  Cc: linux-input, dmitry.torokhov

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Adding `PRP0001` directly to the ACPI match table incorrectly bypasses `compatible` string matching, causing the driver to intercept all PRP0001 I2C devices and spam the kernel log.
--

commit 7d0ee8c935eaadd4f5e66e197d5712a960fd31f5
Author: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>

HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias

This commit adds the PRP0001 ACPI ID to the i2c-hid-acpi driver's match
table and manually defines an OF module alias. This is intended to allow the
driver to probe devices that declare themselves as PRP0001 with a "hid-over-i2c"
compatible string in their _DSD.

> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> index abd700a101f46..515ced22c978d 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> @@ -119,10 +119,30 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
>  static const struct acpi_device_id i2c_hid_acpi_match[] = {
>  	{ "ACPI0C50" },
>  	{ "PNP0C50" },
> +	/*
> +	 * Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
> +	 * declare their I2C HID ACPI touchpad in the DSDT as _HID "PRP0001"
> +	 * with _DSD compatible "hid-over-i2c" instead of the standard
> +	 * "PNP0C50". This worked before i2c-hid was split into i2c-hid-acpi
> +	 * and i2c-hid-of, but PRP0001 devices on the ACPI bus are no longer
> +	 * probed after the split. The _DSM call in probe() naturally rejects
> +	 * PRP0001 devices that are not actually I2C HID, so matching PRP0001
> +	 * here is safe.
> +	 */
> +	{ "PRP0001" },

[Severity: Medium]
Does adding PRP0001 directly to the ACPI match table bypass the OF compatible
string check in __acpi_match_device()?

PRP0001 is a special identifier used by the ACPI subsystem to defer matching
to the OF compatible string within the device's _DSD. When PRP0001 is
explicitly added here, the match returns true immediately without checking the
compatible string.

If a system has other unrelated I2C devices using PRP0001 (like temperature
sensors or RTCs utilizing compatible strings), won't the driver core attempt
to bind i2c-hid-acpi to all of them?

Because i2c_hid_acpi_probe() unconditionally evaluates the HID _DSM which will
fail for these unrelated devices, does this introduce boot delays and pollute
the kernel log with false-positive "Error _DSM call to get HID descriptor
address failed" messages for every non-HID PRP0001 device?

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(acpi, i2c_hid_acpi_match);
>  
> +	/*
> +	 * When an ACPI device has a _DSD "compatible" property, the uevent
> +	 * modalias uses the OF format (of:N<name>T<compatible>) instead of
> +	 * the ACPI format (acpi:<HID>). Add an OF alias so udev can autoload
> +	 * this module for such devices. probe() will reject pure DT devices
> +	 * via the _DSM HID descriptor call.
> +	 */
> +MODULE_ALIAS("of:N*TChid-over-i2c");

[Severity: Medium]
Instead of manually defining this MODULE_ALIAS, could we define an
of_device_id match table containing { .compatible = "hid-over-i2c" }?

Using an of_device_id table would naturally solve the matching logic by
allowing the ACPI core to correctly evaluate the _DSD compatible string.
This prevents intercepting unrelated PRP0001 devices, and the driver core
would automatically generate the correct, robust module alias for autoloading.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_61698D7A14AC9BFE6C9F3FDB797FCA02E309@qq.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
  2026-05-27 15:44 ` Hans de Goede
@ 2026-05-29 12:16   ` 谢致邦 (XIE Zhibang)
  2026-05-29 15:00     ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: 谢致邦 (XIE Zhibang) @ 2026-05-29 12:16 UTC (permalink / raw)
  To: linux-input, hansg
  Cc: Yeking, bentiss, dianders, jikos, linux-kernel, superm1

Before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are
separate modules"), the unified i2c-hid driver handled both PNP0C50 ACPI
devices and hid-over-i2c OF devices. After the split, devices with _HID
"PRP0001" and _DSD compatible "hid-over-i2c" are only probed by
i2c_hid_of, which requires "hid-descr-addr" in the _DSD. Some devices,
for example the Lenovo KaiTian N60d and Inspur CP300L3, provide the HID
descriptor address only through the _DSM method. Fall back to the _DSM
call when the property is absent.

Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
 drivers/hid/i2c-hid/i2c-hid-of.c | 44 ++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
index 57379b77e977..62c089a6455a 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of.c
@@ -19,6 +19,7 @@
  * more details.
  */
 
+#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
@@ -74,6 +75,39 @@ static void i2c_hid_of_power_down(struct i2chid_ops *ops)
 			       ihid_of->supplies);
 }
 
+#ifdef CONFIG_ACPI
+/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
+static guid_t i2c_hid_of_acpi_guid =
+	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
+		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
+
+/*
+ * Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
+ * declare their I2C HID touchpad with _HID "PRP0001" and _DSD compatible
+ * "hid-over-i2c" but lack the "hid-descr-addr" property. Use the _DSM
+ * method to obtain the HID descriptor address.
+ */
+static int i2c_hid_of_acpi_get_descriptor(struct device *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	union acpi_object *obj;
+	u16 addr;
+
+	if (!adev)
+		return -ENODEV;
+
+	obj = acpi_evaluate_dsm_typed(acpi_device_handle(adev),
+				      &i2c_hid_of_acpi_guid, 1, 1, NULL,
+				      ACPI_TYPE_INTEGER);
+	if (!obj)
+		return -ENODEV;
+
+	addr = obj->integer.value;
+	ACPI_FREE(obj);
+	return addr;
+}
+#endif
+
 static int i2c_hid_of_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -92,6 +126,16 @@ static int i2c_hid_of_probe(struct i2c_client *client)
 	ihid_of->ops.power_down = i2c_hid_of_power_down;
 
 	ret = device_property_read_u32(dev, "hid-descr-addr", &val);
+#ifdef CONFIG_ACPI
+	if (ret) {
+		int dsm_ret = i2c_hid_of_acpi_get_descriptor(dev);
+
+		if (dsm_ret >= 0) {
+			val = dsm_ret;
+			ret = 0;
+		}
+	}
+#endif
 	if (ret) {
 		dev_err(dev, "HID register address not provided\n");
 		return -ENODEV;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
  2026-05-29 12:16   ` [PATCH] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing 谢致邦 (XIE Zhibang)
@ 2026-05-29 15:00     ` Hans de Goede
  2026-05-29 19:36       ` Dmitry Torokhov
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2026-05-29 15:00 UTC (permalink / raw)
  To: 谢致邦 (XIE Zhibang), linux-input
  Cc: bentiss, dianders, jikos, linux-kernel, superm1

Hi,

On 29-May-26 14:16, 谢致邦 (XIE Zhibang) wrote:
> Before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are
> separate modules"), the unified i2c-hid driver handled both PNP0C50 ACPI
> devices and hid-over-i2c OF devices. After the split, devices with _HID
> "PRP0001" and _DSD compatible "hid-over-i2c" are only probed by
> i2c_hid_of, which requires "hid-descr-addr" in the _DSD. Some devices,
> for example the Lenovo KaiTian N60d and Inspur CP300L3, provide the HID
> descriptor address only through the _DSM method. Fall back to the _DSM
> call when the property is absent.
> 
> Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>

Thank you for the new patch, this is an interesting approach and better
then the modalias magic from the previous version.

Note I'm not the i2c-hid maintainer, with that said I think this should
be acceptable. But currently it duplicates the _DSM handling code and
that should be fixed.

I think this should be changed to a series of 3 patches:

1. Move the i2c_hid_acpi_blacklist handling out of
   i2c_hid_acpi_get_descriptor() into i2c_hid_acpi_probe()
   to above the devm_kzalloc() call.

2. Move i2c_hid_acpi_get_descriptor() to a generic
   int i2c_hid_acpi_get_descriptor(struct device *dev)
   helper in drivers/hid/i2c-hid/i2c-hid-core.c .
   Wrapped in #ifdef CONFIG_ACPI and with a static inline
   stub in drivers/hid/i2c-hid/i2c-hid.h when CONFIG_ACPI
   is not set, e.g. in drivers/hid/i2c-hid/i2c-hid.h add:

#ifdef CONFIG_ACPI
	int i2c_hid_acpi_get_descriptor_address(struct device *dev);
#else
	static inline int i2c_hid_acpi_get_descriptor_address(struct device *dev)
	{
		return -ENODEV;
	}
#endif

3. Modify i2c-hid-of.c to try i2c_hid_acpi_get_descriptor_address() as
fallback for the missing "hid-descr-addr" property.  Please also add
a comment in the code explaining that this fallback is about ACPI I2C-hid
devices which use a "PRP0001" ACPI _HID with an "hid-over-i2c" compatible.

Regards,

Hans



> ---
>  drivers/hid/i2c-hid/i2c-hid-of.c | 44 ++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
> index 57379b77e977..62c089a6455a 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of.c
> @@ -19,6 +19,7 @@
>   * more details.
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/gpio/consumer.h>
> @@ -74,6 +75,39 @@ static void i2c_hid_of_power_down(struct i2chid_ops *ops)
>  			       ihid_of->supplies);
>  }
>  
> +#ifdef CONFIG_ACPI
> +/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
> +static guid_t i2c_hid_of_acpi_guid =
> +	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
> +		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
> +
> +/*
> + * Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
> + * declare their I2C HID touchpad with _HID "PRP0001" and _DSD compatible
> + * "hid-over-i2c" but lack the "hid-descr-addr" property. Use the _DSM
> + * method to obtain the HID descriptor address.
> + */
> +static int i2c_hid_of_acpi_get_descriptor(struct device *dev)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(dev);
> +	union acpi_object *obj;
> +	u16 addr;
> +
> +	if (!adev)
> +		return -ENODEV;
> +
> +	obj = acpi_evaluate_dsm_typed(acpi_device_handle(adev),
> +				      &i2c_hid_of_acpi_guid, 1, 1, NULL,
> +				      ACPI_TYPE_INTEGER);
> +	if (!obj)
> +		return -ENODEV;
> +
> +	addr = obj->integer.value;
> +	ACPI_FREE(obj);
> +	return addr;
> +}
> +#endif
> +
>  static int i2c_hid_of_probe(struct i2c_client *client)
>  {
>  	struct device *dev = &client->dev;
> @@ -92,6 +126,16 @@ static int i2c_hid_of_probe(struct i2c_client *client)
>  	ihid_of->ops.power_down = i2c_hid_of_power_down;
>  
>  	ret = device_property_read_u32(dev, "hid-descr-addr", &val);
> +#ifdef CONFIG_ACPI
> +	if (ret) {
> +		int dsm_ret = i2c_hid_of_acpi_get_descriptor(dev);
> +
> +		if (dsm_ret >= 0) {
> +			val = dsm_ret;
> +			ret = 0;
> +		}
> +	}
> +#endif
>  	if (ret) {
>  		dev_err(dev, "HID register address not provided\n");
>  		return -ENODEV;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
  2026-05-29 15:00     ` Hans de Goede
@ 2026-05-29 19:36       ` Dmitry Torokhov
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2026-05-29 19:36 UTC (permalink / raw)
  To: Hans de Goede
  Cc: 谢致邦 (XIE Zhibang), linux-input, bentiss,
	dianders, jikos, linux-kernel, superm1

On Fri, May 29, 2026 at 05:00:37PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 29-May-26 14:16, 谢致邦 (XIE Zhibang) wrote:
> > Before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are
> > separate modules"), the unified i2c-hid driver handled both PNP0C50 ACPI
> > devices and hid-over-i2c OF devices. After the split, devices with _HID
> > "PRP0001" and _DSD compatible "hid-over-i2c" are only probed by
> > i2c_hid_of, which requires "hid-descr-addr" in the _DSD. Some devices,
> > for example the Lenovo KaiTian N60d and Inspur CP300L3, provide the HID
> > descriptor address only through the _DSM method. Fall back to the _DSM
> > call when the property is absent.
> > 
> > Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
> > Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> 
> Thank you for the new patch, this is an interesting approach and better
> then the modalias magic from the previous version.
> 
> Note I'm not the i2c-hid maintainer, with that said I think this should
> be acceptable. But currently it duplicates the _DSM handling code and
> that should be fixed.
> 
> I think this should be changed to a series of 3 patches:
> 
> 1. Move the i2c_hid_acpi_blacklist handling out of
>    i2c_hid_acpi_get_descriptor() into i2c_hid_acpi_probe()
>    to above the devm_kzalloc() call.
> 
> 2. Move i2c_hid_acpi_get_descriptor() to a generic
>    int i2c_hid_acpi_get_descriptor(struct device *dev)
>    helper in drivers/hid/i2c-hid/i2c-hid-core.c .
>    Wrapped in #ifdef CONFIG_ACPI and with a static inline
>    stub in drivers/hid/i2c-hid/i2c-hid.h when CONFIG_ACPI
>    is not set, e.g. in drivers/hid/i2c-hid/i2c-hid.h add:
> 
> #ifdef CONFIG_ACPI
> 	int i2c_hid_acpi_get_descriptor_address(struct device *dev);
> #else
> 	static inline int i2c_hid_acpi_get_descriptor_address(struct device *dev)
> 	{
> 		return -ENODEV;
> 	}
> #endif
> 
> 3. Modify i2c-hid-of.c to try i2c_hid_acpi_get_descriptor_address() as
> fallback for the missing "hid-descr-addr" property.  Please also add
> a comment in the code explaining that this fallback is about ACPI I2C-hid
> devices which use a "PRP0001" ACPI _HID with an "hid-over-i2c" compatible.

I think we should also stick a big fat warning if _DSM succeeds in that
code branch: hopefully manufacturers will notice and fix the firmware on
new devices.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-05-29 19:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-27 15:17 [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias 谢致邦 (XIE Zhibang)
2026-05-27 15:44 ` Hans de Goede
2026-05-29 12:16   ` [PATCH] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing 谢致邦 (XIE Zhibang)
2026-05-29 15:00     ` Hans de Goede
2026-05-29 19:36       ` Dmitry Torokhov
2026-05-27 16:00 ` [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox