On 04/08/2024 17:51, Heiko Stübner wrote: > 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@xxxxxxxxxxx> >>>>>>> --- >>>>>>> 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@xxxxxxxxxxx> >>>>>>> + */ >>>>>>> + >>>>>>> +#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"; Just to make it clear - doc clearly says the preferred name is: "clock-frequency". Best regards, Krzysztof