Introduce .get_voltage callback into voltdm

From: Mike Turquette <[email protected]>

This series creates a new callback for struct voltagedomain, .get_voltage. This fetches the voltage from hardware, if possible, and returns it to the caller. We use this call to populate voltdm->nominal_volt at boot time.

The need for this came out of the Adaptive Body-Bias patches. There is a corner case where voltdm->nominal_volt is zero at boot, but the ABB pre-scale function needs to know the current voltage as a part of its logic. Without this series voltdm->nominal_volt will be zero until after voltdm->scale completes, which is too late for the ABB pre-scale logic.

It is possible to have ABB without this series. In the event that voltdm->nominal_volt is zero then the ABB pre-scale function can simply return early. However this opens up a condition that burns power needlessly:

1) The bootloader enables Forward Body-Bias mode on the ABB ldo and runs the voltage domain at a high OPP

2) Linux boots and the first transition of that voltage domain is to a lower OPP.

3) The voltage of that vdd is unknown so the ABB pre-scale function returns early and FBB is left enabled at an OPP that would otherwise not need it.

This scenario is not validated and is best avoided. This series is one step towards that.

Mike Turquette (5):
ARM: omap: vc: replace data_shift with data_mask
ARM: omap: introduce .get_voltage callback
ARM: omap: vc: .get_voltage callback
ARM: omap: vp: .get_voltage callback
ARM: omap: initialize voltdm->nominal_volt

 arch/arm/mach-omap2/vc.c          |   23 ++++++++++++++++++++++-
 arch/arm/mach-omap2/vc.h          |    3 ++-
 arch/arm/mach-omap2/vc3xxx_data.c |    2 +-
 arch/arm/mach-omap2/vc44xx_data.c |    2 +-
 arch/arm/mach-omap2/voltage.c     |   13 +++++++++++++
 arch/arm/mach-omap2/voltage.h     |    1 +
 arch/arm/mach-omap2/vp.c          |   22 ++++++++++++++++++++++
 arch/arm/mach-omap2/vp.h          |    1 +
 8 files changed, 63 insertions(+), 4 deletions(-)

1.7.9.5

From: Mike Turquette <[email protected]>

---
 arch/arm/mach-omap2/vc.c          |    2 +-
 arch/arm/mach-omap2/vc.h          |    2 +-
 arch/arm/mach-omap2/vc3xxx_data.c |    2 +-
 arch/arm/mach-omap2/vc44xx_data.c |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

index 84da34f..2bcac64 100644
@@ -171,7 +171,7 @@ int omap_vc_bypass_scale(struct voltagedomain *voltdm,
 
 	vc_valid = vc->common->valid;
 	vc_bypass_val_reg = vc->common->bypass_val_reg;
-	vc_bypass_value = (target_vsel << vc->common->data_shift) |
+	vc_bypass_value = (target_vsel << __ffs(vc->common->data_mask)) |
 		(vc->volt_reg_addr << vc->common->regaddr_shift) |
 		(vc->i2c_slave_addr << vc->common->slaveaddr_shift);
 

index 478bf6b..84a61b1 100644
@@ -43,8 +43,8 @@ struct voltagedomain;
 struct omap_vc_common {
 	u32 cmd_on_mask;
 	u32 valid;
+	u32 data_mask;
 	u8 bypass_val_reg;
-	u8 data_shift;
 	u8 slaveaddr_shift;
 	u8 regaddr_shift;
 	u8 cmd_on_shift;

index 5d8eaf3..2b35e82 100644
@@ -31,7 +31,7 @@
  */
 static struct omap_vc_common omap3_vc_common = {
 	.bypass_val_reg	 = OMAP3_PRM_VC_BYPASS_VAL_OFFSET,
-	.data_shift	 = OMAP3430_DATA_SHIFT,
+	.data_mask	 = OMAP3430_DATA_MASK,
 	.slaveaddr_shift = OMAP3430_SLAVEADDR_SHIFT,
 	.regaddr_shift	 = OMAP3430_REGADDR_SHIFT,
 	.valid		 = OMAP3430_VALID_MASK,

index d70b930..598edc9 100644
@@ -32,7 +32,7 @@
  */
 static const struct omap_vc_common omap4_vc_common = {
 	.bypass_val_reg = OMAP4_PRM_VC_VAL_BYPASS_OFFSET,
-	.data_shift = OMAP4430_DATA_SHIFT,
+	.data_mask = OMAP4430_DATA_MASK,
 	.slaveaddr_shift = OMAP4430_SLAVEADDR_SHIFT,
 	.regaddr_shift = OMAP4430_REGADDR_SHIFT,
 	.valid = OMAP4430_VALID_MASK,

1.7.9.5

Re: $SUBJECT... ok, I'll bite.... why?

(I'm sure I will find out when I read the rest of the series, but I'm
impatient and picky and would like to know here.) ;)

Kevin

From: Mike Turquette <[email protected]>

Introduces a new callback to struct voltagedomain for retrieving the voltage of the voltage domain from the hardware. This will be used to populate voltdm->nominal_volt during initialization of the voltage code instead of waiting for the first call to voltdm_scale.

In the event of an error or the inability to determine voltage the callback must return zero. It is the callers responsibility to gracefully degrade when presented with a voltage of zero.

---
 arch/arm/mach-omap2/voltage.h |    1 +
 1 file changed, 1 insertion(+)

index 0ac2caf..0ded54f 100644
@@ -85,6 +85,7 @@ struct voltagedomain {
 		u32 rate;
 	} sys_clk;
 
+	unsigned long (*get_voltage) (struct voltagedomain *voltdm);
 	int (*scale) (struct voltagedomain *voltdm,
 		      unsigned long target_volt);
 

1.7.9.5

From: Mike Turquette <[email protected]>

Implement the voltdm->get_voltage callback for the voltage controller driver. This reads the DATA field corresponding to each VC and returns the voltage, after converting it from vsel format.

If DATA is zero (the reset value) then the caller must interpret this as the PMIC running at the default power-on voltage. In such a case DT data for the PMIC is necessary to know the voltage.

---
 arch/arm/mach-omap2/vc.c |   21 +++++++++++++++++++++
 arch/arm/mach-omap2/vc.h |    1 +
 2 files changed, 22 insertions(+)

index 2bcac64..90a9ea6 100644
@@ -101,6 +101,27 @@ static int omap_vc_config_channel(struct voltagedomain *voltdm)
 }
 
 /* Voltage scale and accessory APIs */
+unsigned long omap_vc_get_bypass_data(struct voltagedomain *voltdm)
+{
+	struct omap_vc_channel *vc = voltdm->vc;
+	u32 vc_bypass_value;
+	u8 vsel;
+	unsigned long volt;
+
+	/* sanity */
+	if (!voltdm->pmic || !voltdm->pmic->vsel_to_uv ||
+			!voltdm->read || !voltdm->write)
+		return 0;
+
+	vc_bypass_value = voltdm->read(vc->common->bypass_val_reg);
+	vc_bypass_value &= vc->common->data_mask;
+	vsel = vc_bypass_value >> __ffs(vc->common->data_mask);
+
+	volt = voltdm->pmic->vsel_to_uv(vsel);
+
+	return volt;
+}
+
 int omap_vc_pre_scale(struct voltagedomain *voltdm,
 		      unsigned long target_volt,
 		      u8 *target_vsel, u8 *current_vsel)

index 84a61b1..e7d4719 100644
@@ -112,6 +112,7 @@ extern struct omap_vc_channel omap4_vc_iva;
 extern struct omap_vc_channel omap4_vc_core;
 
 void omap_vc_init_channel(struct voltagedomain *voltdm);
+unsigned long omap_vc_get_bypass_data(struct voltagedomain *voltdm);
 int omap_vc_pre_scale(struct voltagedomain *voltdm,
 		      unsigned long target_volt,
 		      u8 *target_vsel, u8 *current_vsel);

1.7.9.5

nit: since it's not actually used in this series, it should say Impelment the function that will be used as the ->get_voltage() callback...

From: Mike Turquette <[email protected]>

Implement the voltdm->get_voltage callback for the voltage processor driver. This reads the INITVOLTAGE field corresponding to each VP and returns the voltage, after converting it from vsel format.

If INITVOLTAGE is zero (the reset value) then the caller must interpret this as the PMIC running at the default power-on voltage. In such a case DT data for the PMIC is necessary to know the voltage.

---
 arch/arm/mach-omap2/vp.c |   22 ++++++++++++++++++++++
 arch/arm/mach-omap2/vp.h |    1 +
 2 files changed, 23 insertions(+)

index f95c1ba..63dc03e 100644
@@ -9,6 +9,28 @@
 #include "prm-regbits-44xx.h"
 #include "prm44xx.h"
 
+unsigned long omap_vp_get_init_voltage(struct voltagedomain *voltdm)
+{
+	struct omap_vp_instance *vp = voltdm->vp;
+	u32 vpconfig;
+	u8 vsel;
+	unsigned long volt;
+
+	/* sanity */
+	if (!voltdm->pmic || !voltdm->pmic->vsel_to_uv
+			|| !voltdm->read || !voltdm->write)
+		return 0;
+
+	vpconfig = voltdm->read(vp->vpconfig);
+	vpconfig &= vp->common->vpconfig_initvoltage_mask;
+
+	vsel = vpconfig >> __ffs(vp->common->vpconfig_initvoltage_mask);
+
+	volt = voltdm->pmic->vsel_to_uv(vsel);
+
+	return volt;
+}
+
 static u32 _vp_set_init_voltage(struct voltagedomain *voltdm, u32 volt)
 {
 	struct omap_vp_instance *vp = voltdm->vp;

index 7c155d2..0747a78 100644
@@ -120,6 +120,7 @@ extern struct omap_vp_instance omap4_vp_core;
 void omap_vp_init(struct voltagedomain *voltdm);
 void omap_vp_enable(struct voltagedomain *voltdm);
 void omap_vp_disable(struct voltagedomain *voltdm);
+unsigned long omap_vp_get_init_voltage(struct voltagedomain *voltdm);
 int omap_vp_forceupdate_scale(struct voltagedomain *voltdm,
 			      unsigned long target_volt);
 int omap_vp_update_errorgain(struct voltagedomain *voltdm,

1.7.9.5

From: Mike Turquette <[email protected]>

Populate the .get_voltage callbacks for VC & VP. Use these callbacks to populate voltdm->nominal_volt during boot sequence.

This is useful for the Adaptive Body-Bias sequence coming in a subsequent series. Without this patch voltdm->nominal will not be populated until after the first call to voltdm_scale. This is too late to be useful to the pre-scale sequence in the ABB series since that sequence needs to know the voltage prior to the first transition.

Note that the .get_voltage callback will return zero for the corner case wherein the bootloader has not scaling the voltage. In this case it is safe to assume that we are running at the PMIC's default power-on voltage. DT data providing the default voltage for each PMIC is needed in this case.

---
 arch/arm/mach-omap2/voltage.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

index 4dc60e8..f45716c 100644
@@ -244,9 +244,11 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm,
 	switch (voltscale_method) {
 	case VOLTSCALE_VPFORCEUPDATE:
 		voltdm->scale = omap_vp_forceupdate_scale;
+		voltdm->get_voltage = omap_vp_get_init_voltage;
 		return;
 	case VOLTSCALE_VCBYPASS:
 		voltdm->scale = omap_vc_bypass_scale;
+		voltdm->get_voltage = omap_vc_get_bypass_data;
 		return;
 	default:
 		pr_warning("%s: Trying to change the method of voltage scaling"

@@ -288,13 +290,24 @@ int __init omap_voltage_late_init(void)
 
 		if (voltdm->vc) {
 			voltdm->scale = omap_vc_bypass_scale;
+			voltdm->get_voltage = omap_vc_get_bypass_data;
 			omap_vc_init_channel(voltdm);
 		}
 
 		if (voltdm->vp) {
 			voltdm->scale = omap_vp_forceupdate_scale;
+			voltdm->get_voltage = omap_vp_get_init_voltage;
 			omap_vp_init(voltdm);
 		}
+
+		/*
+		 * XXX If voltdm->nominal_volt is zero after calling
+		 * voltdm->get_voltage then we are likely running this
+		 * voltage domain at the default boot voltage of the
+		 * PMIC.  In such a case it would be best to load this
+		 * value from DT.
+		 */
+		voltdm->nominal_volt = voltdm->get_voltage(voltdm);
 	}
 
 	return 0;

1.7.9.5

Hi Mike,

I pointed out a couple nitpicky things on individual patches, but otherwise this direction and motiviation for this series looks OK by me.

Just some minor comments about the structure of the series. I tend to prefer combining the introduction of a new function with it's usage. It makes review and understanding much easier, IMO. If there are reasons to keep them separate, that's fine too. Just describe the reasons in the cover letter.

Thanks,

Kevin

Quoting Kevin Hilman (2012-10-26 11:17:51)

Will do. I've thought on this topic a bit more and this approach isn't quite robust enough. For instance it assumes that the kernel's voltage scaling approach (vp_forceupdate vs. vc_bypass) matches what the bootloader does. This is of course an unsafe assumption.

Also I've seen the Android guys struggle with this a bit and I'd like to review how they approached the problem before sending my next series. Couple with upcoming travel I'm not sure how quickly I'll get a V2 on the list.

Thanks for reviewing,
Mike

Hi Mike, Kevin,

Just checking on the status of this series. Is this superseded by Andrii's driver? Does it need to be reposted for merging? etc.

- Paul

Hi Paul,

Thanks for resurrecting this thread. This series is needed if the ABB transition is triggered by the OMAP VC/VP code. Andrii's approach triggers an ABB transition based on the clock rate-change notifiers but that is still in discussion.

So the short answer is "I don't know yet".

Regards,
Mike