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 - Grygorii Tony, 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. 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 Regards, Keerthy >>> >>> And at least am437x based devices with pixcir_ts will fail to resume >>> to a touchscreen that is configured as the wakeup-source in device >>> tree for these devices. >>> >>> This is because pixcir_ts tries to reconfigure it's registers for >>> noirq suspend which fails. This also leaves i2c-omap in enabled state >>> for suspend. >> >> I didn't fully get this. Does it reconfigure the registers for noirq or >> during noirq? If the former, why exactly does it fail? > > On suspend and resume, pixcir_ts tries to configure registers > during noirq. At that point we already have PM runtime > rpm_check_suspend_allowed() return -EACCESS for PM runtime > which leads to the pixcir -ETIMEDOUT 110 error from the i2c > controller. > > And in general, I don't think we should attempt any i2c during > noirq :) > >>> Let's fix the pixcir_ts issue and make sure i2c-omap is suspended by >>> adding SET_NOIRQ_SYSTEM_SLEEP_PM_OPS. In the long run the solution >>> seems to be to move the handling of suspended adapters to the i2c >>> core, but that still needs some more work. >> >> I don't know if you followed this, but we failed with a generic handling >> within the core. We have helpers now which drivers can apply >> individually. > > Hmm I don't follow you you here. Which helpers are you > talking about here? > > I'm only aware of your pending patches from thread > "[PATCH v2 0/9] i2c: move handling of suspended adapters > to the core". > > Regards, > > Tony >
