On 22/04/2020 15:07, Dmitry Osipenko wrote: ... >> So I think that part of the problem already existed prior to these >> patches. Without your patches I see ... >> >> [ 59.543528] tegra-i2c 7000d000.i2c: i2c transfer timed out > > Does this I2C timeout happen with my patches? Could you please post full > logs of an older and the recent kernel versions? I believe that it does, but I need to check. >> [ 59.549036] vdd_sata,avdd_plle: failed to disable >> >> [ 59.553778] Failed to disable avdd-plle: -110 >> >> [ 59.558150] tegra-pcie 3000.pcie: failed to disable regulators: -110 >> >> >> However, now with your patches the i2c is failing to resume and this >> breaks system suspend completely for Tegra30. So far this is the only >> board that appears to break. >> >> So the above issue needs to be fixed and I will chat to Thierry about this. > > Okay So I have been looking at this some more and this starting to all look a bit of a mess :-( Firstly, the Tegra PCI driver suspends late (noirq) as we know. The PCI driver will warn if it cannot disable the regulators when suspending but does not actually fail suspend. So this warning is just indicating that we were unable to disable the regulators. Now I don't see that we can ever disable the PCI regulators currently when entering suspend because ... 1. We are in the noirq phase and so we will not get the completion interrupt for the I2C transfer. I know that you recently added the atomic I2C transfer support, but we can get the regulator framework to use this (I have not looked in much detail so far). 2. Even if the regulator framework supported atomic I2C transfers, we also have the problem that the I2C controller could be runtime- suspended and pm_runtime_get_sync() will not work during during this phase to resume it correctly. This is a problem that needs to be fixed indeed! 3. Then we also have the possible dependency on the DMA driver that is suspended during the noirq phase. It could be argued that if the PCI regulators are never turned off (currently) then we should not even bother and this will likely resolve this for now. However, really we should try to fix that correctly. What I still don't understand is why your patch breaks resume. Even if the I2C transfer fails, and is deemed harmless by the client driver, we should still be able to suspend and resume correctly. Cheers Jon -- nvpublic
