| Message ID | 20240803125510.4699-2-ziyao@disroot.org |
|---|---|
| Headers | show |
| Series | Add initial support for Rockchip RK3528 SoC | expand |
On Sun, Aug 04, 2024 at 07:40:43AM +0200, Dragan Simic wrote: > Hello all, > > On 2024-08-03 14:55, Yao Zi wrote: > > Rockchip RK3528 is a quad-core ARM Cortex-A53 SoC designed for > > multimedia application. This series add a basic device tree with CPU, > > interrupts and UART nodes for it and is able to boot into a kernel with > > only UART console. > > > > Has been tested on Radxa E20C board[1] with vendor U-boot, successfully > > booted into initramfs with this log[2]. > > I wonder will at least the RK3528 datasheet become available publicly? I found none for now, and I am not someone from Rockchip, thus don't know whether they have a plan to make it public, either. But there has been some devices shipping it already and getting them mainlined will be a neat thing. Just FYI, the vendor kernel is available here[1] on the "develop-5.10" branch. Best regards, Yao Zi [1]: https://github.com/rockchip-linux/kernel > > > [1]: https://docs.radxa.com/en/e/e20c > > [2]: https://gist.github.com/ziyao233/b74523a1e3e8bf36286a572e008ca319 > > > > Yao Zi (4): > > dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528 > > dt-bindings: arm: rockchip: Add Radxa E20C board > > arm64: dts: rockchip: Add base DT for rk3528 SoC > > arm64: dts: rockchip: Add Radxa e20c board > > > > .../devicetree/bindings/arm/rockchip.yaml | 5 + > > .../bindings/serial/snps-dw-apb-uart.yaml | 1 + > > arch/arm64/boot/dts/rockchip/Makefile | 1 + > > .../boot/dts/rockchip/rk3528-radxa-e20c.dts | 22 +++ > > arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 ++++++++++++++++++ > > 5 files changed, 211 insertions(+) > > create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts > > create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi > > > > > > base-commit: 94ede2a3e9135764736221c080ac7c0ad993dc2d
> Wiadomość napisana przez Yao Zi <ziyao@disroot.org> w dniu 04.08.2024, o godz. 08:22: > > On Sun, Aug 04, 2024 at 07:40:43AM +0200, Dragan Simic wrote: >> Hello all, >> >> On 2024-08-03 14:55, Yao Zi wrote: >>> Rockchip RK3528 is a quad-core ARM Cortex-A53 SoC designed for >>> multimedia application. This series add a basic device tree with CPU, >>> interrupts and UART nodes for it and is able to boot into a kernel with >>> only UART console. >>> >>> Has been tested on Radxa E20C board[1] with vendor U-boot, successfully >>> booted into initramfs with this log[2]. >> >> I wonder will at least the RK3528 datasheet become available publicly? > > I found none for now, and I am not someone from Rockchip, thus don't > know whether they have a plan to make it public, either. But there has > been some devices shipping it already and getting them mainlined will > be a neat thing. > Maybe this hight be useful: at some point in time I started to hack with rk3528 support in mainline kernel. HW I’m using is vontar rk3528 tvbox. So far - on mainline 6.10 kernel - I got working: clocks, pin control, uart, sdcard, eth, usb. I started to work on dw hdmi and got to stage with correct mode sets by dw-hdmi but I stuck with vop3 support. Generally - I was back porting code from bsp but rockchip bsp is too divergent from mainline to my back porting skills :-( If anybody can help with vop3 - then we can have quite good initial support of rk3528 in mainline (the, sdcard, usb, eth, hdmi) gpu, hdmi audio should be probably easy… My rk3528 enablements: Bindings: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1100-dt-bindings-clock-add-rk3528-clock-definitions.patch Clocks: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1101-clk-rockchip-add-clock-controller-for-the-RK3528.patch Pincontrol: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1102-pinctrl-rockchip-add-rk3528-support.patch Power domain bindings: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1103-dt-bindings-power-add-RK3528-SoCs-header-for-idle.patch Ethernet support: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1104-ethernet-stmmac-dwmac-rk3528-add-GMAC-support.patch Power domains support: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1105-soc-rockchip-power-domain-add-rk3528-support.patch usb: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1106-phy-rockchip-inno-usb2-add-phy-support-for-rk3528.patch SoC power domains: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1108-soc-rockchip-power-domain-add-rk3528-support.patch Thermal: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1108-thermal-rockchip-add-support-for-rk3528.patch Naneng phy: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1109-phy-rockchip-naneng-combphy-add-support-for-rk3528.patch dw hdmi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1110-phy-rockchip-inno-hdmi-add-support-for-rk3528.patch Inno hdmi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1111-drm-rockchip-dw_hdmi-add-support-for-rk3528.patch Nvmem: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1113-nvmem-rockchip-otp-add-support-for-rk3528.patch Sound soc: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1114-sound-soc-codecs-add-rk3528-support.patch rk3528 dtsi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1150-arm64-dtsi-rockchip-add-3528.dtsi.patch: Vontar tvbox dts: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1151-arm64-dts-rockchip-add-dts-for-vontar_r3.patch It may happen that above patches will not apply clean on vanilla 6.10 as I’m patching kernel for s905/s912/sm1/g12/rpi3/rpi4/rpi5/3328/3399//3566/3568/3588/3528 and rk3528 patches are after applied s905/s912/sm1/g12/rpi3/rpi4/rpi5/3328/3399/3566/3568/3588 patches. If anybody is interested in adding vop3 support - pls give me sign - we can work together… > Just FYI, the vendor kernel is available here[1] on the "develop-5.10" > branch. > > Best regards, > Yao Zi > > [1]: https://github.com/rockchip-linux/kernel > >> >>> [1]: https://docs.radxa.com/en/e/e20c >>> [2]: https://gist.github.com/ziyao233/b74523a1e3e8bf36286a572e008ca319 >>> >>> Yao Zi (4): >>> dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528 >>> dt-bindings: arm: rockchip: Add Radxa E20C board >>> arm64: dts: rockchip: Add base DT for rk3528 SoC >>> arm64: dts: rockchip: Add Radxa e20c board >>> >>> .../devicetree/bindings/arm/rockchip.yaml | 5 + >>> .../bindings/serial/snps-dw-apb-uart.yaml | 1 + >>> arch/arm64/boot/dts/rockchip/Makefile | 1 + >>> .../boot/dts/rockchip/rk3528-radxa-e20c.dts | 22 +++ >>> arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 ++++++++++++++++++ >>> 5 files changed, 211 insertions(+) >>> create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts >>> create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi >>> >>> >>> base-commit: 94ede2a3e9135764736221c080ac7c0ad993dc2d > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
On 03/08/2024 14:55, Yao Zi wrote: > Rockchip RK3528 comes with a snps-dw-apb-uart compatible UART. Document > it in dt-bindings. > > Signed-off-by: Yao Zi <ziyao@disroot.org> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- <form letter> This is an automated instruction, just in case, because many review tags are being ignored. If you know the process, you can skip it (please do not feel offended by me posting it here - no bad intentions intended). If you do not know the process, here is a short explanation: Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions, under or above your Signed-off-by tag. Tag is "received", when provided in a message replied to you on the mailing list. Tools like b4 can help here. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for tags received on the version they apply. https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577 </form letter> Best regards, Krzysztof
On 03/08/2024 14:55, Yao Zi wrote: > This initial device tree describes CPU, interrupts and UART on the chip > and is able to boot into basic kernel with only UART. Cache information > is omitted for now as there is no precise documentation. Support for > other features will be added later. > > Signed-off-by: Yao Zi <ziyao@disroot.org> > --- > arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++ > 1 file changed, 182 insertions(+) > create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi > > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi > new file mode 100644 > index 000000000000..77687d9e7e80 > --- /dev/null > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi > @@ -0,0 +1,182 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2022 Rockchip Electronics Co., Ltd. > + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org> > + */ > + > +#include <dt-bindings/interrupt-controller/arm-gic.h> > +#include <dt-bindings/interrupt-controller/irq.h> > + > +/ { > + compatible = "rockchip,rk3528"; > + > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + aliases { > + serial0 = &uart0; > + serial1 = &uart1; > + serial2 = &uart2; > + serial3 = &uart3; > + serial4 = &uart4; > + serial5 = &uart5; > + serial6 = &uart6; > + serial7 = &uart7; > + }; > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu-map { > + cluster0 { > + core0 { > + cpu = <&cpu0>; > + }; > + core1 { > + cpu = <&cpu1>; > + }; > + core2 { > + cpu = <&cpu2>; > + }; > + core3 { > + cpu = <&cpu3>; > + }; > + }; > + }; > + > + cpu0: cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x0>; > + enable-method = "psci"; > + }; > + > + cpu1: cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x1>; > + enable-method = "psci"; > + }; > + > + cpu2: cpu@2 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x2>; > + enable-method = "psci"; > + }; > + > + cpu3: cpu@3 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x3>; > + enable-method = "psci"; > + }; > + }; > + > + psci { > + compatible = "arm,psci-1.0", "arm,psci-0.2"; > + method = "smc"; > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > + }; > + > + xin24m: xin24m { Please use name for all fixed clocks which matches current format recommendation: 'clock-([0-9]+|[a-z0-9-]+)+' https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1 > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <24000000>; > + clock-output-names = "xin24m"; > + }; > + > + gic: interrupt-controller@fed01000 { Why this all is outside of SoC? Best regards, Krzysztof
Hi Krzysztof, Am Sonntag, 4. August 2024, 12:05:11 CEST schrieb Krzysztof Kozlowski: > On 03/08/2024 14:55, Yao Zi wrote: > > This initial device tree describes CPU, interrupts and UART on the chip > > and is able to boot into basic kernel with only UART. Cache information > > is omitted for now as there is no precise documentation. Support for > > other features will be added later. > > > > Signed-off-by: Yao Zi <ziyao@disroot.org> > > --- > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + clock-frequency = <24000000>; > > + clock-output-names = "xin24m"; > > + }; > > + > > + gic: interrupt-controller@fed01000 { > > Why this all is outside of SoC? I guess you mean outside of a "soc {}" node? Here the rk3528 simply follows all other Rockchip SoCs :-) . Digging into the history, the first rk3066a and initial rk3288 submission did use a soc {} node, which later got removed as suggested by arm-soc maintainers at the time [0]. I guess that changed since then? Heiko [0] https://lore.kernel.org/linux-arm-kernel/20140716005528.GA26207@quad.lixom.net/ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c3030d30d9c99c057b5ddfa289cffa637a2775f5
On Sun, Aug 04, 2024 at 01:27:35PM +0200, Diederik de Haas wrote: > On Saturday, 3 August 2024 14:55:10 CEST Yao Zi wrote: > > + gic: interrupt-controller@fed01000 { > > + compatible = "arm,gic-400"; > > + #interrupt-cells = <3>; > > + #address-cells = <0>; > > + interrupt-controller; > > + reg = <0x0 0xfed01000 0 0x1000>, > > + <0x0 0xfed02000 0 0x2000>, > > + <0x0 0xfed04000 0 0x2000>, > > + <0x0 0xfed06000 0 0x2000>; > > + interrupts = <GIC_PPI 9 > > + (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > > + }; > > + > > + uart0: serial@ff9f0000 { > > + compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart"; > > + reg = <0x0 0xff9f0000 0x0 0x100>; > > + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>; > > + reg-shift = <2>; > > + reg-io-width = <4>; > > + clock-frequency = <24000000>; > > + status = "disabled"; > > + }; > > The properties should be sorted as follows: > - compatible > - reg > - <other properties sorted alphabetically> > - status > > This also applies to the other blocks which I didn't quote. Thanks, will be fixed in next revision. Best regards, Yao Zi
On Sun, Aug 04, 2024 at 11:13:59AM +0200, Piotr Oniszczuk wrote: > > > > Wiadomość napisana przez Yao Zi <ziyao@disroot.org> w dniu 04.08.2024, o godz. 08:22: > > > > On Sun, Aug 04, 2024 at 07:40:43AM +0200, Dragan Simic wrote: > >> Hello all, > >> > >> On 2024-08-03 14:55, Yao Zi wrote: > >>> Rockchip RK3528 is a quad-core ARM Cortex-A53 SoC designed for > >>> multimedia application. This series add a basic device tree with CPU, > >>> interrupts and UART nodes for it and is able to boot into a kernel with > >>> only UART console. > >>> > >>> Has been tested on Radxa E20C board[1] with vendor U-boot, successfully > >>> booted into initramfs with this log[2]. > >> > >> I wonder will at least the RK3528 datasheet become available publicly? > > > > I found none for now, and I am not someone from Rockchip, thus don't > > know whether they have a plan to make it public, either. But there has > > been some devices shipping it already and getting them mainlined will > > be a neat thing. > > > > Maybe this hight be useful: at some point in time I started to hack with rk3528 support in mainline kernel. > HW I’m using is vontar rk3528 tvbox. > > So far - on mainline 6.10 kernel - I got working: clocks, pin control, uart, sdcard, eth, usb. > > I started to work on dw hdmi and got to stage with correct mode sets by dw-hdmi but I stuck with vop3 support. > > Generally - I was back porting code from bsp but rockchip bsp is too divergent from mainline to my back porting skills :-( > > If anybody can help with vop3 - then we can have quite good initial support of rk3528 in mainline (the, sdcard, usb, eth, hdmi) > gpu, hdmi audio should be probably easy… > > My rk3528 enablements: > > Bindings: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1100-dt-bindings-clock-add-rk3528-clock-definitions.patch > > Clocks: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1101-clk-rockchip-add-clock-controller-for-the-RK3528.patch > > Pincontrol: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1102-pinctrl-rockchip-add-rk3528-support.patch > > Power domain bindings: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1103-dt-bindings-power-add-RK3528-SoCs-header-for-idle.patch > > Ethernet support: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1104-ethernet-stmmac-dwmac-rk3528-add-GMAC-support.patch > > Power domains support: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1105-soc-rockchip-power-domain-add-rk3528-support.patch > > usb: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1106-phy-rockchip-inno-usb2-add-phy-support-for-rk3528.patch > > SoC power domains: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1108-soc-rockchip-power-domain-add-rk3528-support.patch > > Thermal: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1108-thermal-rockchip-add-support-for-rk3528.patch > > Naneng phy: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1109-phy-rockchip-naneng-combphy-add-support-for-rk3528.patch > > dw hdmi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1110-phy-rockchip-inno-hdmi-add-support-for-rk3528.patch > > Inno hdmi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1111-drm-rockchip-dw_hdmi-add-support-for-rk3528.patch Nvmem: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1113-nvmem-rockchip-otp-add-support-for-rk3528.patch > > Sound soc: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1114-sound-soc-codecs-add-rk3528-support.patch > > rk3528 dtsi: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1150-arm64-dtsi-rockchip-add-3528.dtsi.patch: FYI, This link is dead. > Vontar tvbox dts: https://github.com/warpme/minimyth2/blob/master/script/kernel/linux-6.10/files/1151-arm64-dts-rockchip-add-dts-for-vontar_r3.patch > > It may happen that above patches will not apply clean on vanilla 6.10 as I’m patching kernel for s905/s912/sm1/g12/rpi3/rpi4/rpi5/3328/3399//3566/3568/3588/3528 and rk3528 patches are after applied s905/s912/sm1/g12/rpi3/rpi4/rpi5/3328/3399/3566/3568/3588 patches. > > If anybody is interested in adding vop3 support - pls give me sign - we can work together… > > > Just FYI, the vendor kernel is available here[1] on the "develop-5.10" > > branch. > > > > Best regards, > > Yao Zi > > > > [1]: https://github.com/rockchip-linux/kernel > > > >> > >>> [1]: https://docs.radxa.com/en/e/e20c > >>> [2]: https://gist.github.com/ziyao233/b74523a1e3e8bf36286a572e008ca319 > >>> > >>> Yao Zi (4): > >>> dt-bindings: serial: snps-dw-apb-uart: Document Rockchip RK3528 > >>> dt-bindings: arm: rockchip: Add Radxa E20C board > >>> arm64: dts: rockchip: Add base DT for rk3528 SoC > >>> arm64: dts: rockchip: Add Radxa e20c board > >>> > >>> .../devicetree/bindings/arm/rockchip.yaml | 5 + > >>> .../bindings/serial/snps-dw-apb-uart.yaml | 1 + > >>> arch/arm64/boot/dts/rockchip/Makefile | 1 + > >>> .../boot/dts/rockchip/rk3528-radxa-e20c.dts | 22 +++ > >>> arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 ++++++++++++++++++ > >>> 5 files changed, 211 insertions(+) > >>> create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts > >>> create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi > >>> > >>> > >>> base-commit: 94ede2a3e9135764736221c080ac7c0ad993dc2d > > > > _______________________________________________ > > Linux-rockchip mailing list > > Linux-rockchip@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-rockchip > Best regards, Yao Zi
On Sun, Aug 04, 2024 at 03:25:47PM +0200, Dragan Simic wrote: > On 2024-08-04 15:20, Yao Zi wrote: > > On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote: > > > On 03/08/2024 14:55, Yao Zi wrote: > > > > This initial device tree describes CPU, interrupts and UART on the chip > > > > and is able to boot into basic kernel with only UART. Cache information > > > > is omitted for now as there is no precise documentation. Support for > > > > other features will be added later. > > > > > > > > Signed-off-by: Yao Zi <ziyao@disroot.org> > > > > --- > > > > arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++ > > > > 1 file changed, 182 insertions(+) > > > > create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi > > > > > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi > > > > new file mode 100644 > > > > index 000000000000..77687d9e7e80 > > > > --- /dev/null > > > > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi > > > > @@ -0,0 +1,182 @@ > > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > +/* > > > > + * Copyright (c) 2022 Rockchip Electronics Co., Ltd. > > > > + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org> > > > > + */ > > > > + > > > > +#include <dt-bindings/interrupt-controller/arm-gic.h> > > > > +#include <dt-bindings/interrupt-controller/irq.h> > > > > + > > > > +/ { > > > > + compatible = "rockchip,rk3528"; > > > > + > > > > + interrupt-parent = <&gic>; > > > > + #address-cells = <2>; > > > > + #size-cells = <2>; > > > > + > > > > + aliases { > > > > + serial0 = &uart0; > > > > + serial1 = &uart1; > > > > + serial2 = &uart2; > > > > + serial3 = &uart3; > > > > + serial4 = &uart4; > > > > + serial5 = &uart5; > > > > + serial6 = &uart6; > > > > + serial7 = &uart7; > > > > + }; > > > > + > > > > + cpus { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + cpu-map { > > > > + cluster0 { > > > > + core0 { > > > > + cpu = <&cpu0>; > > > > + }; > > > > + core1 { > > > > + cpu = <&cpu1>; > > > > + }; > > > > + core2 { > > > > + cpu = <&cpu2>; > > > > + }; > > > > + core3 { > > > > + cpu = <&cpu3>; > > > > + }; > > > > + }; > > > > + }; > > > > + > > > > + cpu0: cpu@0 { > > > > + device_type = "cpu"; > > > > + compatible = "arm,cortex-a53"; > > > > + reg = <0x0>; > > > > + enable-method = "psci"; > > > > + }; > > > > + > > > > + cpu1: cpu@1 { > > > > + device_type = "cpu"; > > > > + compatible = "arm,cortex-a53"; > > > > + reg = <0x1>; > > > > + enable-method = "psci"; > > > > + }; > > > > + > > > > + cpu2: cpu@2 { > > > > + device_type = "cpu"; > > > > + compatible = "arm,cortex-a53"; > > > > + reg = <0x2>; > > > > + enable-method = "psci"; > > > > + }; > > > > + > > > > + cpu3: cpu@3 { > > > > + device_type = "cpu"; > > > > + compatible = "arm,cortex-a53"; > > > > + reg = <0x3>; > > > > + enable-method = "psci"; > > > > + }; > > > > + }; > > > > + > > > > + psci { > > > > + compatible = "arm,psci-1.0", "arm,psci-0.2"; > > > > + method = "smc"; > > > > + }; > > > > + > > > > + timer { > > > > + compatible = "arm,armv8-timer"; > > > > + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > > > > + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > > > > + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > > > > + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > > > > + }; > > > > + > > > > + xin24m: xin24m { > > > > > > Please use name for all fixed clocks which matches current format > > > recommendation: 'clock-([0-9]+|[a-z0-9-]+)+' > > > > Will be fixed in next revision. > > Hmm, why should we apply that rule to the xin24m clock, which is > named exactly like that everywhere else in Rockchip SoC dtsi files? > It's much better to remain consistent. Historical reasons should not affect new code. And if we don't follow the new rules now, they may never get followed. Best regards, Yao Zi > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1 > > > > > > > + compatible = "fixed-clock"; > > > > + #clock-cells = <0>; > > > > + clock-frequency = <24000000>; > > > > + clock-output-names = "xin24m"; > > > > + }; > > > > + > > > > + gic: interrupt-controller@fed01000 { > > > > > > Why this all is outside of SoC? > > > > Just as Heiko says, device tree for all other Rockchip SoCs don't have > > a "soc" node. I didn't know why before but just follow the style. > > > > If you prefer add a soc node, I am willing to.
On 04/08/2024 15:20, Yao Zi wrote: >> >>> + compatible = "fixed-clock"; >>> + #clock-cells = <0>; >>> + clock-frequency = <24000000>; >>> + clock-output-names = "xin24m"; >>> + }; >>> + >>> + gic: interrupt-controller@fed01000 { >> >> Why this all is outside of SoC? > > Just as Heiko says, device tree for all other Rockchip SoCs don't have > a "soc" node. I didn't know why before but just follow the style. > > If you prefer add a soc node, I am willing to. Surprising as usually we expect MMIO nodes being part of SoC to be under soc@, but if that's Rockchip preference then fine. Best regards, Krzysztof
Am Sonntag, 4. August 2024, 15:59:19 CEST schrieb Dragan Simic: > On 2024-08-04 15:44, Heiko Stübner wrote: > > Am Sonntag, 4. August 2024, 15:25:47 CEST schrieb Dragan Simic: > >> On 2024-08-04 15:20, Yao Zi wrote: > >> > On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote: > >> >> On 03/08/2024 14:55, Yao Zi wrote: > >> >> > This initial device tree describes CPU, interrupts and UART on the chip > >> >> > and is able to boot into basic kernel with only UART. Cache information > >> >> > is omitted for now as there is no precise documentation. Support for > >> >> > other features will be added later. > >> >> > > >> >> > Signed-off-by: Yao Zi <ziyao@disroot.org> > >> >> > --- > >> >> > arch/arm64/boot/dts/rockchip/rk3528.dtsi | 182 +++++++++++++++++++++++ > >> >> > 1 file changed, 182 insertions(+) > >> >> > create mode 100644 arch/arm64/boot/dts/rockchip/rk3528.dtsi > >> >> > > >> >> > diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi > >> >> > new file mode 100644 > >> >> > index 000000000000..77687d9e7e80 > >> >> > --- /dev/null > >> >> > +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi > >> >> > @@ -0,0 +1,182 @@ > >> >> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > >> >> > +/* > >> >> > + * Copyright (c) 2022 Rockchip Electronics Co., Ltd. > >> >> > + * Copyright (c) 2024 Yao Zi <ziyao@disroot.org> > >> >> > + */ > >> >> > + > >> >> > +#include <dt-bindings/interrupt-controller/arm-gic.h> > >> >> > +#include <dt-bindings/interrupt-controller/irq.h> > >> >> > + > >> >> > +/ { > >> >> > + compatible = "rockchip,rk3528"; > >> >> > + > >> >> > + interrupt-parent = <&gic>; > >> >> > + #address-cells = <2>; > >> >> > + #size-cells = <2>; > >> >> > + > >> >> > + aliases { > >> >> > + serial0 = &uart0; > >> >> > + serial1 = &uart1; > >> >> > + serial2 = &uart2; > >> >> > + serial3 = &uart3; > >> >> > + serial4 = &uart4; > >> >> > + serial5 = &uart5; > >> >> > + serial6 = &uart6; > >> >> > + serial7 = &uart7; > >> >> > + }; > >> >> > + > >> >> > + cpus { > >> >> > + #address-cells = <1>; > >> >> > + #size-cells = <0>; > >> >> > + > >> >> > + cpu-map { > >> >> > + cluster0 { > >> >> > + core0 { > >> >> > + cpu = <&cpu0>; > >> >> > + }; > >> >> > + core1 { > >> >> > + cpu = <&cpu1>; > >> >> > + }; > >> >> > + core2 { > >> >> > + cpu = <&cpu2>; > >> >> > + }; > >> >> > + core3 { > >> >> > + cpu = <&cpu3>; > >> >> > + }; > >> >> > + }; > >> >> > + }; > >> >> > + > >> >> > + cpu0: cpu@0 { > >> >> > + device_type = "cpu"; > >> >> > + compatible = "arm,cortex-a53"; > >> >> > + reg = <0x0>; > >> >> > + enable-method = "psci"; > >> >> > + }; > >> >> > + > >> >> > + cpu1: cpu@1 { > >> >> > + device_type = "cpu"; > >> >> > + compatible = "arm,cortex-a53"; > >> >> > + reg = <0x1>; > >> >> > + enable-method = "psci"; > >> >> > + }; > >> >> > + > >> >> > + cpu2: cpu@2 { > >> >> > + device_type = "cpu"; > >> >> > + compatible = "arm,cortex-a53"; > >> >> > + reg = <0x2>; > >> >> > + enable-method = "psci"; > >> >> > + }; > >> >> > + > >> >> > + cpu3: cpu@3 { > >> >> > + device_type = "cpu"; > >> >> > + compatible = "arm,cortex-a53"; > >> >> > + reg = <0x3>; > >> >> > + enable-method = "psci"; > >> >> > + }; > >> >> > + }; > >> >> > + > >> >> > + psci { > >> >> > + compatible = "arm,psci-1.0", "arm,psci-0.2"; > >> >> > + method = "smc"; > >> >> > + }; > >> >> > + > >> >> > + timer { > >> >> > + compatible = "arm,armv8-timer"; > >> >> > + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > >> >> > + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > >> >> > + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > >> >> > + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > >> >> > + }; > >> >> > + > >> >> > + xin24m: xin24m { > >> >> > >> >> Please use name for all fixed clocks which matches current format > >> >> recommendation: 'clock-([0-9]+|[a-z0-9-]+)+' > >> > > >> > Will be fixed in next revision. > >> > >> Hmm, why should we apply that rule to the xin24m clock, which is > >> named exactly like that everywhere else in Rockchip SoC dtsi files? > >> It's much better to remain consistent. > > > > bindings or how we write devicetrees evolve over time ... similarly the > > xin24m name comes from more than 10 years ago. > > > > We also name all those regulator nodes regulator-foo now, which in turn > > automatically does enforce a nice sorting rule to keep all the > > regulators > > around the same area ;-) > > > > So I don't see a problem of going with xin24m: clock-xin24m {} > > I agree that using "clock-xin24m" makes more sense in general, but the > trouble is that we can't rename the already existing instances of > "xin24m", > because that has become part of the ABI. Thus, I'm not sure that > breaking > away from the legacy brings benefits in this particular case. In the regulator case, we have _new_ boards using the new _node_-names but I don't see any renaming of old boards and also don't think we should. But that still does not keep us from using the nicer naming convention in new boards ;-) . Same with xin24m. We're talking only about the node-name here. The phandle stays the same and also the actual clock name stays the same and really only the actual node name you need to look for in /proc/device-tree changes ;-) . So I don't see the need to go about changing all the old socs, but new additions should use improved naming conventions. xin24m: clock-xin24m { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <24000000>; clock-output-names = "xin24m"; }; Heiko > >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml?h=v6.11-rc1 > >> >> > >> >> > + compatible = "fixed-clock"; > >> >> > + #clock-cells = <0>; > >> >> > + clock-frequency = <24000000>; > >> >> > + clock-output-names = "xin24m"; > >> >> > + }; > >> >> > + > >> >> > + gic: interrupt-controller@fed01000 { > >> >> > >> >> Why this all is outside of SoC? > >> > > >> > Just as Heiko says, device tree for all other Rockchip SoCs don't have > >> > a "soc" node. I didn't know why before but just follow the style. > >> > > >> > If you prefer add a soc node, I am willing to. > >> > > > > > > > > > > > > _______________________________________________ > > Linux-rockchip mailing list > > Linux-rockchip@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-rockchip >
Hello Yao, On 2024-08-05 18:22, Yao Zi wrote: > On Mon, Aug 05, 2024 at 01:47:45PM +0200, Heiko Stübner wrote: >> Am Montag, 5. August 2024, 13:37:11 CEST schrieb Dragan Simic: >> > On 2024-08-05 12:59, Yao Zi wrote: >> > > On Sun, Aug 04, 2024 at 04:05:24PM +0200, Krzysztof Kozlowski wrote: >> > >> On 04/08/2024 15:20, Yao Zi wrote: >> > >> >> >> > >> >>> + compatible = "fixed-clock"; >> > >> >>> + #clock-cells = <0>; >> > >> >>> + clock-frequency = <24000000>; >> > >> >>> + clock-output-names = "xin24m"; >> > >> >>> + }; >> > >> >>> + >> > >> >>> + gic: interrupt-controller@fed01000 { >> > >> >> >> > >> >> Why this all is outside of SoC? >> > >> > >> > >> > Just as Heiko says, device tree for all other Rockchip SoCs don't have >> > >> > a "soc" node. I didn't know why before but just follow the style. >> > >> > >> > >> > If you prefer add a soc node, I am willing to. >> > >> >> > >> Surprising as usually we expect MMIO nodes being part of SoC to be >> > >> under >> > >> soc@, but if that's Rockchip preference then fine. >> > >> >> > > >> > > Okay, then I would leave it as is. >> > > >> > > For the fixed-clock node, I think "xin24m: clock-24m { }" is okay and >> > > follows the new rule? >> > >> > I find "xin24m: clock-xin24m { }" better, because keeping the "xin24m" >> > part in /sys listing(s), for example, can only be helpful. >> >> I would second that :-) . Like on a number of boards we have for >> example >> 125MHz gmac clock generators ... with 2 gmacs, there are 2 of them. >> >> I'm not sure the preferred name accounts for that? >> >> Similarly we also keep the naming in the regulator node, >> it's regulator-vcc3v3-somename {} instead of just regulator-3v3 {}. > > "clock-xin24m" wouldn't be more descriptive than "clock-24m" and there > are usually only a few fixed clocks in dt, thus finding corresponding > definition isn't a problem I think. Well, using "clock-xin24m" comes with another benefit, which is using the same "xin24m" as in the actual clock name. That way, the same clock name gets used in various /sys listings and in the debug clock summary in /sys. Having that kind of consistency can only be beneficial. > For the gmac case, Krzysztof, do you think something like > "clock-125m-gmac1" is acceptable, just like what has been done for > regulators?
Am Samstag, 3. August 2024, 14:55:08 CEST schrieb Yao Zi: > Rockchip RK3528 comes with a snps-dw-apb-uart compatible UART. Document > it in dt-bindings. > > Signed-off-by: Yao Zi <ziyao@disroot.org> Reviewed-by: Heiko Stuebner <heiko@sntech.de> > --- > Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml > index 4cdb0dcaccf3..4573044be189 100644 > --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml > +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml > @@ -48,6 +48,7 @@ properties: > - rockchip,rk3328-uart > - rockchip,rk3368-uart > - rockchip,rk3399-uart > + - rockchip,rk3528-uart > - rockchip,rk3568-uart > - rockchip,rk3588-uart > - rockchip,rv1108-uart >
On 04/08/2024 16:05, Krzysztof Kozlowski wrote: > On 04/08/2024 15:20, Yao Zi wrote: >>> >>>> + compatible = "fixed-clock"; >>>> + #clock-cells = <0>; >>>> + clock-frequency = <24000000>; >>>> + clock-output-names = "xin24m"; >>>> + }; >>>> + >>>> + gic: interrupt-controller@fed01000 { >>> >>> Why this all is outside of SoC? >> >> Just as Heiko says, device tree for all other Rockchip SoCs don't have >> a "soc" node. I didn't know why before but just follow the style. >> >> If you prefer add a soc node, I am willing to. > > Surprising as usually we expect MMIO nodes being part of SoC to be under > soc@, but if that's Rockchip preference then fine. One more comment, I forgot we actually have it documented long time ago: https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L90 Best regards, Krzysztof
Am Dienstag, 13. August 2024, 18:38:58 CEST schrieb Krzysztof Kozlowski: > On 04/08/2024 16:05, Krzysztof Kozlowski wrote: > > On 04/08/2024 15:20, Yao Zi wrote: > >>> > >>>> + compatible = "fixed-clock"; > >>>> + #clock-cells = <0>; > >>>> + clock-frequency = <24000000>; > >>>> + clock-output-names = "xin24m"; > >>>> + }; > >>>> + > >>>> + gic: interrupt-controller@fed01000 { > >>> > >>> Why this all is outside of SoC? > >> > >> Just as Heiko says, device tree for all other Rockchip SoCs don't have > >> a "soc" node. I didn't know why before but just follow the style. > >> > >> If you prefer add a soc node, I am willing to. > > > > Surprising as usually we expect MMIO nodes being part of SoC to be under > > soc@, but if that's Rockchip preference then fine. > > One more comment, I forgot we actually have it documented long time ago: > > https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L90 Thanks for finding that block. I guess we'll follow that advice then and go with a soc node :-) . Heiko
Am Dienstag, 13. August 2024, 18:38:58 CEST schrieb Krzysztof Kozlowski: > On 04/08/2024 16:05, Krzysztof Kozlowski wrote: > > On 04/08/2024 15:20, Yao Zi wrote: > >>> > >>>> + compatible = "fixed-clock"; > >>>> + #clock-cells = <0>; > >>>> + clock-frequency = <24000000>; > >>>> + clock-output-names = "xin24m"; > >>>> + }; > >>>> + > >>>> + gic: interrupt-controller@fed01000 { > >>> > >>> Why this all is outside of SoC? > >> > >> Just as Heiko says, device tree for all other Rockchip SoCs don't have > >> a "soc" node. I didn't know why before but just follow the style. > >> > >> If you prefer add a soc node, I am willing to. > > > > Surprising as usually we expect MMIO nodes being part of SoC to be under > > soc@, but if that's Rockchip preference then fine. > > One more comment, I forgot we actually have it documented long time ago: > > https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L90 I guess that piece of documentation should move to the dts style guide though? Because it's not about writing bindings but how to structure a dts/dtsi.
On 15/08/2024 18:44, Heiko Stübner wrote: >> >> One more comment, I forgot we actually have it documented long time ago: >> >> https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L90 > > I guess that piece of documentation should move to the dts style > guide though? Because it's not about writing bindings but how > to structure a dts/dtsi. Yes, it should. Best regards, Krzysztof