On Friday 11 January 2019 09:06 PM, Tony Lindgren wrote: > 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. https://pastebin.ubuntu.com/p/Tc7WGVJ7hT/ commit b5f8ffbb6fad9151634805c2001af4afbb884eca I wanted to eliminate ARM: dts: am437x: Add l4 interconnect hierarchy and ti-sysc data to verify if it started erring after ti,sysc migration i could not boot to prompt. - Keerthy > >> 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. Okay > > 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. Agreed. So here is the suspend log with some prints added to i2c-omap runtime ops and Pixcir driver: [ 90.855206] PM: suspend entry (deep) [ 90.860125] PM: Syncing filesystems ... done. [ 90.920632] Freezing user space processes ... (elapsed 0.003 seconds) done. [ 90.932245] OOM killer disabled. [ 90.935668] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 90.946227] printk: Suspending console(s) (use no_console_suspend to debug) [ 91.007332] pixcir_ts 1-005c: pixcir_i2c_ts_suspend called [ 91.008492] omap_i2c 4802a000.i2c: omap_i2c_runtime_resume /* omap_i2c_runtime_resume is called before suspending but no corresponding omap_i2c_runtime_suspend before suspending */ [ 91.080420] cpsw 4a100000.ethernet eth0: Link is Down [ 91.194714] Disabling non-boot CPUs ... [ 91.194892] pm33xx pm33xx: PM: Successfully put all powerdomains to target state [ 91.257081] net eth0: initializing cpsw version 1.15 (0) [ 91.276247] Micrel KSZ9031 Gigabit PHY 4a101000.mdio:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=4a101000.mdio:0) /* So basically we are trying to resume pixcir and i2c has not even suspended properly */ [ 91.294897] pixcir_ts 1-005c: pixcir_i2c_ts_resume called [ 92.342825] omap_i2c 4802a000.i2c: controller timed out [ 92.372732] pixcir_ts 1-005c: pixcir_int_enable: can't read reg 0x34 : -110 [ 92.372779] pixcir_ts 1-005c: Failed to disable interrupt generation: -110 [ 92.372815] pixcir_ts 1-005c: Failed to stop [ 92.372900] dpm_run_callback(): pixcir_i2c_ts_resume+0x0/0xc0 [pixcir_i2c_ts] returns -110 [ 92.372951] PM: Device 1-005c failed to resume: error -110 [ 92.386149] usb usb1: root hub lost power or was reset [ 92.386312] usb usb2: root hub lost power or was reset [ 92.650326] OOM killer enabled. [ 92.653632] Restarting tasks ... done. [ 92.693415] PM: suspend exit > > 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. Yea no need to get back to LEGACY_IDLE mode i believe. > > Regards, > > Tony >
