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

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: <1246055215.10360.273.camel@brutus>
Date:	Fri, 26 Jun 2009 19:26:55 -0300
From:	Daniel Ribeiro <drwyrm@...il.com>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Cc:	Liam Girdwood <lrg@...mlogic.co.uk>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	openezx-devel <openezx-devel@...ts.openezx.org>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	David Brownell <dbrownell@...rs.sourceforge.net>,
	Pierre Ossman <pierre@...man.eu>
Subject: Re: [PATCH] PCAP regulator driver (for 2.6.32).

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

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