OMAP3+: PM: introduce SR class 1.5

OMAP3 SmartReflex IRQs in hwmod structures with the same naming as present in OMAP4. Without these IRQs being registered, SmartReflex driver will be unable to get the IRQ numbers to handle notifications

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +0 -0

index 196a834..2d464bc 100644
@@ -378,6 +378,15 @@ static struct omap_hwmod_ocp_if omap3_l4_core__i2c3 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +8 -0

 /* L4 CORE -> SR1 interface */
 static struct omap_hwmod_addr_space omap3_sr1_addr_space[] = {
 	{

@@ -2925,6 +2934,8 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES2 |
 					CHIP_IS_OMAP3430ES3_0 |
 					CHIP_IS_OMAP3430ES3_1),
+	.mpu_irqs	= omap3_smartreflex_mpu_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3_smartreflex_mpu_irqs),
 	.flags		= HWMOD_SET_DEFAULT_CLOCKACT,
 };
 

@@ -2945,6 +2956,8 @@ static struct omap_hwmod omap36xx_sr1_hwmod = {
 	.slaves		= omap3_sr1_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap3_sr1_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3630ES1),
+	.mpu_irqs	= omap3_smartreflex_mpu_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3_smartreflex_mpu_irqs),
 };
 
 /* SR2 */

@@ -2971,6 +2984,8 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES2 |
 					CHIP_IS_OMAP3430ES3_0 |
 					CHIP_IS_OMAP3430ES3_1),
+	.mpu_irqs	= omap3_smartreflex_core_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3_smartreflex_core_irqs),
 	.flags		= HWMOD_SET_DEFAULT_CLOCKACT,
 };
 

@@ -2991,6 +3006,8 @@ static struct omap_hwmod omap36xx_sr2_hwmod = {
 	.slaves		= omap3_sr2_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap3_sr2_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3630ES1),
+	.mpu_irqs	= omap3_smartreflex_core_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3_smartreflex_core_irqs),
 };
 
 /*

OMAP3 SmartReflex IRQs in hwmod structures with the same naming as present in OMAP4. Without these IRQs being registered, SmartReflex driver will be unable to get the IRQ numbers to handle notifications

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +17 -0

index 196a834..2d464bc 100644
@@ -378,6 +378,15 @@ static struct omap_hwmod_ocp_if omap3_l4_core__i2c3 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
+
+static struct omap_hwmod_irq_info omap3_smartreflex_mpu_irqs[] = {
+	{.name = "sr1_irq", .irq = 18},
+};
+
+static struct omap_hwmod_irq_info omap3_smartreflex_core_irqs[] = {
+	{.name = "sr2_irq", .irq = 19},
+};
+
 /* L4 CORE -> SR1 interface */
 static struct omap_hwmod_addr_space omap3_sr1_addr_space[] = {
 	{

@@ -2925,6 +2934,8 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES2 |
 					CHIP_IS_OMAP3430ES3_0 |
 					CHIP_IS_OMAP3430ES3_1),
+	.mpu_irqs	= omap3_smartreflex_mpu_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3_smartreflex_mpu_irqs),
 	.flags		= HWMOD_SET_DEFAULT_CLOCKACT,
 };
 

@@ -2945,6 +2956,8 @@ static struct omap_hwmod omap36xx_sr1_hwmod = {
 	.slaves		= omap3_sr1_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap3_sr1_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3630ES1),
+	.mpu_irqs	= omap3_smartreflex_mpu_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3_smartreflex_mpu_irqs),
 };
 
 /* SR2 */

@@ -2971,6 +2984,8 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES2 |
 					CHIP_IS_OMAP3430ES3_0 |
 					CHIP_IS_OMAP3430ES3_1),
+	.mpu_irqs	= omap3_smartreflex_core_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3_smartreflex_core_irqs),
 	.flags		= HWMOD_SET_DEFAULT_CLOCKACT,
 };
 

@@ -2991,6 +3006,8 @@ static struct omap_hwmod omap36xx_sr2_hwmod = {
 	.slaves		= omap3_sr2_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap3_sr2_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3630ES1),
+	.mpu_irqs	= omap3_smartreflex_core_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3_smartreflex_core_irqs),
 };
 
 /*

Signature · 6 lines

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Handle the case for a future SoC where sys_ck_name might be
uninitialized. Fixes the build warning:
arch/arm/mach-omap2/voltage.c: In function 'omap_voltage_late_init':
arch/arm/mach-omap2/voltage.c:86:8: warning: 'sys_ck_name' may be used uninitialized in this function

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/voltage.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/voltage.c +6 -2

index e0cbd93..56961df 100644
@@ -91,10 +91,14 @@ static int __init _config_common_vdd_data(struct omap_vdd_info *vdd)
 	 * XXX Clockfw should handle this, or this should be in a
 	 * struct record
 	 */
-	if (cpu_is_omap24xx() || cpu_is_omap34xx())
+	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
 		sys_ck_name = "sys_ck";
-	else if (cpu_is_omap44xx())
+	} else if (cpu_is_omap44xx()) {
 		sys_ck_name = "sys_clkin_ck";
+	} else {
+		pr_err("%s: Unsupported OMAP\n", __func__);
+		return -EINVAL;
+	}
 
 	/*
 	 * Sys clk rate is require to calculate vp timeout value and

Handle the case for a future SoC where sys_ck_name might be
uninitialized. Fixes the build warning:
arch/arm/mach-omap2/voltage.c: In function 'omap_voltage_late_init':
arch/arm/mach-omap2/voltage.c:86:8: warning: 'sys_ck_name' may be used uninitialized in this function

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/voltage.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/voltage.c +6 -2

index e0cbd93..56961df 100644
@@ -91,10 +91,14 @@ static int __init _config_common_vdd_data(struct omap_vdd_info *vdd)
 	 * XXX Clockfw should handle this, or this should be in a
 	 * struct record
 	 */
-	if (cpu_is_omap24xx() || cpu_is_omap34xx())
+	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
 		sys_ck_name = "sys_ck";
-	else if (cpu_is_omap44xx())
+	} else if (cpu_is_omap44xx()) {
 		sys_ck_name = "sys_clkin_ck";
+	} else {
+		pr_err("%s: Unsupported OMAP\n", __func__);
+		return -EINVAL;
+	}
 
 	/*
 	 * Sys clk rate is require to calculate vp timeout value and

Signature · 6 lines

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Blindly setting a 1.2V setting

in the initial structure may not even
match the default voltages stored in the voltage table which are supported for the domain. For example, OMAP3430 core domain does not use 1.2V and ends up generating a warning on the first transition.

Further, since omap2_set_init_voltage is called as part of the pm framework's initialization sequence to configure the voltage required for the current OPP, the call does(and has to) setup the system voltage(curr_volt as a result) using the right mechanisms appropriate for the system at that point of time. This also overrides initialization we are currently doing in voltage.c making it redundant. So, remove the wrong and redundant initialization.

Signed-off-by Nishanth Menon<[email protected]>

Quoted 2 lines

Blindly setting a 1.2V setting

Hm, seems tautological...

Quoted 4 lines

in the initial structure may not even
match the default voltages stored in the voltage table which are supported for the domain. For example, OMAP3430 core domain does not use 1.2V and ends up generating a warning on the first transition.

Quoted 3 lines

Quoted 1 line

Blindly setting a 1.2V setting

Hm, seems tautological...

As a non native English speaker, I would like to understand how we could improve this. I suppose you mean "Blindly setting 1.2V" is better here. Am i correct?

Hello.

Quoted 1 line · Nishanth Menon

Quoted 1 line

Quoted 1 line

Blindly setting a 1.2V setting

Quoted 5 lines

Quoted 1 line

Hm, seems tautological...

As a non native English speaker, I would like to understand how we could improve this. I suppose you mean "Blindly setting 1.2V" is better here. Am i correct?

Yes.

Quoted 6 lines · Sergei Shtylyov

Hello.

Quoted 1 line · Nishanth Menon

Quoted 1 line

Quoted 1 line

Blindly setting a 1.2V setting

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Hm, seems tautological...

As a non native English speaker, I would like to understand how we could improve this. I suppose you mean "Blindly setting 1.2V" is better here. Am i correct?

Yes.

OK, here is the update as per the suggestion:
Regards,
Nishanth Menon
From: Nishanth Menon <[email protected]>
Date: Tue, 15 Feb 2011 09:37:18 +0530
Subject: [PATCH V4 3/19] OMAP3+: voltage: remove initial voltage

Blindly setting 1.2V in the initial structure may not even match the default voltages stored in the voltage table which are supported for the domain. For example, OMAP3430 core domain does not use 1.2V and ends up generating a warning on the first transition.

Further, since omap2_set_init_voltage is called as part of the pm framework's initialization sequence to configure the voltage required for the current OPP, the call does(and has to) setup the system voltage(curr_volt as a result) using the right mechanisms appropriate for the system at that point of time. This also overrides initialization we are currently doing in voltage.c making it redundant. So, remove the wrong and redundant initialization.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/voltage.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/voltage.c +0 -1

index 56961df..1b40aa0 100644
@@ -116,7 +116,6 @@ static int __init _config_common_vdd_data(struct omap_vdd_info *vdd)
 	sys_clk_speed /= 1000;
 
 	/* Generic voltage parameters */
-	vdd->curr_volt = 1200000;
 	vdd->volt_scale = vp_forceupdate_scale_voltage;
 	vdd->vp_enabled = false;
 

Signature · 6 lines

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Blindly setting a 1.2V setting in the initial structure may not even match the default voltages stored in the voltage table which are supported for the domain. For example, OMAP3430 core domain does not use 1.2V and ends up generating a warning on the first transition.

Further, since omap2_set_init_voltage is called as part of the pm framework's initialization sequence to configure the voltage required for the current OPP, the call does(and has to) setup the system voltage(curr_volt as a result) using the right mechanisms appropriate for the system at that point of time. This also overrides initialization we are currently doing in voltage.c making it redundant. So, remove the wrong and redundant initialization.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/voltage.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/voltage.c +0 -1

index 56961df..1b40aa0 100644
@@ -116,7 +116,6 @@ static int __init _config_common_vdd_data(struct omap_vdd_info *vdd)
 	sys_clk_speed /= 1000;
 
 	/* Generic voltage parameters */
-	vdd->curr_volt = 1200000;
 	vdd->volt_scale = vp_forceupdate_scale_voltage;
 	vdd->vp_enabled = false;

cat of debugfs entry for vp_volt provides voltage. The additional pr_notice is just spam on console and provides no additional information.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/voltage.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/voltage.c +0 -1

index 1b40aa0..53c399f 100644
@@ -150,7 +150,6 @@ static int vp_volt_debug_get(void *data, u64 *val)
 	}
 
 	vsel = vdd->read_reg(prm_mod_offs, vdd->vp_data->voltage);
-	pr_notice("curr_vsel = %x\n", vsel);
 
 	if (!vdd->pmic_info->vsel_to_uv) {
 		pr_warning("PMIC function to convert vsel to voltage"

cat of debugfs entry for vp_volt provides voltage. The additional pr_notice is just spam on console and provides no additional information.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/voltage.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/voltage.c +0 -1

index 1b40aa0..53c399f 100644
@@ -150,7 +150,6 @@ static int vp_volt_debug_get(void *data, u64 *val)
 	}
 
 	vsel = vdd->read_reg(prm_mod_offs, vdd->vp_data->voltage);
-	pr_notice("curr_vsel = %x\n", vsel);
 
 	if (!vdd->pmic_info->vsel_to_uv) {
 		pr_warning("PMIC function to convert vsel to voltage"

Signature · 6 lines

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Use IS_ERR_OR_NULL macro instead of !xyz || IS_ERR(xyz) usage.

Signed-off-by Nishanth Menon <[email protected]>

---
?arch/arm/mach-omap2/voltage.c | ? 22 +++++++++++----------- ?1 files changed, 11 insertions(+), 11 deletions(-)

arch/arm/mach-omap2/voltage.c +0 -0

index 53c399f..76bcaee 100644

arch/arm/mach-omap2/voltage.c +10 -11

 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return 0;
 ? ? ? ?}

@@ -703,7 +703,7 @@ unsigned long omap_vp_get_curr_volt(struct voltagedomain *voltdm)
 ? ? ? ?struct omap_vdd_info *vdd;
 ? ? ? ?u8 curr_vsel;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return 0;
 ? ? ? ?}

@@ -738,7 +738,7 @@ void omap_vp_enable(struct voltagedomain *voltdm)
 ? ? ? ?struct omap_vdd_info *vdd;
 ? ? ? ?u32 vpconfig;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return;
 ? ? ? ?}

@@ -776,7 +776,7 @@ void omap_vp_disable(struct voltagedomain *voltdm)
 ? ? ? ?u32 vpconfig;
 ? ? ? ?int timeout;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return;
 ? ? ? ?}

@@ -829,7 +829,7 @@ int omap_voltage_scale_vdd(struct voltagedomain *voltdm,
 ?{
 ? ? ? ?struct omap_vdd_info *vdd;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return -EINVAL;
 ? ? ? ?}

@@ -858,7 +858,7 @@ void omap_voltage_reset(struct voltagedomain *voltdm)
 ?{
 ? ? ? ?unsigned long target_uvdc;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return;
 ? ? ? ?}

@@ -890,7 +890,7 @@ void omap_voltage_get_volttable(struct voltagedomain *voltdm,
 ?{
 ? ? ? ?struct omap_vdd_info *vdd;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return;
 ? ? ? ?}

@@ -921,7 +921,7 @@ struct omap_volt_data *omap_voltage_get_voltdata(struct voltagedomain *voltdm,
 ? ? ? ?struct omap_vdd_info *vdd;
 ? ? ? ?int i;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return ERR_PTR(-EINVAL);
 ? ? ? ?}

@@ -959,7 +959,7 @@ int omap_voltage_register_pmic(struct voltagedomain *voltdm,
 ?{
 ? ? ? ?struct omap_vdd_info *vdd;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return -EINVAL;
 ? ? ? ?}

@@ -986,7 +986,7 @@ struct dentry *omap_voltage_get_dbgdir(struct voltagedomain *voltdm)
 ?{
 ? ? ? ?struct omap_vdd_info *vdd;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return NULL;
 ? ? ? ?}

@@ -1011,7 +1011,7 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm,
 ?{
 ? ? ? ?struct omap_vdd_info *vdd;
 
- ? ? ? if (!voltdm || IS_ERR(voltdm)) {
+ ? ? ? if (IS_ERR_OR_NULL(voltdm)) {
 ? ? ? ? ? ? ? ?pr_warning("%s: VDD specified does not exist!\n", __func__);
 ? ? ? ? ? ? ? ?return;
 ? ? ? ?}
--

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

Quoted 8 lines

arch/arm/mach-omap2/voltage.c +0 -0

index 53c399f..76bcaee 100644
@@ -682,7 +682,7 @@ unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm)
  {
        struct omap_vdd_info *vdd;
 

arch/arm/mach-omap2/voltage.c unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm) · line 685

Quoted 2 lines

arch/arm/mach-omap2/voltage.c +1 -1

-       if (!voltdm || IS_ERR(voltdm)) {
+       if (IS_ERR_OR_NULL(voltdm)) {

I have one comment here. voltdm is received as parameter and it's already checked by IS_ERR(). Is there any specific reason for that?

IS_ERR() doesn't suppose to be a macro to check if the pointer is valid or not, but to know if there's an invalid value with error code in the pointer value. It's useful when you have a function which returns a pointer but it can return an error code when it fails. Please, note that IS_ERR() won't detect invalid pointers which does not represent an error code, so it's not correct to rely on it for this purpose.
IMO, instead of change to IS_ERR_OR_NULL(), you could only remove IS_ERR(). The same apply for the other cases.

my_dumb_func(){
	struct voltagedomain *vdata = NULL;
	if (cpu_is_omap3630()_) {
		vdata = omap_voltage_domain_lookup("mpu")
	}
	/* forgot to add other cpu types or a else case */
	/* do other things */
	me_volt=omap_voltage_get_nom_volt(vdata);
	/* do things with me_volt */
}

Sorry, but i think the check, even if seems superfluous is sane IMHO. even if the above code worked on 3630, it'd fail on o4/3430 without the check, it can even crash. and given that we've all seen our fair share of developers who develop for one platform without consideration that the rest of the world uses others as well... I do feel cases like above example might infact be a reality.

Quoted 11 lines

my_dumb_func(){
	struct voltagedomain *vdata = NULL;
	if (cpu_is_omap3630()_) {
		vdata = omap_voltage_domain_lookup("mpu")
	}
	/* forgot to add other cpu types or a else case */
	/* do other things */
	me_volt=omap_voltage_get_nom_volt(vdata);
	/* do things with me_volt */
}

Quoted 6 lines

Sorry, but i think the check, even if seems superfluous is sane IMHO. even if the above code worked on 3630, it'd fail on o4/3430 without the check, it can even crash. and given that we've all seen our fair share of developers who develop for one platform without consideration that the rest of the world uses others as well... I do feel cases like above example might infact be a reality.

But normal practice is to check the return value from functions before it's used. So:

  my_dumb_func(){
	struct voltagedomain *vdata = NULL;
	if (cpu_is_omap3630()) {
		vdata = omap_voltage_domain_lookup("mpu")
	}
	/* forgot to add other cpu types or a else case */

(quoted diff) +2 -0

+	if (!vdata)
+		return some error;

	/* do other things */
	me_volt=omap_voltage_get_nom_volt(vdata);
	/* do things with me_volt */

}

is the right way to deal with this. Pushing the primary error checking down into sub-functions is stupid - where does it stop? Do you check me_volt for errors? Do you get functions which use me_volt to check for errors from that too?

It's a silly idea. Put the primary error checking in the function which gets the return value. Don't bury it in sub-functions.

Use IS_ERR_OR_NULL macro instead of !xyz || IS_ERR(xyz) usage.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/voltage.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

arch/arm/mach-omap2/voltage.c +11 -11

index 53c399f..76bcaee 100644
@@ -682,7 +682,7 @@ unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm)
 {
 	struct omap_vdd_info *vdd;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return 0;
 	}

@@ -703,7 +703,7 @@ unsigned long omap_vp_get_curr_volt(struct voltagedomain *voltdm)
 	struct omap_vdd_info *vdd;
 	u8 curr_vsel;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return 0;
 	}

@@ -738,7 +738,7 @@ void omap_vp_enable(struct voltagedomain *voltdm)
 	struct omap_vdd_info *vdd;
 	u32 vpconfig;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return;
 	}

@@ -776,7 +776,7 @@ void omap_vp_disable(struct voltagedomain *voltdm)
 	u32 vpconfig;
 	int timeout;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return;
 	}

@@ -829,7 +829,7 @@ int omap_voltage_scale_vdd(struct voltagedomain *voltdm,
 {
 	struct omap_vdd_info *vdd;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return -EINVAL;
 	}

@@ -858,7 +858,7 @@ void omap_voltage_reset(struct voltagedomain *voltdm)
 {
 	unsigned long target_uvdc;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return;
 	}

@@ -890,7 +890,7 @@ void omap_voltage_get_volttable(struct voltagedomain *voltdm,
 {
 	struct omap_vdd_info *vdd;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return;
 	}

@@ -921,7 +921,7 @@ struct omap_volt_data *omap_voltage_get_voltdata(struct voltagedomain *voltdm,
 	struct omap_vdd_info *vdd;
 	int i;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return ERR_PTR(-EINVAL);
 	}

@@ -959,7 +959,7 @@ int omap_voltage_register_pmic(struct voltagedomain *voltdm,
 {
 	struct omap_vdd_info *vdd;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return -EINVAL;
 	}

@@ -986,7 +986,7 @@ struct dentry *omap_voltage_get_dbgdir(struct voltagedomain *voltdm)
 {
 	struct omap_vdd_info *vdd;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return NULL;
 	}

@@ -1011,7 +1011,7 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm,
 {
 	struct omap_vdd_info *vdd;
 
-	if (!voltdm || IS_ERR(voltdm)) {
+	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
 		return;
 	}

Signature · 6 lines

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Voltage values can get confusing in meaning with various SmartReflex classes being active. Depending on the class used, the actual voltage selected might be a variant. For example:
With SmartReflex AVS class 3:
a) If we don't have SR enabled, we will go to volt_nominal.
b) If have SR enabled, we go to volt_nominal, then enable SR and expect it to adjust voltage. We don't really care about the resultant voltage.
Essentially, when we ask voltage layer to scale to voltage x for OPP y, it always means x is the nominal voltage for OPP y.

Now, once we introduce SmartReflex AVS class 1.5:
a) If you are booting for the first time OR if you never enabled SR before, we always go to volt_nominal.
b) If you enable SR and the OPP is calibrated, we will not enable SR again when that OPP is accessed anymore, but when we set voltage for an OPP, the voltage achieved will be volt_calibrated.
c) If recalibration timeout triggers or SR is disabled after a calibration, the calibrated values are not valid anymore, at this point, setting the voltage will mean volt_dynamic_nominal.
So, depending on which state the system is at, voltage for that OPP we are setting has not 1 single value anymore, but 3 possible valid values.

For upper layers(DVFS/cpufreq OMAP SoC layers) to use voltage values, it will need to know which type of voltage AVS strategy is being used and the corresponding system state from voltage layer perspective. This would replicate the role of voltage layer, SmartReflex AVS in the upper layers and make the corresponding implementations complex.

Since each voltage domain contains a set of volt_data structs representing a voltage point that is supported for that domain, volt_data is a more accurate representation of the voltage point we are interested in going to, and the actual translation of this voltage point to the voltage value is done inside the voltage layer. Doing this allows the users of the voltage layer to be blissfully ignorant of any complexity of the underneath layers and simplify the implementation of dependent layers.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/pm.c                 |    3 +-
 arch/arm/mach-omap2/smartreflex-class3.c |    3 +-
 arch/arm/mach-omap2/voltage.c            |   75 +++++++++++++++++-------------
 arch/arm/mach-omap2/voltage.h            |   17 +++++--
 4 files changed, 59 insertions(+), 39 deletions(-)

arch/arm/mach-omap2/pm.c +0 -0

index 2c3a253..2372744 100644

arch/arm/mach-omap2/pm.c +0 -0

 	return 0;
 
 exit:

arch/arm/mach-omap2/smartreflex-class3.c +0 -0

index f438cf4..2ee48af 100644

arch/arm/mach-omap2/smartreflex-class3.c +0 -0

 	if (!volt) {
 		pr_warning("%s: Curr voltage unknown. Cannot enable sr_%s\n",

arch/arm/mach-omap2/voltage.c +9 -2

index 76bcaee..a12ac1e 100644
@@ -58,7 +58,7 @@ static struct dentry *voltage_dir;
 
 /* Init function pointers */
 static int vp_forceupdate_scale_voltage(struct omap_vdd_info *vdd,
-					unsigned long target_volt);
+					struct omap_volt_data *target_volt);
 
 static u32 omap3_voltage_read_reg(u16 mod, u8 offset)
 {

@@ -164,13 +164,20 @@ static int vp_volt_debug_get(void *data, u64 *val)
 static int nom_volt_debug_get(void *data, u64 *val)
 {
 	struct omap_vdd_info *vdd = (struct omap_vdd_info *) data;
+	struct omap_volt_data *volt_data;
 
 	if (!vdd) {
 		pr_warning("Wrong paramater passed\n");
 		return -EINVAL;
 	}
 
-	*val = omap_voltage_get_nom_volt(&vdd->voltdm);
+	volt_data = omap_voltage_get_nom_volt(&vdd->voltdm);
+	if (IS_ERR_OR_NULL(volt_data)) {
+		pr_warning("%s: No voltage/domain?\n", __func__);
+		return -ENODEV;
+	}
+
+	*val = volt_data->volt_nominal;
 
 	return 0;
 }

arch/arm/mach-omap2/voltage.c +0 -0

 	if (!uvdc) {
 		pr_warning("%s: unable to find current voltage for vdd_%s\n",
 			__func__, vdd->voltdm.name);

@@ -302,13 +310,19 @@ static void __init vdd_debugfs_init(struct omap_vdd_info *vdd)
 
 /* Voltage scale and accessory APIs */
 static int _pre_volt_scale(struct omap_vdd_info *vdd,

arch/arm/mach-omap2/voltage.c +5 -0

+			!target_vsel || !current_vsel) {
+		pr_err("%s: invalid parms!\n", __func__);
+		return -EINVAL;
+	}
+
 	vc_common = vdd->vc_data->vc_common;
 	vp_common = vdd->vp_data->vp_common;
 

@@ -332,12 +346,8 @@ static int _pre_volt_scale(struct omap_vdd_info *vdd,
 		return -EINVAL;

arch/arm/mach-omap2/voltage.c +16 -18

 	*current_vsel = vdd->read_reg(prm_mod_offs, vdd->vp_data->voltage);
 
 	/* Setting the ON voltage to the new target voltage */

@@ -347,22 +357,21 @@ static int _pre_volt_scale(struct omap_vdd_info *vdd,
 	vdd->write_reg(vc_cmdval, prm_mod_offs, vdd->vc_data->cmdval_reg);
 
 	/* Setting vp errorgain based on the voltage */
-	if (volt_data) {
-		vp_errgain_val = vdd->read_reg(prm_mod_offs,
-					       vdd->vp_data->vpconfig);
-		vdd->vp_rt_data.vpconfig_errorgain = volt_data->vp_errgain;
-		vp_errgain_val &= ~vp_common->vpconfig_errorgain_mask;
-		vp_errgain_val |= vdd->vp_rt_data.vpconfig_errorgain <<
-			vp_common->vpconfig_errorgain_shift;
-		vdd->write_reg(vp_errgain_val, prm_mod_offs,
-			       vdd->vp_data->vpconfig);
-	}
+	vp_errgain_val = vdd->read_reg(prm_mod_offs,
+				       vdd->vp_data->vpconfig);
+	vdd->vp_rt_data.vpconfig_errorgain = target_volt->vp_errgain;
+	vp_errgain_val &= ~vp_common->vpconfig_errorgain_mask;
+	vp_errgain_val |= vdd->vp_rt_data.vpconfig_errorgain <<
+		vp_common->vpconfig_errorgain_shift;
+	vdd->write_reg(vp_errgain_val, prm_mod_offs,
+		       vdd->vp_data->vpconfig);
 
 	return 0;
 }
 
 static void _post_volt_scale(struct omap_vdd_info *vdd,
-		unsigned long target_volt, u8 target_vsel, u8 current_vsel)
+		struct omap_volt_data *target_volt, u8 target_vsel,
+		u8 current_vsel)
 {
 	u32 smps_steps = 0, smps_delay = 0;
 

@@ -377,7 +386,7 @@ static void _post_volt_scale(struct omap_vdd_info *vdd,
 
 /* vc_bypass_scale_voltage - VC bypass method of voltage scaling */
 static int vc_bypass_scale_voltage(struct omap_vdd_info *vdd,
-		unsigned long target_volt)
+		struct omap_volt_data *target_volt)
 {
 	u32 loop_cnt = 0, retries_cnt = 0;
 	u32 vc_valid, vc_bypass_val_reg, vc_bypass_value;

@@ -429,7 +438,7 @@ static int vc_bypass_scale_voltage(struct omap_vdd_info *vdd,
 
 /* VP force update method of voltage scaling */
 static int vp_forceupdate_scale_voltage(struct omap_vdd_info *vdd,
-		unsigned long target_volt)
+		struct omap_volt_data *target_volt)
 {
 	u32 vpconfig;
 	u8 target_vsel, current_vsel, prm_irqst_reg;

@@ -675,16 +684,15 @@ ovdc_out:
  * omap_voltage_get_nom_volt() - Gets the current non-auto-compensated voltage
  * @voltdm:	pointer to the VDD for which current voltage info is needed
  *
- * API to get the current non-auto-compensated voltage for a VDD.
- * Returns 0 in case of error else returns the current voltage for the VDD.
+ * API to get the current voltage data pointer for a VDD.
  */
-unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm)
+struct omap_volt_data *omap_voltage_get_nom_volt(struct voltagedomain *voltdm)
 {
 	struct omap_vdd_info *vdd;
 
 	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
-		return 0;
+		return ERR_PTR(-ENODATA);
 	}
 
 	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);

@@ -819,18 +827,19 @@ void omap_vp_disable(struct voltagedomain *voltdm)
  * omap_voltage_scale_vdd() - API to scale voltage of a particular
  *				voltage domain.
  * @voltdm:	pointer to the VDD which is to be scaled.
- * @target_volt:	The target voltage of the voltage domain
+ * @target_volt:	The target voltage data for the voltage domain
  *
  * This API should be called by the kernel to do the voltage scaling
  * for a particular voltage domain during dvfs or any other situation.

arch/arm/mach-omap2/voltage.c +3 -1

+		pr_warning("%s: Bad Params vdm=%p tv=%p!\n", __func__,
+				voltdm, target_volt);
 		return -EINVAL;
 	}
 

@@ -856,7 +865,7 @@ int omap_voltage_scale_vdd(struct voltagedomain *voltdm,
  */
 void omap_voltage_reset(struct voltagedomain *voltdm)
 {
-	unsigned long target_uvdc;
+	struct omap_volt_data *target_uvdc;
 
 	if (IS_ERR_OR_NULL(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);

arch/arm/mach-omap2/voltage.h +4 -4

index e9f5408..6e9acd6 100644
@@ -131,25 +131,25 @@ struct omap_vdd_info {
 	const struct omap_vfsm_instance_data *vfsm;
 	struct voltagedomain voltdm;
 	struct dentry *debug_dir;
-	u32 curr_volt;
+	struct omap_volt_data *curr_volt;
 	bool vp_enabled;
 	u32 (*read_reg) (u16 mod, u8 offset);
 	void (*write_reg) (u32 val, u16 mod, u8 offset);
 	int (*volt_scale) (struct omap_vdd_info *vdd,
-		unsigned long target_volt);
+		struct omap_volt_data *target_volt);
 };
 
 unsigned long omap_vp_get_curr_volt(struct voltagedomain *voltdm);
 void omap_vp_enable(struct voltagedomain *voltdm);
 void omap_vp_disable(struct voltagedomain *voltdm);
 int omap_voltage_scale_vdd(struct voltagedomain *voltdm,
-		unsigned long target_volt);
+		struct omap_volt_data *target_volt);
 void omap_voltage_reset(struct voltagedomain *voltdm);
 void omap_voltage_get_volttable(struct voltagedomain *voltdm,
 		struct omap_volt_data **volt_data);
 struct omap_volt_data *omap_voltage_get_voltdata(struct voltagedomain *voltdm,
 		unsigned long volt);
-unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm);
+struct omap_volt_data *omap_voltage_get_nom_volt(struct voltagedomain *voltdm);
 struct dentry *omap_voltage_get_dbgdir(struct voltagedomain *voltdm);
 int __init omap_voltage_early_init(s16 prm_mod, s16 prm_irqst_mod,
 				   struct omap_vdd_info *omap_vdd_array[],

Transdone event in Voltage processor gives us fine grained status on

the current status of the voltage communication with the PMIC. Unfortunately, IRQ generation by VP is based on the start of the transmission to VC from VP, not at the end (or the completion of the voltage setting). Hence any users of voltage layer who need to know fine grained information such as confirmation if the voltage is actually send to PMIC, needs to depend on this status.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/voltage.c |   57 +++++++++++++++++++++++++++++++++++------
 arch/arm/mach-omap2/voltage.h |    2 +
 2 files changed, 51 insertions(+), 8 deletions(-)

arch/arm/mach-omap2/voltage.c +7 -8

index a12ac1e..2d70d13 100644
@@ -455,10 +455,8 @@ static int vp_forceupdate_scale_voltage(struct omap_vdd_info *vdd,
 	 * is <3us
 	 */
 	while (timeout++ < VP_TRANXDONE_TIMEOUT) {
-		vdd->write_reg(vdd->vp_data->prm_irqst_data->tranxdone_status,
-			       prm_irqst_ocp_mod_offs, prm_irqst_reg);
-		if (!(vdd->read_reg(prm_irqst_ocp_mod_offs, prm_irqst_reg) &
-		      vdd->vp_data->prm_irqst_data->tranxdone_status))
+		omap_vp_clear_transdone(&vdd->voltdm);
+		if (!omap_vp_is_transdone(&vdd->voltdm))
 			break;
 		udelay(1);
 	}

@@ -506,10 +504,8 @@ static int vp_forceupdate_scale_voltage(struct omap_vdd_info *vdd,
 	 */
 	timeout = 0;
 	while (timeout++ < VP_TRANXDONE_TIMEOUT) {
-		vdd->write_reg(vdd->vp_data->prm_irqst_data->tranxdone_status,
-			       prm_irqst_ocp_mod_offs, prm_irqst_reg);
-		if (!(vdd->read_reg(prm_irqst_ocp_mod_offs, prm_irqst_reg) &
-		      vdd->vp_data->prm_irqst_data->tranxdone_status))
+		omap_vp_clear_transdone(&vdd->voltdm);
+		if (!omap_vp_is_transdone(&vdd->voltdm))
 			break;
 		udelay(1);
 	}

@@ -824,6 +820,51 @@ void omap_vp_disable(struct voltagedomain *voltdm)
 }
 
 /**
+ * omap_vp_is_transdone() - is voltage transfer done on vp?
+ * @voltdm:	pointer to the VDD which is to be scaled.
+ *

arch/arm/mach-omap2/voltage.c +9 -0

+		pr_warning("%s: Bad Params vdm=%p\n", __func__, voltdm);
+		return false;
+	}
+
+	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
+	return (vdd->read_reg(prm_irqst_ocp_mod_offs,
+		vdd->vp_data->prm_irqst_data->prm_irqst_reg) &
+		vdd->vp_data->prm_irqst_data->tranxdone_status) ? true : false;
+}

arch/arm/mach-omap2/voltage.c +13 -0

+		pr_warning("%s: Bad Params vdm=%p\n", __func__, voltdm);
+		return false;
+	}
+
+	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
+	vdd->write_reg(vdd->vp_data->prm_irqst_data->tranxdone_status,
+			prm_irqst_ocp_mod_offs,
+			vdd->vp_data->prm_irqst_data->prm_irqst_reg);
+
+	return true;
+}
+
+/**
  * omap_voltage_scale_vdd() - API to scale voltage of a particular
  *				voltage domain.
  * @voltdm:	pointer to the VDD which is to be scaled.

arch/arm/mach-omap2/voltage.h +0 -0

index 6e9acd6..5b4e363 100644

Interrupt notification mechanism of SmartReflex can be used by the choice of implementation of the class driver. For example, Class 2 and Class 1.5 of SmartReflex can both use the interrupt notification to identify the transition of voltage or other events.

Hence, the actual class does not matter for notifier. Let the class driver's handling decide how it should be used. SmartReflex driver should provide just the primitives.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/smartreflex.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/smartreflex.c +2 -4

index 2566552..7096300 100644
@@ -142,7 +142,7 @@ static irqreturn_t sr_interrupt(int irq, void *data)
 		sr_write_reg(sr_info, IRQSTATUS, status);
 	}
 
-	if (sr_class->class_type == SR_CLASS2 && sr_class->notify)
+	if (sr_class->notify)
 		sr_class->notify(sr_info->voltdm, status);
 
 	return IRQ_HANDLED;

@@ -257,9 +257,7 @@ static int sr_late_init(struct omap_sr *sr_info)
 	struct resource *mem;
 	int ret = 0;
 
-	if (sr_class->class_type == SR_CLASS2 &&
-		sr_class->notify_flags && sr_info->irq) {
-
+	if (sr_class->notify && sr_class->notify_flags && sr_info->irq) {
 		name = kasprintf(GFP_KERNEL, "sr_%s", sr_info->voltdm->name);
 		if (name == NULL) {
 			ret = -ENOMEM;

We will enable and disable interrupt on a need basis in the class driver. We need to keep the IRQ disabled by default else the forceupdate or vcbypass events could trigger events that we don't need/expect to handle.

This is a preparation for SmartReflex AVS class drivers such as class 2 and class 1.5 which would need to use interrupts. Existing SmartReflex AVS class 3 driver does not require to use interrupts and is not impacted by this change.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/smartreflex.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/smartreflex.c +1 -0

index 7096300..f9057ad 100644
@@ -267,6 +267,7 @@ static int sr_late_init(struct omap_sr *sr_info)
 				0, name, (void *)sr_info);
 		if (ret)
 			goto error;
+		disable_irq(sr_info->irq);
 	}
 
 	if (pdata && pdata->enable_on_init)

Since we already know the state of the autocomp enablement, we can see if the requested state is different from the current state and enable/disable SR only on the need basis.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/smartreflex.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/smartreflex.c +7 -4

index f9057ad..2b6fdc7 100644
@@ -806,10 +806,13 @@ static int omap_sr_autocomp_store(void *data, u64 val)
 		return -EINVAL;
 	}
 
-	if (!val)
-		sr_stop_vddautocomp(sr_info);
-	else
-		sr_start_vddautocomp(sr_info);
+	/* control enable/disable only if there is a delta in value */
+	if (sr_info->autocomp_active != val) {
+		if (!val)
+			sr_stop_vddautocomp(sr_info);
+		else
+			sr_start_vddautocomp(sr_info);
+	}
 
 	return 0;
 }

Error label case seems to have a 2 tab indentation when just 1 is necessary.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/smartreflex.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

arch/arm/mach-omap2/smartreflex.c +10 -10

index 2b6fdc7..1e52d61 100644
@@ -276,16 +276,16 @@ static int sr_late_init(struct omap_sr *sr_info)
 	return ret;
 
 error:
-		iounmap(sr_info->base);
-		mem = platform_get_resource(sr_info->pdev, IORESOURCE_MEM, 0);
-		release_mem_region(mem->start, resource_size(mem));
-		list_del(&sr_info->node);
-		dev_err(&sr_info->pdev->dev, "%s: ERROR in registering"
-			"interrupt handler. Smartreflex will"
-			"not function as desired\n", __func__);
-		kfree(name);
-		kfree(sr_info);
-		return ret;
+	iounmap(sr_info->base);
+	mem = platform_get_resource(sr_info->pdev, IORESOURCE_MEM, 0);
+	release_mem_region(mem->start, resource_size(mem));
+	list_del(&sr_info->node);
+	dev_err(&sr_info->pdev->dev, "%s: ERROR in registering"
+		"interrupt handler. Smartreflex will"
+		"not function as desired\n", __func__);
+	kfree(name);
+	kfree(sr_info);
+	return ret;
 }
 
 static void sr_v1_disable(struct omap_sr *sr)

Thanks, queuing for 2.6.40.

Quoted 13 lines

arch/arm/mach-omap2/smartreflex.c +2 -10

-		iounmap(sr_info->base);
-		mem = platform_get_resource(sr_info->pdev, IORESOURCE_MEM, 0);
-		release_mem_region(mem->start, resource_size(mem));
-		list_del(&sr_info->node);
-		dev_err(&sr_info->pdev->dev, "%s: ERROR in registering"
-			"interrupt handler. Smartreflex will"
-			"not function as desired\n", __func__);
-		kfree(name);
-		kfree(sr_info);
-		return ret;
+	iounmap(sr_info->base);
+	mem = platform_get_resource(sr_info->pdev, IORESOURCE_MEM, 0);

arch/arm/mach-omap2/smartreflex.c static int sr_late_init(struct omap_sr *sr_info) · line 288

Quoted 8 lines

arch/arm/mach-omap2/smartreflex.c +8 -0

+	release_mem_region(mem->start, resource_size(mem));
+	list_del(&sr_info->node);
+	dev_err(&sr_info->pdev->dev, "%s: ERROR in registering"
+		"interrupt handler. Smartreflex will"
+		"not function as desired\n", __func__);
+	kfree(name);
+	kfree(sr_info);
+	return ret;

Hi,

Quoted 3 lines · Kevin Hilman

Thanks, queuing for 2.6.40.

[...]

Quoted 15 lines

Quoted 15 lines

arch/arm/mach-omap2/smartreflex.c +4 -10

-		iounmap(sr_info->base);
-		mem = platform_get_resource(sr_info->pdev, IORESOURCE_MEM, 0);
-		release_mem_region(mem->start, resource_size(mem));
-		list_del(&sr_info->node);
-		dev_err(&sr_info->pdev->dev, "%s: ERROR in registering"
-			"interrupt handler. Smartreflex will"
-			"not function as desired\n", __func__);
-		kfree(name);
-		kfree(sr_info);
-		return ret;
+	iounmap(sr_info->base);
+	mem = platform_get_resource(sr_info->pdev, IORESOURCE_MEM, 0);
+	release_mem_region(mem->start, resource_size(mem));
+	list_del(&sr_info->node);

Quoted 6 lines

arch/arm/mach-omap2/smartreflex.c static int sr_late_init(struct omap_sr *sr_info) · line 288

Quoted 6 lines

arch/arm/mach-omap2/smartreflex.c +6 -0

+	dev_err(&sr_info->pdev->dev, "%s: ERROR in registering"
+		"interrupt handler. Smartreflex will"
+		"not function as desired\n", __func__);
+	kfree(name);
+	kfree(sr_info);
+	return ret;

Just FYI, this code is partly broken and probably need to be fixed in .39.

Some of those resources are also freed by omap_sr_probe() when sr_late_init() fails and that cannot be correct.

Certain class drivers such as class 1.5 drivers, will need specific notification that they have to be started up or stopped independent of smart reflex operation. They also may need private data to be used for operations of their own, provide the same.
This allows the class driver to initialize it's internal data structures on a need basis in preparation for the specific domain's autocomp usage. The resultant operation is as follows:
when autocomp is set to 1 -> class drivers' start is called The existing enable/disable is still used as before by the SmartReflex core driver to enable disable the class operation.
When autocomp is set to 0 -> class drivers' stop is called, signaling the end of usage for that domain.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/smartreflex.c |   14 ++++++++++++++
 arch/arm/mach-omap2/smartreflex.h |    6 ++++++
 2 files changed, 20 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/smartreflex.c +14 -0

index 1e52d61..592e9a0 100644
@@ -220,6 +220,13 @@ static void sr_start_vddautocomp(struct omap_sr *sr)
 		return;
 	}
 
+	if (sr_class->start &&
+	    sr_class->start(sr->voltdm, sr_class->class_priv_data)) {
+		dev_err(&sr->pdev->dev,
+			"%s: SRClass initialization failed\n", __func__);
+		return;
+	}
+
 	if (!sr_class->enable(sr->voltdm))
 		sr->autocomp_active = true;
 }

@@ -235,6 +242,13 @@ static void sr_stop_vddautocomp(struct omap_sr *sr)
 
 	if (sr->autocomp_active) {
 		sr_class->disable(sr->voltdm, 1);
+		if (sr_class->stop &&
+		    sr_class->stop(sr->voltdm,
+			    sr_class->class_priv_data)) {
+			dev_err(&sr->pdev->dev,
+				"%s: SR[%d]Class deinitialization failed\n",
+				__func__, sr->srid);
+		}
 		sr->autocomp_active = false;
 	}
 }

arch/arm/mach-omap2/smartreflex.h +6 -0

index 5f35b9e..d4b8bee 100644
@@ -168,6 +168,8 @@ struct omap_sr_pmic_data {
  *
  * @enable:		API to enable a particular class smaartreflex.
  * @disable:		API to disable a particular class smartreflex.
+ * @start:		API to do class specific initialization (optional)
+ * @stop:		API to do class specific deinitialization (optional)
  * @configure:		API to configure a particular class smartreflex.
  * @notify:		API to notify the class driver about an event in SR.
  *			Not needed for class3.

@@ -175,14 +177,18 @@ struct omap_sr_pmic_data {
  * @class_type:		specify which smartreflex class.
  *			Can be used by the SR driver to take any class
  *			based decisions.
+ * @class_priv_data:	Class specific private data (optional)
  */
 struct omap_sr_class_data {
 	int (*enable)(struct voltagedomain *voltdm);
 	int (*disable)(struct voltagedomain *voltdm, int is_volt_reset);
+	int (*start)(struct voltagedomain *voltdm, void *class_priv_data);
+	int (*stop)(struct voltagedomain *voltdm, void *class_priv_data);
 	int (*configure)(struct voltagedomain *voltdm);
 	int (*notify)(struct voltagedomain *voltdm, u32 status);
 	u8 notify_flags;
 	u8 class_type;
+	void *class_priv_data;
 };
 
 /**

From: Jarkko Nikula <[email protected]>

sr_start_vddautocomp and sr_stop_autocomp functions can be reused from omap_sr_enable, omap_sr_disable and omap_sr_disable_reset_volt and by adding one additional argument sr_stop_autocomp. This allows us to have a single point where SR enable and disable operations are controlled from.

[nm at ti.com: synced it up with class start/stop as well. autocomp flag
should only be disabled when SR is disabled for that domain]

Signed-off-by Nishanth Menon <[email protected]>

Signed-off-by Jarkko Nikula <[email protected]>

---
Side note: Jarkko has'nt seen this rev as I just finished the rebase and tested but adding his signed-off anyways as he is primary contributor to the idea and the patch.

From: Jarkko Nikula <[email protected]>

sr_start_vddautocomp and sr_stop_autocomp functions can be reused from omap_sr_enable, omap_sr_disable and omap_sr_disable_reset_volt and by adding one additional argument sr_stop_autocomp. This allows us to have a single point where SR enable and disable operations are controlled from.

[nm at ti.com: synced it up with class start/stop as well. autocomp flag
should only be disabled when SR is disabled for that domain]

Signed-off-by Nishanth Menon <[email protected]>

Signed-off-by Jarkko Nikula <[email protected]>

---
Side note: Jarkko has'nt seen this rev as I just finished the rebase and tested but adding his signed-off anyways as he is primary contributor to the idea and the patch.

 arch/arm/mach-omap2/smartreflex.c |   66 +++++++++++++------------------------
 1 files changed, 23 insertions(+), 43 deletions(-)

arch/arm/mach-omap2/smartreflex.c +23 -43

index 592e9a0..d839fa6 100644
@@ -211,8 +211,12 @@ static void sr_set_regfields(struct omap_sr *sr)
 	}
 }
 
-static void sr_start_vddautocomp(struct omap_sr *sr)
+static void sr_start_vddautocomp(struct omap_sr *sr, bool class_start)
 {
+	int r;
+	if (sr->autocomp_active)
+		return;
+
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
 		dev_warn(&sr->pdev->dev,
 			"%s: smartreflex class driver not registered\n",

@@ -220,19 +224,24 @@ static void sr_start_vddautocomp(struct omap_sr *sr)
 		return;
 	}
 
-	if (sr_class->start &&
+	if (class_start && sr_class->start &&
 	    sr_class->start(sr->voltdm, sr_class->class_priv_data)) {
 		dev_err(&sr->pdev->dev,
 			"%s: SRClass initialization failed\n", __func__);
 		return;
 	}
 
-	if (!sr_class->enable(sr->voltdm))
+	r = sr_class->enable(sr->voltdm);
+	if (!r && class_start)
 		sr->autocomp_active = true;
 }
 
-static void sr_stop_vddautocomp(struct omap_sr *sr)
+static void sr_stop_vddautocomp(struct omap_sr *sr, bool class_stop,
+		int is_volt_reset)
 {
+	if (!sr->autocomp_active)
+		return;
+
 	if (!sr_class || !(sr_class->disable)) {
 		dev_warn(&sr->pdev->dev,
 			"%s: smartreflex class driver not registered\n",

@@ -240,15 +249,13 @@ static void sr_stop_vddautocomp(struct omap_sr *sr)
 		return;
 	}
 
-	if (sr->autocomp_active) {
-		sr_class->disable(sr->voltdm, 1);
+	sr_class->disable(sr->voltdm, is_volt_reset);
+	if (class_stop) {
 		if (sr_class->stop &&
-		    sr_class->stop(sr->voltdm,
-			    sr_class->class_priv_data)) {
+		    sr_class->stop(sr->voltdm, sr_class->class_priv_data))
 			dev_err(&sr->pdev->dev,
 				"%s: SR[%d]Class deinitialization failed\n",
 				__func__, sr->srid);
-		}
 		sr->autocomp_active = false;
 	}
 }

@@ -285,7 +292,7 @@ static int sr_late_init(struct omap_sr *sr_info)
 	}
 
 	if (pdata && pdata->enable_on_init)
-		sr_start_vddautocomp(sr_info);
+		sr_start_vddautocomp(sr_info, true);
 
 	return ret;
 

@@ -693,16 +700,7 @@ void omap_sr_enable(struct voltagedomain *voltdm)
 		return;
 	}
 
-	if (!sr->autocomp_active)
-		return;
-
-	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
-		dev_warn(&sr->pdev->dev, "%s: smartreflex class driver not"
-			"registered\n", __func__);
-		return;
-	}
-
-	sr_class->enable(voltdm);
+	sr_start_vddautocomp(sr, false);
 }
 
 /**

@@ -726,16 +724,7 @@ void omap_sr_disable(struct voltagedomain *voltdm)
 		return;
 	}
 
-	if (!sr->autocomp_active)
-		return;
-
-	if (!sr_class || !(sr_class->disable)) {
-		dev_warn(&sr->pdev->dev, "%s: smartreflex class driver not"
-			"registered\n", __func__);
-		return;
-	}
-
-	sr_class->disable(voltdm, 0);
+	sr_stop_vddautocomp(sr, false, 0);
 }
 
 /**

@@ -759,16 +748,7 @@ void omap_sr_disable_reset_volt(struct voltagedomain *voltdm)
 		return;
 	}
 
-	if (!sr->autocomp_active)
-		return;
-
-	if (!sr_class || !(sr_class->disable)) {
-		dev_warn(&sr->pdev->dev, "%s: smartreflex class driver not"
-			"registered\n", __func__);
-		return;
-	}
-
-	sr_class->disable(voltdm, 1);
+	sr_stop_vddautocomp(sr, false, 1);
 }
 
 /**

@@ -823,9 +803,9 @@ static int omap_sr_autocomp_store(void *data, u64 val)
 	/* control enable/disable only if there is a delta in value */
 	if (sr_info->autocomp_active != val) {
 		if (!val)
-			sr_stop_vddautocomp(sr_info);
+			sr_stop_vddautocomp(sr_info, true, 1);
 		else
-			sr_start_vddautocomp(sr_info);
+			sr_start_vddautocomp(sr_info, true);
 	}
 
 	return 0;

@@ -987,7 +967,7 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
 	}
 
 	if (sr_info->autocomp_active)
-		sr_stop_vddautocomp(sr_info);
+		sr_stop_vddautocomp(sr_info, true, 1);
 
 	list_del(&sr_info->node);
 	iounmap(sr_info->base);

SmartReflex IP V1 and V2 have different registers and offsets. Currently, we pass the status as is to the class driver. However, since we don't pass the version of the underlying SR hardware to the Class driver, it will not be unable to make consistent sense of the status bits coming over to it.

A class driver should be able to function without dependency on the exact IP version it is actually running on. We hence introduce our own translation in software level for a generic notification flag.

As part of this change, we will now call the notifier iff we get a match with the notifier flags that the class driver requested.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/smartreflex.c |   73 +++++++++++++++++++++++++++++++++++--
 arch/arm/mach-omap2/smartreflex.h |    6 +++
 2 files changed, 76 insertions(+), 3 deletions(-)

arch/arm/mach-omap2/smartreflex.c +0 -0

index d839fa6..21944e2 100644

arch/arm/mach-omap2/smartreflex.c +59 -3

+	return val;
+}
+
+static inline u32 notifier_to_irqen_v2(u8 notify_flags)
+{
+	u32 val;
+	val = (notify_flags & SR_NOTIFY_MCUACCUM) ?
+		IRQENABLE_MCUACCUMINT : 0;
+	val |= (notify_flags & SR_NOTIFY_MCUVALID) ?
+		IRQENABLE_MCUVALIDINT : 0;
+	val |= (notify_flags & SR_NOTIFY_MCUBOUND) ?
+		IRQENABLE_MCUBOUNDSINT : 0;
+	val |= (notify_flags & SR_NOTIFY_MCUDISACK) ?
+		IRQENABLE_MCUDISABLEACKINT : 0;
+	return val;
+}
+
+static inline u8 irqstat_to_notifier_v1(u32 status)
+{
+	u8 val;
+	val = (status & ERRCONFIG_MCUACCUMINTST) ?
+		SR_NOTIFY_MCUACCUM : 0;
+	val |= (status & ERRCONFIG_MCUVALIDINTEN) ?
+		SR_NOTIFY_MCUVALID : 0;
+	val |= (status & ERRCONFIG_MCUBOUNDINTEN) ?
+		SR_NOTIFY_MCUBOUND : 0;
+	val |= (status & ERRCONFIG_MCUDISACKINTEN) ?
+		SR_NOTIFY_MCUDISACK : 0;
+	return val;
+}
+
+static inline u8 irqstat_to_notifier_v2(u32 status)
+{
+	u8 val;
+	val = (status & IRQENABLE_MCUACCUMINT) ?
+		SR_NOTIFY_MCUACCUM : 0;
+	val |= (status & IRQENABLE_MCUVALIDINT) ?
+		SR_NOTIFY_MCUVALID : 0;
+	val |= (status & IRQENABLE_MCUBOUNDSINT) ?
+		SR_NOTIFY_MCUBOUND : 0;
+	val |= (status & IRQENABLE_MCUDISABLEACKINT) ?
+		SR_NOTIFY_MCUDISACK : 0;
+	return val;
+}
+
+
 static irqreturn_t sr_interrupt(int irq, void *data)
 {
 	struct omap_sr *sr_info = (struct omap_sr *)data;
 	u32 status = 0;
+	u32 value = 0;
 
 	if (sr_info->ip_type == SR_TYPE_V1) {
+		/* Status bits are one bit before enable bits in v1 */
+		value = notifier_to_irqen_v1(sr_class->notify_flags) >> 1;
+
 		/* Read the status bits */
 		status = sr_read_reg(sr_info, ERRCONFIG_V1);
+		status &= value;
 
 		/* Clear them by writing back */
-		sr_write_reg(sr_info, ERRCONFIG_V1, status);
+		sr_modify_reg(sr_info, ERRCONFIG_V1, value, status);
+
+		value = irqstat_to_notifier_v1(status);
 	} else if (sr_info->ip_type == SR_TYPE_V2) {
+		value = notifier_to_irqen_v2(sr_class->notify_flags);
 		/* Read the status bits */
-		sr_read_reg(sr_info, IRQSTATUS);
+		status = sr_read_reg(sr_info, IRQSTATUS);
+		status &= value;
 
 		/* Clear them by writing back */
 		sr_write_reg(sr_info, IRQSTATUS, status);
+		value = irqstat_to_notifier_v2(status);
 	}
 
 	if (sr_class->notify)
-		sr_class->notify(sr_info->voltdm, status);
+		sr_class->notify(sr_info->voltdm, value);
 
 	return IRQ_HANDLED;
 }

arch/arm/mach-omap2/smartreflex.h +6 -0

index d4b8bee..2976bf6 100644
@@ -142,6 +142,12 @@
 #define OMAP3430_SR_ERRWEIGHT		0x04
 #define OMAP3430_SR_ERRMAXLIMIT		0x02
 
+/* Smart reflex notifiers for class drivers to use */
+#define SR_NOTIFY_MCUDISACK		BIT(3)
+#define SR_NOTIFY_MCUBOUND		BIT(2)
+#define SR_NOTIFY_MCUVALID		BIT(1)
+#define SR_NOTIFY_MCUACCUM		BIT(0)
+
 /**
  * struct omap_sr_pmic_data - Strucutre to be populated by pmic code to pass
  *				pmic specific info to smartreflex driver

We need some mechanism from class drivers to control when notifiers should be triggered and when not, currently we have none, which makes Class driver usage of the interrupt events almost impossible.

Introduce an SmartReflex driver API for doing the same. This is useful for SmartReflex AVS class 1.5 or 2 drivers.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/smartreflex.c |   57 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/smartreflex.h |    8 +++++
 2 files changed, 65 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/smartreflex.c +4 -0

index 21944e2..f6e44a1 100644
@@ -712,6 +712,63 @@ void sr_disable(struct voltagedomain *voltdm)
 }
 
 /**
+ * sr_notifier_control() - control the notifier mechanism
+ * @voltdm:	VDD pointer to which the SR module to be configured belongs to.
+ * @enable:	true to enable notifiers and false to disable the same
+ *

arch/arm/mach-omap2/smartreflex.c +17 -0

+		return -EINVAL;
+	}
+
+	switch (sr->ip_type) {
+	case SR_TYPE_V1:
+		value = notifier_to_irqen_v1(sr_class->notify_flags);
+		sr_modify_reg(sr, ERRCONFIG_V1, value,
+				(enable) ? value : 0);
+		break;
+	case SR_TYPE_V2:
+		value = notifier_to_irqen_v2(sr_class->notify_flags);
+		sr_write_reg(sr, (enable) ? IRQENABLE_SET : IRQENABLE_CLR,
+				value);
+		break;
+	default:
+		 dev_warn(&sr->pdev->dev, "%s: unknown type of sr??\n",
+				 __func__);

arch/arm/mach-omap2/smartreflex.c +1 -0

+/**
  * sr_register_class() - API to register a smartreflex class parameters.
  * @class_data:	The structure containing various sr class specific data.
  *

arch/arm/mach-omap2/smartreflex.h +8 -0

index 2976bf6..4a1ada4 100644
@@ -242,6 +242,7 @@ void omap_sr_register_pmic(struct omap_sr_pmic_data *pmic_data);
 /* Smartreflex driver hooks to be called from Smartreflex class driver */
 int sr_enable(struct voltagedomain *voltdm, unsigned long volt);
 void sr_disable(struct voltagedomain *voltdm);
+int sr_notifier_control(struct voltagedomain *voltdm, bool enable);
 int sr_configure_errgen(struct voltagedomain *voltdm);
 int sr_configure_minmax(struct voltagedomain *voltdm);
 

@@ -250,6 +251,13 @@ int sr_register_class(struct omap_sr_class_data *class_data);
 #else
 static inline void omap_sr_enable(struct voltagedomain *voltdm) {}
 static inline void omap_sr_disable(struct voltagedomain *voltdm) {}
+
+static inline int sr_notifier_control(struct voltagedomain *voltdm,
+		bool enable)
+{
+	return -EINVAL;
+}
+
 static inline void omap_sr_disable_reset_volt(
 		struct voltagedomain *voltdm) {}
 static inline void omap_sr_register_pmic(

Passing the volt_data pointers across allows us to save us the effort of looking up the voltage data pointer from the voltage value at multiple layers, we need to look at the voltage data in DVFS layer for further processing, so modify the APIs to pass the voltage data pointer all the way through to lower layers to the SmartReflex AVS class drivers.

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/smartreflex-class3.c |   13 +++----------
 arch/arm/mach-omap2/smartreflex.c        |   22 ++++++++++------------
 arch/arm/mach-omap2/smartreflex.h        |    8 +++++---
 3 files changed, 18 insertions(+), 25 deletions(-)

arch/arm/mach-omap2/smartreflex-class3.c +3 -10

index 2ee48af..1d3eb11 100644
@@ -13,19 +13,12 @@
 
 #include "smartreflex.h"
 
-static int sr_class3_enable(struct voltagedomain *voltdm)
+static int sr_class3_enable(struct voltagedomain *voltdm,
+		struct omap_volt_data *volt_data)
 {
-	unsigned long volt = omap_get_operation_voltage(
-		omap_voltage_get_nom_volt(voltdm));
-
-	if (!volt) {
-		pr_warning("%s: Curr voltage unknown. Cannot enable sr_%s\n",
-				__func__, voltdm->name);
-		return -ENODATA;
-	}
 
 	omap_vp_enable(voltdm);
-	return sr_enable(voltdm, volt);
+	return sr_enable(voltdm, volt_data);
 }
 
 static int sr_class3_disable(struct voltagedomain *voltdm, int is_volt_reset)

arch/arm/mach-omap2/smartreflex.c +10 -12

index b1a7cfe..5c549b9 100644
@@ -306,7 +306,7 @@ static void sr_start_vddautocomp(struct omap_sr *sr, bool class_start)
 		return;
 	}
 
-	r = sr_class->enable(sr->voltdm);
+	r = sr_class->enable(sr->voltdm, omap_voltage_get_nom_volt(sr->voltdm));
 	if (!r && class_start)
 		sr->autocomp_active = true;
 }

@@ -626,7 +626,7 @@ int sr_configure_minmax(struct voltagedomain *voltdm)
 /**
  * sr_enable() - Enables the smartreflex module.
  * @voltdm:	VDD pointer to which the SR module to be configured belongs to.
- * @volt:	The voltage at which the Voltage domain associated with
+ * @volt_data:	The voltage at which the Voltage domain associated with
  *		the smartreflex module is operating at.
  *		This is required only to program the correct Ntarget value.
  *

@@ -634,10 +634,9 @@ int sr_configure_minmax(struct voltagedomain *voltdm)
  * enable a smartreflex module. Returns 0 on success. Returns error
  * value if the voltage passed is wrong or if ntarget value is wrong.
  */
-int sr_enable(struct voltagedomain *voltdm, unsigned long volt)
+int sr_enable(struct voltagedomain *voltdm, struct omap_volt_data *volt_data)
 {
 	u32 nvalue_reciprocal;
-	struct omap_volt_data *volt_data;
 	struct omap_sr *sr = _sr_lookup(voltdm);
 	int ret;
 

@@ -647,19 +646,16 @@ int sr_enable(struct voltagedomain *voltdm, unsigned long volt)
 		return -EINVAL;
 	}
 
-	volt_data = omap_voltage_get_voltdata(sr->voltdm, volt);
-
-	if (IS_ERR(volt_data)) {
-		dev_warn(&sr->pdev->dev, "%s: Unable to get voltage table"
-			"for nominal voltage %ld\n", __func__, volt);
-		return -ENODATA;
+	if (IS_ERR_OR_NULL(volt_data)) {
+		dev_warn(&sr->pdev->dev, "%s: bad voltage data\n", __func__);
+		return -EINVAL;
 	}
 
 	nvalue_reciprocal = sr_retrieve_nvalue(sr, volt_data->sr_efuse_offs);
 
 	if (!nvalue_reciprocal) {
 		dev_warn(&sr->pdev->dev, "%s: NVALUE = 0 at voltage %ld\n",
-			__func__, volt);
+			__func__, omap_get_operation_voltage(volt_data));
 		return -ENODATA;
 	}
 

@@ -816,13 +812,15 @@ int sr_register_class(struct omap_sr_class_data *class_data)
  * omap_sr_enable() -  API to enable SR clocks and to call into the
  *			registered smartreflex class enable API.
  * @voltdm:	VDD pointer to which the SR module to be configured belongs to.
+ * @volt_data:	Voltage data to go to
  *
  * This API is to be called from the kernel in order to enable
  * a particular smartreflex module. This API will do the initial
  * configurations to turn on the smartreflex module and in turn call
  * into the registered smartreflex class enable API.
  */
-void omap_sr_enable(struct voltagedomain *voltdm)
+void omap_sr_enable(struct voltagedomain *voltdm,
+			struct omap_volt_data *volt_data)
 {
 	struct omap_sr *sr = _sr_lookup(voltdm);
 

arch/arm/mach-omap2/smartreflex.h +5 -3

index 4a1ada4..812e86d 100644
@@ -186,7 +186,8 @@ struct omap_sr_pmic_data {
  * @class_priv_data:	Class specific private data (optional)
  */
 struct omap_sr_class_data {
-	int (*enable)(struct voltagedomain *voltdm);
+	int (*enable)(struct voltagedomain *voltdm,
+			struct omap_volt_data *volt_data);
 	int (*disable)(struct voltagedomain *voltdm, int is_volt_reset);
 	int (*start)(struct voltagedomain *voltdm, void *class_priv_data);
 	int (*stop)(struct voltagedomain *voltdm, void *class_priv_data);

@@ -232,7 +233,8 @@ struct omap_sr_data {
 };
 
 /* Smartreflex module enable/disable interface */
-void omap_sr_enable(struct voltagedomain *voltdm);
+void omap_sr_enable(struct voltagedomain *voltdm,
+			struct omap_volt_data *volt_data);
 void omap_sr_disable(struct voltagedomain *voltdm);
 void omap_sr_disable_reset_volt(struct voltagedomain *voltdm);
 

@@ -240,7 +242,7 @@ void omap_sr_disable_reset_volt(struct voltagedomain *voltdm);
 void omap_sr_register_pmic(struct omap_sr_pmic_data *pmic_data);
 
 /* Smartreflex driver hooks to be called from Smartreflex class driver */
-int sr_enable(struct voltagedomain *voltdm, unsigned long volt);
+int sr_enable(struct voltagedomain *voltdm, struct omap_volt_data *volt_data);
 void sr_disable(struct voltagedomain *voltdm);
 int sr_notifier_control(struct voltagedomain *voltdm, bool enable);
 int sr_configure_errgen(struct voltagedomain *voltdm);