Skip to contentSkip to conversation

OMAP3+: SR Layer Cleanup

PATCH Superseded Shweta Gulati 4 participants 8 messages May 13, 2011 – May 25, 2011

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH v4 1/1 OMAP3+: SR Layer Cleanup

To set sr ntarget values for all volt_domain, volt_table is retrieved by doing a look_up of 'vdd_name' field from omap_hwmod but voltage domain pointer does not belong to omap_hwmod and is not used anywhere else. As a part of voltage layer and SR Layer clean up volt pointer is removed from omap_hwmod and added in dev attributes of SR.

Tested on OMAP3630 SDP and OMAP4430 SDP Board

Acked by: Nishanth Menon <[email protected]>
Cc: Benoit Cousson <[email protected]>
Cc: Paul Walmsley <[email protected]>
---

V4:
Fixed comments like checking for NULL pointers and following conventions in naming the instances recommended by Todd Poynor and Benoit Cousson.

V3:
Changed the Subject and Rephrased Commit log as reviewed by Nishanth Menon.
V2:
Rebased on latest 'pm-wip/voltdm_a' branch.

 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c   |   17 +++++++++++++----
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c   |   19 ++++++++++++++++---
 arch/arm/mach-omap2/smartreflex.h            |   10 ++++++++++
 arch/arm/mach-omap2/sr_device.c              |   11 +++++++----
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 -
 5 files changed, 46 insertions(+), 12 deletions(-)
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +13 -4
index 3cd91ac..b1b11de 100644
@@ -29,6 +29,7 @@
 
 #include "omap_hwmod_common_data.h"
 
+#include "smartreflex.h"
 #include "prm-regbits-34xx.h"
 #include "cm-regbits-34xx.h"
 #include "wd_timer.h"
@@ -2904,6 +2905,10 @@ static struct omap_hwmod_class omap36xx_smartreflex_hwmod_class = {
 };
 
 /* SR1 */
+static struct omap_smartreflex_dev_attr sr1_dev_attr = {
+	.sensor_voltdm_name   = "mpu_iva",
+};
+
 static struct omap_hwmod_ocp_if *omap3_sr1_slaves[] = {
 	&omap3_l4_core__sr1,
 };
@@ -2912,7 +2917,6 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
 	.name		= "sr1_hwmod",
 	.class		= &omap34xx_smartreflex_hwmod_class,
 	.main_clk	= "sr1_fck",
-	.vdd_name	= "mpu_iva",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -2924,6 +2928,7 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
 	},
 	.slaves		= omap3_sr1_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap3_sr1_slaves),
+	.dev_attr       = &sr1_dev_attr,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES2 |
 					CHIP_IS_OMAP3430ES3_0 |
 					CHIP_IS_OMAP3430ES3_1),
@@ -2934,7 +2939,6 @@ static struct omap_hwmod omap36xx_sr1_hwmod = {
 	.name		= "sr1_hwmod",
 	.class		= &omap36xx_smartreflex_hwmod_class,
 	.main_clk	= "sr1_fck",
-	.vdd_name	= "mpu_iva",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -2946,10 +2950,15 @@ static struct omap_hwmod omap36xx_sr1_hwmod = {
 	},
 	.slaves		= omap3_sr1_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap3_sr1_slaves),
+	.dev_attr       = &sr1_dev_attr,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3630ES1),
 };
 
 /* SR2 */
+static struct omap_smartreflex_dev_attr sr2_dev_attr = {
+	.sensor_voltdm_name    = "core",
+};
+
 static struct omap_hwmod_ocp_if *omap3_sr2_slaves[] = {
 	&omap3_l4_core__sr2,
 };
@@ -2958,7 +2967,6 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
 	.name		= "sr2_hwmod",
 	.class		= &omap34xx_smartreflex_hwmod_class,
 	.main_clk	= "sr2_fck",
-	.vdd_name	= "core",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -2970,6 +2978,7 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
 	},
 	.slaves		= omap3_sr2_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap3_sr2_slaves),
+	.dev_attr       = &sr2_dev_attr,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES2 |
 					CHIP_IS_OMAP3430ES3_0 |
 					CHIP_IS_OMAP3430ES3_1),
@@ -2980,7 +2989,6 @@ static struct omap_hwmod omap36xx_sr2_hwmod = {
 	.name		= "sr2_hwmod",
 	.class		= &omap36xx_smartreflex_hwmod_class,
 	.main_clk	= "sr2_fck",
-	.vdd_name	= "core",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -2992,6 +3000,7 @@ static struct omap_hwmod omap36xx_sr2_hwmod = {
 	},
 	.slaves		= omap3_sr2_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap3_sr2_slaves),
+	.dev_attr       = &sr2_dev_attr,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3630ES1),
 };
 
arch/arm/mach-omap2/omap_hwmod_44xx_data.c +16 -3
index 3e88dd3..78b6860 100644
@@ -30,6 +30,7 @@
 
 #include "omap_hwmod_common_data.h"
 
+#include "smartreflex.h"
 #include "cm1_44xx.h"
 #include "cm2_44xx.h"
 #include "prm44xx.h"
@@ -3775,6 +3776,10 @@ static struct omap_hwmod_class omap44xx_smartreflex_hwmod_class = {
 };
 
 /* smartreflex_core */
+static struct omap_smartreflex_dev_attr smartreflex_core_dev_attr = {
+	.sensor_voltdm_name   = "core",
+};
+
 static struct omap_hwmod omap44xx_smartreflex_core_hwmod;
 static struct omap_hwmod_irq_info omap44xx_smartreflex_core_irqs[] = {
 	{ .irq = 19 + OMAP44XX_IRQ_GIC_START },
@@ -3809,7 +3814,6 @@ static struct omap_hwmod omap44xx_smartreflex_core_hwmod = {
 	.mpu_irqs	= omap44xx_smartreflex_core_irqs,
 	.mpu_irqs_cnt	= ARRAY_SIZE(omap44xx_smartreflex_core_irqs),
 	.main_clk	= "smartreflex_core_fck",
-	.vdd_name	= "core",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_reg = OMAP4430_CM_ALWON_SR_CORE_CLKCTRL,
@@ -3817,10 +3821,15 @@ static struct omap_hwmod omap44xx_smartreflex_core_hwmod = {
 	},
 	.slaves		= omap44xx_smartreflex_core_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_smartreflex_core_slaves),
+	.dev_attr       = &smartreflex_core_dev_attr,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
 };
 
 /* smartreflex_iva */
+static struct omap_smartreflex_dev_attr smartreflex_iva_dev_attr = {
+	.sensor_voltdm_name   = "iva",
+};
+
 static struct omap_hwmod omap44xx_smartreflex_iva_hwmod;
 static struct omap_hwmod_irq_info omap44xx_smartreflex_iva_irqs[] = {
 	{ .irq = 102 + OMAP44XX_IRQ_GIC_START },
@@ -3855,7 +3864,6 @@ static struct omap_hwmod omap44xx_smartreflex_iva_hwmod = {
 	.mpu_irqs	= omap44xx_smartreflex_iva_irqs,
 	.mpu_irqs_cnt	= ARRAY_SIZE(omap44xx_smartreflex_iva_irqs),
 	.main_clk	= "smartreflex_iva_fck",
-	.vdd_name	= "iva",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_reg = OMAP4430_CM_ALWON_SR_IVA_CLKCTRL,
@@ -3863,10 +3871,15 @@ static struct omap_hwmod omap44xx_smartreflex_iva_hwmod = {
 	},
 	.slaves		= omap44xx_smartreflex_iva_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_smartreflex_iva_slaves),
+	.dev_attr       = &smartreflex_iva_dev_attr,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
 };
 
 /* smartreflex_mpu */
+static struct omap_smartreflex_dev_attr smartreflex_mpu_dev_attr = {
+	.sensor_voltdm_name   = "mpu",
+};
+
 static struct omap_hwmod omap44xx_smartreflex_mpu_hwmod;
 static struct omap_hwmod_irq_info omap44xx_smartreflex_mpu_irqs[] = {
 	{ .irq = 18 + OMAP44XX_IRQ_GIC_START },
@@ -3901,7 +3914,6 @@ static struct omap_hwmod omap44xx_smartreflex_mpu_hwmod = {
 	.mpu_irqs	= omap44xx_smartreflex_mpu_irqs,
 	.mpu_irqs_cnt	= ARRAY_SIZE(omap44xx_smartreflex_mpu_irqs),
 	.main_clk	= "smartreflex_mpu_fck",
-	.vdd_name	= "mpu",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_reg = OMAP4430_CM_ALWON_SR_MPU_CLKCTRL,
@@ -3909,6 +3921,7 @@ static struct omap_hwmod omap44xx_smartreflex_mpu_hwmod = {
 	},
 	.slaves		= omap44xx_smartreflex_mpu_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_smartreflex_mpu_slaves),
+	.dev_attr       = &smartreflex_mpu_dev_attr,
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
 };
 
arch/arm/mach-omap2/smartreflex.h +10 -0
index 5f35b9e..aeeec5f 100644
@@ -197,6 +197,16 @@ struct omap_sr_nvalue_table {
 };
 
 /**
+ * struct omap_smartreflex_dev_attr - Smartreflex Device attribute.
+ *
+ * @sensor_voltdm_name:       Name of voltdomain of SR instance
+ */
+
+struct omap_smartreflex_dev_attr {
+	const char      *sensor_voltdm_name;
+};
+
+/**
  * struct omap_sr_data - Smartreflex platform data.
  *
  * @ip_type:		Smartreflex IP type.
arch/arm/mach-omap2/sr_device.c +7 -4
index 2782d3f..0b74f5b 100644
@@ -82,6 +82,7 @@ static int sr_dev_init(struct omap_hwmod *oh, void *user)
 	struct omap_sr_data *sr_data;
 	struct omap_device *od;
 	struct omap_volt_data *volt_data;
+	struct omap_smartreflex_dev_attr *sr_dev_attr;
 	char *name = "smartreflex";
 	static int i;
 
@@ -92,9 +93,11 @@ static int sr_dev_init(struct omap_hwmod *oh, void *user)
 		return -ENOMEM;
 	}
 
-	if (!oh->vdd_name) {
+	sr_dev_attr = (struct omap_smartreflex_dev_attr *)oh->dev_attr;
+	if (!sr_dev_attr || !sr_dev_attr->sensor_voltdm_name) {
 		pr_err("%s: No voltage domain specified for %s."
-			"Cannot initialize\n", __func__, oh->name);
+				"Cannot initialize\n", __func__,
+					oh->name);
 		goto exit;
 	}
 
@@ -102,10 +105,10 @@ static int sr_dev_init(struct omap_hwmod *oh, void *user)
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
-	sr_data->voltdm = voltdm_lookup(oh->vdd_name);
+	sr_data->voltdm = voltdm_lookup(sr_dev_attr->sensor_voltdm_name);
 	if (IS_ERR(sr_data->voltdm)) {
 		pr_err("%s: Unable to get voltage domain pointer for VDD %s\n",
-			__func__, oh->vdd_name);
+			__func__, sr_dev_attr->sensor_voltdm_name);
 		goto exit;
 	}
 
arch/arm/plat-omap/include/plat/omap_hwmod.h +0 -1
index a5fa7c1..39d809a 100644
@@ -519,7 +519,6 @@ struct omap_hwmod {
 	const char			*main_clk;
 	struct clk			*_clk;
 	struct omap_hwmod_opt_clk	*opt_clks;
-	char				*vdd_name;
 	struct omap_hwmod_ocp_if	**masters; /* connect to *_IA */
 	struct omap_hwmod_ocp_if	**slaves;  /* connect to *_TA */
 	void				*dev_attr;
Signature · 1 line
1.7.0.4
#1 lore raw

PATCH v4 1/1 OMAP3+: SR Layer Cleanup

Quoted 8 lines · Shweta Gulati

To set sr ntarget values for all volt_domain, volt_table is retrieved by doing a look_up of 'vdd_name' field from omap_hwmod but voltage domain pointer does not belong to omap_hwmod and is not used anywhere else. As a part of voltage layer and SR Layer clean up volt pointer is removed from omap_hwmod and added in dev attributes of SR.

Quoted 6 lines

Tested on OMAP3630 SDP and OMAP4430 SDP Board

Acked by: Nishanth Menon <[email protected]>
Cc: Benoit Cousson <[email protected]>
Cc: Paul Walmsley <[email protected]>

As this isn't a consolidation/cleanup, I'm not going to push for 2.6.40, but with an ack from Benoit, I'll queue for 2.6.40+.

Also it has some minor conflicts with some other SR cleanups I have queued for 2.6.40+. Please rebase on to my for_2.6.40/pm-misc branch.

Thanks,

Kevin

#2 lore raw

PATCH v4 1/1 OMAP3+: SR Layer Cleanup

Hi,

Quoted 17 lines · Kevin Hilman
Quoted 8 lines · Shweta Gulati

To set sr ntarget values ?for all volt_domain, volt_table is retrieved by doing a look_up of 'vdd_name' field from omap_hwmod but voltage domain pointer does not belong to omap_hwmod and is not used anywhere else. As a part of voltage layer and SR Layer clean up volt pointer is removed from omap_hwmod and added in dev attributes of SR.

Quoted 6 lines

Tested on OMAP3630 SDP and OMAP4430 SDP Board

Acked by: Nishanth Menon <[email protected]>
Cc: Benoit Cousson <[email protected]>
Cc: Paul <[email protected]>

Quoted 5 lines

As this isn't a consolidation/cleanup, I'm not going to push for 2.6.40, but with an ack from Benoit, I'll queue for 2.6.40+.

Also it has some minor conflicts with some other SR cleanups I have queued for 2.6.40+. ?Please rebase on to my for_2.6.40/pm-misc branch.

Ok, I will rebase it to 2.6.40/pm-misc branch.

Quoted 4 lines

Thanks,

Kevin

Signature · 3 lines
Thanks,
Regards,
Shweta
#3 lore raw

PATCH v4 1/1 OMAP3+: SR Layer Cleanup

In reply to Shweta Gulati

On Fri, May 13, 2011 at 01:27, Shweta Gulati <[email protected]> wrote:
[...]

Quoted 19 lines
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +0 -0
index 3cd91ac..b1b11de 100644
@@ -29,6 +29,7 @@
 

?#include "omap_hwmod_common_data.h"

+#include "smartreflex.h"
?#include "prm-regbits-34xx.h"
?#include "cm-regbits-34xx.h"
?#include "wd_timer.h"

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +0 -0
@@ -2904,6 +2905,10 @@ static struct omap_hwmod_class omap36xx_smartreflex_hwmod_class = {
 ?};
 

?/* SR1 */

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +3 -0
+static struct omap_smartreflex_dev_attr sr1_dev_attr = {
+ ? ? ? .sensor_voltdm_name ? = "mpu_iva",
+};

[...]

Quoted 12 lines
arch/arm/mach-omap2/smartreflex.h +0 -0
index 5f35b9e..aeeec5f 100644
@@ -197,6 +197,16 @@ struct omap_sr_nvalue_table {
 ?};
 

?/**
+ * struct omap_smartreflex_dev_attr - Smartreflex Device attribute.
+ *
+ * @sensor_voltdm_name: ? ? ? Name of voltdomain of SR instance
+ */

arch/arm/mach-omap2/smartreflex.h struct omap_sr_nvalue_table { · line 208

Quoted 5 lines
arch/arm/mach-omap2/smartreflex.h +5 -0
+
+struct omap_smartreflex_dev_attr {
+ ? ? ? const char ? ? ?*sensor_voltdm_name;
+};
+

This structure is in #ifdef CONFIG_SMART_REFLEX and breaks hwmod_data.c when built without SR.
please move it out of the #ifdef in your next rev.

Regards,
Nishanth Menon

#4 lore raw

PATCH v4 1/1 OMAP3+: SR Layer Cleanup

In reply to Shweta Gulati

Hi Shweta,

Quoted 16 lines · Shweta

To set sr ntarget values for all volt_domain, volt_table is retrieved by doing a look_up of 'vdd_name' field from omap_hwmod but voltage domain pointer does not belong to omap_hwmod and is not used anywhere else. As a part of voltage layer and SR Layer clean up volt pointer is removed from omap_hwmod and added in dev attributes of SR.

Tested on OMAP3630 SDP and OMAP4430 SDP Board

Acked by: Nishanth Menon<[email protected]>
Cc: Benoit Cousson<[email protected]>
Cc: Paul Walmsley<[email protected]>
---

Quoted 4 lines

V4:
Fixed comments like checking for NULL pointers and following conventions in naming the instances recommended by Todd Poynor and Benoit Cousson.

It looks like you missed at least two comments I did on the previous version whereas you did agree to fix them.

Thanks,
Benoit

#5 lore raw

PATCH v4 1/1 OMAP3+: SR Layer Cleanup

Benoit,

Quoted 20 lines · Benoit

Hi Shweta,

Quoted 17 lines · Shweta

To set sr ntarget values ?for all volt_domain, volt_table is retrieved by doing a look_up of 'vdd_name' field from omap_hwmod but voltage domain pointer does not belong to omap_hwmod and is not used anywhere else. As a part of voltage layer and SR Layer clean up volt pointer is removed from omap_hwmod and added in dev attributes of SR.

Tested on OMAP3630 SDP and OMAP4430 SDP Board

Acked by: Nishanth Menon<[email protected]>
Cc: Benoit Cousson<[email protected]>
Cc: Paul Walmsley<[email protected]>
---

Quoted 7 lines
Quoted 4 lines

V4:
? Fixed comments like checking for NULL pointers ? and following conventions in naming the instances ? recommended by Todd Poynor and Benoit Cousson.

It looks like you missed at least two comments I did on the previous version whereas you did agree to fix them.

You told me to move smartrefex_xxxx_dev_attr above omap44xx_smartreflex_XXX_hwmod struct,
which is already in that order and the other thing you suggested was to move .dev_attr before .slaves,
that I have not done explaining that, to follow the standard as in all defined hwmod struct .dev_attr are placed after .slaves.

Quoted 3 lines

Thanks,
Benoit

Signature · 3 lines
Thanks,
Regards,
Shweta
#6 lore raw

PATCH v4 1/1 OMAP3+: SR Layer Cleanup

Quoted 28 lines · Shweta

Benoit,

Quoted 25 lines · Benoit

Hi Shweta,

Quoted 21 lines · Shweta

To set sr ntarget values for all volt_domain, volt_table is retrieved by doing a look_up of 'vdd_name' field from omap_hwmod but voltage domain pointer does not belong to omap_hwmod and is not used anywhere else. As a part of voltage layer and SR Layer clean up volt pointer is removed from omap_hwmod and added in dev attributes of SR.

Tested on OMAP3630 SDP and OMAP4430 SDP Board

Acked by: Nishanth Menon<[email protected]>
Cc: Benoit Cousson<[email protected]>
Cc: Paul Walmsley<[email protected]>
---

V4:
Fixed comments like checking for NULL pointers and following conventions in naming the instances recommended by Todd Poynor and Benoit Cousson.

Quoted 4 lines
Quoted 2 lines

It looks like you missed at least two comments I did on the previous version whereas you did agree to fix them.

You told me to move smartrefex_xxxx_dev_attr above omap44xx_smartreflex_XXX_hwmod struct,

Just before the struct which is not the case.

Quoted 4 lines

which is already in that order and the other thing you suggested was to move .dev_attr before .slaves,
that I have not done explaining that, to follow the standard as in all defined hwmod struct .dev_attr are placed after .slaves.

Could you give some example in the OMAP4 data file?

In the generic form, you should have opt_clk attribute then dev_attr then .slaves.

Anyway, do not say you will fix something if you decide not to fix them after. Or in case of valid concern, please provide some explanation in the cover letter.

Regards,
Benoit

#7 lore raw

PATCH v4 1/1 OMAP3+: SR Layer Cleanup

Hi,

Quoted 31 lines · Benoit
Quoted 30 lines · Shweta

Benoit,

Quoted 26 lines · On Fri, May 20, 2011 at 7:19 PM, Cousson, Benoit<[email protected]> ?wrote:

Hi Shweta,

Quoted 21 lines · Shweta

To set sr ntarget values ?for all volt_domain, volt_table is retrieved by doing a look_up of 'vdd_name' field from omap_hwmod but voltage domain pointer does not belong to omap_hwmod and is not used anywhere else. As a part of voltage layer and SR Layer clean up volt pointer is removed from omap_hwmod and added in dev attributes of SR.

Tested on OMAP3630 SDP and OMAP4430 SDP Board

Acked by: Nishanth Menon<[email protected]>
Cc: Benoit Cousson<[email protected]>
Cc: Paul Walmsley<[email protected]>
---

V4:
? Fixed comments like checking for NULL pointers ? and following conventions in naming the instances ? recommended by Todd Poynor and Benoit Cousson.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

It looks like you missed at least two comments I did on the previous version
whereas you did agree to fix them.

You told me to move smartrefex_xxxx_dev_attr above omap44xx_smartreflex_XXX_hwmod struct,

Just before the struct which is not the case.

Ok will do , just above that struct.

Quoted 6 lines
Quoted 4 lines

which is already in that order and the other thing you suggested was to move .dev_attr before .slaves,
that I have not done explaining that, to follow the standard as in all defined hwmod struct .dev_attr are placed after .slaves.

Could you give some example in the OMAP4 data file?

Benoit, I checked in OMAP3 data file, So it that in OMAP3 and OMAP4 data file the order should be different, what do you suggest?

Quoted 6 lines

In the generic form, you should have opt_clk attribute then dev_attr then .slaves.

Anyway, do not say you will fix something if you decide not to fix them after. Or in case of valid concern, please provide some explanation in the cover letter.

Benoit, I did send a mail asking for this concern earlier too, Anyways we can decide order for OMAP3 and OMAP 4data files and I would change that in V5.

Thanks.

Quoted 5 lines

Regards,
Benoit

Signature · 3 lines
Thanks,
Regards,
Shweta