* 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