Messages in this thread | | | Subject | Re: [PATCH] PCAP regulator driver (for 2.6.32). | From | Daniel Ribeiro <> | Date | Fri, 26 Jun 2009 19:26:55 -0300 |
| |
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] | |