Messages in this thread | | | Subject | Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support | From | Joseph Lo <> | Date | Sun, 3 Feb 2019 00:04:44 +0800 |
| |
On 2/2/19 9:30 PM, Dmitry Osipenko wrote: > 01.02.2019 18:37, Joseph Lo пишет: >> On 2/1/19 11:13 PM, Dmitry Osipenko wrote: >>> 01.02.2019 17:13, Joseph Lo пишет: >>>> On 2/1/19 9:54 PM, Jon Hunter wrote: >>>>> >>>>> On 01/02/2019 13:11, Dmitry Osipenko wrote: >>>>>> 01.02.2019 16:06, Dmitry Osipenko пишет: >>>>>>> 01.02.2019 6:36, Joseph Lo пишет: >>>>>>>> Add support for the Tegra210 timer that runs at oscillator clock >>>>>>>> (TMR10-TMR13). We need these timers to work as clock event device and to >>>>>>>> replace the ARMv8 architected timer due to it can't survive across the >>>>>>>> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up >>>>>>>> source when CPU suspends in power down state. >>>>>>>> >>>>>>>> Also convert the original driver to use timer-of API. >>>>>>>> >>>>>>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org> >>>>>>>> Cc: Thomas Gleixner <tglx@linutronix.de> >>>>>>>> Cc: linux-kernel@vger.kernel.org >>>>>>>> Signed-off-by: Joseph Lo <josephl@nvidia.com> >>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com> >>>>>>>> --- >> snip. >>>>>>>> +} >>>>>>>> +TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_timer_init); >>>>>>>> +#else /* CONFIG_ARM */ >>>>>>>> +static int __init tegra20_init_timer(struct device_node *np) >>>>>>>> +{ >>>>>>> What about T132? Isn't it ARM64 which uses tegra20-timer IP? At least T132 DT suggests so and seems this change will break it. >>>>>>> >>>>>>> [snip] >>>>>>> >>>>>> >>>>>> Ah, noticed the "depends on ARM" in Kconfig.. Seems okay then. >>>>>> >>>>> >>>>> >>>>> This is a good point, because even though we had 'depends on ARM', this >>>>> still means that the Tegra132 DT is incorrect. >>>>> >>>>> Joseph, can you take a quick look at Tegra132? >>>> >>>> Hi Jon and Dmitry, >>>> >>>> No worry about T132, T132 uses arch timer (v7). The tegra20 timer driver has never been used. We should fix the dtsi file later. >>> >>> Hi Joseph, >>> >>> So is T132 HW actually incompatible with the tegra20-timer? If it's compatible, then I think the driver's code should be made more universal to support T132. >>> >> >> From HW point of view, the TIMER1 ~ TIMER4 is compatible with "nvidia,tegra20-timer". But Tegra132 actually has 10 timers which are exactly the same as Tegra30. So it should backward compatible with "nvidia,tegra30-timer", which is tegra_wdt driver now. And Tegra132 should never use this driver. > > Then shouldn't device tree look like this? Why TMR7-TMR0 are not defined there? Yeah, they need to revisit and fix. > > timer@60005000 { > compatible = "nvidia,tegra124-timer", "nvidia,tegra30-timer", "nvidia,tegra20-timer"; > reg = <0x0 0x60005000 0x0 0x400>; > interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>; > <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>; > <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>; > <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&tegra_car TEGRA124_CLK_TIMER>; > clock-names = "timer"; > }; > > TMR 0,6,7,8,9 should define a shared interrupt as well, but seems the shared interrupt provider is not supported in upstream. > > Also note that seems T124/132 device tree has a typo (I'm looking at TK1 TRM), TMR6 IRQ is 152 and not 122. > > And T30 device tree looks incorrect, TRM says that TMR1-TMR5 have a "dedicated interrupt bit", but not TMR6. > Yeah, noticed that as well. Because the wdt driver doesn't need IRQ support and they (Tegra114/124/132) use arch timer. So everything just works fine.
Thanks, Joseph
| |