| Message ID | 1485260203-14216-4-git-send-email-al.kochet@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 24, 2017 at 03:16:38PM +0300, Alexander Kochetkov wrote: > The patch add two timers to all rk3188 based boards. > > The first timer is from alive subsystem and it act as a backup > for the local timers at sleep time. It act the same as other > SoC rockchip timers already present in kernel. It is up to the RTC to wake up the system from suspend and there is no idle state stopping the local timers on these SoCs. So, the rockchip timers won't be ever used on the rk3188, right ? > The second timer is from CPU subsystem and act as replacement > for the arm-global-timer clocksource and sched clock. It run > at stable frequency 24MHz. This patch should go after the rockchip timer patches. > Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com> > Reviwed-by: Heiko Stübner <heiko@sntech.de> > --- > arch/arm/boot/dts/rk3188.dtsi | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi > index 869e189..bcf8e03 100644 > --- a/arch/arm/boot/dts/rk3188.dtsi > +++ b/arch/arm/boot/dts/rk3188.dtsi > @@ -106,6 +106,22 @@ > }; > }; > > + timer3: timer@2000e000 { > + compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer"; > + reg = <0x2000e000 0x20>; > + interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cru SCLK_TIMER3>, <&cru PCLK_TIMER3>; > + clock-names = "timer", "pclk"; > + }; > + > + timer6: timer@200380a0 { > + compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer"; > + reg = <0x200380a0 0x20>; > + interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cru SCLK_TIMER6>, <&cru PCLK_TIMER0>; > + clock-names = "timer", "pclk"; > + }; > + > i2s0: i2s@1011a000 { > compatible = "rockchip,rk3188-i2s", "rockchip,rk3066-i2s"; > reg = <0x1011a000 0x2000>; > -- > 1.7.9.5 >
> 30 янв. 2017 г., в 14:04, Daniel Lezcano <daniel.lezcano@linaro.org> написал(а): > > t is up to the RTC to wake up the system from suspend and there is no > idle state stopping the local timers on these SoCs. So, the rockchip timers > won't be ever used on the rk3188, right ? No rockchip timers where used on rk3188 before. Actually, I don’t know how to test backup timer functionality. rk3228 backup timer is from alive subsystem. It is unclear from TRM what this mean, I assume that somehow related to CLK during suspend. So I pick the timer from alive subsystem on rk3188 also. DT timer configuration where tested: interrupts and IO works. > This patch should go after the rockchip timer patches Ok, let me know, when I should send v6. Or may be you change the order of the patches during merge. And thank you for fixing commit message for previous patch. Feel free to tell me than I write something really unclear or incorrect. Alexander.
On Mon, Jan 30, 2017 at 02:20:13PM +0300, Alexander Kochetkov wrote: > > > 30 янв. 2017 г., в 14:04, Daniel Lezcano <daniel.lezcano@linaro.org> написал(а): > > > > t is up to the RTC to wake up the system from suspend and there is no > > idle state stopping the local timers on these SoCs. So, the rockchip timers > > won't be ever used on the rk3188, right ? > > No rockchip timers where used on rk3188 before. > > Actually, I don’t know how to test backup timer functionality. rk3228 backup timer is from alive > subsystem. It is unclear from TRM what this mean, It means it is on an always-on power domain, so it will be always powered even in suspend mode. There are different clockevents on the SoC, the local timers (smp_twd) and the rockchip timer. They are not used together, the local timers will be used as default because they are per cpu. So the rockchip timer is not used in this case. Then there is the situation when the cpu is powered down, the local timers, which belongs to the same power domain, will be powered down also. If there is a timer set for this CPU, an alternate, backup timer must be used, and that is the rockchip timer. However this situation happens: - when a CPU is offlined. But the timers are moved to another CPU. - when the CPU enters a deep idle state. But such state does not exist on rk3188 - when the system goes to suspend. But the timers are stopped in any case and CPU0 is always on. There is no case when the rockchip timer is used for the clockevent. If I'm not wrong, you can check /proc/interrupts and see there is no interrupt on the rockchip timer. So the board is typically a case where we want to use a clocksource from one IP and the clockevents from another IP (just looping back with the clkevt macro patch I answered a few minutes ago). That does not call in question this patch as it describes the hardware. > I assume that somehow related to CLK > during suspend. So I pick the timer from alive subsystem on rk3188 also. > DT timer configuration where tested: interrupts and IO works. > > > This patch should go after the rockchip timer patches > > > Ok, let me know, when I should send v6. Or may be you change the order of the patches > during merge. I have to review the last 3 patches of the series. I will let you know for a V6. Thanks. -- Daniel
> 30 янв. 2017 г., в 15:04, Daniel Lezcano <daniel.lezcano@linaro.org> написал(а): > > There is no case when the rockchip timer is used for the clockevent. The is already timer entry for rk3228 in the DT. And it act as clockevent. I guess it work as backup, but I cannot test it also. In order to not break DT compatibility I had to provide one timer to be initialized as clockevent. And implemented implicit rule the driver (first DT timer - clockevent, second DT timer - clocksource) already exists for other timers. > If I'm not wrong, you can check /proc/interrupts and see there is no interrupt > on the rockchip timer. Yes, you right here. I’ve temporary disable smp_twd to test rockchip timer interrupts. There is no interrupt on the rockchip timer during normal work. > - when the CPU enters a deep idle state. But such state does not exist on rk3188 ARM chip/revision specific? > - when the system goes to suspend. But the timers are stopped in any case and > CPU0 is always on. There is timer for rk3228 in the DT. I guess there are situations where it can be used. May be the situations are also acceptable for rk3188? May be something like suspend to RAM or suspend to HDD? I am not expert in that questions. I can do some tests if you give hint/link. So, as I understood you suggest to leave only one timer what can be used as clocksource only. I can implement that, but there should be DT rule what allow to setup timer as clocksource only. I cannot do more without timer framework support. Looks, like I have to wait your patch to implement that. Alexander.
On Mon, Jan 30, 2017 at 04:13:07PM +0300, Alexander Kochetkov wrote: > > > 30 янв. 2017 г., в 15:04, Daniel Lezcano <daniel.lezcano@linaro.org> написал(а): > > > > There is no case when the rockchip timer is used for the clockevent. > The is already timer entry for rk3228 in the DT. And it act as clockevent. I guess it work as backup, > but I cannot test it also. In order to not break DT compatibility I had to provide one timer to be > initialized as clockevent. And implemented implicit rule the driver (first DT timer - clockevent, > second DT timer - clocksource) already exists for other timers. > > > If I'm not wrong, you can check /proc/interrupts and see there is no interrupt > > on the rockchip timer. > > Yes, you right here. I’ve temporary disable smp_twd to test rockchip timer interrupts. > There is no interrupt on the rockchip timer during normal work. > > > - when the CPU enters a deep idle state. But such state does not exist on rk3188 > ARM chip/revision specific? rk3188 specific. Applies also for rk3288 AFAICT. Without entering in the details, the SoC can't handle that. > > - when the system goes to suspend. But the timers are stopped in any case and > > CPU0 is always on. > There is timer for rk3228 in the DT. I guess there are situations where it can be used. > May be the situations are also acceptable for rk3188? May be something like suspend to RAM or > suspend to HDD? I am not expert in that questions. I can do some tests if you give hint/link. Nope, rockchip clockevents won't be used, smp_twd will instead. You can check by commenting the local-timer in the DT, and you should see the /proc/interrupts line for the rockchip incrementing while the 'twd' are zero. > So, as I understood you suggest to leave only one timer what can be used as clocksource > only. I can implement that, but there should be DT rule what allow to setup timer > as clocksource only. I cannot do more without timer framework support. > Looks, like I have to wait your patch to implement that. No, actually I meant the rockchip clockevents won't be used for both rk3188 and rk3288, they are not needed. It is for information, this patch is ok. -- Daniel
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi index 869e189..bcf8e03 100644 --- a/arch/arm/boot/dts/rk3188.dtsi +++ b/arch/arm/boot/dts/rk3188.dtsi @@ -106,6 +106,22 @@ }; }; + timer3: timer@2000e000 { + compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer"; + reg = <0x2000e000 0x20>; + interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cru SCLK_TIMER3>, <&cru PCLK_TIMER3>; + clock-names = "timer", "pclk"; + }; + + timer6: timer@200380a0 { + compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer"; + reg = <0x200380a0 0x20>; + interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cru SCLK_TIMER6>, <&cru PCLK_TIMER0>; + clock-names = "timer", "pclk"; + }; + i2s0: i2s@1011a000 { compatible = "rockchip,rk3188-i2s", "rockchip,rk3066-i2s"; reg = <0x1011a000 0x2000>;
The patch add two timers to all rk3188 based boards. The first timer is from alive subsystem and it act as a backup for the local timers at sleep time. It act the same as other SoC rockchip timers already present in kernel. The second timer is from CPU subsystem and act as replacement for the arm-global-timer clocksource and sched clock. It run at stable frequency 24MHz. Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com> Reviwed-by: Heiko Stübner <heiko@sntech.de> --- arch/arm/boot/dts/rk3188.dtsi | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)