Hi, * Keerthy <j-keerthy@xxxxxx> [190111 10:05]: > On Friday 11 January 2019 05:15 AM, Tony Lindgren wrote: > > * Wolfram Sang <wsa@xxxxxxxxxxxxx> [190110 23:31]: > >> Hi Tony, > >> > >> On Thu, Jan 10, 2019 at 07:59:16AM -0800, Tony Lindgren wrote: > >>> We currently get the following error with pixcir_ts driver during a > >>> suspend resume cycle: > >>> > >>> omap_i2c 4802a000.i2c: controller timed out > >>> pixcir_ts 1-005c: pixcir_int_enable: can't read reg 0x34 : -110 > >>> pixcir_ts 1-005c: Failed to disable interrupt generation: -110 > >>> pixcir_ts 1-005c: Failed to stop > >>> dpm_run_callback(): pixcir_i2c_ts_resume+0x0/0x98 > >>> [pixcir_i2c_ts] returns -110 > >>> PM: Device 1-005c failed to resume: error -110 > > I spent some time today bisecting the issue. Unfortunately could not get to a > proper commit but i believe this error got introduced after the ti,sysc > migration series while i started bisecting it am437x stopped booting. May be it > would worth looking at the dt nodes after sysc migration. Hmm care to post the bisect commit that was not booting for you? Would be good to know. > This patch surely solves that error but may be if you can look at the migration > patches a bit. So this error starts appearing in 5.0 only. 4.20 has no issues > from my testing on am437x We have legacy _od_suspend_noirq() do pm_generic_suspend_noirq(), and on errors call pm_generic_runtime_suspend() anyways to idle the module. So that explains how i2c-omap gets idled earlier. With i2c-omap probing with ti-sysc, we can idle i2c-omap simply with SET_NOIRQ_SYSTEM_SLEEP_PM_OPS in the driver and have the generic code take care of things for us. Currently we don't have anything in place to idle i2c-omap for us. There's no _od_suspend_noirq() or SET_NOIRQ_SYSTEM_SLEEP_PM_OPS to idle i2-omap. So we start seeing the pixcir errors when it tries to access i2c-bus but it's too late at that point. We could go back to _od_suspend_noirq() by moving i2c out of DEBUG in sysc_revision_quirks and tag it with SYSC_QUIRK_LEGACY_IDLE. But I'd rather limit SYSC_QUIRK_LEGACY_IDLE usage to the remaining cases of pm_runtime_irq_safe_use(). And then we can eventually get rid of SYSC_QUIRK_LEGACY_IDLE too. Regards, Tony
