[U-Boot] [PATCH] mx5/6 timer: Use defined CONFIG_SYS_MX*_CLK32

[Stefano Babic]

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 at advansee.com>
>>> Cc: Stefano Babic <sbabic at 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
=====================================================================