Re: [PATCH] PCAP regulator driver (for 2.6.32).

[Daniel Ribeiro]

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

Attachment: signature.asc
Description: Esta =?ISO-8859-1?Q?=E9?= uma parte de mensagemassinada digitalmente