From mboxrd@z Thu Jan 1 00:00:00 1970 From: "santosh.shilimkar@oracle.com" Subject: Re: [RFC/RFT PATCH 2/2] gpio: omap: ensure that runtime pm will disable unused gpio banks Date: Wed, 29 Apr 2015 07:59:04 -0700 Message-ID: <5540F1B8.5030407@oracle.com> References: <1425670017-19598-1-git-send-email-grygorii.strashko@linaro.org> <1425670017-19598-2-git-send-email-grygorii.strashko@linaro.org> <20150306231513.GB2651@atomide.com> <20150319230326.GP31346@atomide.com> <20150416162941.GI18048@atomide.com> <55314396.20907@linaro.org> <20150429143320.GA24469@atomide.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150429143320.GA24469@atomide.com> Sender: linux-gpio-owner@vger.kernel.org To: Tony Lindgren , "Grygorii.Strashko@linaro.org" Cc: Linus Walleij , ssantosh@kernel.org, Javier Martinez Canillas , Alexandre Courbot , linux-omap@vger.kernel.org, linux-gpio@vger.kernel.org List-Id: linux-omap@vger.kernel.org On 4/29/15 7:33 AM, Tony Lindgren wrote: > * Grygorii.Strashko@linaro.org [150417 10:33]: >> Hi Tony, >> On 04/16/2015 07:29 PM, Tony Lindgren wrote: >>> * Tony Lindgren [150319 16:08]: >>>> * Tony Lindgren [150307 00:08]: >>>>> * grygorii.strashko@linaro.org [150306 11:27]: >>>>>> From: Grygorii Strashko >>>>>> >>>>>> 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. > The issue is across OMAPs. Those stupid debounce clocks won't let the GPIO block to idle. They are called optional clocks but actually they aren't. Regards, Santosh