| Message ID | 20230418095344.274025-4-cristian.ciocaltea@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable rk3588 timer support | expand |
On 4/18/23 11:53, Cristian Ciocaltea wrote: > Add DT node for Rockchip RK3588/RK3588S SoC timer. > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > index 657c019d27fa..acd89a55374a 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > @@ -1400,6 +1400,14 @@ i2c5: i2c@fead0000 { > status = "disabled"; > }; > > + rktimer: timer@feae0000 { There are multiple timers. Use a label in line with the TRM. Maybe change your label to "timer0" in that trend? > + compatible = "rockchip,rk3588-timer", "rockchip,rk3288-timer"; > + reg = <0x0 0xfeae0000 0x0 0x20>; > + clocks = <&cru PCLK_BUSTIMER0>, <&cru CLK_BUSTIMER0>; > + clock-names = "pclk", "timer"; > + interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH 0>; Heiko's sort rules: compatible reg interrupts [alphabetical] status [if needed] > + }; > + > wdt: watchdog@feaf0000 { > compatible = "rockchip,rk3588-wdt", "snps,dw-wdt"; > reg = <0x0 0xfeaf0000 0x0 0x100>;
On 4/18/23 14:29, Johan Jonker wrote: > > > On 4/18/23 11:53, Cristian Ciocaltea wrote: >> Add DT node for Rockchip RK3588/RK3588S SoC timer. >> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi >> index 657c019d27fa..acd89a55374a 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi >> @@ -1400,6 +1400,14 @@ i2c5: i2c@fead0000 { >> status = "disabled"; >> }; >> > >> + rktimer: timer@feae0000 { > > There are multiple timers. > Use a label in line with the TRM. > Maybe change your label to "timer0" in that trend? Sure, will use "timer0". >> + compatible = "rockchip,rk3588-timer", "rockchip,rk3288-timer"; >> + reg = <0x0 0xfeae0000 0x0 0x20>; > >> + clocks = <&cru PCLK_BUSTIMER0>, <&cru CLK_BUSTIMER0>; >> + clock-names = "pclk", "timer"; >> + interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH 0>; > > Heiko's sort rules: > > compatible > reg > interrupts > [alphabetical] > status [if needed] Thanks for pointing this out! The sort rule was not obvious as there are many other nodes that don't conform.
Am Dienstag, 18. April 2023, 13:53:23 CEST schrieb Cristian Ciocaltea: > > On 4/18/23 14:29, Johan Jonker wrote: > > > > > > On 4/18/23 11:53, Cristian Ciocaltea wrote: > >> Add DT node for Rockchip RK3588/RK3588S SoC timer. > >> > >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > >> --- > >> arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 8 ++++++++ > >> 1 file changed, 8 insertions(+) > >> > >> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > >> index 657c019d27fa..acd89a55374a 100644 > >> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > >> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > >> @@ -1400,6 +1400,14 @@ i2c5: i2c@fead0000 { > >> status = "disabled"; > >> }; > >> > > > >> + rktimer: timer@feae0000 { > > > > There are multiple timers. > > Use a label in line with the TRM. > > Maybe change your label to "timer0" in that trend? > > Sure, will use "timer0". > > >> + compatible = "rockchip,rk3588-timer", "rockchip,rk3288-timer"; > >> + reg = <0x0 0xfeae0000 0x0 0x20>; > > > >> + clocks = <&cru PCLK_BUSTIMER0>, <&cru CLK_BUSTIMER0>; > >> + clock-names = "pclk", "timer"; > >> + interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH 0>; > > > > Heiko's sort rules: > > > > compatible > > reg > > interrupts > > [alphabetical] > > status [if needed] > > Thanks for pointing this out! The sort rule was not obvious as there are > many other nodes that don't conform. hmm, that is probably an oversight then :-) . Though looking through rk3588s.dtsi and rk3588.dtsi, it looks like most peripherals follow that sorting quite nicely. But there is also a bit of leeway ... aka there can be an argument made that assigned-clocks might want to live near clocks or regulator-properties could be sorted somewhat differently. So general ideal is alphabetically for the random properties to give some guidance on sorting. And compatible, regs, interrupts at the top + status at the bottom makes it way easier to establish a reading pattern, when looking for something in the file :-) . Heiko
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi index 657c019d27fa..acd89a55374a 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi @@ -1400,6 +1400,14 @@ i2c5: i2c@fead0000 { status = "disabled"; }; + rktimer: timer@feae0000 { + compatible = "rockchip,rk3588-timer", "rockchip,rk3288-timer"; + reg = <0x0 0xfeae0000 0x0 0x20>; + clocks = <&cru PCLK_BUSTIMER0>, <&cru CLK_BUSTIMER0>; + clock-names = "pclk", "timer"; + interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH 0>; + }; + wdt: watchdog@feaf0000 { compatible = "rockchip,rk3588-wdt", "snps,dw-wdt"; reg = <0x0 0xfeaf0000 0x0 0x100>;
Add DT node for Rockchip RK3588/RK3588S SoC timer. Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> --- arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)