All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-04 16:47 ` jean.pihet at newoldbits.com
  0 siblings, 0 replies; 34+ messages in thread
From: jean.pihet @ 2012-10-04 16:47 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, tony, Kevin Hilman, Anton Vorontsov
  Cc: J Keerthy, Jean Pihet

From: Jean Pihet <j-pihet@ti.com>

Remove the device dependent code (ex. cpu_is_xxx()) and settings
from the driver code and instead pass them via the platform
data. This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Note about the smartreflex functional clocks: the smartreflex fclks
are derived from sys_clk and have the same name as the main_clk from
the hwmod entry, in order for the SmartReflex driver to request the
fclk (using clk_get(dev, "fck")).

Based on mainline 3.6.0. Boot tested on OMAP3&4 platforms.

Jean Pihet (2):
  ARM: OMAP: hwmod: align the SmartReflex fck names
  ARM: OMAP: SmartReflex: pass device dependent data via platform data

 arch/arm/mach-omap2/clock33xx_data.c       |   12 +++----
 arch/arm/mach-omap2/clock3xxx_data.c       |   12 +++----
 arch/arm/mach-omap2/clock44xx_data.c       |    6 ++--
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    8 ++---
 arch/arm/mach-omap2/sr_device.c            |   13 +++++++
 drivers/power/avs/smartreflex.c            |   54 +++++++++-------------------
 include/linux/power/smartreflex.h          |   14 ++++++--
 7 files changed, 61 insertions(+), 58 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 0/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-04 16:47 ` jean.pihet at newoldbits.com
  0 siblings, 0 replies; 34+ messages in thread
From: jean.pihet at newoldbits.com @ 2012-10-04 16:47 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jean Pihet <j-pihet@ti.com>

Remove the device dependent code (ex. cpu_is_xxx()) and settings
from the driver code and instead pass them via the platform
data. This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Note about the smartreflex functional clocks: the smartreflex fclks
are derived from sys_clk and have the same name as the main_clk from
the hwmod entry, in order for the SmartReflex driver to request the
fclk (using clk_get(dev, "fck")).

Based on mainline 3.6.0. Boot tested on OMAP3&4 platforms.

Jean Pihet (2):
  ARM: OMAP: hwmod: align the SmartReflex fck names
  ARM: OMAP: SmartReflex: pass device dependent data via platform data

 arch/arm/mach-omap2/clock33xx_data.c       |   12 +++----
 arch/arm/mach-omap2/clock3xxx_data.c       |   12 +++----
 arch/arm/mach-omap2/clock44xx_data.c       |    6 ++--
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    8 ++---
 arch/arm/mach-omap2/sr_device.c            |   13 +++++++
 drivers/power/avs/smartreflex.c            |   54 +++++++++-------------------
 include/linux/power/smartreflex.h          |   14 ++++++--
 7 files changed, 61 insertions(+), 58 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/2] ARM: OMAP: hwmod: align the SmartReflex fck names
  2012-10-04 16:47 ` jean.pihet at newoldbits.com
@ 2012-10-04 16:47   ` jean.pihet at newoldbits.com
  -1 siblings, 0 replies; 34+ messages in thread
From: jean.pihet @ 2012-10-04 16:47 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, tony, Kevin Hilman, Anton Vorontsov
  Cc: J Keerthy, Jean Pihet

From: Jean Pihet <j-pihet@ti.com>

Rename the smartreflex fck names for consistency and better readability;
rename the clock aliases so that they match the hwmod main_clk names.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/clock33xx_data.c       |   12 ++++++------
 arch/arm/mach-omap2/clock3xxx_data.c       |   12 ++++++------
 arch/arm/mach-omap2/clock44xx_data.c       |    6 +++---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    8 ++++----
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c
index 2026311..4fa2dd9 100644
--- a/arch/arm/mach-omap2/clock33xx_data.c
+++ b/arch/arm/mach-omap2/clock33xx_data.c
@@ -548,16 +548,16 @@ static struct clk mcasp1_fck = {
 	.recalc		= &followparent_recalc,
 };
 
-static struct clk smartreflex0_fck = {
-	.name		= "smartreflex0_fck",
+static struct clk smartreflex_mpu_fck = {
+	.name		= "smartreflex_mpu_fck",
 	.clkdm_name	= "l4_wkup_clkdm",
 	.parent		= &sys_clkin_ck,
 	.ops		= &clkops_null,
 	.recalc		= &followparent_recalc,
 };
 
-static struct clk smartreflex1_fck = {
-	.name		= "smartreflex1_fck",
+static struct clk smartreflex_core_fck = {
+	.name		= "smartreflex_core_fck",
 	.clkdm_name	= "l4_wkup_clkdm",
 	.parent		= &sys_clkin_ck,
 	.ops		= &clkops_null,
@@ -1036,8 +1036,8 @@ static struct omap_clk am33xx_clks[] = {
 	CLK("davinci-mcasp.1",  NULL,           &mcasp1_fck,    CK_AM33XX),
 	CLK("NULL",	"mmc2_fck",		&mmc2_fck,	CK_AM33XX),
 	CLK(NULL,	"mmu_fck",		&mmu_fck,	CK_AM33XX),
-	CLK(NULL,	"smartreflex0_fck",	&smartreflex0_fck,	CK_AM33XX),
-	CLK(NULL,	"smartreflex1_fck",	&smartreflex1_fck,	CK_AM33XX),
+	CLK(NULL,	"smartreflex_mpu_fck",	&smartreflex_mpu_fck,	CK_AM33XX),
+	CLK(NULL,	"smartreflex_core_fck",	&smartreflex_core_fck,	CK_AM33XX),
 	CLK(NULL,	"timer1_fck",		&timer1_fck,	CK_AM33XX),
 	CLK(NULL,	"timer2_fck",		&timer2_fck,	CK_AM33XX),
 	CLK(NULL,	"timer3_fck",		&timer3_fck,	CK_AM33XX),
diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index 700317a..81f6a15 100644
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -3050,8 +3050,8 @@ static struct clk traceclk_fck = {
 /* SR clocks */
 
 /* SmartReflex fclk (VDD1) */
-static struct clk sr1_fck = {
-	.name		= "sr1_fck",
+static struct clk smartreflex_mpu_iva_fck = {
+	.name		= "smartreflex_mpu_iva_fck",
 	.ops		= &clkops_omap2_dflt_wait,
 	.parent		= &sys_ck,
 	.enable_reg	= OMAP_CM_REGADDR(WKUP_MOD, CM_FCLKEN),
@@ -3061,8 +3061,8 @@ static struct clk sr1_fck = {
 };
 
 /* SmartReflex fclk (VDD2) */
-static struct clk sr2_fck = {
-	.name		= "sr2_fck",
+static struct clk smartreflex_core_fck = {
+	.name		= "smartreflex_core_fck",
 	.ops		= &clkops_omap2_dflt_wait,
 	.parent		= &sys_ck,
 	.enable_reg	= OMAP_CM_REGADDR(WKUP_MOD, CM_FCLKEN),
@@ -3448,8 +3448,8 @@ static struct omap_clk omap3xxx_clks[] = {
 	CLK(NULL,	"atclk_fck",	&atclk_fck,	CK_3XXX),
 	CLK(NULL,	"traceclk_src_fck", &traceclk_src_fck, CK_3XXX),
 	CLK(NULL,	"traceclk_fck",	&traceclk_fck,	CK_3XXX),
-	CLK(NULL,	"sr1_fck",	&sr1_fck,	CK_34XX | CK_36XX),
-	CLK(NULL,	"sr2_fck",	&sr2_fck,	CK_34XX | CK_36XX),
+	CLK(NULL,	"smartreflex_mpu_iva_fck",	&smartreflex_mpu_iva_fck,	CK_34XX | CK_36XX),
+	CLK(NULL,	"smartreflex_core_fck",	&smartreflex_core_fck,	CK_34XX | CK_36XX),
 	CLK(NULL,	"sr_l4_ick",	&sr_l4_ick,	CK_34XX | CK_36XX),
 	CLK(NULL,	"secure_32k_fck", &secure_32k_fck, CK_3XXX),
 	CLK(NULL,	"gpt12_fck",	&gpt12_fck,	CK_3XXX),
diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 500682c..9852ecb 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -3224,9 +3224,9 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"slimbus2_fclk_0",		&slimbus2_fclk_0,	CK_443X),
 	CLK(NULL,	"slimbus2_slimbus_clk",		&slimbus2_slimbus_clk,	CK_443X),
 	CLK(NULL,	"slimbus2_fck",			&slimbus2_fck,	CK_443X),
-	CLK(NULL,	"smartreflex_core_fck",		&smartreflex_core_fck,	CK_443X),
-	CLK(NULL,	"smartreflex_iva_fck",		&smartreflex_iva_fck,	CK_443X),
-	CLK(NULL,	"smartreflex_mpu_fck",		&smartreflex_mpu_fck,	CK_443X),
+	CLK(NULL,	"smartreflex_core_fck",	&smartreflex_core_fck,	CK_443X),
+	CLK(NULL,	"smartreflex_iva_fck",	&smartreflex_iva_fck,	CK_443X),
+	CLK(NULL,	"smartreflex_mpu_fck",	&smartreflex_mpu_fck,	CK_443X),
 	CLK(NULL,	"timer1_fck",			&timer1_fck,	CK_443X),
 	CLK(NULL,	"timer10_fck",			&timer10_fck,	CK_443X),
 	CLK(NULL,	"timer11_fck",			&timer11_fck,	CK_443X),
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 94b38af..f1095a6 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -1368,7 +1368,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_mpu_irqs[] = {
 static struct omap_hwmod omap34xx_sr1_hwmod = {
 	.name		= "smartreflex_mpu_iva",
 	.class		= &omap34xx_smartreflex_hwmod_class,
-	.main_clk	= "sr1_fck",
+	.main_clk	= "smartreflex_mpu_iva_fck",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -1386,7 +1386,7 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
 static struct omap_hwmod omap36xx_sr1_hwmod = {
 	.name		= "smartreflex_mpu_iva",
 	.class		= &omap36xx_smartreflex_hwmod_class,
-	.main_clk	= "sr1_fck",
+	.main_clk	= "smartreflex_mpu_iva_fck",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -1413,7 +1413,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_core_irqs[] = {
 static struct omap_hwmod omap34xx_sr2_hwmod = {
 	.name		= "smartreflex_core",
 	.class		= &omap34xx_smartreflex_hwmod_class,
-	.main_clk	= "sr2_fck",
+	.main_clk	= "smartreflex_core_fck",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -1431,7 +1431,7 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
 static struct omap_hwmod omap36xx_sr2_hwmod = {
 	.name		= "smartreflex_core",
 	.class		= &omap36xx_smartreflex_hwmod_class,
-	.main_clk	= "sr2_fck",
+	.main_clk	= "smartreflex_core_fck",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 1/2] ARM: OMAP: hwmod: align the SmartReflex fck names
@ 2012-10-04 16:47   ` jean.pihet at newoldbits.com
  0 siblings, 0 replies; 34+ messages in thread
From: jean.pihet at newoldbits.com @ 2012-10-04 16:47 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jean Pihet <j-pihet@ti.com>

Rename the smartreflex fck names for consistency and better readability;
rename the clock aliases so that they match the hwmod main_clk names.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/clock33xx_data.c       |   12 ++++++------
 arch/arm/mach-omap2/clock3xxx_data.c       |   12 ++++++------
 arch/arm/mach-omap2/clock44xx_data.c       |    6 +++---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    8 ++++----
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c
index 2026311..4fa2dd9 100644
--- a/arch/arm/mach-omap2/clock33xx_data.c
+++ b/arch/arm/mach-omap2/clock33xx_data.c
@@ -548,16 +548,16 @@ static struct clk mcasp1_fck = {
 	.recalc		= &followparent_recalc,
 };
 
-static struct clk smartreflex0_fck = {
-	.name		= "smartreflex0_fck",
+static struct clk smartreflex_mpu_fck = {
+	.name		= "smartreflex_mpu_fck",
 	.clkdm_name	= "l4_wkup_clkdm",
 	.parent		= &sys_clkin_ck,
 	.ops		= &clkops_null,
 	.recalc		= &followparent_recalc,
 };
 
-static struct clk smartreflex1_fck = {
-	.name		= "smartreflex1_fck",
+static struct clk smartreflex_core_fck = {
+	.name		= "smartreflex_core_fck",
 	.clkdm_name	= "l4_wkup_clkdm",
 	.parent		= &sys_clkin_ck,
 	.ops		= &clkops_null,
@@ -1036,8 +1036,8 @@ static struct omap_clk am33xx_clks[] = {
 	CLK("davinci-mcasp.1",  NULL,           &mcasp1_fck,    CK_AM33XX),
 	CLK("NULL",	"mmc2_fck",		&mmc2_fck,	CK_AM33XX),
 	CLK(NULL,	"mmu_fck",		&mmu_fck,	CK_AM33XX),
-	CLK(NULL,	"smartreflex0_fck",	&smartreflex0_fck,	CK_AM33XX),
-	CLK(NULL,	"smartreflex1_fck",	&smartreflex1_fck,	CK_AM33XX),
+	CLK(NULL,	"smartreflex_mpu_fck",	&smartreflex_mpu_fck,	CK_AM33XX),
+	CLK(NULL,	"smartreflex_core_fck",	&smartreflex_core_fck,	CK_AM33XX),
 	CLK(NULL,	"timer1_fck",		&timer1_fck,	CK_AM33XX),
 	CLK(NULL,	"timer2_fck",		&timer2_fck,	CK_AM33XX),
 	CLK(NULL,	"timer3_fck",		&timer3_fck,	CK_AM33XX),
diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index 700317a..81f6a15 100644
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -3050,8 +3050,8 @@ static struct clk traceclk_fck = {
 /* SR clocks */
 
 /* SmartReflex fclk (VDD1) */
-static struct clk sr1_fck = {
-	.name		= "sr1_fck",
+static struct clk smartreflex_mpu_iva_fck = {
+	.name		= "smartreflex_mpu_iva_fck",
 	.ops		= &clkops_omap2_dflt_wait,
 	.parent		= &sys_ck,
 	.enable_reg	= OMAP_CM_REGADDR(WKUP_MOD, CM_FCLKEN),
@@ -3061,8 +3061,8 @@ static struct clk sr1_fck = {
 };
 
 /* SmartReflex fclk (VDD2) */
-static struct clk sr2_fck = {
-	.name		= "sr2_fck",
+static struct clk smartreflex_core_fck = {
+	.name		= "smartreflex_core_fck",
 	.ops		= &clkops_omap2_dflt_wait,
 	.parent		= &sys_ck,
 	.enable_reg	= OMAP_CM_REGADDR(WKUP_MOD, CM_FCLKEN),
@@ -3448,8 +3448,8 @@ static struct omap_clk omap3xxx_clks[] = {
 	CLK(NULL,	"atclk_fck",	&atclk_fck,	CK_3XXX),
 	CLK(NULL,	"traceclk_src_fck", &traceclk_src_fck, CK_3XXX),
 	CLK(NULL,	"traceclk_fck",	&traceclk_fck,	CK_3XXX),
-	CLK(NULL,	"sr1_fck",	&sr1_fck,	CK_34XX | CK_36XX),
-	CLK(NULL,	"sr2_fck",	&sr2_fck,	CK_34XX | CK_36XX),
+	CLK(NULL,	"smartreflex_mpu_iva_fck",	&smartreflex_mpu_iva_fck,	CK_34XX | CK_36XX),
+	CLK(NULL,	"smartreflex_core_fck",	&smartreflex_core_fck,	CK_34XX | CK_36XX),
 	CLK(NULL,	"sr_l4_ick",	&sr_l4_ick,	CK_34XX | CK_36XX),
 	CLK(NULL,	"secure_32k_fck", &secure_32k_fck, CK_3XXX),
 	CLK(NULL,	"gpt12_fck",	&gpt12_fck,	CK_3XXX),
diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 500682c..9852ecb 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -3224,9 +3224,9 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK(NULL,	"slimbus2_fclk_0",		&slimbus2_fclk_0,	CK_443X),
 	CLK(NULL,	"slimbus2_slimbus_clk",		&slimbus2_slimbus_clk,	CK_443X),
 	CLK(NULL,	"slimbus2_fck",			&slimbus2_fck,	CK_443X),
-	CLK(NULL,	"smartreflex_core_fck",		&smartreflex_core_fck,	CK_443X),
-	CLK(NULL,	"smartreflex_iva_fck",		&smartreflex_iva_fck,	CK_443X),
-	CLK(NULL,	"smartreflex_mpu_fck",		&smartreflex_mpu_fck,	CK_443X),
+	CLK(NULL,	"smartreflex_core_fck",	&smartreflex_core_fck,	CK_443X),
+	CLK(NULL,	"smartreflex_iva_fck",	&smartreflex_iva_fck,	CK_443X),
+	CLK(NULL,	"smartreflex_mpu_fck",	&smartreflex_mpu_fck,	CK_443X),
 	CLK(NULL,	"timer1_fck",			&timer1_fck,	CK_443X),
 	CLK(NULL,	"timer10_fck",			&timer10_fck,	CK_443X),
 	CLK(NULL,	"timer11_fck",			&timer11_fck,	CK_443X),
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 94b38af..f1095a6 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -1368,7 +1368,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_mpu_irqs[] = {
 static struct omap_hwmod omap34xx_sr1_hwmod = {
 	.name		= "smartreflex_mpu_iva",
 	.class		= &omap34xx_smartreflex_hwmod_class,
-	.main_clk	= "sr1_fck",
+	.main_clk	= "smartreflex_mpu_iva_fck",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -1386,7 +1386,7 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
 static struct omap_hwmod omap36xx_sr1_hwmod = {
 	.name		= "smartreflex_mpu_iva",
 	.class		= &omap36xx_smartreflex_hwmod_class,
-	.main_clk	= "sr1_fck",
+	.main_clk	= "smartreflex_mpu_iva_fck",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -1413,7 +1413,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_core_irqs[] = {
 static struct omap_hwmod omap34xx_sr2_hwmod = {
 	.name		= "smartreflex_core",
 	.class		= &omap34xx_smartreflex_hwmod_class,
-	.main_clk	= "sr2_fck",
+	.main_clk	= "smartreflex_core_fck",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
@@ -1431,7 +1431,7 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
 static struct omap_hwmod omap36xx_sr2_hwmod = {
 	.name		= "smartreflex_core",
 	.class		= &omap36xx_smartreflex_hwmod_class,
-	.main_clk	= "sr2_fck",
+	.main_clk	= "smartreflex_core_fck",
 	.prcm		= {
 		.omap2 = {
 			.prcm_reg_id = 1,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-10-04 16:47 ` jean.pihet at newoldbits.com
@ 2012-10-04 16:47   ` jean.pihet at newoldbits.com
  -1 siblings, 0 replies; 34+ messages in thread
From: jean.pihet @ 2012-10-04 16:47 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, tony, Kevin Hilman, Anton Vorontsov
  Cc: J Keerthy, Jean Pihet

From: Jean Pihet <j-pihet@ti.com>

Remove the device dependent code (ex. cpu_is_xxx()) and settings
from the driver code and instead pass them via the platform
data. This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Note about the smartreflex functional clocks: the smartreflex fclks
are derived from sys_clk and have the same name as the main_clk from
the hwmod entry, in order for the SmartReflex driver to request the
fclk (using clk_get(dev, "fck")).

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   13 +++++++++
 drivers/power/avs/smartreflex.c   |   54 ++++++++++++-------------------------
 include/linux/power/smartreflex.h |   14 ++++++++--
 3 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index cbeae56..06de443 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -121,6 +121,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 24768a2..4c4519e 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -130,24 +130,21 @@ static irqreturn_t sr_interrupt(int irq, void *data)
 
 static void sr_set_clk_length(struct omap_sr *sr)
 {
-	struct clk *sys_ck;
-	u32 sys_clk_speed;
+	struct clk *fck;
+	u32 fclk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	fck = clk_get(&sr->pdev->dev, "fck");
 
-	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+	if (IS_ERR(fck)) {
+		dev_err(&sr->pdev->dev, "%s: unable to get fck for device %s\n",
+				__func__, dev_name(&sr->pdev->dev));
 		return;
 	}
 
-	sys_clk_speed = clk_get_rate(sys_ck);
-	clk_put(sys_ck);
+	fclk_speed = clk_get_rate(fck);
+	clk_put(fck);
 
-	switch (sys_clk_speed) {
+	switch (fclk_speed) {
 	case 12000000:
 		sr->clk_length = SRCLKLENGTH_12MHZ_SYSCLK;
 		break;
@@ -164,34 +161,12 @@ static void sr_set_clk_length(struct omap_sr *sr)
 		sr->clk_length = SRCLKLENGTH_38MHZ_SYSCLK;
 		break;
 	default:
-		dev_err(&sr->pdev->dev, "%s: Invalid sysclk value: %d\n",
-			__func__, sys_clk_speed);
+		dev_err(&sr->pdev->dev, "%s: Invalid fclk rate: %d\n",
+			__func__, fclk_speed);
 		break;
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -924,8 +899,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
@@ -937,7 +918,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 4a496eb..c0f44c2 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -260,8 +260,13 @@ struct omap_sr_nvalue_table {
  *
  * @name:		instance name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -274,6 +279,11 @@ struct omap_sr_data {
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-04 16:47   ` jean.pihet at newoldbits.com
  0 siblings, 0 replies; 34+ messages in thread
From: jean.pihet at newoldbits.com @ 2012-10-04 16:47 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jean Pihet <j-pihet@ti.com>

Remove the device dependent code (ex. cpu_is_xxx()) and settings
from the driver code and instead pass them via the platform
data. This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Note about the smartreflex functional clocks: the smartreflex fclks
are derived from sys_clk and have the same name as the main_clk from
the hwmod entry, in order for the SmartReflex driver to request the
fclk (using clk_get(dev, "fck")).

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   13 +++++++++
 drivers/power/avs/smartreflex.c   |   54 ++++++++++++-------------------------
 include/linux/power/smartreflex.h |   14 ++++++++--
 3 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index cbeae56..06de443 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -121,6 +121,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 24768a2..4c4519e 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -130,24 +130,21 @@ static irqreturn_t sr_interrupt(int irq, void *data)
 
 static void sr_set_clk_length(struct omap_sr *sr)
 {
-	struct clk *sys_ck;
-	u32 sys_clk_speed;
+	struct clk *fck;
+	u32 fclk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	fck = clk_get(&sr->pdev->dev, "fck");
 
-	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+	if (IS_ERR(fck)) {
+		dev_err(&sr->pdev->dev, "%s: unable to get fck for device %s\n",
+				__func__, dev_name(&sr->pdev->dev));
 		return;
 	}
 
-	sys_clk_speed = clk_get_rate(sys_ck);
-	clk_put(sys_ck);
+	fclk_speed = clk_get_rate(fck);
+	clk_put(fck);
 
-	switch (sys_clk_speed) {
+	switch (fclk_speed) {
 	case 12000000:
 		sr->clk_length = SRCLKLENGTH_12MHZ_SYSCLK;
 		break;
@@ -164,34 +161,12 @@ static void sr_set_clk_length(struct omap_sr *sr)
 		sr->clk_length = SRCLKLENGTH_38MHZ_SYSCLK;
 		break;
 	default:
-		dev_err(&sr->pdev->dev, "%s: Invalid sysclk value: %d\n",
-			__func__, sys_clk_speed);
+		dev_err(&sr->pdev->dev, "%s: Invalid fclk rate: %d\n",
+			__func__, fclk_speed);
 		break;
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -924,8 +899,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
@@ -937,7 +918,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 4a496eb..c0f44c2 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -260,8 +260,13 @@ struct omap_sr_nvalue_table {
  *
  * @name:		instance name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -274,6 +279,11 @@ struct omap_sr_data {
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-10-04 16:47 ` jean.pihet at newoldbits.com
@ 2012-10-04 23:40   ` Kevin Hilman
  -1 siblings, 0 replies; 34+ messages in thread
From: Kevin Hilman @ 2012-10-04 23:40 UTC (permalink / raw)
  To: jean.pihet
  Cc: linux-omap, linux-arm-kernel, tony, Anton Vorontsov, J Keerthy,
	Jean Pihet

jean.pihet@newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> Remove the device dependent code (ex. cpu_is_xxx()) and settings
> from the driver code and instead pass them via the platform
> data. This allows a clean separation of the driver code and the platform
> code, as required by the move of the platform header files to
> include/linux/platform_data.
>
> Note about the smartreflex functional clocks: the smartreflex fclks
> are derived from sys_clk and have the same name as the main_clk from
> the hwmod entry, in order for the SmartReflex driver to request the
> fclk (using clk_get(dev, "fck")).
>
> Based on mainline 3.6.0. Boot tested on OMAP3&4 platforms.

Thanks, queuing this version for v3.8 (branch: for_3.8/pm/sr)

Kevin

P.S. in the future, It helps reviewers and maintainers if there's some
versioning in the patches (e.g. PATCH v3 0/2]), especially when updated
versions come in quick succession.  Thanks.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 0/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-04 23:40   ` Kevin Hilman
  0 siblings, 0 replies; 34+ messages in thread
From: Kevin Hilman @ 2012-10-04 23:40 UTC (permalink / raw)
  To: linux-arm-kernel

jean.pihet at newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> Remove the device dependent code (ex. cpu_is_xxx()) and settings
> from the driver code and instead pass them via the platform
> data. This allows a clean separation of the driver code and the platform
> code, as required by the move of the platform header files to
> include/linux/platform_data.
>
> Note about the smartreflex functional clocks: the smartreflex fclks
> are derived from sys_clk and have the same name as the main_clk from
> the hwmod entry, in order for the SmartReflex driver to request the
> fclk (using clk_get(dev, "fck")).
>
> Based on mainline 3.6.0. Boot tested on OMAP3&4 platforms.

Thanks, queuing this version for v3.8 (branch: for_3.8/pm/sr)

Kevin

P.S. in the future, It helps reviewers and maintainers if there's some
versioning in the patches (e.g. PATCH v3 0/2]), especially when updated
versions come in quick succession.  Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-10-04 23:40   ` Kevin Hilman
@ 2012-10-05  8:10     ` Jean Pihet
  -1 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-10-05  8:10 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-omap, linux-arm-kernel, tony, Anton Vorontsov, J Keerthy,
	Jean Pihet

On Fri, Oct 5, 2012 at 1:40 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> jean.pihet@newoldbits.com writes:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> Remove the device dependent code (ex. cpu_is_xxx()) and settings
>> from the driver code and instead pass them via the platform
>> data. This allows a clean separation of the driver code and the platform
>> code, as required by the move of the platform header files to
>> include/linux/platform_data.
>>
>> Note about the smartreflex functional clocks: the smartreflex fclks
>> are derived from sys_clk and have the same name as the main_clk from
>> the hwmod entry, in order for the SmartReflex driver to request the
>> fclk (using clk_get(dev, "fck")).
>>
>> Based on mainline 3.6.0. Boot tested on OMAP3&4 platforms.
>
> Thanks, queuing this version for v3.8 (branch: for_3.8/pm/sr)

Thanks!
Jean

>
> Kevin
>
> P.S. in the future, It helps reviewers and maintainers if there's some
> versioning in the patches (e.g. PATCH v3 0/2]), especially when updated
> versions come in quick succession.  Thanks.
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 0/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-05  8:10     ` Jean Pihet
  0 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-10-05  8:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 5, 2012 at 1:40 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> jean.pihet at newoldbits.com writes:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> Remove the device dependent code (ex. cpu_is_xxx()) and settings
>> from the driver code and instead pass them via the platform
>> data. This allows a clean separation of the driver code and the platform
>> code, as required by the move of the platform header files to
>> include/linux/platform_data.
>>
>> Note about the smartreflex functional clocks: the smartreflex fclks
>> are derived from sys_clk and have the same name as the main_clk from
>> the hwmod entry, in order for the SmartReflex driver to request the
>> fclk (using clk_get(dev, "fck")).
>>
>> Based on mainline 3.6.0. Boot tested on OMAP3&4 platforms.
>
> Thanks, queuing this version for v3.8 (branch: for_3.8/pm/sr)

Thanks!
Jean

>
> Kevin
>
> P.S. in the future, It helps reviewers and maintainers if there's some
> versioning in the patches (e.g. PATCH v3 0/2]), especially when updated
> versions come in quick succession.  Thanks.
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-10-03 14:29         ` Kevin Hilman
@ 2012-10-03 15:51           ` Jean Pihet
  -1 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-10-03 15:51 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: tony, J Keerthy, Anton Vorontsov, linux-omap, Jean Pihet,
	linux-arm-kernel

Kevin,

On Wed, Oct 3, 2012 at 4:29 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> jean.pihet@newoldbits.com writes:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> Remove the device dependent code (ex. cpu_is_xxx()) and settings
>> from the driver code and instead pass them via the platform
>> data. This allows a clean separation of the driver code and the platform
>> code, as required by the move of the platform header files to
>> include/linux/platform_data.
>>
>> Note about the smartreflex functional clocks: the smartreflex fclks
>> are derived from sys_clk and are named "smartreflex.%d". Since the
>> smartreflex device names and the functional clock names are identical
>> the device driver code uses them to control the functional clocks.
>
> Thanks for adding this part.
>
> One more nit below, then please resend this patch as a combined series
> with the "align fclk names" patch.
Just re-sent the new series.

> (note: The previous patch 1 from this
> series I've queued separately as a fix for v3.7-rc.  )
Thanks! The new series is based on mainline 3.6.0 with this patch applied.

...
>> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
>> index 24768a2..829467f 100644
>> --- a/drivers/power/avs/smartreflex.c
>> +++ b/drivers/power/avs/smartreflex.c
>> @@ -133,14 +133,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
>>       struct clk *sys_ck;
>>       u32 sys_clk_speed;
>>
>> -     if (cpu_is_omap34xx())
>> -             sys_ck = clk_get(NULL, "sys_ck");
>> -     else
>> -             sys_ck = clk_get(NULL, "sys_clkin_ck");
>> +     sys_ck = clk_get(&sr->pdev->dev, "fck");
>
> nit: since this isn't the sys_clk anymore, could you s/sys_ck/fck/  ?
Done!

>
> Thanks,
>
> Kevin
>

Thanks,
Jean

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-03 15:51           ` Jean Pihet
  0 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-10-03 15:51 UTC (permalink / raw)
  To: linux-arm-kernel

Kevin,

On Wed, Oct 3, 2012 at 4:29 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> jean.pihet at newoldbits.com writes:
>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> Remove the device dependent code (ex. cpu_is_xxx()) and settings
>> from the driver code and instead pass them via the platform
>> data. This allows a clean separation of the driver code and the platform
>> code, as required by the move of the platform header files to
>> include/linux/platform_data.
>>
>> Note about the smartreflex functional clocks: the smartreflex fclks
>> are derived from sys_clk and are named "smartreflex.%d". Since the
>> smartreflex device names and the functional clock names are identical
>> the device driver code uses them to control the functional clocks.
>
> Thanks for adding this part.
>
> One more nit below, then please resend this patch as a combined series
> with the "align fclk names" patch.
Just re-sent the new series.

> (note: The previous patch 1 from this
> series I've queued separately as a fix for v3.7-rc.  )
Thanks! The new series is based on mainline 3.6.0 with this patch applied.

...
>> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
>> index 24768a2..829467f 100644
>> --- a/drivers/power/avs/smartreflex.c
>> +++ b/drivers/power/avs/smartreflex.c
>> @@ -133,14 +133,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
>>       struct clk *sys_ck;
>>       u32 sys_clk_speed;
>>
>> -     if (cpu_is_omap34xx())
>> -             sys_ck = clk_get(NULL, "sys_ck");
>> -     else
>> -             sys_ck = clk_get(NULL, "sys_clkin_ck");
>> +     sys_ck = clk_get(&sr->pdev->dev, "fck");
>
> nit: since this isn't the sys_clk anymore, could you s/sys_ck/fck/  ?
Done!

>
> Thanks,
>
> Kevin
>

Thanks,
Jean

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-10-03 15:47 jean.pihet
@ 2012-10-03 15:47   ` jean.pihet at newoldbits.com
  0 siblings, 0 replies; 34+ messages in thread
From: jean.pihet @ 2012-10-03 15:47 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, tony, Kevin Hilman, Anton Vorontsov
  Cc: J Keerthy, Jean Pihet

From: Jean Pihet <j-pihet@ti.com>

Remove the device dependent code (ex. cpu_is_xxx()) and settings
from the driver code and instead pass them via the platform
data. This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Note about the smartreflex functional clocks: the smartreflex fclks
are derived from sys_clk and are named "smartreflex.%d". Since the
smartreflex device names and the functional clock names are identical
the device driver code uses them to control the functional clocks.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   13 +++++++++
 drivers/power/avs/smartreflex.c   |   54 ++++++++++++-------------------------
 include/linux/power/smartreflex.h |   14 ++++++++--
 3 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index cbeae56..06de443 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -121,6 +121,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 24768a2..c983e85 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -130,24 +130,21 @@ static irqreturn_t sr_interrupt(int irq, void *data)
 
 static void sr_set_clk_length(struct omap_sr *sr)
 {
-	struct clk *sys_ck;
-	u32 sys_clk_speed;
+	struct clk *fck;
+	u32 fclk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	fck = clk_get(&sr->pdev->dev, "fck");
 
-	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+	if (IS_ERR(fck)) {
+		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
+				__func__, dev_name(&sr->pdev->dev));
 		return;
 	}
 
-	sys_clk_speed = clk_get_rate(sys_ck);
-	clk_put(sys_ck);
+	fclk_speed = clk_get_rate(fck);
+	clk_put(fck);
 
-	switch (sys_clk_speed) {
+	switch (fclk_speed) {
 	case 12000000:
 		sr->clk_length = SRCLKLENGTH_12MHZ_SYSCLK;
 		break;
@@ -164,34 +161,12 @@ static void sr_set_clk_length(struct omap_sr *sr)
 		sr->clk_length = SRCLKLENGTH_38MHZ_SYSCLK;
 		break;
 	default:
-		dev_err(&sr->pdev->dev, "%s: Invalid sysclk value: %d\n",
-			__func__, sys_clk_speed);
+		dev_err(&sr->pdev->dev, "%s: Invalid fclk rate: %d\n",
+			__func__, fclk_speed);
 		break;
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -924,8 +899,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
@@ -937,7 +918,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 4a496eb..c0f44c2 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -260,8 +260,13 @@ struct omap_sr_nvalue_table {
  *
  * @name:		instance name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -274,6 +279,11 @@ struct omap_sr_data {
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-03 15:47   ` jean.pihet at newoldbits.com
  0 siblings, 0 replies; 34+ messages in thread
From: jean.pihet at newoldbits.com @ 2012-10-03 15:47 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jean Pihet <j-pihet@ti.com>

Remove the device dependent code (ex. cpu_is_xxx()) and settings
from the driver code and instead pass them via the platform
data. This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Note about the smartreflex functional clocks: the smartreflex fclks
are derived from sys_clk and are named "smartreflex.%d". Since the
smartreflex device names and the functional clock names are identical
the device driver code uses them to control the functional clocks.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   13 +++++++++
 drivers/power/avs/smartreflex.c   |   54 ++++++++++++-------------------------
 include/linux/power/smartreflex.h |   14 ++++++++--
 3 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index cbeae56..06de443 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -121,6 +121,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 24768a2..c983e85 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -130,24 +130,21 @@ static irqreturn_t sr_interrupt(int irq, void *data)
 
 static void sr_set_clk_length(struct omap_sr *sr)
 {
-	struct clk *sys_ck;
-	u32 sys_clk_speed;
+	struct clk *fck;
+	u32 fclk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	fck = clk_get(&sr->pdev->dev, "fck");
 
-	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+	if (IS_ERR(fck)) {
+		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
+				__func__, dev_name(&sr->pdev->dev));
 		return;
 	}
 
-	sys_clk_speed = clk_get_rate(sys_ck);
-	clk_put(sys_ck);
+	fclk_speed = clk_get_rate(fck);
+	clk_put(fck);
 
-	switch (sys_clk_speed) {
+	switch (fclk_speed) {
 	case 12000000:
 		sr->clk_length = SRCLKLENGTH_12MHZ_SYSCLK;
 		break;
@@ -164,34 +161,12 @@ static void sr_set_clk_length(struct omap_sr *sr)
 		sr->clk_length = SRCLKLENGTH_38MHZ_SYSCLK;
 		break;
 	default:
-		dev_err(&sr->pdev->dev, "%s: Invalid sysclk value: %d\n",
-			__func__, sys_clk_speed);
+		dev_err(&sr->pdev->dev, "%s: Invalid fclk rate: %d\n",
+			__func__, fclk_speed);
 		break;
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -924,8 +899,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
@@ -937,7 +918,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 4a496eb..c0f44c2 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -260,8 +260,13 @@ struct omap_sr_nvalue_table {
  *
  * @name:		instance name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -274,6 +279,11 @@ struct omap_sr_data {
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-10-03 13:32       ` jean.pihet at newoldbits.com
@ 2012-10-03 14:29         ` Kevin Hilman
  -1 siblings, 0 replies; 34+ messages in thread
From: Kevin Hilman @ 2012-10-03 14:29 UTC (permalink / raw)
  To: jean.pihet
  Cc: linux-omap, linux-arm-kernel, tony, Anton Vorontsov, J Keerthy,
	Jean Pihet

jean.pihet@newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> Remove the device dependent code (ex. cpu_is_xxx()) and settings
> from the driver code and instead pass them via the platform
> data. This allows a clean separation of the driver code and the platform
> code, as required by the move of the platform header files to
> include/linux/platform_data.
>
> Note about the smartreflex functional clocks: the smartreflex fclks
> are derived from sys_clk and are named "smartreflex.%d". Since the
> smartreflex device names and the functional clock names are identical
> the device driver code uses them to control the functional clocks.

Thanks for adding this part.

One more nit below, then please resend this patch as a combined series
with the "align fclk names" patch.  (note: The previous patch 1 from this
series I've queued separately as a fix for v3.7-rc.  )

> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
>  arch/arm/mach-omap2/sr_device.c   |   13 +++++++++++++
>  drivers/power/avs/smartreflex.c   |   38 +++++++++----------------------------
>  include/linux/power/smartreflex.h |   14 ++++++++++++--
>  3 files changed, 34 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
> index cbeae56..06de443 100644
> --- a/arch/arm/mach-omap2/sr_device.c
> +++ b/arch/arm/mach-omap2/sr_device.c
> @@ -121,6 +121,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
>  	sr_data->senn_mod = 0x1;
>  	sr_data->senp_mod = 0x1;
>  
> +	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> +		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
> +		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
> +		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
> +		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
> +			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
> +			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
> +		} else {
> +			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
> +			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
> +		}
> +	}
> +
>  	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",
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index 24768a2..829467f 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -133,14 +133,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
>  	struct clk *sys_ck;
>  	u32 sys_clk_speed;
>  
> -	if (cpu_is_omap34xx())
> -		sys_ck = clk_get(NULL, "sys_ck");
> -	else
> -		sys_ck = clk_get(NULL, "sys_clkin_ck");
> +	sys_ck = clk_get(&sr->pdev->dev, "fck");

nit: since this isn't the sys_clk anymore, could you s/sys_ck/fck/  ?

Thanks,

Kevin


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-03 14:29         ` Kevin Hilman
  0 siblings, 0 replies; 34+ messages in thread
From: Kevin Hilman @ 2012-10-03 14:29 UTC (permalink / raw)
  To: linux-arm-kernel

jean.pihet at newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> Remove the device dependent code (ex. cpu_is_xxx()) and settings
> from the driver code and instead pass them via the platform
> data. This allows a clean separation of the driver code and the platform
> code, as required by the move of the platform header files to
> include/linux/platform_data.
>
> Note about the smartreflex functional clocks: the smartreflex fclks
> are derived from sys_clk and are named "smartreflex.%d". Since the
> smartreflex device names and the functional clock names are identical
> the device driver code uses them to control the functional clocks.

Thanks for adding this part.

One more nit below, then please resend this patch as a combined series
with the "align fclk names" patch.  (note: The previous patch 1 from this
series I've queued separately as a fix for v3.7-rc.  )

> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
>  arch/arm/mach-omap2/sr_device.c   |   13 +++++++++++++
>  drivers/power/avs/smartreflex.c   |   38 +++++++++----------------------------
>  include/linux/power/smartreflex.h |   14 ++++++++++++--
>  3 files changed, 34 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
> index cbeae56..06de443 100644
> --- a/arch/arm/mach-omap2/sr_device.c
> +++ b/arch/arm/mach-omap2/sr_device.c
> @@ -121,6 +121,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
>  	sr_data->senn_mod = 0x1;
>  	sr_data->senp_mod = 0x1;
>  
> +	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> +		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
> +		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
> +		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
> +		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
> +			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
> +			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
> +		} else {
> +			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
> +			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
> +		}
> +	}
> +
>  	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",
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index 24768a2..829467f 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -133,14 +133,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
>  	struct clk *sys_ck;
>  	u32 sys_clk_speed;
>  
> -	if (cpu_is_omap34xx())
> -		sys_ck = clk_get(NULL, "sys_ck");
> -	else
> -		sys_ck = clk_get(NULL, "sys_clkin_ck");
> +	sys_ck = clk_get(&sr->pdev->dev, "fck");

nit: since this isn't the sys_clk anymore, could you s/sys_ck/fck/  ?

Thanks,

Kevin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-09-24 14:16     ` Jean Pihet
@ 2012-10-03 13:32       ` jean.pihet at newoldbits.com
  -1 siblings, 0 replies; 34+ messages in thread
From: jean.pihet @ 2012-10-03 13:32 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, tony, Anton Vorontsov, Kevin Hilman
  Cc: J Keerthy, Jean Pihet

From: Jean Pihet <j-pihet@ti.com>

Remove the device dependent code (ex. cpu_is_xxx()) and settings
from the driver code and instead pass them via the platform
data. This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Note about the smartreflex functional clocks: the smartreflex fclks
are derived from sys_clk and are named "smartreflex.%d". Since the
smartreflex device names and the functional clock names are identical
the device driver code uses them to control the functional clocks.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   13 +++++++++++++
 drivers/power/avs/smartreflex.c   |   38 +++++++++----------------------------
 include/linux/power/smartreflex.h |   14 ++++++++++++--
 3 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index cbeae56..06de443 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -121,6 +121,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 24768a2..829467f 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -133,14 +133,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	struct clk *sys_ck;
 	u32 sys_clk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	sys_ck = clk_get(&sr->pdev->dev, "fck");
 
 	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
+				__func__, dev_name(&sr->pdev->dev));
 		return;
 	}
 
@@ -170,28 +167,6 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -924,8 +899,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
@@ -937,7 +918,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 4a496eb..c0f44c2 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -260,8 +260,13 @@ struct omap_sr_nvalue_table {
  *
  * @name:		instance name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -274,6 +279,11 @@ struct omap_sr_data {
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-03 13:32       ` jean.pihet at newoldbits.com
  0 siblings, 0 replies; 34+ messages in thread
From: jean.pihet at newoldbits.com @ 2012-10-03 13:32 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jean Pihet <j-pihet@ti.com>

Remove the device dependent code (ex. cpu_is_xxx()) and settings
from the driver code and instead pass them via the platform
data. This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Note about the smartreflex functional clocks: the smartreflex fclks
are derived from sys_clk and are named "smartreflex.%d". Since the
smartreflex device names and the functional clock names are identical
the device driver code uses them to control the functional clocks.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   13 +++++++++++++
 drivers/power/avs/smartreflex.c   |   38 +++++++++----------------------------
 include/linux/power/smartreflex.h |   14 ++++++++++++--
 3 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index cbeae56..06de443 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -121,6 +121,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 24768a2..829467f 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -133,14 +133,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	struct clk *sys_ck;
 	u32 sys_clk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	sys_ck = clk_get(&sr->pdev->dev, "fck");
 
 	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
+				__func__, dev_name(&sr->pdev->dev));
 		return;
 	}
 
@@ -170,28 +167,6 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -924,8 +899,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
@@ -937,7 +918,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 4a496eb..c0f44c2 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -260,8 +260,13 @@ struct omap_sr_nvalue_table {
  *
  * @name:		instance name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -274,6 +279,11 @@ struct omap_sr_data {
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-10-02 22:21       ` Kevin Hilman
@ 2012-10-03 13:05         ` Jean Pihet
  -1 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-10-03 13:05 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: tony, J Keerthy, Anton Vorontsov, linux-omap, Jean Pihet,
	linux-arm-kernel

Hi Kevin,

On Wed, Oct 3, 2012 at 12:21 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Hi Jean,
>
> Jean Pihet <jean.pihet@newoldbits.com> writes:
>
>> Remove the device dependent settings (cpu_is_xxx(), IP fck etc.)
>> from the driver code and pass them instead via the platform
>> data.
>> This allows a clean separation of the driver code and the platform
>> code, as required by the move of the platform header files to
>> include/linux/platform_data.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>
> Could you make pdata change and the clock change should be two different
> patches?  Also, your previous patch to align SR clock names should be
> combined with the changes made here.
>
> Some comments on the clock change below...
>
>> ---
>>  arch/arm/mach-omap2/sr_device.c   |   13 ++++++++++++
>>  drivers/power/avs/smartreflex.c   |   40 ++++++++++--------------------------
>>  include/linux/power/smartreflex.h |   14 +++++++++++-
>>  3 files changed, 36 insertions(+), 31 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
>> index d033a65..2885a77 100644
>> --- a/arch/arm/mach-omap2/sr_device.c
>> +++ b/arch/arm/mach-omap2/sr_device.c
>> @@ -122,6 +122,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
>>       sr_data->senn_mod = 0x1;
>>       sr_data->senp_mod = 0x1;
>>
>> +     if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
>> +             sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
>> +             sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
>> +             sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
>> +             if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
>> +                     sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
>> +                     sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
>> +             } else {
>> +                     sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
>> +                     sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
>> +             }
>> +     }
>> +
>>       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",
>> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
>> index 92f6728..7c03c90 100644
>> --- a/drivers/power/avs/smartreflex.c
>> +++ b/drivers/power/avs/smartreflex.c
>> @@ -128,17 +128,16 @@ static irqreturn_t sr_interrupt(int irq, void *data)
>>
>>  static void sr_set_clk_length(struct omap_sr *sr)
>>  {
>> +     char fck_name[16]; /* "smartreflex.0" fits in 16 chars */
>>       struct clk *sys_ck;
>>       u32 sys_clk_speed;
>>
>> -     if (cpu_is_omap34xx())
>> -             sys_ck = clk_get(NULL, "sys_ck");
>> -     else
>> -             sys_ck = clk_get(NULL, "sys_clkin_ck");
>> +     sprintf(fck_name, "smartreflex.%d", sr->srid);
>
> hmm, isn't this the same as dev_name(&sr->pdev.dev) ?
Yes. Atfer the previous patch "ARM: OMAP: hwmod: align the SmartReflex
fck names" there is a direct mapping between the device name and the
IP functional clock. Note: the mapping is based on the order of the
hwmod entries and so it is important not to move them around.

> Combined with your earlier patch to align clock names, this should just
> be:
>
>         sys_ck = clk_get(&sr->pdev.dev, "fck");
Great! That works great and the code is much more elegant.

> Also note that you've changed this from sys_clk to the SR functional
> clock, which seems to be the same clock on 34xx and 44xx, but that change
> should be clearly documented in the changelog.
Ok.

Updated patch in a bit.

>
> Kevin

Thanks for reviewing,
Jean

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-03 13:05         ` Jean Pihet
  0 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-10-03 13:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Kevin,

On Wed, Oct 3, 2012 at 12:21 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Hi Jean,
>
> Jean Pihet <jean.pihet@newoldbits.com> writes:
>
>> Remove the device dependent settings (cpu_is_xxx(), IP fck etc.)
>> from the driver code and pass them instead via the platform
>> data.
>> This allows a clean separation of the driver code and the platform
>> code, as required by the move of the platform header files to
>> include/linux/platform_data.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>
> Could you make pdata change and the clock change should be two different
> patches?  Also, your previous patch to align SR clock names should be
> combined with the changes made here.
>
> Some comments on the clock change below...
>
>> ---
>>  arch/arm/mach-omap2/sr_device.c   |   13 ++++++++++++
>>  drivers/power/avs/smartreflex.c   |   40 ++++++++++--------------------------
>>  include/linux/power/smartreflex.h |   14 +++++++++++-
>>  3 files changed, 36 insertions(+), 31 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
>> index d033a65..2885a77 100644
>> --- a/arch/arm/mach-omap2/sr_device.c
>> +++ b/arch/arm/mach-omap2/sr_device.c
>> @@ -122,6 +122,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
>>       sr_data->senn_mod = 0x1;
>>       sr_data->senp_mod = 0x1;
>>
>> +     if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
>> +             sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
>> +             sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
>> +             sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
>> +             if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
>> +                     sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
>> +                     sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
>> +             } else {
>> +                     sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
>> +                     sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
>> +             }
>> +     }
>> +
>>       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",
>> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
>> index 92f6728..7c03c90 100644
>> --- a/drivers/power/avs/smartreflex.c
>> +++ b/drivers/power/avs/smartreflex.c
>> @@ -128,17 +128,16 @@ static irqreturn_t sr_interrupt(int irq, void *data)
>>
>>  static void sr_set_clk_length(struct omap_sr *sr)
>>  {
>> +     char fck_name[16]; /* "smartreflex.0" fits in 16 chars */
>>       struct clk *sys_ck;
>>       u32 sys_clk_speed;
>>
>> -     if (cpu_is_omap34xx())
>> -             sys_ck = clk_get(NULL, "sys_ck");
>> -     else
>> -             sys_ck = clk_get(NULL, "sys_clkin_ck");
>> +     sprintf(fck_name, "smartreflex.%d", sr->srid);
>
> hmm, isn't this the same as dev_name(&sr->pdev.dev) ?
Yes. Atfer the previous patch "ARM: OMAP: hwmod: align the SmartReflex
fck names" there is a direct mapping between the device name and the
IP functional clock. Note: the mapping is based on the order of the
hwmod entries and so it is important not to move them around.

> Combined with your earlier patch to align clock names, this should just
> be:
>
>         sys_ck = clk_get(&sr->pdev.dev, "fck");
Great! That works great and the code is much more elegant.

> Also note that you've changed this from sys_clk to the SR functional
> clock, which seems to be the same clock on 34xx and 44xx, but that change
> should be clearly documented in the changelog.
Ok.

Updated patch in a bit.

>
> Kevin

Thanks for reviewing,
Jean

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-09-24 14:16     ` Jean Pihet
@ 2012-10-02 22:21       ` Kevin Hilman
  -1 siblings, 0 replies; 34+ messages in thread
From: Kevin Hilman @ 2012-10-02 22:21 UTC (permalink / raw)
  To: Jean Pihet
  Cc: linux-omap, linux-arm-kernel, tony, Anton Vorontsov, J Keerthy,
	Jean Pihet

Hi Jean,

Jean Pihet <jean.pihet@newoldbits.com> writes:

> Remove the device dependent settings (cpu_is_xxx(), IP fck etc.)
> from the driver code and pass them instead via the platform
> data.
> This allows a clean separation of the driver code and the platform
> code, as required by the move of the platform header files to
> include/linux/platform_data.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>

Could you make pdata change and the clock change should be two different
patches?  Also, your previous patch to align SR clock names should be
combined with the changes made here.

Some comments on the clock change below...

> ---
>  arch/arm/mach-omap2/sr_device.c   |   13 ++++++++++++
>  drivers/power/avs/smartreflex.c   |   40 ++++++++++--------------------------
>  include/linux/power/smartreflex.h |   14 +++++++++++-
>  3 files changed, 36 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
> index d033a65..2885a77 100644
> --- a/arch/arm/mach-omap2/sr_device.c
> +++ b/arch/arm/mach-omap2/sr_device.c
> @@ -122,6 +122,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
>  	sr_data->senn_mod = 0x1;
>  	sr_data->senp_mod = 0x1;
>  
> +	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> +		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
> +		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
> +		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
> +		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
> +			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
> +			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
> +		} else {
> +			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
> +			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
> +		}
> +	}
> +
>  	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",
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index 92f6728..7c03c90 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -128,17 +128,16 @@ static irqreturn_t sr_interrupt(int irq, void *data)
>  
>  static void sr_set_clk_length(struct omap_sr *sr)
>  {
> +	char fck_name[16]; /* "smartreflex.0" fits in 16 chars */
>  	struct clk *sys_ck;
>  	u32 sys_clk_speed;
>  
> -	if (cpu_is_omap34xx())
> -		sys_ck = clk_get(NULL, "sys_ck");
> -	else
> -		sys_ck = clk_get(NULL, "sys_clkin_ck");
> +	sprintf(fck_name, "smartreflex.%d", sr->srid);

hmm, isn't this the same as dev_name(&sr->pdev.dev) ?

Combined with your earlier patch to align clock names, this should just
be:

        sys_ck = clk_get(&sr->pdev.dev, "fck");

Also note that you've changed this from sys_clk to the SR functional
clock, which seems to be the same clock on 34xx and 44xx, but that change
should be clearly documented in the changelog.

Kevin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-10-02 22:21       ` Kevin Hilman
  0 siblings, 0 replies; 34+ messages in thread
From: Kevin Hilman @ 2012-10-02 22:21 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Jean,

Jean Pihet <jean.pihet@newoldbits.com> writes:

> Remove the device dependent settings (cpu_is_xxx(), IP fck etc.)
> from the driver code and pass them instead via the platform
> data.
> This allows a clean separation of the driver code and the platform
> code, as required by the move of the platform header files to
> include/linux/platform_data.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>

Could you make pdata change and the clock change should be two different
patches?  Also, your previous patch to align SR clock names should be
combined with the changes made here.

Some comments on the clock change below...

> ---
>  arch/arm/mach-omap2/sr_device.c   |   13 ++++++++++++
>  drivers/power/avs/smartreflex.c   |   40 ++++++++++--------------------------
>  include/linux/power/smartreflex.h |   14 +++++++++++-
>  3 files changed, 36 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
> index d033a65..2885a77 100644
> --- a/arch/arm/mach-omap2/sr_device.c
> +++ b/arch/arm/mach-omap2/sr_device.c
> @@ -122,6 +122,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
>  	sr_data->senn_mod = 0x1;
>  	sr_data->senp_mod = 0x1;
>  
> +	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> +		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
> +		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
> +		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
> +		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
> +			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
> +			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
> +		} else {
> +			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
> +			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
> +		}
> +	}
> +
>  	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",
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index 92f6728..7c03c90 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -128,17 +128,16 @@ static irqreturn_t sr_interrupt(int irq, void *data)
>  
>  static void sr_set_clk_length(struct omap_sr *sr)
>  {
> +	char fck_name[16]; /* "smartreflex.0" fits in 16 chars */
>  	struct clk *sys_ck;
>  	u32 sys_clk_speed;
>  
> -	if (cpu_is_omap34xx())
> -		sys_ck = clk_get(NULL, "sys_ck");
> -	else
> -		sys_ck = clk_get(NULL, "sys_clkin_ck");
> +	sprintf(fck_name, "smartreflex.%d", sr->srid);

hmm, isn't this the same as dev_name(&sr->pdev.dev) ?

Combined with your earlier patch to align clock names, this should just
be:

        sys_ck = clk_get(&sr->pdev.dev, "fck");

Also note that you've changed this from sys_clk to the SR functional
clock, which seems to be the same clock on 34xx and 44xx, but that change
should be clearly documented in the changelog.

Kevin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-09-24 14:16 ` [PATCH 0/2] " Jean Pihet
@ 2012-09-24 14:16     ` Jean Pihet
  0 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-09-24 14:16 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, tony, Anton Vorontsov
  Cc: Kevin Hilman, J Keerthy, Jean Pihet

Remove the device dependent settings (cpu_is_xxx(), IP fck etc.)
from the driver code and pass them instead via the platform
data.
This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   13 ++++++++++++
 drivers/power/avs/smartreflex.c   |   40 ++++++++++--------------------------
 include/linux/power/smartreflex.h |   14 +++++++++++-
 3 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index d033a65..2885a77 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -122,6 +122,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 92f6728..7c03c90 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -128,17 +128,16 @@ static irqreturn_t sr_interrupt(int irq, void *data)
 
 static void sr_set_clk_length(struct omap_sr *sr)
 {
+	char fck_name[16]; /* "smartreflex.0" fits in 16 chars */
 	struct clk *sys_ck;
 	u32 sys_clk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	sprintf(fck_name, "smartreflex.%d", sr->srid);
+	sys_ck = clk_get(NULL, fck_name);
 
 	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
+			__func__, fck_name);
 		return;
 	}
 
@@ -168,28 +167,6 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -922,8 +899,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
@@ -935,7 +918,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 3101e62..09d5efc 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -260,8 +260,13 @@ struct omap_sr_nvalue_table {
  *
  * @name:		instance name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -274,6 +279,11 @@ struct omap_sr_data {
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-09-24 14:16     ` Jean Pihet
  0 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-09-24 14:16 UTC (permalink / raw)
  To: linux-arm-kernel

Remove the device dependent settings (cpu_is_xxx(), IP fck etc.)
from the driver code and pass them instead via the platform
data.
This allows a clean separation of the driver code and the platform
code, as required by the move of the platform header files to
include/linux/platform_data.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   13 ++++++++++++
 drivers/power/avs/smartreflex.c   |   40 ++++++++++--------------------------
 include/linux/power/smartreflex.h |   14 +++++++++++-
 3 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index d033a65..2885a77 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -122,6 +122,19 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 92f6728..7c03c90 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -128,17 +128,16 @@ static irqreturn_t sr_interrupt(int irq, void *data)
 
 static void sr_set_clk_length(struct omap_sr *sr)
 {
+	char fck_name[16]; /* "smartreflex.0" fits in 16 chars */
 	struct clk *sys_ck;
 	u32 sys_clk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	sprintf(fck_name, "smartreflex.%d", sr->srid);
+	sys_ck = clk_get(NULL, fck_name);
 
 	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
+			__func__, fck_name);
 		return;
 	}
 
@@ -168,28 +167,6 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -922,8 +899,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
@@ -935,7 +918,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 3101e62..09d5efc 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -260,8 +260,13 @@ struct omap_sr_nvalue_table {
  *
  * @name:		instance name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -274,6 +279,11 @@ struct omap_sr_data {
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-09-21 19:07         ` Tony Lindgren
@ 2012-09-24 14:10           ` Jean Pihet
  -1 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-09-24 14:10 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-omap, linux-arm-kernel, Anton Vorontsov, Kevin Hilman,
	J Keerthy, Jean Pihet

Hi Tony,

On Fri, Sep 21, 2012 at 9:07 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Jean Pihet <jean.pihet@newoldbits.com> [120920 23:31]:
>> On Fri, Sep 21, 2012 at 12:15 AM, Tony Lindgren <tony@atomide.com> wrote:
>> >
>> > You should be able to make this even simpler and not have to pass
>> > the clock name around at all. Just do:
>> >
>> > syc_ck = clk_get(NULL, "fck);
>> > ...
>> The problem is that the system has multiple instances of the
>> smartreflex module, each having its own fck. On OMAP3/4 the fck's are
>> derived from sys_clk via muxes and latches.
>> The proposed code uses the fck's as defined in the .main_clk field of
>> the hwmod entries, so that it takes the muxes and latches into account
>> and also has a consistent clock naming.
>
> If the same system has multiple clocks, then you could have them matched
> by the smartreflex driver instance number.
>
> Or if you mean different source clocks for various omaps, then
> you just need to set multiple aliases for those clocks.
>
>> > In the driver, and add the necessary entries to the clock alias
>> > table. That way it's up to the SoC to set up the necessary clocks
>> > and the driver stays generic.
>> Got it. The only solution would be to use an unique fck for all
>> smartreflex modules in all configurations. Is that acceptable?
>
> Hmm maybe I don't follow you, but sounds like you just need to
> use the driver instance like we do for timers:
>
> $ grep omap_timer arch/arm/mach-omap2/clock*data*.c
> arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.1",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
> arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.2",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
> arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.3",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
> ...
Ok.

I have a new version that implements this, re-submitting in a bit.

>
> If you need multiple clocks for a driver instance, then they
> typically are just "fck" and "ick".
>
> Regards,
>
> Tony

Thanks,
Jean

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-09-24 14:10           ` Jean Pihet
  0 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-09-24 14:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Tony,

On Fri, Sep 21, 2012 at 9:07 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Jean Pihet <jean.pihet@newoldbits.com> [120920 23:31]:
>> On Fri, Sep 21, 2012 at 12:15 AM, Tony Lindgren <tony@atomide.com> wrote:
>> >
>> > You should be able to make this even simpler and not have to pass
>> > the clock name around at all. Just do:
>> >
>> > syc_ck = clk_get(NULL, "fck);
>> > ...
>> The problem is that the system has multiple instances of the
>> smartreflex module, each having its own fck. On OMAP3/4 the fck's are
>> derived from sys_clk via muxes and latches.
>> The proposed code uses the fck's as defined in the .main_clk field of
>> the hwmod entries, so that it takes the muxes and latches into account
>> and also has a consistent clock naming.
>
> If the same system has multiple clocks, then you could have them matched
> by the smartreflex driver instance number.
>
> Or if you mean different source clocks for various omaps, then
> you just need to set multiple aliases for those clocks.
>
>> > In the driver, and add the necessary entries to the clock alias
>> > table. That way it's up to the SoC to set up the necessary clocks
>> > and the driver stays generic.
>> Got it. The only solution would be to use an unique fck for all
>> smartreflex modules in all configurations. Is that acceptable?
>
> Hmm maybe I don't follow you, but sounds like you just need to
> use the driver instance like we do for timers:
>
> $ grep omap_timer arch/arm/mach-omap2/clock*data*.c
> arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.1",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
> arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.2",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
> arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.3",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
> ...
Ok.

I have a new version that implements this, re-submitting in a bit.

>
> If you need multiple clocks for a driver instance, then they
> typically are just "fck" and "ick".
>
> Regards,
>
> Tony

Thanks,
Jean

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-09-21  6:30       ` Jean Pihet
@ 2012-09-21 19:07         ` Tony Lindgren
  -1 siblings, 0 replies; 34+ messages in thread
From: Tony Lindgren @ 2012-09-21 19:07 UTC (permalink / raw)
  To: Jean Pihet
  Cc: linux-omap, linux-arm-kernel, Anton Vorontsov, Kevin Hilman,
	J Keerthy, Jean Pihet

* Jean Pihet <jean.pihet@newoldbits.com> [120920 23:31]:
> On Fri, Sep 21, 2012 at 12:15 AM, Tony Lindgren <tony@atomide.com> wrote:
> >
> > You should be able to make this even simpler and not have to pass
> > the clock name around at all. Just do:
> >
> > syc_ck = clk_get(NULL, "fck);
> > ...
> The problem is that the system has multiple instances of the
> smartreflex module, each having its own fck. On OMAP3/4 the fck's are
> derived from sys_clk via muxes and latches.
> The proposed code uses the fck's as defined in the .main_clk field of
> the hwmod entries, so that it takes the muxes and latches into account
> and also has a consistent clock naming.

If the same system has multiple clocks, then you could have them matched
by the smartreflex driver instance number.

Or if you mean different source clocks for various omaps, then
you just need to set multiple aliases for those clocks.
 
> > In the driver, and add the necessary entries to the clock alias
> > table. That way it's up to the SoC to set up the necessary clocks
> > and the driver stays generic.
> Got it. The only solution would be to use an unique fck for all
> smartreflex modules in all configurations. Is that acceptable?

Hmm maybe I don't follow you, but sounds like you just need to
use the driver instance like we do for timers:

$ grep omap_timer arch/arm/mach-omap2/clock*data*.c
arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.1",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.2",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.3",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
...

If you need multiple clocks for a driver instance, then they
typically are just "fck" and "ick".

Regards,

Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-09-21 19:07         ` Tony Lindgren
  0 siblings, 0 replies; 34+ messages in thread
From: Tony Lindgren @ 2012-09-21 19:07 UTC (permalink / raw)
  To: linux-arm-kernel

* Jean Pihet <jean.pihet@newoldbits.com> [120920 23:31]:
> On Fri, Sep 21, 2012 at 12:15 AM, Tony Lindgren <tony@atomide.com> wrote:
> >
> > You should be able to make this even simpler and not have to pass
> > the clock name around at all. Just do:
> >
> > syc_ck = clk_get(NULL, "fck);
> > ...
> The problem is that the system has multiple instances of the
> smartreflex module, each having its own fck. On OMAP3/4 the fck's are
> derived from sys_clk via muxes and latches.
> The proposed code uses the fck's as defined in the .main_clk field of
> the hwmod entries, so that it takes the muxes and latches into account
> and also has a consistent clock naming.

If the same system has multiple clocks, then you could have them matched
by the smartreflex driver instance number.

Or if you mean different source clocks for various omaps, then
you just need to set multiple aliases for those clocks.
 
> > In the driver, and add the necessary entries to the clock alias
> > table. That way it's up to the SoC to set up the necessary clocks
> > and the driver stays generic.
> Got it. The only solution would be to use an unique fck for all
> smartreflex modules in all configurations. Is that acceptable?

Hmm maybe I don't follow you, but sounds like you just need to
use the driver instance like we do for timers:

$ grep omap_timer arch/arm/mach-omap2/clock*data*.c
arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.1",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.2",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
arch/arm/mach-omap2/clock44xx_data.c:   CLK("omap_timer.3",     "timer_sys_ck", &sys_clkin_ck,  CK_443X),
...

If you need multiple clocks for a driver instance, then they
typically are just "fck" and "ick".

Regards,

Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-09-20 22:15     ` Tony Lindgren
@ 2012-09-21  6:30       ` Jean Pihet
  -1 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-09-21  6:30 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-omap, linux-arm-kernel, Anton Vorontsov, Kevin Hilman,
	J Keerthy, Jean Pihet

Hi Tony,

On Fri, Sep 21, 2012 at 12:15 AM, Tony Lindgren <tony@atomide.com> wrote:
> Hi,
>
> * Jean Pihet <jean.pihet@newoldbits.com> [120920 07:48]:
>> --- a/drivers/power/avs/smartreflex.c
>> +++ b/drivers/power/avs/smartreflex.c
>> @@ -131,14 +131,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
>>       struct clk *sys_ck;
>>       u32 sys_clk_speed;
>>
>> -     if (cpu_is_omap34xx())
>> -             sys_ck = clk_get(NULL, "sys_ck");
>> -     else
>> -             sys_ck = clk_get(NULL, "sys_clkin_ck");
>> +     sys_ck = clk_get(NULL, sr->fck_name);
>>
>>       if (IS_ERR(sys_ck)) {
>> -             dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
>> -                     __func__);
>> +             dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
>> +                     __func__, sr->fck_name);
>>               return;
>>       }
>>
>
> You should be able to make this even simpler and not have to pass
> the clock name around at all. Just do:
>
> syc_ck = clk_get(NULL, "fck);
> ...
The problem is that the system has multiple instances of the
smartreflex module, each having its own fck. On OMAP3/4 the fck's are
derived from sys_clk via muxes and latches.
The proposed code uses the fck's as defined in the .main_clk field of
the hwmod entries, so that it takes the muxes and latches into account
and also has a consistent clock naming.

> In the driver, and add the necessary entries to the clock alias
> table. That way it's up to the SoC to set up the necessary clocks
> and the driver stays generic.
Got it. The only solution would be to use an unique fck for all
smartreflex modules in all configurations. Is that acceptable?

>
>> @@ -1049,6 +1039,7 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
>>
>>       list_del(&sr_info->node);
>>       iounmap(sr_info->base);
>> +     kfree(sr_info->fck_name);
>>       kfree(sr_info->name);
>>       kfree(sr_info);
>>       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> Then there's no need for the kfree of the fck_name
> either.
>
> There's an example of a similar patch done for twl-core.c as commit
> defa6be1 (mfd: Fix compile for twl-core.c by removing cpu_is_omap usage)
> in current linux next, except with smartreflex you probably don't
> need to do any of the platform_device_alloc trickery like twl-core.c
> neded to get around using the i2c numbers as names.

Thanks!

Regards,
Jean

>
> Regards,
>
> Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-09-21  6:30       ` Jean Pihet
  0 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-09-21  6:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Tony,

On Fri, Sep 21, 2012 at 12:15 AM, Tony Lindgren <tony@atomide.com> wrote:
> Hi,
>
> * Jean Pihet <jean.pihet@newoldbits.com> [120920 07:48]:
>> --- a/drivers/power/avs/smartreflex.c
>> +++ b/drivers/power/avs/smartreflex.c
>> @@ -131,14 +131,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
>>       struct clk *sys_ck;
>>       u32 sys_clk_speed;
>>
>> -     if (cpu_is_omap34xx())
>> -             sys_ck = clk_get(NULL, "sys_ck");
>> -     else
>> -             sys_ck = clk_get(NULL, "sys_clkin_ck");
>> +     sys_ck = clk_get(NULL, sr->fck_name);
>>
>>       if (IS_ERR(sys_ck)) {
>> -             dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
>> -                     __func__);
>> +             dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
>> +                     __func__, sr->fck_name);
>>               return;
>>       }
>>
>
> You should be able to make this even simpler and not have to pass
> the clock name around at all. Just do:
>
> syc_ck = clk_get(NULL, "fck);
> ...
The problem is that the system has multiple instances of the
smartreflex module, each having its own fck. On OMAP3/4 the fck's are
derived from sys_clk via muxes and latches.
The proposed code uses the fck's as defined in the .main_clk field of
the hwmod entries, so that it takes the muxes and latches into account
and also has a consistent clock naming.

> In the driver, and add the necessary entries to the clock alias
> table. That way it's up to the SoC to set up the necessary clocks
> and the driver stays generic.
Got it. The only solution would be to use an unique fck for all
smartreflex modules in all configurations. Is that acceptable?

>
>> @@ -1049,6 +1039,7 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
>>
>>       list_del(&sr_info->node);
>>       iounmap(sr_info->base);
>> +     kfree(sr_info->fck_name);
>>       kfree(sr_info->name);
>>       kfree(sr_info);
>>       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> Then there's no need for the kfree of the fck_name
> either.
>
> There's an example of a similar patch done for twl-core.c as commit
> defa6be1 (mfd: Fix compile for twl-core.c by removing cpu_is_omap usage)
> in current linux next, except with smartreflex you probably don't
> need to do any of the platform_device_alloc trickery like twl-core.c
> neded to get around using the i2c numbers as names.

Thanks!

Regards,
Jean

>
> Regards,
>
> Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-09-20 14:47   ` Jean Pihet
@ 2012-09-20 22:15     ` Tony Lindgren
  -1 siblings, 0 replies; 34+ messages in thread
From: Tony Lindgren @ 2012-09-20 22:15 UTC (permalink / raw)
  To: Jean Pihet
  Cc: linux-omap, linux-arm-kernel, Anton Vorontsov, Kevin Hilman,
	J Keerthy, Jean Pihet

Hi,

* Jean Pihet <jean.pihet@newoldbits.com> [120920 07:48]:
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -131,14 +131,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
>  	struct clk *sys_ck;
>  	u32 sys_clk_speed;
>  
> -	if (cpu_is_omap34xx())
> -		sys_ck = clk_get(NULL, "sys_ck");
> -	else
> -		sys_ck = clk_get(NULL, "sys_clkin_ck");
> +	sys_ck = clk_get(NULL, sr->fck_name);
>  
>  	if (IS_ERR(sys_ck)) {
> -		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
> -			__func__);
> +		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
> +			__func__, sr->fck_name);
>  		return;
>  	}
>  

You should be able to make this even simpler and not have to pass
the clock name around at all. Just do:

syc_ck = clk_get(NULL, "fck);
...

In the driver, and add the necessary entries to the clock alias
table. That way it's up to the SoC to set up the necessary clocks
and the driver stays generic.

> @@ -1049,6 +1039,7 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
>  
>  	list_del(&sr_info->node);
>  	iounmap(sr_info->base);
> +	kfree(sr_info->fck_name);
>  	kfree(sr_info->name);
>  	kfree(sr_info);
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);

Then there's no need for the kfree of the fck_name
either.

There's an example of a similar patch done for twl-core.c as commit
defa6be1 (mfd: Fix compile for twl-core.c by removing cpu_is_omap usage)
in current linux next, except with smartreflex you probably don't
need to do any of the platform_device_alloc trickery like twl-core.c
neded to get around using the i2c numbers as names.

Regards,

Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-09-20 22:15     ` Tony Lindgren
  0 siblings, 0 replies; 34+ messages in thread
From: Tony Lindgren @ 2012-09-20 22:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

* Jean Pihet <jean.pihet@newoldbits.com> [120920 07:48]:
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -131,14 +131,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
>  	struct clk *sys_ck;
>  	u32 sys_clk_speed;
>  
> -	if (cpu_is_omap34xx())
> -		sys_ck = clk_get(NULL, "sys_ck");
> -	else
> -		sys_ck = clk_get(NULL, "sys_clkin_ck");
> +	sys_ck = clk_get(NULL, sr->fck_name);
>  
>  	if (IS_ERR(sys_ck)) {
> -		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
> -			__func__);
> +		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
> +			__func__, sr->fck_name);
>  		return;
>  	}
>  

You should be able to make this even simpler and not have to pass
the clock name around at all. Just do:

syc_ck = clk_get(NULL, "fck);
...

In the driver, and add the necessary entries to the clock alias
table. That way it's up to the SoC to set up the necessary clocks
and the driver stays generic.

> @@ -1049,6 +1039,7 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
>  
>  	list_del(&sr_info->node);
>  	iounmap(sr_info->base);
> +	kfree(sr_info->fck_name);
>  	kfree(sr_info->name);
>  	kfree(sr_info);
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);

Then there's no need for the kfree of the fck_name
either.

There's an example of a similar patch done for twl-core.c as commit
defa6be1 (mfd: Fix compile for twl-core.c by removing cpu_is_omap usage)
in current linux next, except with smartreflex you probably don't
need to do any of the platform_device_alloc trickery like twl-core.c
neded to get around using the i2c numbers as names.

Regards,

Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
  2012-09-20 14:47 [PATCH 0/2] " Jean Pihet
@ 2012-09-20 14:47   ` Jean Pihet
  2012-09-24 14:16 ` [PATCH 0/2] " Jean Pihet
  1 sibling, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-09-20 14:47 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, tony, Anton Vorontsov,
	Kevin Hilman, J Keerthy
  Cc: Jean Pihet

Remove the device dependent settings (cpu_is_xxx(), IP fck etc.)
from the driver code and pass them instead via the platform
data.
This allows a clean separation of the driver code and the platform
code.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   14 ++++++++++
 drivers/power/avs/smartreflex.c   |   51 +++++++++++++++---------------------
 include/linux/power/smartreflex.h |   17 +++++++++++-
 3 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index d033a65..12d95c8 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -118,10 +118,24 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	}
 
 	sr_data->name = oh->name;
+	sr_data->fck_name = oh->main_clk;
 	sr_data->ip_type = oh->class->rev;
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu_iva"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 92f6728..f09e8df 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -131,14 +131,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	struct clk *sys_ck;
 	u32 sys_clk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	sys_ck = clk_get(NULL, sr->fck_name);
 
 	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
+			__func__, sr->fck_name);
 		return;
 	}
 
@@ -168,28 +165,6 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -915,6 +890,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		goto err_release_region;
 	}
 
+	sr_info->fck_name = kasprintf(GFP_KERNEL, "%s", pdata->fck_name);
+	if (!sr_info->fck_name) {
+		dev_err(&pdev->dev, "%s: Unable to alloc SR instance fck name\n",
+			__func__);
+		ret = -ENOMEM;
+		goto err_free_name;
+	}
+
 	sr_info->pdev = pdev;
 	sr_info->srid = pdev->id;
 	sr_info->voltdm = pdata->voltdm;
@@ -922,20 +905,25 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
 		ret = -ENOMEM;
-		goto err_free_name;
+		goto err_free_fck_name;
 	}
 
 	if (irq)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
@@ -1014,6 +1002,8 @@ err_debugfs:
 err_iounmap:
 	list_del(&sr_info->node);
 	iounmap(sr_info->base);
+err_free_fck_name:
+	kfree(sr_info->fck_name);
 err_free_name:
 	kfree(sr_info->name);
 err_release_region:
@@ -1049,6 +1039,7 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
 
 	list_del(&sr_info->node);
 	iounmap(sr_info->base);
+	kfree(sr_info->fck_name);
 	kfree(sr_info->name);
 	kfree(sr_info);
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 3101e62..7b16c3c 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -145,6 +145,7 @@
 
 struct omap_sr {
 	char				*name;
+	char				*fck_name;
 	struct list_head		node;
 	struct platform_device		*pdev;
 	struct omap_sr_nvalue_table	*nvalue_table;
@@ -259,9 +260,15 @@ struct omap_sr_nvalue_table {
  * struct omap_sr_data - Smartreflex platform data.
  *
  * @name:		instance name
+ * @fck_name:		IP block fck name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -271,9 +278,15 @@ struct omap_sr_nvalue_table {
  */
 struct omap_sr_data {
 	const char			*name;
+	const char			*fck_name;
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data
@ 2012-09-20 14:47   ` Jean Pihet
  0 siblings, 0 replies; 34+ messages in thread
From: Jean Pihet @ 2012-09-20 14:47 UTC (permalink / raw)
  To: linux-arm-kernel

Remove the device dependent settings (cpu_is_xxx(), IP fck etc.)
from the driver code and pass them instead via the platform
data.
This allows a clean separation of the driver code and the platform
code.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/sr_device.c   |   14 ++++++++++
 drivers/power/avs/smartreflex.c   |   51 +++++++++++++++---------------------
 include/linux/power/smartreflex.h |   17 +++++++++++-
 3 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index d033a65..12d95c8 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -118,10 +118,24 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 	}
 
 	sr_data->name = oh->name;
+	sr_data->fck_name = oh->main_clk;
 	sr_data->ip_type = oh->class->rev;
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
 
+	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
+		sr_data->err_weight = OMAP3430_SR_ERRWEIGHT;
+		sr_data->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
+		sr_data->accum_data = OMAP3430_SR_ACCUMDATA;
+		if (!(strcmp(sr_data->name, "smartreflex_mpu_iva"))) {
+			sr_data->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
+		} else {
+			sr_data->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
+			sr_data->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
+		}
+	}
+
 	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",
diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
index 92f6728..f09e8df 100644
--- a/drivers/power/avs/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -131,14 +131,11 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	struct clk *sys_ck;
 	u32 sys_clk_speed;
 
-	if (cpu_is_omap34xx())
-		sys_ck = clk_get(NULL, "sys_ck");
-	else
-		sys_ck = clk_get(NULL, "sys_clkin_ck");
+	sys_ck = clk_get(NULL, sr->fck_name);
 
 	if (IS_ERR(sys_ck)) {
-		dev_err(&sr->pdev->dev, "%s: unable to get sys clk\n",
-			__func__);
+		dev_err(&sr->pdev->dev, "%s: unable to get smartreflex fck %s\n",
+			__func__, sr->fck_name);
 		return;
 	}
 
@@ -168,28 +165,6 @@ static void sr_set_clk_length(struct omap_sr *sr)
 	}
 }
 
-static void sr_set_regfields(struct omap_sr *sr)
-{
-	/*
-	 * For time being these values are defined in smartreflex.h
-	 * and populated during init. May be they can be moved to board
-	 * file or pmic specific data structure. In that case these structure
-	 * fields will have to be populated using the pdata or pmic structure.
-	 */
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
-		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
-		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
-			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
-		} else {
-			sr->senn_avgweight = OMAP3430_SR2_SENNAVGWEIGHT;
-			sr->senp_avgweight = OMAP3430_SR2_SENPAVGWEIGHT;
-		}
-	}
-}
-
 static void sr_start_vddautocomp(struct omap_sr *sr)
 {
 	if (!sr_class || !(sr_class->enable) || !(sr_class->configure)) {
@@ -915,6 +890,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		goto err_release_region;
 	}
 
+	sr_info->fck_name = kasprintf(GFP_KERNEL, "%s", pdata->fck_name);
+	if (!sr_info->fck_name) {
+		dev_err(&pdev->dev, "%s: Unable to alloc SR instance fck name\n",
+			__func__);
+		ret = -ENOMEM;
+		goto err_free_name;
+	}
+
 	sr_info->pdev = pdev;
 	sr_info->srid = pdev->id;
 	sr_info->voltdm = pdata->voltdm;
@@ -922,20 +905,25 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	sr_info->nvalue_count = pdata->nvalue_count;
 	sr_info->senn_mod = pdata->senn_mod;
 	sr_info->senp_mod = pdata->senp_mod;
+	sr_info->err_weight = pdata->err_weight;
+	sr_info->err_maxlimit = pdata->err_maxlimit;
+	sr_info->accum_data = pdata->accum_data;
+	sr_info->senn_avgweight = pdata->senn_avgweight;
+	sr_info->senp_avgweight = pdata->senp_avgweight;
 	sr_info->autocomp_active = false;
 	sr_info->ip_type = pdata->ip_type;
+
 	sr_info->base = ioremap(mem->start, resource_size(mem));
 	if (!sr_info->base) {
 		dev_err(&pdev->dev, "%s: ioremap fail\n", __func__);
 		ret = -ENOMEM;
-		goto err_free_name;
+		goto err_free_fck_name;
 	}
 
 	if (irq)
 		sr_info->irq = irq->start;
 
 	sr_set_clk_length(sr_info);
-	sr_set_regfields(sr_info);
 
 	list_add(&sr_info->node, &sr_list);
 
@@ -1014,6 +1002,8 @@ err_debugfs:
 err_iounmap:
 	list_del(&sr_info->node);
 	iounmap(sr_info->base);
+err_free_fck_name:
+	kfree(sr_info->fck_name);
 err_free_name:
 	kfree(sr_info->name);
 err_release_region:
@@ -1049,6 +1039,7 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
 
 	list_del(&sr_info->node);
 	iounmap(sr_info->base);
+	kfree(sr_info->fck_name);
 	kfree(sr_info->name);
 	kfree(sr_info);
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 3101e62..7b16c3c 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -145,6 +145,7 @@
 
 struct omap_sr {
 	char				*name;
+	char				*fck_name;
 	struct list_head		node;
 	struct platform_device		*pdev;
 	struct omap_sr_nvalue_table	*nvalue_table;
@@ -259,9 +260,15 @@ struct omap_sr_nvalue_table {
  * struct omap_sr_data - Smartreflex platform data.
  *
  * @name:		instance name
+ * @fck_name:		IP block fck name
  * @ip_type:		Smartreflex IP type.
- * @senp_mod:		SENPENABLE value for the sr
- * @senn_mod:		SENNENABLE value for sr
+ * @senp_mod:		SENPENABLE value of the sr CONFIG register
+ * @senn_mod:		SENNENABLE value for sr CONFIG register
+ * @err_weight		ERRWEIGHT value of the sr ERRCONFIG register
+ * @err_maxlimit	ERRMAXLIMIT value of the sr ERRCONFIG register
+ * @accum_data		ACCUMDATA value of the sr CONFIG register
+ * @senn_avgweight	SENNAVGWEIGHT value of the sr AVGWEIGHT register
+ * @senp_avgweight	SENPAVGWEIGHT value of the sr AVGWEIGHT register
  * @nvalue_count:	Number of distinct nvalues in the nvalue table
  * @enable_on_init:	whether this sr module needs to enabled at
  *			boot up or not.
@@ -271,9 +278,15 @@ struct omap_sr_nvalue_table {
  */
 struct omap_sr_data {
 	const char			*name;
+	const char			*fck_name;
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
+	u32				err_weight;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
 	int				nvalue_count;
 	bool				enable_on_init;
 	struct omap_sr_nvalue_table	*nvalue_table;
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2012-10-05  8:10 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-04 16:47 [PATCH 0/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data jean.pihet
2012-10-04 16:47 ` jean.pihet at newoldbits.com
2012-10-04 16:47 ` [PATCH 1/2] ARM: OMAP: hwmod: align the SmartReflex fck names jean.pihet
2012-10-04 16:47   ` jean.pihet at newoldbits.com
2012-10-04 16:47 ` [PATCH 2/2] ARM: OMAP: SmartReflex: pass device dependent data via platform data jean.pihet
2012-10-04 16:47   ` jean.pihet at newoldbits.com
2012-10-04 23:40 ` [PATCH 0/2] " Kevin Hilman
2012-10-04 23:40   ` Kevin Hilman
2012-10-05  8:10   ` Jean Pihet
2012-10-05  8:10     ` Jean Pihet
  -- strict thread matches above, loose matches on Subject: below --
2012-10-03 15:47 jean.pihet
2012-10-03 15:47 ` [PATCH 2/2] " jean.pihet
2012-10-03 15:47   ` jean.pihet at newoldbits.com
2012-09-20 14:47 [PATCH 0/2] " Jean Pihet
2012-09-20 14:47 ` [PATCH 2/2] " Jean Pihet
2012-09-20 14:47   ` Jean Pihet
2012-09-20 22:15   ` Tony Lindgren
2012-09-20 22:15     ` Tony Lindgren
2012-09-21  6:30     ` Jean Pihet
2012-09-21  6:30       ` Jean Pihet
2012-09-21 19:07       ` Tony Lindgren
2012-09-21 19:07         ` Tony Lindgren
2012-09-24 14:10         ` Jean Pihet
2012-09-24 14:10           ` Jean Pihet
2012-09-24 14:16 ` [PATCH 0/2] " Jean Pihet
2012-09-24 14:16   ` [PATCH 2/2] " Jean Pihet
2012-09-24 14:16     ` Jean Pihet
2012-10-02 22:21     ` Kevin Hilman
2012-10-02 22:21       ` Kevin Hilman
2012-10-03 13:05       ` Jean Pihet
2012-10-03 13:05         ` Jean Pihet
2012-10-03 13:32     ` jean.pihet
2012-10-03 13:32       ` jean.pihet at newoldbits.com
2012-10-03 14:29       ` Kevin Hilman
2012-10-03 14:29         ` Kevin Hilman
2012-10-03 15:51         ` Jean Pihet
2012-10-03 15:51           ` Jean Pihet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.