HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing

[PATCH] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing

Posted by 谢致邦 (XIE Zhibang) 1 week, 2 days ago

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

Re: [PATCH] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing

Posted by Hans de Goede 1 week, 2 days ago

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;

Re: [PATCH] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing

Posted by Dmitry Torokhov 1 week, 2 days ago

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

[PATCH v2 0/3] HID: i2c-hid: Fix some PRP0001 touchpads probe after OF/ACPI split

Posted by 谢致邦 (XIE Zhibang) 6 days, 16 hours ago

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 and thus fail to probe.

Patch 1 moves the blacklist check so the function can return early
without wasting an allocation.

Patch 2 moves the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
i2c-hid-acpi.c and i2c-hid-of.c can use it.

Patch 3 calls the common helper as a fallback when "hid-descr-addr" is
missing, and sets safe post-power-on and post-reset-deassert delays.

谢致邦 (XIE Zhibang) (3):
  HID: i2c-hid-acpi: Move blacklist check to probe() before
    devm_kzalloc()
  HID: i2c-hid: Move common ACPI _DSM helper into core
  HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing

 drivers/hid/i2c-hid/i2c-hid-acpi.c | 41 +++++++-----------------------
 drivers/hid/i2c-hid/i2c-hid-core.c | 35 +++++++++++++++++++++++++
 drivers/hid/i2c-hid/i2c-hid-of.c   | 30 ++++++++++++++++++++++
 drivers/hid/i2c-hid/i2c-hid.h      | 11 ++++++++
 4 files changed, 85 insertions(+), 32 deletions(-)

-- 
2.54.0

[PATCH 0/3] HID: i2c-hid: Fix some PRP0001 touchpads probe after OF/ACPI split

Posted by 谢致邦 (XIE Zhibang) 6 days, 17 hours ago

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 and thus fail to probe.

Patch 1 moves the blacklist check so the function can return early
without wasting an allocation.

Patch 2 moves the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
i2c-hid-acpi.c and i2c-hid-of.c can use it.

Patch 3 calls the common helper as a fallback when "hid-descr-addr" is
missing, and sets safe post-power-on and post-reset-deassert delays.

谢致邦 (XIE Zhibang) (3):
  HID: i2c-hid-acpi: Move blacklist check to probe() before
    devm_kzalloc()
  HID: i2c-hid: Move common ACPI _DSM helper into core
  HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing

 drivers/hid/i2c-hid/i2c-hid-acpi.c | 42 +++++++-----------------------
 drivers/hid/i2c-hid/i2c-hid-core.c | 35 +++++++++++++++++++++++++
 drivers/hid/i2c-hid/i2c-hid-of.c   | 30 +++++++++++++++++++++
 drivers/hid/i2c-hid/i2c-hid.h      | 11 ++++++++
 4 files changed, 86 insertions(+), 32 deletions(-)

-- 
2.54.0