* [U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32
@ 2012-08-14 15:01 Benoît Thébaudeau
2012-08-17 19:43 ` Stefano Babic
0 siblings, 1 reply; 6+ messages in thread
From: Benoît Thébaudeau @ 2012-08-14 15:01 UTC (permalink / raw)
To: u-boot
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;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32
2012-08-14 15:01 [U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32 Benoît Thébaudeau
@ 2012-08-17 19:43 ` Stefano Babic
2012-08-17 19:52 ` Benoît Thébaudeau
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Babic @ 2012-08-17 19:43 UTC (permalink / raw)
To: u-boot
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
--
=====================================================================
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
=====================================================================
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32
2012-08-17 19:43 ` Stefano Babic
@ 2012-08-17 19:52 ` Benoît Thébaudeau
2012-08-17 20:51 ` Stefano Babic
0 siblings, 1 reply; 6+ messages in thread
From: Benoît Thébaudeau @ 2012-08-17 19:52 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32
2012-08-17 19:52 ` Benoît Thébaudeau
@ 2012-08-17 20:51 ` Stefano Babic
2012-08-17 21:03 ` Benoît Thébaudeau
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Babic @ 2012-08-17 20:51 UTC (permalink / raw)
To: u-boot
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
--
=====================================================================
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
=====================================================================
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32
2012-08-17 20:51 ` Stefano Babic
@ 2012-08-17 21:03 ` Benoît Thébaudeau
2012-08-17 21:09 ` Stefano Babic
0 siblings, 1 reply; 6+ messages in thread
From: Benoît Thébaudeau @ 2012-08-17 21:03 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32
2012-08-17 21:03 ` Benoît Thébaudeau
@ 2012-08-17 21:09 ` Stefano Babic
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Babic @ 2012-08-17 21:09 UTC (permalink / raw)
To: u-boot
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
--
=====================================================================
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
=====================================================================
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-08-17 21:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-14 15:01 [U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32 Benoît Thébaudeau
2012-08-17 19:43 ` Stefano Babic
2012-08-17 19:52 ` Benoît Thébaudeau
2012-08-17 20:51 ` Stefano Babic
2012-08-17 21:03 ` Benoît Thébaudeau
2012-08-17 21:09 ` Stefano Babic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox