Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: 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
References: <20200921144027.2135390-1-sashal@kernel.org>
 <20200921144027.2135390-17-sashal@kernel.org>
 <1977b57b-fae6-d9d4-e6bf-3d4013619537@ideasonboard.com>
 <bbeb7cae-d856-bb25-4602-8dd3bae62773@redhat.com>
Message-ID: <d1318f56-e610-b095-6d8c-37f94444882e@redhat.com>
Date:   Wed, 14 Oct 2020 15:46:08 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <bbeb7cae-d856-bb25-4602-8dd3bae62773@redhat.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi all,

On 10/14/20 1:23 PM, Hans de Goede wrote:
> 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 ... ?

Thanks to Maximilian Luz sharp eyes this is explained now,
despite the name of the i2c_acpi_install_space_handler()
it also had a acpi_walk_dep_device_list() call hidden in
there, so the "i2c: core: Call i2c_acpi_install_space_handler()
before i2c_acpi_register_devices()" also moved that
acpi_walk_dep_device_list() earlier.

I've given Kieran a patch to test which in essence reverts
the part where the acpi_walk_dep_device_list() call is also
moved earlier and that fixes the Surface Go 2 not booting.

I will submit this fix upstream right away and I'll also
send a separate mail to Greg / stable@vger to see if Greg
is willing to make an exception and at this to the stable
series before it hits Linus' tree.

Regards,

Hans