Re: [PATCH] ACPI / LPSS: Don't skip late system PM ops for hibernate on BYT/CHT

[Robert R. Howell]

Hi Hans

On 5/13/19 2:41 AM, Hans de Goede wrote:

> 
> Hi Robert,
> 
> On 09-05-19 20:09, Robert R. Howell wrote:
>> Hi Hans
>>
>> On 5/9/19 2:50 AM, Hans de Goede wrote:
>>
>>>
>>> Hi,
>>>
>>> On 09-05-19 06:24, Robert R. Howell wrote:
>>>> On 4/30/19 8:39 AM, Hans de Goede wrote:
>>>>>
>>
>>>>>
>>>>> I've just tried to reproduce the "Error i2c_dw_xfer call while suspended" error
>>>>> on suspend/resume on my own T100TA and I could not reproduce this.
>>>>>
>>>>> Can you try without the BT keyboard paired and waking up from suspend using the
>>>>> tablet part's power-button ?
>>>>>
>>>>> Also do you still have the scripts to rmmod some modules before suspend ?
>>>>>
>>>>
>>>> The T100TA keyboard is actually a hardwired connection rather than Bluetooth but I
>>>> did physically disconnect the keyboard, and also unpaired all the actual Bluetooth
>>>> devices (such as the mouse) and then powered down the T100TA bluetooth adapter.
>>>> When I suspend, then resume using the tablet power button, I still get the
>>>> i2c_dw_xfererror error during the resume. But whatever causes this error isn't fatal,
>>>> in the sense that after resume the sound and other i2c functions do still work OK.
>>>>
>>>> While I always get this i2c_dw_xfer error on resume from suspend or hibernation on the T100TA,
>>>> I also have a T100TAM and curiously, it NEVER shows that error -- although all the
>>>> other suspend and hibernate behavior seems similar. I'm not sure if the following could
>>>> be the difference, but the T100TA uses an i2c connected ATML1000 touchscreen controller
>>>> while the T100TAM uses an i2c connected SIS0817 touchscreen controller. Other than that
>>>> the hardware seems almost identical.
>>>
>>> I've been testing on an actual T100TA, with the ATML1000 touchscreen controller.
>>>
>>> Maybe it is a difference in BIOS version, my T100TA is running the latest BIOS, what
>>> is the output of:
>>>
>>> cat /sys/class/dmi/id/bios_version /sys/class/dmi/id/bios_date
>>>
>> On the T100TA which shows the i2c_dw_xfer error the above cat reports:
>> T100TA.307
>> 05/09/2014
>>
>> while the T100TA which does NOT show the i2c_dw_xfer error reports:
>> T100TAM.205
>> 07/25/2014
>>>
>>> Also do you perhaps have a microsd card inserted? (I'm trying to figure out the
>>> different between our setups so that I can hopefully reproduce the issue myself).
>>>
>> I do have a microsd card inserted in both the T100TA and the T100TAM.
> 
> Ah, ok I already suspected that and I think that is the difference between our
> 2 setups. I will try to reproduce the suspend/resume problem again with a microsd
> card inserted and mounted.
> 
>>>> Regarding scripts, while I do still need a systemd hibernate script which removes the
>>>> brcmfmac and the hci_uart (bluetooth related) drivers, I've found that I no longer need
>>>> any script for suspend.
>>>
>>> Ok, so you are not doing any rmmod-s on suspend, right?
>>>
>> Correct -- I am NOT using a script and am not doing any explicit rmmod's on suspend, just on hibernate.
>>> Regards,
>>>
>>> Hans
>>
>> All my previous tests were done using a 5.1.0-rc5, 5.1.0-rc3, or earlier kernel.
>> But I just compiled the released 5.1.0 kernel and the behavior has changed for the T100TA,
>> resulting in a different i2c error and a call trace. (I still continue to NOT see any
>> suspend/resume errors on the T100TAM.)
> 
> This is expected we changed / improved the code generating the:
> "i2c_designware 80860F41:00: Transfer while suspended"
> 
> Warning to include a trace, so that we know which code initiated the transfer,
> which, as more or less expected, is the ACPI subsystem, like some power_on (_PS0)
> or off (_PS3) method.
> 
>> Note that for all the tests described in this message I'm applying your patch
>> regarding .poweroff_noirq and .restore_noirq, and I'm applying my own patch removing the
>> DPM_FLAG_SMART_SUSPEND flag. I haven't yet tried to explore varying those patches
>> for the 5.1.0 release as I did for the earlier rc's, as described in previous messages.
> 
> Hmm, for future testing please leave out the patch removing the DPM_FLAG_SMART_SUSPEND
> flag. Usually when asking you to test something we assume you are using a pristine kernel.
> What does that patch attempt to fix and what happens during suspend/resume without it ?
> 
Removing the DPM_FLAG_SMART_SUSPEND fixes an i2c_designware timeout on the T100TA and TAM systems 
when resuming from hibernation.  That's detailed in an earlier message of mine in this thread, and also 
is quoted more completely in a reply I'm about to send to Rafael.  From a usability perspective the
resume-from-hibernate i2c_designware timeout errors were more serious, since they disabled sound 
till I rebooted, while the suspend/resume "Transfer while suspended" error was just a minor 
annoyance in the log, since it didn't SEEM to cause subsequent problems.

If the patch that Rafael just provided doesn't fix these issues (I'm traveling again so it will be a 
couple days before I'm home and can test it) I'll try some further experiments and I might have a 
T100TA where I can install a more recent BIOS and see what affect that has. 
 
> Regards,
> 
> Hans
> 

Thanks again for testing this on your machine.

Bob Howell