Re: [RFC/RFT PATCH 2/2] gpio: omap: ensure that runtime pm will disable unused gpio banks

[Tony Lindgren <tony@xxxxxxxxxxx>]

* Grygorii.Strashko@xxxxxxxxxx <grygorii.strashko@xxxxxxxxxx> [150417 10:33]:
> Hi Tony,
> On 04/16/2015 07:29 PM, Tony Lindgren wrote:
> > * Tony Lindgren <tony@xxxxxxxxxxx> [150319 16:08]:
> >> * Tony Lindgren <tony@xxxxxxxxxxx> [150307 00:08]:
> >>> * grygorii.strashko@xxxxxxxxxx <grygorii.strashko@xxxxxxxxxx> [150306 11:27]:
> >>>> From: Grygorii Strashko <grygorii.strashko@xxxxxxxxxx>
> >>>>
> >>>> Now there are two points related to Runtime PM usage:
> >>>> 1) bank state doesn't need to be checked in places where
> >>>> Rintime PM is used, bacause Runtime PM maintains its
> >>>> own usage counter:
> >>>>        if (!BANK_USED(bank))
> >>>>                 pm_runtime_get_sync(bank->dev);
> >>>> so, it's safe to drop such checks.
> >>>>
> >>>> 2) There is a call of pm_runtime_get_sync() in omap_gpio_irq_type(),
> >>>> but no corresponding put. As result, GPIO devices could be
> >>>> powered on forever if at least one GPIO was used as IRQ.
> >>>> Hence, allow powering off GPIO banks by adding missed
> >>>> pm_runtime_put(bank->dev) at the end of omap_gpio_irq_type().
> >>>
> >>> Nice to see this happening, but I think before merging this we need
> >>> to test to be sure that the pm_runtime calls actually match.. I'm
> >>> not convinced right now.. We may still have uninitialized entry
> >>> points similar to 3d009c8c61f9 ("gpio: omap: Fix bad device
> >>> access with setup_irq()").
> >>
> >> OK so I finally got around testing this along with your bank
> >> removal set. Looks like this patch causes a regression at least
> >> with n900 keyboard LEDs with off-idle. The LED won't come back
> >> on after restore from off-idle. Anyways, now we have something
> >> reproducable :) So I'll try to debug it further at some point,
> >> might be few days before I get to it.
> > 
> > Sorry for the delay, finally got around to this one :)
> > 
> > Here's what I came up with, only lightly tested so far. Note that
> > we need to keep the PM runtime per bank, not per GPIO. Will repost
> > a proper patch after some more testing.
> 
> I've had a opposite idea actually ;) - use Runtime PM on per-gpio basis as
> it maintains all needed counters inside and we can be sure that
> PM runtime callbacks will be called once per bank.

Sorry looks like I missed this email earlier, the mail got buried
in all the threads in my inbox.

Anyways, we still need per bank pm runtime for a while until we
can make it per GPIO.

> Also, I've thought about moving the code from omap_enable_gpio_module()
> into Runtime PM callbacks.
> So, final goal - get rid of BANK_USED & LINE_USED.
> 
> Were you able to identify broken calls sequence?

No, but it breaks things for omap3 off idle. And then I noticed all
the duplicate code as discussed in the newer thread.
 
> Also, Pay attention pls, that omap2_gpio_prepare_for/resume_after_idle()
> will be most probably a NOP now.

Looks like we can't do that change yet, it breaks omap3 off idle.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html