linux-kernel - Re: [PATCH v9 01/25] gpio/omap: remove dependency on gpio_bank

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120206064043.GB15652@legolas.emea.dhcp.ti.com>
Date:	Mon, 6 Feb 2012 08:40:45 +0200
From:	Felipe Balbi <balbi@...com>
To:	"Shilimkar, Santosh" <santosh.shilimkar@...com>
Cc:	balbi@...com, "Varadarajan, Charulatha" <charu@...com>,
	Kevin Hilman <khilman@...com>,
	"Cousson, Benoit" <b-cousson@...com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Tarun Kanti DebBarma <tarun.kanti@...com>,
	linux-omap@...r.kernel.org, tony@...mide.com,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [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@...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

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)