diff mbox

[4/4] regulator: arizona-ldo1: Do not control clocking from regulator

Message ID 20140609150435.GE5229@opensource.wolfsonmicro.com (mailing list archive)
State Accepted
Commit 69a6582eeb17dc083b2510f1ca2eaa54ff679b49
Headers show

Commit Message

Richard Fitzgerald June 9, 2014, 3:04 p.m. UTC
Using the driver for the internal regulator to also control
the clock frequency of blocks inside the codec is an
unexpected side-effect for a regulator, and also means that
the core clocks won't be changed as expected if an external
regulator is used to power the codec.

The clocking control is now handled by the core arizona MFD
driver so can be removed from the LDO1 driver.

Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
 drivers/regulator/arizona-ldo1.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

Comments

Mark Brown June 9, 2014, 6:43 p.m. UTC | #1
On Mon, Jun 09, 2014 at 04:04:35PM +0100, Richard Fitzgerald wrote:
> Using the driver for the internal regulator to also control
> the clock frequency of blocks inside the codec is an
> unexpected side-effect for a regulator, and also means that
> the core clocks won't be changed as expected if an external
> regulator is used to power the codec.

IIRC this was deliberately coded in this fashion on advice from the
hardware engineers - there was more going on with that register than
there might at first appear and some actual sync with the LDO.  I
believe there was some different process to follow (possibly just
setting this mode all the time) when using an external regulator, though
it's also possible the hardware guys were just unsure at the time.
Charles Keepax June 11, 2014, 10:59 a.m. UTC | #2
On Mon, Jun 09, 2014 at 07:43:14PM +0100, Mark Brown wrote:
> On Mon, Jun 09, 2014 at 04:04:35PM +0100, Richard Fitzgerald wrote:
> > Using the driver for the internal regulator to also control
> > the clock frequency of blocks inside the codec is an
> > unexpected side-effect for a regulator, and also means that
> > the core clocks won't be changed as expected if an external
> > regulator is used to power the codec.
> 
> IIRC this was deliberately coded in this fashion on advice from the
> hardware engineers - there was more going on with that register than
> there might at first appear and some actual sync with the LDO.  I
> believe there was some different process to follow (possibly just
> setting this mode all the time) when using an external regulator, though
> it's also possible the hardware guys were just unsure at the time.

I have had a good chat with the hardware engineers here and they
are pretty adamant that the only constraint here is that we
should never enable SUBSYS without 1.8V being supplied to the
core.

They also believe that the external case should be handled the
same as the internal one, although admittedly I haven't tested
that personally.

This series needs a slight rebase and fixing up for my comment on
one of the patches, but otherwise I think should be good in my
opinion.

Thanks,
Charles
Mark Brown June 11, 2014, 1:54 p.m. UTC | #3
On Wed, Jun 11, 2014 at 11:59:26AM +0100, Charles Keepax wrote:
> On Mon, Jun 09, 2014 at 07:43:14PM +0100, Mark Brown wrote:

> > IIRC this was deliberately coded in this fashion on advice from the
> > hardware engineers - there was more going on with that register than
> > there might at first appear and some actual sync with the LDO.  I
> > believe there was some different process to follow (possibly just
> > setting this mode all the time) when using an external regulator, though
> > it's also possible the hardware guys were just unsure at the time.

> I have had a good chat with the hardware engineers here and they
> are pretty adamant that the only constraint here is that we
> should never enable SUBSYS without 1.8V being supplied to the
> core.

OK, that sounds like the advice when the device was first produced was
not accurate - might be worth checking your datasheets here.  Might also
be worth updating the commit log.
diff mbox

Patch

diff --git a/drivers/regulator/arizona-ldo1.c b/drivers/regulator/arizona-ldo1.c
index d3787e1..ea4fbce 100644
--- a/drivers/regulator/arizona-ldo1.c
+++ b/drivers/regulator/arizona-ldo1.c
@@ -77,11 +77,6 @@  static int arizona_ldo1_hc_set_voltage_sel(struct regulator_dev *rdev,
 	if (ret != 0)
 		return ret;
 
-	ret = regmap_update_bits(regmap, ARIZONA_DYNAMIC_FREQUENCY_SCALING_1,
-				 ARIZONA_SUBSYS_MAX_FREQ, val);
-	if (ret != 0)
-		return ret;
-
 	if (val)
 		return 0;