lkml.org 
[lkml]   [2009]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] PCAP regulator driver (for 2.6.32).
From
Date
Em Sex, 2009-06-26 às 16:08 +0100, Mark Brown escreveu:
> > If the above is not possible, then regulator_is_enabled() doesn't match
> > regulator_enable()/regulator_disable() pair. Maybe the API should make
> > this clear?
>
> Frankly I'm not sure how much any documentation is going to help here.
> There's already a note about the fact that the regulator might've been
> enabled elsewhere, it could be strengthened a bit but it still relies on
> people reading it.

I wasn't talking about documentation.

> Fundamentally, if your consumer is trying to explicitly force the
> regulator off then it's not able to cope with the regulator being
> shared. I suspect that if someone does add a non-shared API then the
> problem will go away, half the problem is with consumers thinking they
> have exclusive use of the regulator.

The consumer (pxamci.c with the logic implemented on mmc/core.c) is not
trying to explicitly force the regulator off. It is trying to know if
itself has previously enabled the regulator.

The problem is that regulator_is_enabled returns the regulator
_hardware_ state, and regulator_enable/regulator_disable are used to
update the use_count. This is an API inconsistency as the consumer
should keep an internal use_count and _not_ rely on
regulator_is_enabled.

I see no point in exporting regulator_is_enabled() as it is now. There
is no use in a consumer driver to know if the regulator _hardware_ is
enabled (as it may be shared).

So, if the regulator framework has no bugs regarding regulators left on
by the bootloader, then maybe the buggy code is mmc/core.c?

int mmc_regulator_set_ocr(struct regulator *supply, unsigned short vdd_bit)
{
...
int enabled;

enabled = regulator_is_enabled(supply);
...
if (vdd_bit) {
...
if (result == 0 && !enabled)
result = regulator_enable(supply);
} else if (enabled) {
result = regulator_disable(supply);
}
return result;
}

Anyway, I don't have more time to spend on this issue, so i will just do
as you request, remove the workaround from pcap_regulator.c and put it
on pxamci.c, even if I think that this is the ugliest solution so far.

--
Daniel Ribeiro
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2009-06-27 00:31    [W:0.047 / U:0.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site