Re: [PATCH AUTOSEL 5.8 17/20] i2c: core: Call i2c_acpi_install_space_handler() before i2c_acpi_register_devices()

From: Hans de Goede <hdegoede@redhat.com>
To: kieran.bingham@ideasonboard.com, Sasha Levin <sashal@kernel.org>,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	Wolfram Sang <wsa@kernel.org>,
	linux-i2c@vger.kernel.org
Subject: Re: [PATCH AUTOSEL 5.8 17/20] i2c: core: Call i2c_acpi_install_space_handler() before i2c_acpi_register_devices()
Date: Wed, 14 Oct 2020 13:23:54 +0200	[thread overview]
Message-ID: <bbeb7cae-d856-bb25-4602-8dd3bae62773@redhat.com> (raw)
In-Reply-To: <1977b57b-fae6-d9d4-e6bf-3d4013619537@ideasonboard.com>

Hi,

On 10/14/20 1:09 PM, Kieran Bingham wrote:
> Hi Hans, Sasha,
> 
> As mentioned on https://github.com/linux-surface/kernel/issues/63, I'm
> afraid I've bisected a boot time issue on the Microsoft Surface Go 2 to
> this commit on the stable 5.8 tree.
> 
> The effect as reported there is that the boot process stalls just after
> loading the usbhid module.
> 
> Typing, or interacting with the Keyboard (Type Cover) at that point
> appears to cause usb bus resets, but I don't know if that's a related
> symptom or just an effect of some underlying root cause.
> 
> I have been running a linux-media kernel on this device without issue.
> 
> Is this commit in 5.9? I'll build a vanilla v5.9 kernel and see if it
> occurs there too.

Yes the commit is in 5.9 too. Still would be interesting to see if 5.9 hits
this issue too. I guess it will, but as I mentioned in:

https://github.com/linux-surface/kernel/issues/63

I do not understand why this commit is causing this issue.

So I just checked and the whole acpidump is not using I2C
opregion stuff at all:

[hans@x1 microsoft-surface-go2]$ ack GenericSerialBus *.dsl
[hans@x1 microsoft-surface-go2]$

And there is only 1 _REG handler which is for the
embedded-controller.

So this patch should not make a difference at all on the GO2,
other then maybe a subtle timing difference somewhere ... ?

Regards,

Hans

> On 21/09/2020 15:40, Sasha Levin wrote:
>> From: Hans de Goede <hdegoede@redhat.com>
>>
>> [ Upstream commit 21653a4181ff292480599dad996a2b759ccf050f ]
>>
>> Some ACPI i2c-devices _STA method (which is used to detect if the device
>> is present) use autodetection code which probes which device is present
>> over i2c. This requires the I2C ACPI OpRegion handler to be registered
>> before we enumerate i2c-clients under the i2c-adapter.
>>
>> This fixes the i2c touchpad on the Lenovo ThinkBook 14-IIL and
>> ThinkBook 15 IIL not getting an i2c-client instantiated and thus not
>> working.
>>
>> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1842039
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> Signed-off-by: Wolfram Sang <wsa@kernel.org>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>>   drivers/i2c/i2c-core-base.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
>> index 4f09d4c318287..7031393c74806 100644
>> --- a/drivers/i2c/i2c-core-base.c
>> +++ b/drivers/i2c/i2c-core-base.c
>> @@ -1336,8 +1336,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>>   
>>   	/* create pre-declared device nodes */
>>   	of_i2c_register_devices(adap);
>> -	i2c_acpi_register_devices(adap);
>>   	i2c_acpi_install_space_handler(adap);
>> +	i2c_acpi_register_devices(adap);
>>   
>>   	if (adap->nr < __i2c_first_dynamic_bus_num)
>>   		i2c_scan_static_board_info(adap);
>>
> 
> 