mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32

PATCH New Benoît Thébaudeau 2 participants 6 messages Aug 14, 2012 – Aug 17, 2012

Subsystems (2)

Arm (32-bit) Arm i.MX and Layerscape

lore

Share & tools

lore

Benoît Thébaudeau

author

2012-08-14 15:01 UTC #0 lore raw

PATCH 1/1 mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32

Signed-off-by Beno?t Th?baudeau <[email protected]>

Cc Stefano Babic <[email protected]>

---
 .../arch/arm/cpu/armv7/imx-common/timer.c          |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/imx-common/timer.c +5 -1

index 1645ff8..ad67367 100644
@@ -44,7 +44,11 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR;
 #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
 #define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */
 #define GPTCR_TEN		1		/* Timer enable */
-#define CLK_32KHZ		32768		/* 32Khz input */
+#if defined(CONFIG_MX51) || defined(CONFIG_MX53)
+#define CLK_32KHZ		CONFIG_SYS_MX5_CLK32
+#elif defined(CONFIG_MX6Q)
+#define CLK_32KHZ		CONFIG_SYS_MX6_CLK32
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;

Stefano Babic

2012-08-17 19:43 UTC #1 lore raw

PATCH 1/1 mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32

Quoted 3 lines · Beno?t Th?baudeau

Signed-off-by Beno?t Th?baudeau <[email protected]>
Cc Stefano Babic <[email protected]>
---

Hi Beno?t,

Quoted 10 lines

 .../arch/arm/cpu/armv7/imx-common/timer.c          |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/imx-common/timer.c +0 -0

index 1645ff8..ad67367 100644
@@ -44,7 +44,11 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR;
 #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
 #define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */

arch/arm/cpu/armv7/imx-common/timer.c static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR; · line 54

Quoted 10 lines

arch/arm/cpu/armv7/imx-common/timer.c +5 -1

 #define GPTCR_TEN		1		/* Timer enable */
-#define CLK_32KHZ		32768		/* 32Khz input */
+#if defined(CONFIG_MX51) || defined(CONFIG_MX53)
+#define CLK_32KHZ		CONFIG_SYS_MX5_CLK32
+#elif defined(CONFIG_MX6Q)
+#define CLK_32KHZ		CONFIG_SYS_MX6_CLK32
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;

Frankly I do not see the advantage to use the CONFIG_SYS defines. On the other hand, checking this patch I see that CONFIG_SYS_MX5_CLK32 and CONFIG_SYS_MX6_CLK32 are dead code.

I do not find drivers using, but all boards define them:

grep -r CONFIG_SYS_MX5_CLK32 *
include/configs/efikamx.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53loco.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53ard.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx51evk.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/vision2.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53evk.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/ima3-mx53.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53smd.h:#define CONFIG_SYS_MX5_CLK32 32768

and

include/configs/mx6qsabrelite.h.orig:#define CONFIG_SYS_MX6_CLK32
32768
include/configs/mx6qarm2.h:#define CONFIG_SYS_MX6_CLK32 32768
include/configs/mx6qsabrelite.h:#define CONFIG_SYS_MX6_CLK32 32768

We have two defines, both for nothing. I prefer a patch dropping completely this dead code as to try to use it...

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Benoît Thébaudeau

author

2012-08-17 19:52 UTC #2 lore raw

PATCH 1/1 mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32

Hi Stefano,

Quoted 28 lines · Beno?t Th?baudeau

Quoted 3 lines
Signed-off-by Beno?t Th?baudeau <[email protected]>
Cc Stefano Babic <[email protected]>
---
Hi Beno?t,
Quoted 21 lines
 .../arch/arm/cpu/armv7/imx-common/timer.c          |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/imx-common/timer.c u-boot-4d3c95f/arch/arm/cpu/armv7/imx-common/timer.c index 1645ff8..ad67367 100644
arch/arm/cpu/armv7/imx-common/timer.c +5 -1
@@ -44,7 +44,11 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt
 *)GPT1_BASE_ADDR;
 #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
 #define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */
 #define GPTCR_TEN		1		/* Timer enable */
-#define CLK_32KHZ		32768		/* 32Khz input */
+#if defined(CONFIG_MX51) || defined(CONFIG_MX53)
+#define CLK_32KHZ		CONFIG_SYS_MX5_CLK32
+#elif defined(CONFIG_MX6Q)
+#define CLK_32KHZ		CONFIG_SYS_MX6_CLK32
+#endif
 

Quoted 4 lines

arch/arm/cpu/armv7/imx-common/timer.c static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR; · line 53
Quoted 2 lines
arch/arm/cpu/armv7/imx-common/timer.c +0 -0
 DECLARE_GLOBAL_DATA_PTR;
Frankly I do not see the advantage to use the CONFIG_SYS defines.

It can be useful if 32000 Hz is used instead of 32768 Hz for some boards.

Quoted 17 lines

On
the
other hand, checking this patch I see that CONFIG_SYS_MX5_CLK32 and CONFIG_SYS_MX6_CLK32 are dead code.
I do not find drivers using, but all boards define them:
grep -r CONFIG_SYS_MX5_CLK32 *
include/configs/efikamx.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53loco.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53ard.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx51evk.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/vision2.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53evk.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/ima3-mx53.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53smd.h:#define CONFIG_SYS_MX5_CLK32 32768

Quoted 7 lines

and
include/configs/mx6qsabrelite.h.orig:#define CONFIG_SYS_MX6_CLK32
32768
include/configs/mx6qarm2.h:#define CONFIG_SYS_MX6_CLK32 32768
include/configs/mx6qsabrelite.h:#define CONFIG_SYS_MX6_CLK32
	       32768

Indeed.

Quoted 2 lines

We have two defines, both for nothing. I prefer a patch dropping completely this dead code as to try to use it...

See my use case above, and tell me if it made you change your mind.

Best regards,
Beno?t

Stefano Babic

2012-08-17 20:51 UTC #3 lore raw

PATCH 1/1 mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32

Quoted 7 lines · Beno?t Th?baudeau

Hi Stefano,
Quoted 5 lines · Beno?t Th?baudeau
Quoted 3 lines
Signed-off-by Beno?t Th?baudeau <[email protected]>
Cc Stefano Babic <[email protected]>
---

Hi Beno?t,

Quoted 22 lines

 .../arch/arm/cpu/armv7/imx-common/timer.c          |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/imx-common/timer.c u-boot-4d3c95f/arch/arm/cpu/armv7/imx-common/timer.c index 1645ff8..ad67367 100644

arch/arm/cpu/armv7/imx-common/timer.c +5 -1

@@ -44,7 +44,11 @@ static struct mxc_gpt *cur_gpt = (struct mxc_gpt
 *)GPT1_BASE_ADDR;
 #define GPTCR_FRR		(1 << 9)	/* Freerun / restart */
 #define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source */
 #define GPTCR_TEN		1		/* Timer enable */
-#define CLK_32KHZ		32768		/* 32Khz input */
+#if defined(CONFIG_MX51) || defined(CONFIG_MX53)
+#define CLK_32KHZ		CONFIG_SYS_MX5_CLK32
+#elif defined(CONFIG_MX6Q)
+#define CLK_32KHZ		CONFIG_SYS_MX6_CLK32
+#endif

Quoted 6 lines

Quoted 4 lines
arch/arm/cpu/armv7/imx-common/timer.c static struct mxc_gpt *cur_gpt = (struct mxc_gpt *)GPT1_BASE_ADDR; · line 53
Quoted 2 lines
arch/arm/cpu/armv7/imx-common/timer.c +0 -0
 DECLARE_GLOBAL_DATA_PTR;
Frankly I do not see the advantage to use the CONFIG_SYS defines.
It can be useful if 32000 Hz is used instead of 32768 Hz for some boards.

Then it is the exception, and we should introduce a CONFIG_ that only *that* board should set, and not that all boards must have.

Quoted 20 lines

Quoted 20 lines
On
the
other hand, checking this patch I see that CONFIG_SYS_MX5_CLK32 and CONFIG_SYS_MX6_CLK32 are dead code.
I do not find drivers using, but all boards define them:
grep -r CONFIG_SYS_MX5_CLK32 *
include/configs/efikamx.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53loco.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53ard.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx51evk.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/vision2.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53evk.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/ima3-mx53.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53smd.h:#define CONFIG_SYS_MX5_CLK32 32768
and

Quoted 7 lines

Quoted 5 lines
include/configs/mx6qsabrelite.h.orig:#define CONFIG_SYS_MX6_CLK32
32768
include/configs/mx6qarm2.h:#define CONFIG_SYS_MX6_CLK32 32768
include/configs/mx6qsabrelite.h:#define CONFIG_SYS_MX6_CLK32
	       32768
Indeed.

All boards here use 32768, none of them set it to 32000.

Quoted 4 lines

Quoted 2 lines
We have two defines, both for nothing. I prefer a patch dropping completely this dead code as to try to use it...
See my use case above, and tell me if it made you change your mind.

Your use case is surely for the tx25. But I do not see the case here. This patch alignes the MX5/MX6 to the MX25, where maybe this case could be treated better. All MX25 board must set CONFIG_MX25_CLK32 - instead that only the tx25 set a different value.

Best regards,
Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Benoît Thébaudeau

author

2012-08-17 21:03 UTC #4 lore raw

PATCH 1/1 mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32

Hi Stefano,

Quoted 80 lines · Beno?t Th?baudeau

Quoted 7 lines
Hi Stefano,
Quoted 5 lines · Beno?t Th?baudeau
Quoted 3 lines
Signed-off-by Beno?t Th?baudeau <[email protected]>
Cc Stefano Babic <[email protected]>
---
Hi Beno?t,
Quoted 23 lines
Quoted 23 lines
Quoted 23 lines
 .../arch/arm/cpu/armv7/imx-common/timer.c          |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/imx-common/timer.c u-boot-4d3c95f/arch/arm/cpu/armv7/imx-common/timer.c index 1645ff8..ad67367 100644
arch/arm/cpu/armv7/imx-common/timer.c +0 -0
@@ -44,7 +44,11 @@ static struct mxc_gpt *cur_gpt = (struct
mxc_gpt
*)GPT1_BASE_ADDR;
#define GPTCR_FRR (1 << 9) /* Freerun / restart */
#define GPTCR_CLKSOURCE_32 (4 << 6) /* Clock source */
#define GPTCR_TEN 1 /* Timer enable */
arch/arm/cpu/armv7/imx-common/timer.c +5 -1
-#define CLK_32KHZ		32768		/* 32Khz input */
+#if defined(CONFIG_MX51) || defined(CONFIG_MX53)
+#define CLK_32KHZ		CONFIG_SYS_MX5_CLK32
+#elif defined(CONFIG_MX6Q)
+#define CLK_32KHZ		CONFIG_SYS_MX6_CLK32
+#endif
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
DECLARE_GLOBAL_DATA_PTR;
Frankly I do not see the advantage to use the CONFIG_SYS defines.
It can be useful if 32000 Hz is used instead of 32768 Hz for some boards.
Then it is the exception, and we should introduce a CONFIG_ that only *that* board should set, and not that all boards must have.
Quoted 21 lines
Quoted 21 lines
On
the
other hand, checking this patch I see that CONFIG_SYS_MX5_CLK32 and
CONFIG_SYS_MX6_CLK32 are dead code.
I do not find drivers using, but all boards define them:
grep -r CONFIG_SYS_MX5_CLK32 *
include/configs/efikamx.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53loco.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53ard.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx51evk.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/vision2.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53evk.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/ima3-mx53.h:#define CONFIG_SYS_MX5_CLK32 32768
include/configs/mx53smd.h:#define CONFIG_SYS_MX5_CLK32 32768
and
Quoted 7 lines
Quoted 5 lines
include/configs/mx6qsabrelite.h.orig:#define CONFIG_SYS_MX6_CLK32
32768
include/configs/mx6qarm2.h:#define CONFIG_SYS_MX6_CLK32 32768
include/configs/mx6qsabrelite.h:#define CONFIG_SYS_MX6_CLK32
	       32768
Indeed.
All boards here use 32768, none of them set it to 32000.
Quoted 3 lines
Quoted 2 lines
We have two defines, both for nothing. I prefer a patch dropping completely this dead code as to try to use it...

Quoted 8 lines

Quoted 1 line
See my use case above, and tell me if it made you change your mind.
Your use case is surely for the tx25. But I do not see the case here. This patch alignes the MX5/MX6 to the MX25, where maybe this case could
be treated better. All MX25 board must set CONFIG_MX25_CLK32 - instead
that only the tx25 set a different value.

OK, then for mx25/35/5/6, would you like in timer.c something like:
#ifdef CONFIG_SYS_MX{*}_CLK32
#define CLK_32KHZ CONFIG_SYS_MX{*}_CLK32
#else
#define CLK_32KHZ 32768
#endif

{*}: 25, 35, 5 or 6

In that way, this definition could be removed from most boards, and that would still allow exceptions and be future-proof.

Best regards,
Beno?t

Stefano Babic

2012-08-17 21:09 UTC #5 lore raw

PATCH 1/1 mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32

Quoted 8 lines · Beno?t Th?baudeau

OK, then for mx25/35/5/6, would you like in timer.c something like:
#ifdef CONFIG_SYS_MX{*}_CLK32
#define CLK_32KHZ CONFIG_SYS_MX{*}_CLK32
#else
#define CLK_32KHZ 32768
#endif

Quoted 4 lines

{*}: 25, 35, 5 or 6
In that way, this definition could be removed from most boards, and that would still allow exceptions and be future-proof.

Yes, this is exactly what I meant.

Best regards,
Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================