Re: [PATCH v2 09/11] arm64: allwinner: A100: add the basical Allwinner A100 DTSI file
From: Maxime Ripard <hidden>
Date: 2020-06-29 18:51:52
Also in:
linux-arm-kernel, linux-clk, linux-gpio, linux-pm, lkml
Hi, On Mon, Jun 22, 2020 at 10:59:05AM +0800, Frank Lee wrote:
quoted hunk ↗ jump to hunk
Allwinner A100 is a new SoC with Cortex-A53 cores, this commit adds the basical DTSI file of it, including the clock, i2c, pins, sid, ths, and UART support. Signed-off-by: Frank Lee <redacted> --- arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi | 337 +++++++++++++++++++++++++ 1 file changed, 337 insertions(+) create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a100.dtsidiff --git a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi new file mode 100644 index 0000000..5133897 --- /dev/null +++ b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi@@ -0,0 +1,337 @@ +// SPDX-License-Identifier: (GPL-2.0+ or MIT) +/* + * Copyright (c) 2020 Frank Lee <frank@allwinnertech.com> + */ + +#include <dt-bindings/interrupt-controller/arm-gic.h> +#include <dt-bindings/clock/sun50i-a100-ccu.h> +#include <dt-bindings/clock/sun50i-a100-r-ccu.h> +#include <dt-bindings/reset/sun50i-a100-ccu.h> +#include <dt-bindings/reset/sun50i-a100-r-ccu.h> + +/ { + interrupt-parent = <&gic>; + #address-cells = <2>; + #size-cells = <2>; + + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu0: cpu@0 { + compatible = "arm,armv8"; + device_type = "cpu"; + reg = <0x0>; + enable-method = "psci"; + }; + + cpu@1 { + compatible = "arm,armv8"; + device_type = "cpu"; + reg = <0x1>; + enable-method = "psci"; + }; + + cpu@2 { + compatible = "arm,armv8"; + device_type = "cpu"; + reg = <0x2>; + enable-method = "psci"; + }; + + cpu@3 { + compatible = "arm,armv8"; + device_type = "cpu"; + reg = <0x3>; + enable-method = "psci"; + }; + }; + + psci { + compatible = "arm,psci-1.0"; + method = "smc"; + }; + + iosc: internal-osc-clk { + compatible = "fixed-clock"; + clock-frequency = <16000000>; + clock-accuracy = <300000000>; + clock-output-names = "iosc"; + #clock-cells = <0>; + }; + + dcxo24M: dcxo24M_clk {
You shouldn't have underscores in the node names.
+ compatible = "fixed-clock";
+ clock-frequency = <24000000>;
+ clock-output-names = "dcxo24M";
+ #clock-cells = <0>;
+ };
+
+ osc32k: osc32k_clk {Same thing here Also, ordering by node name here would be nice
+ compatible = "fixed-clock";
+ clock-frequency = <32768>;
+ clock-output-names = "osc32k";
+ #clock-cells = <0>;
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupts = <GIC_PPI 13
+ (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+ <GIC_PPI 14
+ (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+ <GIC_PPI 11
+ (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+ <GIC_PPI 10
+ (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
+ };Does it suffer from the same time instability than the one in the A64? If so, you probably want to enable the workaround too.
+ soc: soc {Do you really need a label for that node?
+ compatible = "simple-bus"; + #address-cells = <2>; + #size-cells = <2>;
Why do you need cells with 2 items here? You don't seem to be using them.
+ r_i2c0: i2c@7081400 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x0 0x07081400 0x0 0x400>;
+ interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2C0>;
+ resets = <&ccu RST_BUS_I2C0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&r_i2c0_pins>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ r_i2c1: i2c@7081800 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x0 0x07081800 0x0 0x400>;
+ interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2C1>;
+ resets = <&ccu RST_BUS_I2C1>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&r_i2c1_pins>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };The clocks and resets phandles and IDs don't look right
+ thermal-zones {
+ cpu_thermal_zone {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+ thermal-sensors = <&ths 0>;
+ };Please add a new line here
+ gpu_thermal_zone{You should have a space here ^
+ polling-delay-passive = <0>; + polling-delay = <0>; + thermal-sensors = <&ths 1>; + };
newline
+ ddr_thermal_zone{space ^ Thanks! Maxime
Attachments
- signature.asc [application/pgp-signature] 228 bytes