lkml.org 
[lkml]   [2012]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v9 01/25] gpio/omap: remove dependency on gpio_bank_count
    On Sun, Feb 05, 2012 at 06:05:37PM +0530, Shilimkar, Santosh wrote:
    > On Sun, Feb 5, 2012 at 5:05 PM, Felipe Balbi <balbi@ti.com> wrote:
    > > 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 ?
    > >
    > Ahh.. Now I see your point. I miss-understood the point first time
    > and thought that we have disconnect on the pm resources from
    > number of GPIO perspective.
    >
    > What you are saying is to use pm runtime reference counters rather
    > than creating local ones for GPIO which seems to be right thing to
    > do. Sorry for the noise.

    no problem.

    > The agressive clock cutting was tried initially without much success
    > and may be we can revisit this one more time.

    I think one issue will be wrt to IRQ handling. If you want pm_runtime_*
    calls to be irq_safe, they will keep the parent always on, so we need to
    find a way to make that part work properly, I guess.

    > As per as this series is concerned, we would like to fix
    > the build error pointed by Kevin and queue it for 3.4.

    sure, go ahead. No problems with that at all.

    --
    balbi
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2012-02-06 07:45    [W:5.868 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site