* Tony Lindgren <tony@xxxxxxxxxxx> [190113 16:23]:
> * 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.
So we don't have any suspend state flag for i2c-omap, and
this patch is still needed as a fix. Do you need any changes
done?
BTW, you already have another similar fix queued as 81d696c7c4ff
("i2c: rcar: Fix clients using i2c from suspend callback") so
it's not like it's a unique problem :)
Regards,
Tony
