Re: [Bug 209627] Touchscreen doesn't work anymore since commit 21653a4181ff292480599dad996a2b759ccf050f (regression)

[Hans de Goede <hdegoede@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2667 bytes --]

Hi,

On 10/13/20 5:45 PM, Hans de Goede wrote:
> Hi,
> 
> On 10/12/20 9:52 PM, Rainer Finke wrote:
>> After upgrading from Linux 5.4.68 to Linux 5.4.69, the touchscreen of my Huawei Matebook 12 doesn't work anymore. The same issue happens with Linux >= 5.8.13.
>>
>> I've compiled Linux from git to verify if it was fixed, but it doesn't help. But when reverting the commit 21653a4181ff292480599dad996a2b759ccf050f the touchscreen works fine again.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v5.8.13&id=953fc770d069b167266d9d9ccfef0455fcfdc070
>>
>> For reference my bug reports:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=209627
>>
>> https://bugs.archlinux.org/task/68178#comment193400
>>
>>
>> Hardware:
>>
>> - CPU Intel Core m3-6Y30
>>
>> - GPU Intel Graphics 515
> 
> Thank you for your bug report and I'm sorry to hear about this problem.
> 
> The commit in question fixes the touchpad not working on several
> recent Lenovo models. What it does it makes the method (opregion) to
> access the i2c bus from ACPI code available to the ACPI code before
> calling the status method of ACPI devices on that i2c-bus.
> This status method tells us if the device is actually present or not
> and on those Thinkpads the status method did an i2c check, so we
> needed to register the i2c opregion before checking for new devices.
> 
> Registering the i2c opregion earlier seemed like an obvious
> solution, but I was already afraid we would hit an issue on some
> device because of this, because of ACPI being ACPI.
> 
> It seems that the ACPI status method for your device probably
> also does something with the i2c bus when the i2c opregion
> is available, but for some reason that is not working...
> 
> The next step in debugging this would be to take a look at
> the ACPI tables for your device, can you please run:
> 
> sudo acpidump -o acpidump.Huawei-Matebook-12
> 
> And then send out an email with the generated
> acpidump.Huawei-Matebook-12 file attached?
> 
> Note please drop the list from the Cc when sending the
> email with the attachment.
> 
> What would also be useful (for a possible workaround) would
> be the output of:
> 
> grep . /sys/class/dmi/id/* 2> /dev/null
> 
> Please run this as a normal user (*) and copy and paste
> the output into your next email.

In the mean time another issue triggered by the same
commit has shown up on Microsoft Surface Go models.

With te help fo the reporters of that issue I have found
on issue with that commit.

Can you please try a kernel with the troublesome commit
*included* with the attached patch added on top and see
if that fixes things?

Regards,

Hans

[-- Attachment #2: 0001-i2c-core-Restore-acpi_walk_dep_device_list-getting-c.patch --]
[-- Type: text/x-patch, Size: 2468 bytes --]

From 3f52f91e8b7a11f96c815ea9c86632b797bdfe97 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Wed, 14 Oct 2020 13:59:06 +0200
Subject: [PATCH] i2c: core: Restore acpi_walk_dep_device_list() getting called
 after registering the ACPI i2c devs

Commit 21653a4181ff ("i2c: core: Call i2c_acpi_install_space_handler()
before i2c_acpi_register_devices()")'s intention was to only move the
acpi_install_address_space_handler() call to the point before where
the ACPI declared i2c-children of the adapter where instantiated by
i2c_acpi_register_devices().

But i2c_acpi_install_space_handler() had a call to
acpi_walk_dep_device_list() hidden (that is I missed it) at the end
of it, so as an unwanted side-effect now acpi_walk_dep_device_list()
was also being called before i2c_acpi_register_devices().

Move the acpi_walk_dep_device_list() call to the end of
i2c_acpi_register_devices(), so that it is once again called *after*
the i2c_client-s hanging of the adapter have been created.

This fixes the Microsoft Surface Go 2 hanging at boot.

Fixes: 21653a4181ff ("i2c: core: Call i2c_acpi_install_space_handler() before i2c_acpi_register_devices()")
Suggested-by: Maximilian Luz <luzmaximilian@gmail.com>
Reported-and-tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/i2c/i2c-core-acpi.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index e627d7b2790f..37c510d9347a 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -264,6 +264,7 @@ static acpi_status i2c_acpi_add_device(acpi_handle handle, u32 level,
 void i2c_acpi_register_devices(struct i2c_adapter *adap)
 {
 	acpi_status status;
+	acpi_handle handle;
 
 	if (!has_acpi_companion(&adap->dev))
 		return;
@@ -274,6 +275,15 @@ void i2c_acpi_register_devices(struct i2c_adapter *adap)
 				     adap, NULL);
 	if (ACPI_FAILURE(status))
 		dev_warn(&adap->dev, "failed to enumerate I2C slaves\n");
+
+	if (!adap->dev.parent)
+		return;
+
+	handle = ACPI_HANDLE(adap->dev.parent);
+	if (!handle)
+		return;
+
+	acpi_walk_dep_device_list(handle);
 }
 
 static const struct acpi_device_id i2c_acpi_force_400khz_device_ids[] = {
@@ -719,7 +729,6 @@ int i2c_acpi_install_space_handler(struct i2c_adapter *adapter)
 		return -ENOMEM;
 	}
 
-	acpi_walk_dep_device_list(handle);
 	return 0;
 }
 
-- 
2.28.0