[PATCH 0/2] regulator: cpcap: Fix standby mode

[PATCH 0/2] regulator: cpcap: Fix standby mode

[Sebastian Reichel]

Hi,

This fixes standby mode of vaudio on Motorola Droid 4.

-- Sebastian

Sebastian Reichel (2):
  regulator: cpcap: Fix standby mode
  ARM: dts: motorola-cpcap-mapphone: set initial mode for vaudio

 arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi |  1 +
 drivers/regulator/cpcap-regulator.c            | 17 ++++++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

-- 
2.13.2

[PATCH 1/2] regulator: cpcap: Fix standby mode

[Sebastian Reichel]

While working on the audio-codec I noticed, that the
low power mode of the regulators are not properly
supported. This fixes the issue for vaudio.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/regulator/cpcap-regulator.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/cpcap-regulator.c b/drivers/regulator/cpcap-regulator.c
index cc98aceed1c1..65da6dba0b82 100644
--- a/drivers/regulator/cpcap-regulator.c
+++ b/drivers/regulator/cpcap-regulator.c
@@ -121,6 +121,7 @@ struct cpcap_regulator {
 		.enable_val = (mode_val),				\
 		.disable_val = (off_val),				\
 		.ramp_delay = (volt_trans_time),			\
+		.of_map_mode = cpcap_map_mode,				\
 	},								\
 	.assign_reg = (assignment_reg),					\
 	.assign_mask = (assignment_mask),				\
@@ -211,13 +212,23 @@ static int cpcap_regulator_disable(struct regulator_dev *rdev)
 	return error;
 }
 
+static unsigned int cpcap_map_mode(unsigned int mode)
+{
+	switch (mode) {
+	case CPCAP_BIT_AUDIO_LOW_PWR:
+		return REGULATOR_MODE_STANDBY;
+	default:
+		return REGULATOR_MODE_NORMAL;
+	}
+}
+
 static unsigned int cpcap_regulator_get_mode(struct regulator_dev *rdev)
 {
 	int value;
 
 	regmap_read(rdev->regmap, rdev->desc->enable_reg, &value);
 
-	if (!(value & CPCAP_BIT_AUDIO_LOW_PWR))
+	if (value & CPCAP_BIT_AUDIO_LOW_PWR)
 		return REGULATOR_MODE_STANDBY;
 
 	return REGULATOR_MODE_NORMAL;
@@ -230,10 +241,10 @@ static int cpcap_regulator_set_mode(struct regulator_dev *rdev,
 
 	switch (mode) {
 	case REGULATOR_MODE_NORMAL:
-		value = CPCAP_BIT_AUDIO_LOW_PWR;
+		value = 0;
 		break;
 	case REGULATOR_MODE_STANDBY:
-		value = 0;
+		value = CPCAP_BIT_AUDIO_LOW_PWR;
 		break;
 	default:
 		return -EINVAL;
-- 
2.13.2

[PATCH 2/2] ARM: dts: motorola-cpcap-mapphone: set initial mode for vaudio

[Sebastian Reichel]

Set default mode for vaudio, which may be left in standby mode
if the system is booted via kexec from Android.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
index 86033aee8a29..7c6d61fb5cf2 100644
--- a/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
+++ b/arch/arm/boot/dts/motorola-cpcap-mapphone.dtsi
@@ -261,5 +261,6 @@
 		regulator-min-microvolt = <2775000>;
 		regulator-max-microvolt = <2775000>;
 		regulator-enable-ramp-delay = <1000>;
+		regulator-initial-mode = <0x00>; /* NORMAL */
 	};
 };
-- 
2.13.2

Re: [PATCH 1/2] regulator: cpcap: Fix standby mode

[Tony Lindgren]

* Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170707 13:08]:
> While working on the audio-codec I noticed, that the
> low power mode of the regulators are not properly
> supported. This fixes the issue for vaudio.

Yeah good catch:

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCH 2/2] ARM: dts: motorola-cpcap-mapphone: set initial mode for vaudio

[Tony Lindgren]

* Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170707 13:08]:
> Set default mode for vaudio, which may be left in standby mode
> if the system is booted via kexec from Android.

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCH 1/2] regulator: cpcap: Fix standby mode

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 854 bytes --]

On Fri, Jul 07, 2017 at 10:08:12PM +0200, Sebastian Reichel wrote:

> While working on the audio-codec I noticed, that the
> low power mode of the regulators are not properly
> supported. This fixes the issue for vaudio.

In what way is it not properly supported and how does this change ensure
that it is properly supported?

> +static unsigned int cpcap_map_mode(unsigned int mode)
> +{
> +	switch (mode) {
> +	case CPCAP_BIT_AUDIO_LOW_PWR:
> +		return REGULATOR_MODE_STANDBY;
> +	default:
> +		return REGULATOR_MODE_NORMAL;
> +	}
> +}

This function is being added but is never referenced AFAICT, it should
either be removed or used.  If it is being used it should be changed so
that it doesn't accept the default case but instead accepts only
specific values so that the mapping of modes to regulator modes stays
1:1, unknown modes should error out.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] regulator: cpcap: Fix standby mode

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

Hi,

On Mon, Jul 10, 2017 at 01:23:45PM +0100, Mark Brown wrote:
> On Fri, Jul 07, 2017 at 10:08:12PM +0200, Sebastian Reichel wrote:
> 
> > While working on the audio-codec I noticed, that the
> > low power mode of the regulators are not properly
> > supported. This fixes the issue for vaudio.
> 
> In what way is it not properly supported and how does this change ensure
> that it is properly supported?
> 
> > +static unsigned int cpcap_map_mode(unsigned int mode)
> > +{
> > +	switch (mode) {
> > +	case CPCAP_BIT_AUDIO_LOW_PWR:
> > +		return REGULATOR_MODE_STANDBY;
> > +	default:
> > +		return REGULATOR_MODE_NORMAL;
> > +	}
> > +}
> 
> This function is being added but is never referenced AFAICT, it should
> either be removed or used.

You probably skipped over the patch to fast, it is referenced in the
patch section before the one adding the function:

+               .of_map_mode = cpcap_map_mode,                          \

> If it is being used it should be changed so that it doesn't accept
> the default case but instead accepts only specific values so that
> the mapping of modes to regulator modes stays 1:1, unknown modes
> should error out.

Fair point.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] regulator: cpcap: Fix standby mode

[Tony Lindgren]

* Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170710 06:45]:
> Hi,
> 
> On Mon, Jul 10, 2017 at 01:23:45PM +0100, Mark Brown wrote:
> > On Fri, Jul 07, 2017 at 10:08:12PM +0200, Sebastian Reichel wrote:
> > 
> > > While working on the audio-codec I noticed, that the
> > > low power mode of the regulators are not properly
> > > supported. This fixes the issue for vaudio.
> > 
> > In what way is it not properly supported and how does this change ensure
> > that it is properly supported?

Just to clarify this, I had inverted use of a register bit bug in my
original patch that $subject patch fixes.

Regards,

Tony