Hi Marek Vasut,
>>
>> I have attached an updated patch below which hopefully addresses the
>> other issues you highlighted.
>
> Hehe, you should RTFM ;-) http://www.denx.de/wiki/U-Boot/Patches
>
> Generally, use git send-email to send the patch.
>
Thanks for your feedback. I have submitted an updated patch using git
send-email. I think I got the formatting right this time. :)
>>
>> The s3c24x0 timer has been updated to use the global_data struct.
>> Restructured code based on other timer.c files.
>> Updated comments and several parameters.
>>
>> Signed-off-by: Mark Norman <mpnor...@gmail.com>
>> ---
>> arch/arm/cpu/arm920t/s3c24x0/timer.c | 155
>> +++++++++++++-------------------- arch/arm/include/asm/global_data.h |
>> 4 +
>> 2 files changed, 65 insertions(+), 94 deletions(-)
>>
>> diff --git a/arch/arm/cpu/arm920t/s3c24x0/timer.c
>> b/arch/arm/cpu/arm920t/s3c24x0/timer.c
>> index 9571870..5695c62 100644
>> --- a/arch/arm/cpu/arm920t/s3c24x0/timer.c
>> +++ b/arch/arm/cpu/arm920t/s3c24x0/timer.c
>> @@ -35,142 +35,109 @@
>> #include <asm/io.h>
>> #include <asm/arch/s3c24x0_cpu.h>
>>
>> -int timer_load_val = 0;
>> -static ulong timer_clk;
>> +DECLARE_GLOBAL_DATA_PTR;
>>
>> -/* macro to read the 16 bit timer */
>> -static inline ulong READ_TIMER(void)
>> +/* Read the 16 bit timer */
>> +static inline ulong read_timer(void)
>> {
>> struct s3c24x0_timers *timers = s3c24x0_get_base_timers();
>>
>> return readl(&timers->tcnto4) & 0xffff;
>> }
>>
>> -static ulong timestamp;
>> -static ulong lastdec;
>> -
>> int timer_init(void)
>> {
>> + /*
>> + * PWM Timer 4 is used because it has no output.
>> + * Prescaler is hard fixed at 250, divider at 2.
>> + * This generates a Timer clock frequency of 100kHz (@PCLK=50MHz) and
>> + * therefore 10us timer ticks.
>> + */
>> + const ulong prescaler = 250;
>> struct s3c24x0_timers *timers = s3c24x0_get_base_timers();
>> ulong tmr;
>>
>> - /* use PWM Timer 4 because it has no output */
>> - /* prescaler for Timer 4 is 16 */
>> - writel(0x0f00, &timers->tcfg0);
>> - if (timer_load_val == 0) {
>> - /*
>> - * for 10 ms clock period @ PCLK with 4 bit divider = 1/2
>> - * (default) and prescaler = 16. Should be 10390
>> - * @33.25MHz and 15625 @ 50 MHz
>> - */
>> - timer_load_val = get_PCLK() / (2 * 16 * 100);
>> - timer_clk = get_PCLK() / (2 * 16);
>> - }
>> - /* load value for 10 ms timeout */
>> - lastdec = timer_load_val;
>> - writel(timer_load_val, &timers->tcntb4);
>> - /* auto load, manual update of timer 4 */
>> + /* Set prescaler for Timer 4 */
>> + writel((prescaler-1) << 8, &timers->tcfg0);
>
> Can we get rid of the magic numbers please?
I have declared some bitfields to remove the magic numbers.
>
>> +
>> + /* Calculate timer freq, approx 100kHz @ PCLK=50MHz. */
>> + gd->timer_rate_hz = get_PCLK() / (2 * prescaler);
>
> Can get_PCLK() be renamed to be lower-case only?
I haven't addressed this in my latest patch as it would be quite a
large change that I though might be worthy of its own patch (there are
several related functions get_FCLK, get_HCLK, etc. which could be
updated at the same time).
>> diff --git a/arch/arm/include/asm/global_data.h
>> b/arch/arm/include/asm/global_data.h
>> index fac98d5..b836915 100644
>> --- a/arch/arm/include/asm/global_data.h
>> +++ b/arch/arm/include/asm/global_data.h
>> @@ -67,6 +67,10 @@ typedef struct global_data {
>> #ifdef CONFIG_IXP425
>> unsigned long timestamp;
>> #endif
>> +#ifdef CONFIG_S3C24X0
>> + unsigned long lastdec;
>> + unsigned long timestamp;
>> +#endif
>
> I wonder about this change ...
>
The latest patch no longer creates new variables in the global_data
struct and instead reuses some of the existing ones.
Any more feedback would be welcomed.
Cheers
Mark Norman
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
