lkml.org 
[lkml]   [2013]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] regulator: anatop: add is_enabled interface
On Tue, Dec 17, 2013 at 12:38:33PM +0000, Anson.Huang@freescale.com wrote:

> > better to add the error checking there wouldn't it?

> Okay, than what about other functions? there is such condition check
> there is other functions too, that is why I add it here. if you think
> it is no necessary, I will remove this check in my patch?

It shouldn't be in the other functions either.

> > This sounds like you need to have some higher level synchronisation
> > between whatever is managing this supply and your cpufreq driver - if
> > you rely on reading back the current status from the hardware there will
> > always be races between reading the state and the other thing doing the
> > enable or disable.

> yes, you are right. but I think we have handled that, all the
> operations of this LDO will via regulator interface, and regulator
> framework already has mutex lock. so there should be no such race in
> kernel as long as we all use regulator interface to access anatop LDO.

No, that's not going to work. Consider this sequence:

cpufreq other driver
disable()
is_enabled()
enable()

The locking the regulator core does won't help you here, nothing stops
the state of the regulator changing after it's read. Your cpufreq
driver should just change the voltage and not worry if the regulator is
enabled, the voltage change won't have any effect while the regulator is
off and presumably if it does get enabled then it needs to be enabled at
whatever voltage cpufreq set anyway.

In any case I'd be much happier with this patch if it implemented the
enable and disable operations as well.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2013-12-17 14:41    [W:0.056 / U:0.808 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site