[PATCHv2] i2c: omap: Use noirq system sleep pm ops to idle device for suspend

Message ID 20190110155916.41266-1-tony@atomide.com
State New
Headers show
Series
  • [PATCHv2] i2c: omap: Use noirq system sleep pm ops to idle device for suspend
Related show

Commit Message

Tony Lindgren Jan. 10, 2019, 3:59 p.m.
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

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.

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.

Let's also get rid of some ifdefs while at it and replace them with
__maybe_unused as SET_RUNTIME_PM_OPS and SET_NOIRQ_SYSTEM_SLEEP_PM_OPS
already deal with the various PM Kconfig options.

Cc: Dave Gerlach <d-gerlach@ti.com>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Reported-by: Keerthy <j-keerthy@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

Wolfram, this is still pending, can you please apply this a fix for
the -rc series if no objections?

Changes since v1:

- Updated comments with the pixcir_ts related error messages

---
 drivers/i2c/busses/i2c-omap.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

Wolfram Sang Jan. 10, 2019, 11:31 p.m. | #1
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
> 
> 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?

> 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.

> Let's also get rid of some ifdefs while at it and replace them with
> __maybe_unused as SET_RUNTIME_PM_OPS and SET_NOIRQ_SYSTEM_SLEEP_PM_OPS
> already deal with the various PM Kconfig options.
> 
> Cc: Dave Gerlach <d-gerlach@ti.com>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Tero Kristo <t-kristo@ti.com>
> Reported-by: Keerthy <j-keerthy@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> 
> Wolfram, this is still pending, can you please apply this a fix for
> the -rc series if no objections?

Yes, I can do that (when I have no further questions ;))

Regards,

   Wolfram
Tony Lindgren Jan. 10, 2019, 11:45 p.m. | #2
* Wolfram Sang <wsa@the-dreams.de> [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
> > 
> > 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
Keerthy Jan. 11, 2019, 10:05 a.m. | #3
On Friday 11 January 2019 05:15 AM, Tony Lindgren wrote:
> * Wolfram Sang <wsa@the-dreams.de> [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
>
Tony Lindgren Jan. 11, 2019, 3:36 p.m. | #4
Hi,

* Keerthy <j-keerthy@ti.com> [190111 10:05]:
> On Friday 11 January 2019 05:15 AM, Tony Lindgren wrote:
> > * Wolfram Sang <wsa@the-dreams.de> [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
Wolfram Sang Jan. 12, 2019, 10:50 a.m. | #5
> 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 :)

I totally agree. But wouldn't the proper solution be to fix the
pixcir_ts driver to not do transfers during noirq in this specific case?

Recalling your original patch from last year, you mentioned RTC devices.
So, another reason for this patch is to enforce suspending the adapter,
even when there is a child device connected which is for good reason not
going to be suspended?

This is to make sure I understand all this correctly.

> > 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".

Yes, but the v2 approach caused problems, see the discussion there. I
merged v1 (with helpers) a few days ago:

[PATCH 00/10] i2c: move handling of suspended adapters to the core
Tony Lindgren Jan. 13, 2019, 4:23 p.m. | #6
* Wolfram Sang <wsa@the-dreams.de> [190112 10:50]:
> 
> > 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 :)
> 
> I totally agree. But wouldn't the proper solution be to fix the
> pixcir_ts driver to not do transfers during noirq in this specific case?

Well AFAIK, pixcir_ts is doing the right thing. But because of the
currently missing pm_generic_runtime_suspend() that's been done
earlier in _od_suspend_noirq(), pixcir_ts now ends up trying to
configure things too late.

> Recalling your original patch from last year, you mentioned RTC devices.
> So, another reason for this patch is to enforce suspending the adapter,
> even when there is a child device connected which is for good reason not
> going to be suspended?

Hmm not sure if I've seen this happen with RTC yet. We just do what
pm_generic_runtime_suspend() does, returning -EBUSY is fine too.
Then the controller will just stay powered during the suspend if
necessary.

For a non-i2c example, we have devices where a gpio instance powers
the DDR memory :) In that case the related gpio instance just stays
stays powered during the suspend.

> This is to make sure I understand all this correctly.

Maybe the commit message should mention the currently missing
pm_generic_runtime_suspend() if that might clarify things.

> > > 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".
> 
> Yes, but the v2 approach caused problems, see the discussion there. I
> merged v1 (with helpers) a few days ago:
> 
> [PATCH 00/10] i2c: move handling of suspended adapters to the core

Oh OK great, I'll take a look.

Regards,

Tony
Keerthy Jan. 14, 2019, 6:30 a.m. | #7
On Friday 11 January 2019 09:06 PM, Tony Lindgren wrote:
> Hi,
> 
> * Keerthy <j-keerthy@ti.com> [190111 10:05]:
>> On Friday 11 January 2019 05:15 AM, Tony Lindgren wrote:
>>> * Wolfram Sang <wsa@the-dreams.de> [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
>
Tony Lindgren Jan. 15, 2019, 4:17 p.m. | #8
* Keerthy <j-keerthy@ti.com> [190114 06:30]:
> 
> 
> On Friday 11 January 2019 09:06 PM, Tony Lindgren wrote:
> > Hi,
> > 
> > * Keerthy <j-keerthy@ti.com> [190111 10:05]:
> >> On Friday 11 January 2019 05:15 AM, Tony Lindgren wrote:
> >>> * Wolfram Sang <wsa@the-dreams.de> [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.

OK seems like some later fix is needed to carry through the bisect
there then. We have all the critical clock and driver fixes merged
since v4.19. But looks like git bisect threw you back into v4.19-rc1
as it took few merge windows to get all this clkctrl and sysc stuff
merged:

$ git show b5f8ffbb6fad9151:Makefile | head -n5
# SPDX-License-Identifier: GPL-2.0
VERSION = 4
PATCHLEVEL = 19
SUBLEVEL = 0
EXTRAVERSION = -rc1

> >> 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

Yeah, thanks for confirming it on your hardware. I have
slightly different hardware with a different touchscreen
controller so I've been only able to emulate the issue.

Thanks,

Tony

Patch

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1500,8 +1500,7 @@  static int omap_i2c_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int omap_i2c_runtime_suspend(struct device *dev)
+static int __maybe_unused omap_i2c_runtime_suspend(struct device *dev)
 {
 	struct omap_i2c_dev *omap = dev_get_drvdata(dev);
 
@@ -1527,7 +1526,7 @@  static int omap_i2c_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int omap_i2c_runtime_resume(struct device *dev)
+static int __maybe_unused omap_i2c_runtime_resume(struct device *dev)
 {
 	struct omap_i2c_dev *omap = dev_get_drvdata(dev);
 
@@ -1542,20 +1541,18 @@  static int omap_i2c_runtime_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops omap_i2c_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				      pm_runtime_force_resume)
 	SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend,
 			   omap_i2c_runtime_resume, NULL)
 };
-#define OMAP_I2C_PM_OPS (&omap_i2c_pm_ops)
-#else
-#define OMAP_I2C_PM_OPS NULL
-#endif /* CONFIG_PM */
 
 static struct platform_driver omap_i2c_driver = {
 	.probe		= omap_i2c_probe,
 	.remove		= omap_i2c_remove,
 	.driver		= {
 		.name	= "omap_i2c",
-		.pm	= OMAP_I2C_PM_OPS,
+		.pm	= &omap_i2c_pm_ops,
 		.of_match_table = of_match_ptr(omap_i2c_of_match),
 	},
 };