On Jun 03 2026, Hans de Goede wrote:

Hi Benjamin,

On 3-Jun-26 11:43 AM, Benjamin Tissoires wrote:

quoted

On Jun 01 2026, 谢致邦 (XIE Zhibang) wrote:

quoted

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) <redacted>
---
 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).

quoted

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