Re: [PATCH v2] HID: i2c-hid-acpi: restore touchscreen IRQ routing on HP OmniBook Flip laptops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid: i2c-hid-acpi: ACPI IRQ routing workaround bypassed on resume if wakeup enabled
--

commit 395f14993542089fc82210b5b3e47d5cf6164163
Author: Waqar Gulzar <iamwaqargulzar@gmail.com>

HID: i2c-hid-acpi: restore touchscreen IRQ routing on HP OmniBook Flip laptops

This patch works around a firmware bug on several HP OmniBook Flip laptops
where the touchscreen ACPI _ON method incorrectly disconnects the interrupt
from the IO-APIC. It attempts to re-enable the correct IRQ routing during
device initialization and resume using the i2c_hid_acpi driver's power_up
hook.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=220854
Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2142384
Link: https://github.com/testyfishy/hp-omnibook-flip16-touchscreen-fix
Link: https://github.com/iamwaqargulzar/hp-omnibook-x-flip-16-linux-touchscreen-pen-fix

> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> index 9371db200fc4d..b307d9a3d0455 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c

[ ... ]

> @@ -147,6 +147,9 @@ static int i2c_hid_acpi_probe(struct i2c_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;
> +	if (acpi_dev_hid_uid_match(ihid_acpi->adev, "ELAN2514", NULL) &&
> +	    dmi_check_system(i2c_hid_acpi_irq_route_dmi))
> +		ihid_acpi->ops.power_up = i2c_hid_acpi_restore_irq_route;

[Severity: High]
Does this workaround get bypassed on system resume if the device is
configured as a wakeup source?

Looking at i2c_hid_core_resume() in drivers/hid/i2c-hid/i2c-hid-core.c,
the power_up hook is skipped if the device may wake the system:

drivers/hid/i2c-hid/i2c-hid-core.c:i2c_hid_core_resume() {
    ...
	if (!device_may_wakeup(&client->dev))
		i2c_hid_core_power_up(ihid);
    ...
}

If the user or system configures the touchscreen to wake the system from
sleep, the ACPI _ON method will run during the transition back to D0 and
disconnect the IRQ routing, but the power_up hook won't be called to
restore it.

Could this cause the interrupt storm to return immediately upon enabling
the IRQ on system resume?

>  
>  	acpi_device_fix_up_power(adev);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-b4-elan2514-irq-route-v2-1-432d95eb7348@gmail.com?part=1