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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Wolfram Sang <wsa@xxxxxxxxxxxxx> [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






[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux