| Message ID | 20190107032810.13522-4-josephl@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add CPUidle support for Tegra210 | expand |
On 07/01/2019 03:28, Joseph Lo wrote: > Fix timer node to make it work with Tegra210 timer driver. And backward > compatible with the Tegra watchdog driver. > > Signed-off-by: Joseph Lo <josephl@nvidia.com> > --- > arch/arm64/boot/dts/nvidia/tegra210.dtsi | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi > index b5858b5ea052..143bd103c923 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi > +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi > @@ -384,14 +384,12 @@ > }; > > timer@60005000 { > - compatible = "nvidia,tegra210-timer", "nvidia,tegra20-timer"; > + compatible = "nvidia,tegra210-timer", "nvidia,tegra30-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 122 IRQ_TYPE_LEVEL_HIGH>; > + interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&tegra_car TEGRA210_CLK_TIMER>; > clock-names = "timer"; > }; Hmmm ... I can't say I understand this. So now the timer devices are compatible with two different drivers? Begs the question why did we not just extend the existing tegra20-timer rather than adding a new one? Also you did not explain why we make them compatible with the watchdog driver? We have other watchdogs timer and so why do we need to make them compatible? Cheers Jon
On 1/24/19 7:16 PM, Jon Hunter wrote: > > On 07/01/2019 03:28, Joseph Lo wrote: >> Fix timer node to make it work with Tegra210 timer driver. And backward >> compatible with the Tegra watchdog driver. >> >> Signed-off-by: Joseph Lo <josephl@nvidia.com> >> --- >> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 12 +++++------- >> 1 file changed, 5 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi >> index b5858b5ea052..143bd103c923 100644 >> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi >> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi >> @@ -384,14 +384,12 @@ >> }; >> >> timer@60005000 { >> - compatible = "nvidia,tegra210-timer", "nvidia,tegra20-timer"; >> + compatible = "nvidia,tegra210-timer", "nvidia,tegra30-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 122 IRQ_TYPE_LEVEL_HIGH>; >> + interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH>; >> clocks = <&tegra_car TEGRA210_CLK_TIMER>; >> clock-names = "timer"; >> }; > > Hmmm ... I can't say I understand this. So now the timer devices are > compatible with two different drivers? The tegra210-timer is for clock event device and the tegra30-timer is for watchdog timer, that is the usage for Tegra platform before Tegra210. But... (see below) > > Begs the question why did we not just extend the existing tegra20-timer > rather than adding a new one? The tegra20-timer was designed for very early stage that the arm-twd driver not ready yet. So it was the clock event device and sched timer for tegra20/30 at that stage. And I believe this driver was replaced and deprecated by arm-twd and armv7 timer for tegra20/30/114/124. So basically, I think we can remove this driver now. So back to the question above, as we see the similar timer node binding in tegra20/30/114/124/132 dts file. It actually didn't use tegra20 timer driver at all but instead of arm-twd and armv7 timer. The binding only makes it work with the tegra-wdt driver, which is the compatible string of "nvidia,tegra30-timer". Maybe we should fix them all together. > > Also you did not explain why we make them compatible with the watchdog > driver? We have other watchdogs timer and so why do we need to make them > compatible? > The detail explanation is above. In here, I just follow the legacy binding convention that we used in other tegra dts files. This binding makes it work with TMR5, which is the watchdog timer that we used to use for Tegra. If we don't prefer this way, I can remove that for now. And we need to find a different way to merge watchdog timer driver and timer driver into one driver. Let me know which one is acceptable for now. Thanks, Joseph
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi index b5858b5ea052..143bd103c923 100644 --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi @@ -384,14 +384,12 @@ }; timer@60005000 { - compatible = "nvidia,tegra210-timer", "nvidia,tegra20-timer"; + compatible = "nvidia,tegra210-timer", "nvidia,tegra30-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 122 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH>; clocks = <&tegra_car TEGRA210_CLK_TIMER>; clock-names = "timer"; };
Fix timer node to make it work with Tegra210 timer driver. And backward compatible with the Tegra watchdog driver. Signed-off-by: Joseph Lo <josephl@nvidia.com> --- arch/arm64/boot/dts/nvidia/tegra210.dtsi | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)