Hi, On Sun, Feb 05, 2012 at 02:46:19PM +0530, Shilimkar, Santosh wrote: > >> bank->mod_usage check is used to take care of doing pm_runtime_get*/put* only > >> if all the GPIOs in a particular bank are enabled or disabled respectively. > > > > and why should you care about that ? The first get will enable the > > resources you need, the second get will just increase a counter and so > > on. So if you have 32 gets, you will disable the module when you have 32 > > puts. > > > Am not sure if it is clear to you that the GPIO resources like clock, > debounce clk are per bank wise and not per GPIO line. So doing 32 this is just one more reason to have usage counters. > get/put per bank is stupid. runtime pm is for managing pm what's stupid is trying to use the pm usage counters as a binary flag, see below. > resources and if the pm resource is per bank, it has to be > handled per bank. hehe, what are you talking about Santosh ? That's the whole point of reference counting. If there are 32 users for 1 resource, you want to make sure that you only free the resources (clocks, or whatever resource you want) after all 32 users are done with it. Trying to use the pm usage counter as a binary flag, that's the stupid part of the OMAP GPIO driver. Instead of adding checks such as: if (module_isnt_used()) enable_clocks(); on get and: if (module_isnt_needed_anymore()) disable_clcoks() on put is the most useless piece of code on that driver. Because such checks are already available on PM core through usage counters. The way that part of the code works is as follow: get() { if (pm_counter_is_zero(dev)) { atomic_inc(); rpm_resume(); } } put() { atomic_dec(); if (pm_counter_is_zero(dev)) { rpm_suspend(); } } Do you not see that you're duplicating functionality by trying to use the pm counter a binary flag ? > >> With the above change, pm_runtime_put*/get* would be called for every > >> gpio_request() > >> /_free() (that is, for upto 32 pins in OMAP3/4) in a bank irrespective > >> of whether other > > > > so ? > It's useless. no, it's not. > >> GPIO pins are enabled or disabled in the same bank. Hence it is > >> required to have a > >> check based on mod_usage. > > > > unnecessary. > > > I don't think so. then show that to me with code. Looking at the usage counters on pm runtime, you're duplicating functionality with it and trying to use it as a binary flag. Preventing multiple gets/puts breaks the whole idea of _having_ usage counters to start with. -- balbi
Attachment:
signature.asc
Description: Digital signature