[U-Boot] [PATCH v5 01/21] rockchip: add timer driver

[hl <hl@rock-chips.com>]

Hi Ben,

On 12/11/15 10:04, Ben Chan wrote:
> On Tue, Nov 10, 2015 at 2:24 AM, Lin Huang <hl@rock-chips.com> wrote:
>> some rockchip soc will not include lib/timer.c in SPL stage,
>> so implement timer driver for some soc can use us delay function in SPL.
>>
>> Signed-off-by: Lin Huang <hl@rock-chips.com>
>> Acked-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v1: None
>> Changes in v2:
>> - add udelay function
>> Changes in v3:
>> - fix some coding style
>> Changes in v4: None
>> Changes in v5: None
>>
>>   arch/arm/include/asm/arch-rockchip/timer.h | 22 ++++++++++++++
>>   arch/arm/mach-rockchip/Makefile            |  1 +
>>   arch/arm/mach-rockchip/board-spl.c         | 21 ++-----------
>>   arch/arm/mach-rockchip/rk_timer.c          | 48 ++++++++++++++++++++++++++++++
>>   include/configs/rk3288_common.h            |  3 +-
>>   5 files changed, 75 insertions(+), 20 deletions(-)
>>   create mode 100644 arch/arm/include/asm/arch-rockchip/timer.h
>>   create mode 100644 arch/arm/mach-rockchip/rk_timer.c
>>
>> diff --git a/arch/arm/include/asm/arch-rockchip/timer.h b/arch/arm/include/asm/arch-rockchip/timer.h
>> new file mode 100644
>> index 0000000..59444d4
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-rockchip/timer.h
>> @@ -0,0 +1,22 @@
>> +/*
>> + * (C) Copyright 2015 Rockchip Electronics Co., Ltd
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#ifndef __ASM_ARCH_TIMER_H
>> +#define __ASM_ARCH_TIMER_H
>> +
>> +struct rk_timer {
> question: should these use sized integer type?
     you mean use u32, i think unsigned int is the same with u32
>> +       unsigned int timer_load_count0;
> nit: it may be clearer to use _l / _h suffix instead of 0 / 1
     it is correspond to TRM register name,  and  it can easy to find 
register in TRM.
>
>> +       unsigned int timer_load_count1;
>> +       unsigned int timer_curr_value0;
>> +       unsigned int timer_curr_value1;
>> +       unsigned int timer_ctrl_reg;
>> +       unsigned int timer_int_status;
>> +};
>> +
>> +void rockchip_timer_init(void);
>> +void rockchip_udelay(unsigned int us);
>> +
>> +#endif
>> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
>> index 5a4e383..abe03a8 100644
>> --- a/arch/arm/mach-rockchip/Makefile
>> +++ b/arch/arm/mach-rockchip/Makefile
>> @@ -10,4 +10,5 @@ else
>>   obj-y += board.o
>>   endif
>>   obj-y += common.o
>> +obj-y += rk_timer.o
>>   obj-$(CONFIG_ROCKCHIP_RK3288) += rk3288/
>> diff --git a/arch/arm/mach-rockchip/board-spl.c b/arch/arm/mach-rockchip/board-spl.c
>> index 28c3949..0426adf 100644
>> --- a/arch/arm/mach-rockchip/board-spl.c
>> +++ b/arch/arm/mach-rockchip/board-spl.c
>> @@ -18,6 +18,7 @@
>>   #include <asm/arch/hardware.h>
>>   #include <asm/arch/periph.h>
>>   #include <asm/arch/sdram.h>
>> +#include <asm/arch/timer.h>
>>   #include <dm/pinctrl.h>
>>   #include <dm/root.h>
>>   #include <dm/test.h>
>> @@ -110,24 +111,6 @@ static void configure_l2ctlr(void)
>>          write_l2ctlr(l2ctlr);
>>   }
>>
>> -struct rk3288_timer {
>> -       u32 timer_load_count0;
>> -       u32 timer_load_count1;
>> -       u32 timer_curr_value0;
>> -       u32 timer_curr_value1;
>> -       u32 timer_ctrl_reg;
>> -       u32 timer_int_status;
>> -};
>> -
>> -void init_timer(void)
>> -{
>> -       struct rk3288_timer * const timer7_ptr = (void *)TIMER7_BASE;
>> -
>> -       writel(0xffffffff, &timer7_ptr->timer_load_count0);
>> -       writel(0xffffffff, &timer7_ptr->timer_load_count1);
>> -       writel(1, &timer7_ptr->timer_ctrl_reg);
>> -}
>> -
>>   static int configure_emmc(struct udevice *pinctrl)
>>   {
>>          struct gpio_desc desc;
>> @@ -197,7 +180,7 @@ void board_init_f(ulong dummy)
>>                  hang();
>>          }
>>
>> -       init_timer();
>> +       rockchip_timer_init();
>>          configure_l2ctlr();
>>
>>          ret = uclass_get_device(UCLASS_CLK, 0, &dev);
>> diff --git a/arch/arm/mach-rockchip/rk_timer.c b/arch/arm/mach-rockchip/rk_timer.c
>> new file mode 100644
>> index 0000000..ae693c0
>> --- /dev/null
>> +++ b/arch/arm/mach-rockchip/rk_timer.c
>> @@ -0,0 +1,48 @@
>> +/*
>> + * (C) Copyright 2015 Rockchip Electronics Co., Ltd
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <asm/arch/timer.h>
>> +#include <asm/io.h>
>> +#include <common.h>
>> +#include <linux/types.h>
>> +
>> +struct rk_timer * const timer_ptr = (void *)CONFIG_SYS_TIMER_BASE;
>> +
>> +static uint64_t rockchip_get_ticks(void)
>> +{
>> +       uint64_t timebase_h, timebase_l;
>> +
>> +       timebase_l = readl(&timer_ptr->timer_curr_value0);
>> +       timebase_h = readl(&timer_ptr->timer_curr_value1);
>> +
>> +       return timebase_h << 32 | timebase_l;
>> +}
>> +
>> +static unsigned int usec_to_tick(unsigned long usec)
> should probably use 'uint64_t' for 'tick' and the return value,  and
> use 'unsigned int' for 'usec'
     Yes, you are right, will correct it next version.
>
>> +{
>> +       unsigned int tick = usec;
>> +       tick *= CONFIG_SYS_TIMER_RATE / (1000 * 1000);
>> +       return tick;
>> +}
>> +
>> +void rockchip_udelay(unsigned int us)
> nit: perhaps use 'usec' instead of 'us' for consistency with usec_to_tick
     Okay, will modify next version.
>
>> +{
>> +       uint64_t tmp;
>> +
>> +       /* get current timestamp */
> nit: the comment is kind of misleadning as 'tmp' isn't the current timestamp
>
>> +       tmp = rockchip_get_ticks() + usec_to_tick(us);
>> +
>> +       /* loop till event */
>> +       while (rockchip_get_ticks() < tmp+1)
>> +               ;
>> +}
>> +
>> +void rockchip_timer_init(void)
>> +{
>> +       writel(0xffffffff, &timer_ptr->timer_load_count0);
>> +       writel(0xffffffff, &timer_ptr->timer_load_count1);
>> +       writel(1, &timer_ptr->timer_ctrl_reg);
>> +}
>> diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h
>> index 36408b9..cdea6cb 100644
>> --- a/include/configs/rk3288_common.h
>> +++ b/include/configs/rk3288_common.h
>> @@ -23,7 +23,8 @@
>>   #define CONFIG_DISPLAY_BOARDINFO
>>
>>   #define CONFIG_SYS_TIMER_RATE          (24 * 1000 * 1000)
>> -#define CONFIG_SYS_TIMER_COUNTER       (TIMER7_BASE + 8)
>> +#define        CONFIG_SYS_TIMER_BASE           0xff810020 /* TIMER7 */
> nit: the code around it seems to use a space instead of a tab after #define
     oh, sorry, i will correct it next version.
>
>> +#define CONFIG_SYS_TIMER_COUNTER       (CONFIG_SYS_TIMER_BASE + 8)
>>
>>   #define CONFIG_SPL_FRAMEWORK
>>   #define CONFIG_SPL_LIBCOMMON_SUPPORT
>> --
>> 1.9.1
>>
>
>

-- 
Lin Huang