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

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

[谢致邦 (XIE Zhibang)]

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

Re: [PATCH] HID: i2c-hid-acpi: Add PRP0001 to match table and OF alias

[Hans de Goede]

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",

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

[谢致邦 (XIE Zhibang)]

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

[Hans de Goede]

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

[Dmitry Torokhov]

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 0/3] HID: i2c-hid: Fix some PRP0001 touchpads probe after OF/ACPI split

[谢致邦 (XIE Zhibang)]

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

[PATCH 1/3] HID: i2c-hid-acpi: Move blacklist check to probe() before devm_kzalloc()

[谢致邦 (XIE Zhibang)]

Move the check so the function can return early without wasting an
allocation. This is a pure refactoring, no functional change.

Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
 drivers/hid/i2c-hid/i2c-hid-acpi.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index abd700a101f4..acfe0fb9e99a 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -60,9 +60,6 @@ static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
 	union acpi_object *obj;
 	u16 hid_descriptor_address;
 
-	if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
-		return -ENODEV;
-
 	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
 				      ACPI_TYPE_INTEGER);
 	if (!obj) {
@@ -93,15 +90,20 @@ static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
 static int i2c_hid_acpi_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
+	struct acpi_device *adev;
 	struct i2c_hid_acpi *ihid_acpi;
 	u16 hid_descriptor_address;
 	int ret;
 
+	adev = ACPI_COMPANION(dev);
+	if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
+		return -ENODEV;
+
 	ihid_acpi = devm_kzalloc(&client->dev, sizeof(*ihid_acpi), GFP_KERNEL);
 	if (!ihid_acpi)
 		return -ENOMEM;
 
-	ihid_acpi->adev = ACPI_COMPANION(dev);
+	ihid_acpi->adev = adev;
 	ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
 	ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
 
-- 
2.54.0

[PATCH 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core

[谢致邦 (XIE Zhibang)]

Move 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.

Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
 drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
 drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
 drivers/hid/i2c-hid/i2c-hid.h      | 11 ++++++++++
 3 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index acfe0fb9e99a..bd888134f4cd 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -25,12 +25,12 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pm.h>
-#include <linux/uuid.h>
 
 #include "i2c-hid.h"
 
 struct i2c_hid_acpi {
 	struct i2chid_ops ops;
+	struct i2c_client *client;
 	struct acpi_device *adev;
 };
 
@@ -48,36 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
 	{ }
 };
 
-/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
-static guid_t i2c_hid_guid =
-	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
-		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
-
-static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
-{
-	struct acpi_device *adev = ihid_acpi->adev;
-	acpi_handle handle = acpi_device_handle(adev);
-	union acpi_object *obj;
-	u16 hid_descriptor_address;
-
-	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
-				      ACPI_TYPE_INTEGER);
-	if (!obj) {
-		acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
-		return -ENODEV;
-	}
-
-	hid_descriptor_address = obj->integer.value;
-	ACPI_FREE(obj);
-
-	return hid_descriptor_address;
-}
-
 static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
 {
 	struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
 
-	i2c_hid_acpi_get_descriptor(ihid_acpi);
+	i2c_hid_core_acpi_get_descriptor(&ihid_acpi->client->dev);
 }
 
 static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
@@ -103,11 +78,12 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
 	if (!ihid_acpi)
 		return -ENOMEM;
 
+	ihid_acpi->client = client;
 	ihid_acpi->adev = adev;
 	ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
 	ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
 
-	ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
+	ret = i2c_hid_core_acpi_get_descriptor(dev);
 	if (ret < 0)
 		return ret;
 	hid_descriptor_address = ret;
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 3adb16366e93..1e1a8df5686d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1405,6 +1405,41 @@ const struct dev_pm_ops i2c_hid_core_pm = {
 };
 EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
 
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+
+/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
+static guid_t i2c_hid_guid =
+	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
+		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
+
+int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	acpi_handle handle;
+	union acpi_object *obj;
+	u16 hid_descriptor_address;
+
+	if (!adev)
+		return -ENODEV;
+
+	handle = acpi_device_handle(adev);
+	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
+				      ACPI_TYPE_INTEGER);
+	if (!obj) {
+		acpi_handle_err(handle,
+				"Error _DSM call to get HID descriptor address failed\n");
+		return -ENODEV;
+	}
+
+	hid_descriptor_address = obj->integer.value;
+	ACPI_FREE(obj);
+
+	return hid_descriptor_address;
+}
+EXPORT_SYMBOL_GPL(i2c_hid_core_acpi_get_descriptor);
+#endif
+
 MODULE_DESCRIPTION("HID over I2C core driver");
 MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
 MODULE_LICENSE("GPL");
diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
index 1724a435c783..bc8661c65b1a 100644
--- a/drivers/hid/i2c-hid/i2c-hid.h
+++ b/drivers/hid/i2c-hid/i2c-hid.h
@@ -44,4 +44,15 @@ void i2c_hid_core_shutdown(struct i2c_client *client);
 
 extern const struct dev_pm_ops i2c_hid_core_pm;
 
+#ifdef CONFIG_ACPI
+struct device;
+int i2c_hid_core_acpi_get_descriptor(struct device *dev);
+#else
+struct device;
+static inline int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+	return -ENODEV;
+}
+#endif
+
 #endif
-- 
2.54.0

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

[谢致邦 (XIE Zhibang)]

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. Call the common
i2c_hid_core_acpi_get_descriptor() helper as a fallback, and set safe
post-power-on and post-reset-deassert delays.

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 | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
index 57379b77e977..e925e2d2cfe0 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of.c
@@ -92,6 +92,36 @@ 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);
+	if (ret) {
+		/*
+		 * 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. Fall back to _DSM to obtain the
+		 * HID descriptor address.
+		 */
+		int dsm_ret = i2c_hid_core_acpi_get_descriptor(dev);
+
+		if (dsm_ret >= 0) {
+			dev_warn(dev,
+				 "hid-descr-addr NOT found, using _DSM fallback. Contact vendor for firmware update!\n");
+			val = dsm_ret;
+
+			/*
+			 * Firmware providing the descriptor address only
+			 * through _DSM may also lack "post-power-on-delay-ms"
+			 * or "post-reset-deassert-delay-ms", leaving the
+			 * driver without enough delay before the first HID
+			 * descriptor read. Set safe defaults to avoid reading
+			 * the descriptor before the device has finished its
+			 * internal power-on reset.
+			 */
+			ihid_of->post_power_delay_ms = 250;
+			ihid_of->post_reset_delay_ms = 250;
+
+			ret = 0;
+		}
+	}
 	if (ret) {
 		dev_err(dev, "HID register address not provided\n");
 		return -ENODEV;
-- 
2.54.0

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

[谢致邦 (XIE Zhibang)]

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 v2 1/3] HID: i2c-hid-acpi: Move blacklist check to probe() before devm_kzalloc()

[谢致邦 (XIE Zhibang)]

Move the check so the function can return early without wasting an
allocation. This is a pure refactoring, no functional change.

Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
v2: Merge declaration into assignment.

 drivers/hid/i2c-hid/i2c-hid-acpi.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index abd700a101f4..f65fb6396b69 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -60,9 +60,6 @@ static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
 	union acpi_object *obj;
 	u16 hid_descriptor_address;
 
-	if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
-		return -ENODEV;
-
 	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
 				      ACPI_TYPE_INTEGER);
 	if (!obj) {
@@ -93,15 +90,19 @@ static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
 static int i2c_hid_acpi_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
+	struct acpi_device *adev = ACPI_COMPANION(dev);
 	struct i2c_hid_acpi *ihid_acpi;
 	u16 hid_descriptor_address;
 	int ret;
 
+	if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
+		return -ENODEV;
+
 	ihid_acpi = devm_kzalloc(&client->dev, sizeof(*ihid_acpi), GFP_KERNEL);
 	if (!ihid_acpi)
 		return -ENOMEM;
 
-	ihid_acpi->adev = ACPI_COMPANION(dev);
+	ihid_acpi->adev = adev;
 	ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
 	ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
 
-- 
2.54.0

[PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core

[谢致邦 (XIE Zhibang)]

Move 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.

Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
 drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
 drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
 drivers/hid/i2c-hid/i2c-hid.h      | 11 ++++++++++
 3 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index f65fb6396b69..234789a07047 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -25,12 +25,12 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pm.h>
-#include <linux/uuid.h>
 
 #include "i2c-hid.h"
 
 struct i2c_hid_acpi {
 	struct i2chid_ops ops;
+	struct i2c_client *client;
 	struct acpi_device *adev;
 };
 
@@ -48,36 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
 	{ }
 };
 
-/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
-static guid_t i2c_hid_guid =
-	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
-		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
-
-static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
-{
-	struct acpi_device *adev = ihid_acpi->adev;
-	acpi_handle handle = acpi_device_handle(adev);
-	union acpi_object *obj;
-	u16 hid_descriptor_address;
-
-	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
-				      ACPI_TYPE_INTEGER);
-	if (!obj) {
-		acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
-		return -ENODEV;
-	}
-
-	hid_descriptor_address = obj->integer.value;
-	ACPI_FREE(obj);
-
-	return hid_descriptor_address;
-}
-
 static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
 {
 	struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
 
-	i2c_hid_acpi_get_descriptor(ihid_acpi);
+	i2c_hid_core_acpi_get_descriptor(&ihid_acpi->client->dev);
 }
 
 static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
@@ -102,11 +77,12 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
 	if (!ihid_acpi)
 		return -ENOMEM;
 
+	ihid_acpi->client = client;
 	ihid_acpi->adev = adev;
 	ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
 	ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
 
-	ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
+	ret = i2c_hid_core_acpi_get_descriptor(dev);
 	if (ret < 0)
 		return ret;
 	hid_descriptor_address = ret;
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 3adb16366e93..1e1a8df5686d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1405,6 +1405,41 @@ const struct dev_pm_ops i2c_hid_core_pm = {
 };
 EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
 
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+
+/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
+static guid_t i2c_hid_guid =
+	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
+		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
+
+int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	acpi_handle handle;
+	union acpi_object *obj;
+	u16 hid_descriptor_address;
+
+	if (!adev)
+		return -ENODEV;
+
+	handle = acpi_device_handle(adev);
+	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
+				      ACPI_TYPE_INTEGER);
+	if (!obj) {
+		acpi_handle_err(handle,
+				"Error _DSM call to get HID descriptor address failed\n");
+		return -ENODEV;
+	}
+
+	hid_descriptor_address = obj->integer.value;
+	ACPI_FREE(obj);
+
+	return hid_descriptor_address;
+}
+EXPORT_SYMBOL_GPL(i2c_hid_core_acpi_get_descriptor);
+#endif
+
 MODULE_DESCRIPTION("HID over I2C core driver");
 MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
 MODULE_LICENSE("GPL");
diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
index 1724a435c783..bc8661c65b1a 100644
--- a/drivers/hid/i2c-hid/i2c-hid.h
+++ b/drivers/hid/i2c-hid/i2c-hid.h
@@ -44,4 +44,15 @@ void i2c_hid_core_shutdown(struct i2c_client *client);
 
 extern const struct dev_pm_ops i2c_hid_core_pm;
 
+#ifdef CONFIG_ACPI
+struct device;
+int i2c_hid_core_acpi_get_descriptor(struct device *dev);
+#else
+struct device;
+static inline int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+	return -ENODEV;
+}
+#endif
+
 #endif
-- 
2.54.0

Re: [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core

[Benjamin Tissoires]

On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:
> Move 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.
> 
> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> ---
>  drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
>  drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++

I'm not a big fan of removing 25% of the i2c-hid-acpi.c code and inject
it in i2c-hid-core.c.

There's something I can't really understand in the problem:
- we are talking about non arm architecture, which should not have OF in
	them
- the current compatible hid-over-i2c loads the OF version?
- how can you make the device working without the couple of ACPI
	functions that are in the i2c-hid-acpi.c driver for powering up and
	down the device?

If the platform is indeed ACPI + x86(_64), then shouldn't we tackle the
problem in the i2c-hid-acpi driver, or add a secondary leaf driver for
this particular platform/device? Kind of what we have with
i2c-hid-of-elan.c

Cheers,
Benjamin

>  drivers/hid/i2c-hid/i2c-hid.h      | 11 ++++++++++
>  3 files changed, 50 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> index f65fb6396b69..234789a07047 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> @@ -25,12 +25,12 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/pm.h>
> -#include <linux/uuid.h>
>  
>  #include "i2c-hid.h"
>  
>  struct i2c_hid_acpi {
>  	struct i2chid_ops ops;
> +	struct i2c_client *client;
>  	struct acpi_device *adev;
>  };
>  
> @@ -48,36 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
>  	{ }
>  };
>  
> -/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
> -static guid_t i2c_hid_guid =
> -	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
> -		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
> -
> -static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
> -{
> -	struct acpi_device *adev = ihid_acpi->adev;
> -	acpi_handle handle = acpi_device_handle(adev);
> -	union acpi_object *obj;
> -	u16 hid_descriptor_address;
> -
> -	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
> -				      ACPI_TYPE_INTEGER);
> -	if (!obj) {
> -		acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
> -		return -ENODEV;
> -	}
> -
> -	hid_descriptor_address = obj->integer.value;
> -	ACPI_FREE(obj);
> -
> -	return hid_descriptor_address;
> -}
> -
>  static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
>  {
>  	struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
>  
> -	i2c_hid_acpi_get_descriptor(ihid_acpi);
> +	i2c_hid_core_acpi_get_descriptor(&ihid_acpi->client->dev);
>  }
>  
>  static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
> @@ -102,11 +77,12 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
>  	if (!ihid_acpi)
>  		return -ENOMEM;
>  
> +	ihid_acpi->client = client;
>  	ihid_acpi->adev = adev;
>  	ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
>  	ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
>  
> -	ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
> +	ret = i2c_hid_core_acpi_get_descriptor(dev);
>  	if (ret < 0)
>  		return ret;
>  	hid_descriptor_address = ret;
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 3adb16366e93..1e1a8df5686d 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1405,6 +1405,41 @@ const struct dev_pm_ops i2c_hid_core_pm = {
>  };
>  EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
>  
> +#ifdef CONFIG_ACPI
> +#include <linux/acpi.h>
> +
> +/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
> +static guid_t i2c_hid_guid =
> +	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
> +		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
> +
> +int i2c_hid_core_acpi_get_descriptor(struct device *dev)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(dev);
> +	acpi_handle handle;
> +	union acpi_object *obj;
> +	u16 hid_descriptor_address;
> +
> +	if (!adev)
> +		return -ENODEV;
> +
> +	handle = acpi_device_handle(adev);
> +	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
> +				      ACPI_TYPE_INTEGER);
> +	if (!obj) {
> +		acpi_handle_err(handle,
> +				"Error _DSM call to get HID descriptor address failed\n");
> +		return -ENODEV;
> +	}
> +
> +	hid_descriptor_address = obj->integer.value;
> +	ACPI_FREE(obj);
> +
> +	return hid_descriptor_address;
> +}
> +EXPORT_SYMBOL_GPL(i2c_hid_core_acpi_get_descriptor);
> +#endif
> +
>  MODULE_DESCRIPTION("HID over I2C core driver");
>  MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
> index 1724a435c783..bc8661c65b1a 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.h
> +++ b/drivers/hid/i2c-hid/i2c-hid.h
> @@ -44,4 +44,15 @@ void i2c_hid_core_shutdown(struct i2c_client *client);
>  
>  extern const struct dev_pm_ops i2c_hid_core_pm;
>  
> +#ifdef CONFIG_ACPI
> +struct device;
> +int i2c_hid_core_acpi_get_descriptor(struct device *dev);
> +#else
> +struct device;
> +static inline int i2c_hid_core_acpi_get_descriptor(struct device *dev)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
>  #endif
> -- 
> 2.54.0
> 
> 

Re: [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core

[Hans de Goede]

Hi Benjamin,

On 3-Jun-26 11:43 AM, Benjamin Tissoires wrote:
> On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:
>> Move 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.
>>
>> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
>> ---
>>  drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
>>  drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
> 
> I'm not a big fan of removing 25% of the i2c-hid-acpi.c code and inject
> it in i2c-hid-core.c.

It is only 35 new lines in i2c-hid-core.c, so it is not that big / much code.

> There's something I can't really understand in the problem:
> - we are talking about non arm architecture, which should not have OF in
> 	them
> - the current compatible hid-over-i2c loads the OF version?
> - how can you make the device working without the couple of ACPI
> 	functions that are in the i2c-hid-acpi.c driver for powering up and
> 	down the device?
> 
> If the platform is indeed ACPI + x86(_64), then shouldn't we tackle the
> problem in the i2c-hid-acpi driver, or add a secondary leaf driver for
> this particular platform/device? Kind of what we have with
> i2c-hid-of-elan.c

As part of the work to use ACPI on ARM systems, it is possible now a days
to inject DT snippets into ACPI tables.

The problem on these Loongson laptops is that they have created this
very ugly mixed-mode where they put a PRP0001 ACPI HID on the ACPI
node for the touchscreen, which means it contains an embedded DT
snippet and in that snipped out "compatible=hid-over-i2c" but NOT
also "i2c-hid-descr-addr=x". To get the i2c-hid descriptor address
the implemented the ACPI _DSM on the same ACPI device/fwnode.

The ACPI core sees HID=PRP0001 so it creates an of-compatible 
modalias / match and not an ACPI one, so the i2c-hid-of driver will
bind.

Note these "fixed" (as in we cannot fix them) ACPI tables use
the generic i2c-over-hid OF/DT compatible _not_ something
more specific so we can also not do a more specific driver like
i2c-hid-of-elan.c. If we could fix the ACPI tables we would just
change the ACPI HID to the standard PNPxxxx ACPI HID for i2c-hid
devices.

The first version of this patch-set added an of match table
to i2c-hid-acpi making it also load and probe i2c-hid devices
described in devicetree on devicetree only systems which IMHO
is very messy.

So this new series is the least ugly way to deal with this.

Regards,

Hans






> 
> Cheers,
> Benjamin
> 
>>  drivers/hid/i2c-hid/i2c-hid.h      | 11 ++++++++++
>>  3 files changed, 50 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
>> index f65fb6396b69..234789a07047 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
>> @@ -25,12 +25,12 @@
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/pm.h>
>> -#include <linux/uuid.h>
>>  
>>  #include "i2c-hid.h"
>>  
>>  struct i2c_hid_acpi {
>>  	struct i2chid_ops ops;
>> +	struct i2c_client *client;
>>  	struct acpi_device *adev;
>>  };
>>  
>> @@ -48,36 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
>>  	{ }
>>  };
>>  
>> -/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
>> -static guid_t i2c_hid_guid =
>> -	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
>> -		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
>> -
>> -static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
>> -{
>> -	struct acpi_device *adev = ihid_acpi->adev;
>> -	acpi_handle handle = acpi_device_handle(adev);
>> -	union acpi_object *obj;
>> -	u16 hid_descriptor_address;
>> -
>> -	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
>> -				      ACPI_TYPE_INTEGER);
>> -	if (!obj) {
>> -		acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
>> -		return -ENODEV;
>> -	}
>> -
>> -	hid_descriptor_address = obj->integer.value;
>> -	ACPI_FREE(obj);
>> -
>> -	return hid_descriptor_address;
>> -}
>> -
>>  static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
>>  {
>>  	struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
>>  
>> -	i2c_hid_acpi_get_descriptor(ihid_acpi);
>> +	i2c_hid_core_acpi_get_descriptor(&ihid_acpi->client->dev);
>>  }
>>  
>>  static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
>> @@ -102,11 +77,12 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
>>  	if (!ihid_acpi)
>>  		return -ENOMEM;
>>  
>> +	ihid_acpi->client = client;
>>  	ihid_acpi->adev = adev;
>>  	ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
>>  	ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
>>  
>> -	ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
>> +	ret = i2c_hid_core_acpi_get_descriptor(dev);
>>  	if (ret < 0)
>>  		return ret;
>>  	hid_descriptor_address = ret;
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>> index 3adb16366e93..1e1a8df5686d 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>> @@ -1405,6 +1405,41 @@ const struct dev_pm_ops i2c_hid_core_pm = {
>>  };
>>  EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
>>  
>> +#ifdef CONFIG_ACPI
>> +#include <linux/acpi.h>
>> +
>> +/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
>> +static guid_t i2c_hid_guid =
>> +	GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
>> +		  0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
>> +
>> +int i2c_hid_core_acpi_get_descriptor(struct device *dev)
>> +{
>> +	struct acpi_device *adev = ACPI_COMPANION(dev);
>> +	acpi_handle handle;
>> +	union acpi_object *obj;
>> +	u16 hid_descriptor_address;
>> +
>> +	if (!adev)
>> +		return -ENODEV;
>> +
>> +	handle = acpi_device_handle(adev);
>> +	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
>> +				      ACPI_TYPE_INTEGER);
>> +	if (!obj) {
>> +		acpi_handle_err(handle,
>> +				"Error _DSM call to get HID descriptor address failed\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	hid_descriptor_address = obj->integer.value;
>> +	ACPI_FREE(obj);
>> +
>> +	return hid_descriptor_address;
>> +}
>> +EXPORT_SYMBOL_GPL(i2c_hid_core_acpi_get_descriptor);
>> +#endif
>> +
>>  MODULE_DESCRIPTION("HID over I2C core driver");
>>  MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
>>  MODULE_LICENSE("GPL");
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
>> index 1724a435c783..bc8661c65b1a 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.h
>> +++ b/drivers/hid/i2c-hid/i2c-hid.h
>> @@ -44,4 +44,15 @@ void i2c_hid_core_shutdown(struct i2c_client *client);
>>  
>>  extern const struct dev_pm_ops i2c_hid_core_pm;
>>  
>> +#ifdef CONFIG_ACPI
>> +struct device;
>> +int i2c_hid_core_acpi_get_descriptor(struct device *dev);
>> +#else
>> +struct device;
>> +static inline int i2c_hid_core_acpi_get_descriptor(struct device *dev)
>> +{
>> +	return -ENODEV;
>> +}
>> +#endif
>> +
>>  #endif
>> -- 
>> 2.54.0
>>
>>

Re: [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core

[Benjamin Tissoires]

On Jun 03 2026, Hans de Goede wrote:
> Hi Benjamin,
> 
> On 3-Jun-26 11:43 AM, Benjamin Tissoires wrote:
> > On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:
> >> Move 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.
> >>
> >> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> >> ---
> >>  drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
> >>  drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
> > 
> > I'm not a big fan of removing 25% of the i2c-hid-acpi.c code and inject
> > it in i2c-hid-core.c.
> 
> It is only 35 new lines in i2c-hid-core.c, so it is not that big / much code.

My concern is not so much about these 35 lines of code, but the fact
that i2c-hid-acpi.c is now down to only 3 functions:
- i2c_hid_acpi_probe()
- i2c_hid_acpi_shutdown_tail()
- i2c_hid_acpi_restore_sequence()

So then, what's the point of keeping it as a separate driver? (more
rethorical question than anything).

> 
> > There's something I can't really understand in the problem:
> > - we are talking about non arm architecture, which should not have OF in
> > 	them
> > - the current compatible hid-over-i2c loads the OF version?
> > - how can you make the device working without the couple of ACPI
> > 	functions that are in the i2c-hid-acpi.c driver for powering up and
> > 	down the device?
> > 
> > If the platform is indeed ACPI + x86(_64), then shouldn't we tackle the
> > problem in the i2c-hid-acpi driver, or add a secondary leaf driver for
> > this particular platform/device? Kind of what we have with
> > i2c-hid-of-elan.c
> 
> As part of the work to use ACPI on ARM systems, it is possible now a days
> to inject DT snippets into ACPI tables.
> 
> The problem on these Loongson laptops is that they have created this
> very ugly mixed-mode where they put a PRP0001 ACPI HID on the ACPI
> node for the touchscreen, which means it contains an embedded DT
> snippet and in that snipped out "compatible=hid-over-i2c" but NOT
> also "i2c-hid-descr-addr=x". To get the i2c-hid descriptor address
> the implemented the ACPI _DSM on the same ACPI device/fwnode.
> 
> The ACPI core sees HID=PRP0001 so it creates an of-compatible 
> modalias / match and not an ACPI one, so the i2c-hid-of driver will
> bind.

But if this fix is for just one platform, why not keeping the design
clean and have a dmi_match instead in a separate driver, like the
i2c-hid-of-elan does for Elan touchpads/touchscreens?
i2c-hid-acpi-loongson.c for instance.

> 
> Note these "fixed" (as in we cannot fix them) ACPI tables use
> the generic i2c-over-hid OF/DT compatible _not_ something
> more specific so we can also not do a more specific driver like
> i2c-hid-of-elan.c.

Do they use anything else than the compatible DT entry? regulators and
others? If not, then the device is pure ACPI, and should not be handled
by i2c-hid-of.c at all, no?

> If we could fix the ACPI tables we would just
> change the ACPI HID to the standard PNPxxxx ACPI HID for i2c-hid
> devices.
> 
> The first version of this patch-set added an of match table
> to i2c-hid-acpi making it also load and probe i2c-hid devices
> described in devicetree on devicetree only systems which IMHO
> is very messy.

Yeah, this one is slightly less messy than the previous versions. It is
just sad to mess up with a clean separation and make the i2c-hid-acpi
driver just a placeholder for a probe function.

> 
> So this new series is the least ugly way to deal with this.

so far :)

Cheers,
Benjamin

Re: [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core

[Hans de Goede]

Hi,

On 3-Jun-26 1:59 PM, Benjamin Tissoires wrote:
> On Jun 03 2026, Hans de Goede wrote:
>> Hi Benjamin,
>>
>> On 3-Jun-26 11:43 AM, Benjamin Tissoires wrote:
>>> On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:
>>>> Move 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.
>>>>
>>>> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
>>>> ---
>>>>  drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
>>>>  drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
>>>
>>> I'm not a big fan of removing 25% of the i2c-hid-acpi.c code and inject
>>> it in i2c-hid-core.c.
>>
>> It is only 35 new lines in i2c-hid-core.c, so it is not that big / much code.
> 
> My concern is not so much about these 35 lines of code, but the fact
> that i2c-hid-acpi.c is now down to only 3 functions:
> - i2c_hid_acpi_probe()
> - i2c_hid_acpi_shutdown_tail()
> - i2c_hid_acpi_restore_sequence()
> 
> So then, what's the point of keeping it as a separate driver? (more
> rethorical question than anything).
> 
>>
>>> There's something I can't really understand in the problem:
>>> - we are talking about non arm architecture, which should not have OF in
>>> 	them
>>> - the current compatible hid-over-i2c loads the OF version?
>>> - how can you make the device working without the couple of ACPI
>>> 	functions that are in the i2c-hid-acpi.c driver for powering up and
>>> 	down the device?
>>>
>>> If the platform is indeed ACPI + x86(_64), then shouldn't we tackle the
>>> problem in the i2c-hid-acpi driver, or add a secondary leaf driver for
>>> this particular platform/device? Kind of what we have with
>>> i2c-hid-of-elan.c
>>
>> As part of the work to use ACPI on ARM systems, it is possible now a days
>> to inject DT snippets into ACPI tables.
>>
>> The problem on these Loongson laptops is that they have created this
>> very ugly mixed-mode where they put a PRP0001 ACPI HID on the ACPI
>> node for the touchscreen, which means it contains an embedded DT
>> snippet and in that snipped out "compatible=hid-over-i2c" but NOT
>> also "i2c-hid-descr-addr=x". To get the i2c-hid descriptor address
>> the implemented the ACPI _DSM on the same ACPI device/fwnode.
>>
>> The ACPI core sees HID=PRP0001 so it creates an of-compatible 
>> modalias / match and not an ACPI one, so the i2c-hid-of driver will
>> bind.
> 
> But if this fix is for just one platform, why not keeping the design
> clean and have a dmi_match instead in a separate driver, like the
> i2c-hid-of-elan does for Elan touchpads/touchscreens?
> i2c-hid-acpi-loongson.c for instance.

Ok, so let me see if I've got this right:

1. We create a i2c-hid-acpi-loongson.c with a DMI
MODULE_DEVICE_TABLE() for auto-module-loading

2. This will have a struct of_device_id table with
the "hid-over-i2c" compatible. But *no* / without a
MODULE_DEVICE_TABLE() for this so that it will not
autoload on regular DT platforms with regular
"hid-over-i2c" compatible touchscreens.

3. We will rely on i2c-hid-of.c error-ing out
due to the missing "hid-descr-addr" and we are ok
with the dev_err() this logs.

4. The new i2c-hid-acpi-loongson.c will in essence
be a copy of i2c-hid-acpi.c without the acpi_match
and with the DMI id + of match as described above.

Yes I think that should work, do you want to just copy
i2c_hid_acpi_get_descriptor() or do you want
i2c-hid-acpi to export this and have i2c-hid-acpi-longsoon
depend on i2c-hid-acpi for this ?

>> Note these "fixed" (as in we cannot fix them) ACPI tables use
>> the generic i2c-over-hid OF/DT compatible _not_ something
>> more specific so we can also not do a more specific driver like
>> i2c-hid-of-elan.c.
> 
> Do they use anything else than the compatible DT entry? regulators and
> others? If not, then the device is pure ACPI, and should not be handled
> by i2c-hid-of.c at all, no?

No they don't use anything other then the compatible. This is
really just a very bad / messy decision by whomever created
the ACPI tables for these 2 laptops. But they are out there, so
we somehow have to deal with them.

<snip>

> so far :)

True, see above.

Regards,

Hans

Re: [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core

[Benjamin Tissoires]

On Jun 03 2026, Hans de Goede wrote:
> Hi,
> 
> On 3-Jun-26 1:59 PM, Benjamin Tissoires wrote:
> > On Jun 03 2026, Hans de Goede wrote:
> >> Hi Benjamin,
> >>
> >> On 3-Jun-26 11:43 AM, Benjamin Tissoires wrote:
> >>> On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:
> >>>> Move 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.
> >>>>
> >>>> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> >>>> ---
> >>>>  drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
> >>>>  drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
> >>>
> >>> I'm not a big fan of removing 25% of the i2c-hid-acpi.c code and inject
> >>> it in i2c-hid-core.c.
> >>
> >> It is only 35 new lines in i2c-hid-core.c, so it is not that big / much code.
> > 
> > My concern is not so much about these 35 lines of code, but the fact
> > that i2c-hid-acpi.c is now down to only 3 functions:
> > - i2c_hid_acpi_probe()
> > - i2c_hid_acpi_shutdown_tail()
> > - i2c_hid_acpi_restore_sequence()
> > 
> > So then, what's the point of keeping it as a separate driver? (more
> > rethorical question than anything).
> > 
> >>
> >>> There's something I can't really understand in the problem:
> >>> - we are talking about non arm architecture, which should not have OF in
> >>> 	them
> >>> - the current compatible hid-over-i2c loads the OF version?
> >>> - how can you make the device working without the couple of ACPI
> >>> 	functions that are in the i2c-hid-acpi.c driver for powering up and
> >>> 	down the device?
> >>>
> >>> If the platform is indeed ACPI + x86(_64), then shouldn't we tackle the
> >>> problem in the i2c-hid-acpi driver, or add a secondary leaf driver for
> >>> this particular platform/device? Kind of what we have with
> >>> i2c-hid-of-elan.c
> >>
> >> As part of the work to use ACPI on ARM systems, it is possible now a days
> >> to inject DT snippets into ACPI tables.
> >>
> >> The problem on these Loongson laptops is that they have created this
> >> very ugly mixed-mode where they put a PRP0001 ACPI HID on the ACPI
> >> node for the touchscreen, which means it contains an embedded DT
> >> snippet and in that snipped out "compatible=hid-over-i2c" but NOT
> >> also "i2c-hid-descr-addr=x". To get the i2c-hid descriptor address
> >> the implemented the ACPI _DSM on the same ACPI device/fwnode.
> >>
> >> The ACPI core sees HID=PRP0001 so it creates an of-compatible 
> >> modalias / match and not an ACPI one, so the i2c-hid-of driver will
> >> bind.
> > 
> > But if this fix is for just one platform, why not keeping the design
> > clean and have a dmi_match instead in a separate driver, like the
> > i2c-hid-of-elan does for Elan touchpads/touchscreens?
> > i2c-hid-acpi-loongson.c for instance.
> 
> Ok, so let me see if I've got this right:
> 
> 1. We create a i2c-hid-acpi-loongson.c with a DMI
> MODULE_DEVICE_TABLE() for auto-module-loading

Yes

> 
> 2. This will have a struct of_device_id table with
> the "hid-over-i2c" compatible. But *no* / without a
> MODULE_DEVICE_TABLE() for this so that it will not
> autoload on regular DT platforms with regular
> "hid-over-i2c" compatible touchscreens.

Yes

> 
> 3. We will rely on i2c-hid-of.c error-ing out
> due to the missing "hid-descr-addr" and we are ok
> with the dev_err() this logs.

Yes, if we want the big fat warning that Dmitry was mentioning.
Otherwise we could also have a deny list table, but that's more work :)

> 
> 4. The new i2c-hid-acpi-loongson.c will in essence
> be a copy of i2c-hid-acpi.c without the acpi_match
> and with the DMI id + of match as described above.

Sort of, yeah

> 
> Yes I think that should work, do you want to just copy
> i2c_hid_acpi_get_descriptor() or do you want
> i2c-hid-acpi to export this and have i2c-hid-acpi-longsoon
> depend on i2c-hid-acpi for this ?

Export i2c_hid_acpi_get_descriptor() from i2c-hid-acpi and have
i2c-hid-acpi-longsoon depend on it.

Also, this would have the good side effect of being able to set the post
power and post reset delays based on the DMI. Kind of like a compatible
driver in the DT world.

> 
> >> Note these "fixed" (as in we cannot fix them) ACPI tables use
> >> the generic i2c-over-hid OF/DT compatible _not_ something
> >> more specific so we can also not do a more specific driver like
> >> i2c-hid-of-elan.c.
> > 
> > Do they use anything else than the compatible DT entry? regulators and
> > others? If not, then the device is pure ACPI, and should not be handled
> > by i2c-hid-of.c at all, no?
> 
> No they don't use anything other then the compatible. This is
> really just a very bad / messy decision by whomever created
> the ACPI tables for these 2 laptops. But they are out there, so
> we somehow have to deal with them.

Yeah, so I think we better have this special driver for the Loongson
platform, and have device crap handled in this place without polluting
the rest.

Cheers,
Benjamin

> 
> <snip>
> 
> > so far :)
> 
> True, see above.
> 
> Regards,
> 
> Hans
> 
> 
> 
> 

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

[谢致邦 (XIE Zhibang)]

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. Call the common
i2c_hid_core_acpi_get_descriptor() helper as a fallback, and set safe
post-power-on and post-reset-deassert delays.

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 | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
index 57379b77e977..e925e2d2cfe0 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of.c
@@ -92,6 +92,36 @@ 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);
+	if (ret) {
+		/*
+		 * 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. Fall back to _DSM to obtain the
+		 * HID descriptor address.
+		 */
+		int dsm_ret = i2c_hid_core_acpi_get_descriptor(dev);
+
+		if (dsm_ret >= 0) {
+			dev_warn(dev,
+				 "hid-descr-addr NOT found, using _DSM fallback. Contact vendor for firmware update!\n");
+			val = dsm_ret;
+
+			/*
+			 * Firmware providing the descriptor address only
+			 * through _DSM may also lack "post-power-on-delay-ms"
+			 * or "post-reset-deassert-delay-ms", leaving the
+			 * driver without enough delay before the first HID
+			 * descriptor read. Set safe defaults to avoid reading
+			 * the descriptor before the device has finished its
+			 * internal power-on reset.
+			 */
+			ihid_of->post_power_delay_ms = 250;
+			ihid_of->post_reset_delay_ms = 250;
+
+			ret = 0;
+		}
+	}
 	if (ret) {
 		dev_err(dev, "HID register address not provided\n");
 		return -ENODEV;
-- 
2.54.0