On 01/13/2015 12:20 AM, Heiko Stübner wrote:
Hi Daniel, sorry it took a bit longer to reply.
No problem :)
Generally it looks good to me, just some minor issues inline below.
Thanks for the review.
It would also be nice to include the rockchip list (linux- rockchip@xxxxxxxxxxxxxxxxxxx) for future patches.
Yes, sure. I will do that in the future. [...]
+- clock-frequency: the frequency the timer is running + +Example: + timer: timer@ff810000 { + compatible = "rockchip,rk3288-timer"; + reg = <0xff810000 0x20>; + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; + clock-frequency = <24000000>;wouldn't it make sense to use the actual supplying clock? For the timer you want to use it is just the non-gateable &xin24m, but the timers in the other block (timer0-5) actually do have gateable clocks. Similarly there is a pclk_timer supplying at least one of the two actual blocks. I'll try to inquire how the blocks are actually supplied.
Ok, are you suggesting I should use another timer so we can gate it for power efficiency ?
+ }; diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi index 2a878a3..7a7db48 100644 --- a/arch/arm/boot/dts/rk3288.dtsi +++ b/arch/arm/boot/dts/rk3288.dtsi @@ -149,6 +149,13 @@ clock-frequency = <24000000>; }; + timer: timer@ff810000 { + compatible = "rockchip,rk3288-timer"; + reg = <0xff810000 0x20>; + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; + clock-frequency = <24000000>; + }; + sdmmc: dwmmc@ff0c0000 { compatible = "rockchip,rk3288-dw-mshc"; clock-freq-min-max = <400000 150000000>; diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index ac5803c..4c3fa7e 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -11,6 +11,7 @@ config ARCH_ROCKCHIP select HAVE_ARM_SCU if SMP select HAVE_ARM_TWD if SMP select DW_APB_TIMER_OF + select RK3288_TIMER select ARM_GLOBAL_TIMER select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK help diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index fc01ec2..6ed97a6 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -26,6 +26,10 @@ config DW_APB_TIMER_OF select DW_APB_TIMER select CLKSRC_OF +config RK3288_TIMER + bool + select CLKSRC_OF +the config symbol to compile rockchip_timer.c is RK3288_TIMER? I'd think it could match a bit more ... ROCKCHIP_TIMER -> rockchip_timer.c or RK3288_TIMER -> rk3288_timer.c
>
This timer-block is actually also present on the rk3188, but not on the rk3066 which uses an unmodified dw_apb_timer.
Ok, then rockchip_timer.c fits better. I will do the change.
config ARMADA_370_XP_TIMER bool select CLKSRC_OF diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile + + if (of_property_read_u32(np, "clock-frequency", &bc_timer.freq)) {formatting issue with spaces instead of tabs in the 5 lines above
Copy that.
+ pr_err("Failed to read the frequency for '%s'\n", TIMER_NAME); + return; + }
Thanks! -- Daniel -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
