[U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Stefano Babic sbabic@denx.de --- .../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 --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/imx-common/timer.c +++ u-boot-4d3c95f/arch/arm/cpu/armv7/imx-common/timer.c @@ -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;
On 14/08/2012 17:01, Benoît Thébaudeau wrote:
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Stefano Babic sbabic@denx.de
Hi Benoît,
.../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 --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/imx-common/timer.c +++ u-boot-4d3c95f/arch/arm/cpu/armv7/imx-common/timer.c @@ -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;
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
Hi Stefano,
On 14/08/2012 17:01, Benoît Thébaudeau wrote:
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Stefano Babic sbabic@denx.de
Hi Benoît,
.../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 --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/imx-common/timer.c +++ u-boot-4d3c95f/arch/arm/cpu/armv7/imx-common/timer.c @@ -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;
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.
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
Indeed.
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
On 17/08/2012 21:52, Benoît Thébaudeau wrote:
Hi Stefano,
On 14/08/2012 17:01, Benoît Thébaudeau wrote:
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Stefano Babic sbabic@denx.de
Hi Benoît,
.../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 --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/imx-common/timer.c +++ u-boot-4d3c95f/arch/arm/cpu/armv7/imx-common/timer.c @@ -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;
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.
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
Indeed.
All boards here use 32768, none of them set it to 32000.
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
Hi Stefano,
On 17/08/2012 21:52, Benoît Thébaudeau wrote:
Hi Stefano,
On 14/08/2012 17:01, Benoît Thébaudeau wrote:
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Stefano Babic sbabic@denx.de
Hi Benoît,
.../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 --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/imx-common/timer.c +++ u-boot-4d3c95f/arch/arm/cpu/armv7/imx-common/timer.c @@ -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;
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.
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
Indeed.
All boards here use 32768, none of them set it to 32000.
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.
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
On 17/08/2012 23:03, Benoît Thébaudeau wrote:
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.
Yes, this is exactly what I meant.
Best regards, Stefano
participants (2)
-
Benoît Thébaudeau -
Stefano Babic