[4/4,v2] regulator: arizona-ldo1: Do not control clocking from regulator
diff mbox

Message ID 1403275273-22713-5-git-send-email-ckeepax@opensource.wolfsonmicro.com
State New, archived
Headers show

Commit Message

Charles Keepax June 20, 2014, 2:41 p.m. UTC
From: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>

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>
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/regulator/arizona-ldo1.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

Comments

Mark Brown June 21, 2014, 8:53 p.m. UTC | #1
On Fri, Jun 20, 2014 at 03:41:13PM +0100, Charles Keepax wrote:
> From: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> 
> 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.

I would expect this to be squashed into the first patch since otherwise
there's a bisection issue with both drivers trying to maintain the same
thing at the same time.
Charles Keepax June 23, 2014, 3:53 p.m. UTC | #2
On Sat, Jun 21, 2014 at 09:53:49PM +0100, Mark Brown wrote:
> On Fri, Jun 20, 2014 at 03:41:13PM +0100, Charles Keepax wrote:
> > From: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> > 
> > 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.
> 
> I would expect this to be squashed into the first patch since otherwise
> there's a bisection issue with both drivers trying to maintain the same
> thing at the same time.

Whilst there are two things controlling it for a couple of
patches they both set the same value one after another, so it
should be safe. If it goes into the first patch nothing is
controlling the DVFS for a few patches so that actually creates a
bisection issue. It could be squashed into the second last patch
but this way feels cleaner?

Thanks,
Charles
Charles Keepax June 23, 2014, 4:29 p.m. UTC | #3
On Mon, Jun 23, 2014 at 04:53:37PM +0100, Charles Keepax wrote:
> On Sat, Jun 21, 2014 at 09:53:49PM +0100, Mark Brown wrote:
> > On Fri, Jun 20, 2014 at 03:41:13PM +0100, Charles Keepax wrote:
> > > From: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> > > 
> > > 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.
> > 
> > I would expect this to be squashed into the first patch since otherwise
> > there's a bisection issue with both drivers trying to maintain the same
> > thing at the same time.
> 
> Whilst there are two things controlling it for a couple of
> patches they both set the same value one after another, so it
> should be safe. If it goes into the first patch nothing is
> controlling the DVFS for a few patches so that actually creates a
> bisection issue. It could be squashed into the second last patch
> but this way feels cleaner?

Although looking at that again it can happily merge into the
second patch in the series so I will do that.

Thanks,
Charles
Mark Brown June 23, 2014, 4:35 p.m. UTC | #4
On Mon, Jun 23, 2014 at 05:29:22PM +0100, Charles Keepax wrote:
> On Mon, Jun 23, 2014 at 04:53:37PM +0100, Charles Keepax wrote:

> > Whilst there are two things controlling it for a couple of
> > patches they both set the same value one after another, so it
> > should be safe. If it goes into the first patch nothing is
> > controlling the DVFS for a few patches so that actually creates a
> > bisection issue. It could be squashed into the second last patch
> > but this way feels cleaner?

> Although looking at that again it can happily merge into the
> second patch in the series so I will do that.

OK, or if you do want to leave them separate I'd suggest noting why it's
safe in the commit log.

Patch
diff mbox

diff --git a/drivers/regulator/arizona-ldo1.c b/drivers/regulator/arizona-ldo1.c
index 04f262a..47b48c8 100644
--- a/drivers/regulator/arizona-ldo1.c
+++ b/drivers/regulator/arizona-ldo1.c
@@ -78,11 +78,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;