On 11/04/2022 19:28, William Zhang wrote:
> Add dts for ARMv7 based broadband SoC BCM47622. bcm47622.dtsi is the
> SoC description dts header and bcm947622.dts is a simple dts file for
> Broadcom BCM947622 Reference board that only enable the UART port.
>
Thank you for your patch. There is something to discuss/improve.
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> + compatible = "brcm,bcm47622";
This does not match your bindings. Even if it is not used.
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + interrupt-parent = <&gic>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + CA7_0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a7";
> + reg = <0x0>;
> + next-level-cache = <&L2_0>;
> + enable-method = "psci";
> + };
> +
> + CA7_1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a7";
> + reg = <0x1>;
> + next-level-cache = <&L2_0>;
> + enable-method = "psci";
> + };
> + CA7_2: cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a7";
> + reg = <0x2>;
> + next-level-cache = <&L2_0>;
> + enable-method = "psci";
> + };
> + CA7_3: cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a7";
> + reg = <0x3>;
> + next-level-cache = <&L2_0>;
> + enable-method = "psci";
> + };
> + L2_0: l2-cache0 {
> + compatible = "cache";
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv7-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)>;
> + arm,cpu-registers-not-fw-configured;
> + };
> +
> + pmu: pmu {
> + compatible = "arm,cortex-a7-pmu";
> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-affinity = <&CA7_0>, <&CA7_1>,
> + <&CA7_2>, <&CA7_3>;
> + };
> +
> + clocks: clocks {
> + periph_clk: periph_clk {
No underscores in node names.
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <200000000>;
> + };
> + uart_clk: uart_clk {
> + compatible = "fixed-factor-clock";
> + #clock-cells = <0>;
> + clocks = <&periph_clk>;
> + clock-div = <4>;
> + clock-mult = <1>;
> + };
> + };
> +
> + psci {
> + compatible = "arm,psci-0.2";
> + method = "smc";
> + cpu_off = <1>;
> + cpu_on = <2>;
> + };
> +
> + axi@81000000 {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x81000000 0x818000>;
> +
> + gic: interrupt-controller@1000 {
> + compatible = "arm,cortex-a7-gic";
> + #interrupt-cells = <3>;
> + #address-cells = <0>;
> + interrupt-controller;
> + reg = <0x1000 0x1000>,
> + <0x2000 0x2000>;
> + };
> + };
> +
> + periph-bus@ff800000 {
just "bus" to be generic?
Best regards,
Krzysztof
