[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <03ac0a67-bd1f-12ca-74f7-8d5b05857ea7@linaro.org>
Date: Thu, 12 May 2022 16:10:48 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Jacky Huang <ychuang3@...oton.com>, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, ychuang570808@...il.com
Cc: robh+dt@...nel.org, sboyd@...nel.org, krzk+dt@...nel.org,
arnd@...db.de, olof@...om.net, catalin.marinas@....com,
will@...nel.org, soc@...nel.org, cfli0@...oton.com
Subject: Re: [PATCH V4 3/5] arm64: dts: nuvoton: Add initial support for
MA35D1
On 10/05/2022 05:25, Jacky Huang wrote:
> Add the initial device tree files for Nuvoton MA35D1 Soc.
>
> Signed-off-by: Jacky Huang <ychuang3@...oton.com>
> ---
> arch/arm64/boot/dts/Makefile | 1 +
> arch/arm64/boot/dts/nuvoton/Makefile | 2 +
> arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts | 24 +++++
> arch/arm64/boot/dts/nuvoton/ma35d1.dtsi | 120 +++++++++++++++++++++
> 4 files changed, 147 insertions(+)
> create mode 100644 arch/arm64/boot/dts/nuvoton/Makefile
> create mode 100644 arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts
> create mode 100644 arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
>
> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> index 1ba04e31a438..7b107fa7414b 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -19,6 +19,7 @@ subdir-y += lg
> subdir-y += marvell
> subdir-y += mediatek
> subdir-y += microchip
> +subdir-y += nuvoton
> subdir-y += nvidia
> subdir-y += qcom
> subdir-y += realtek
> diff --git a/arch/arm64/boot/dts/nuvoton/Makefile b/arch/arm64/boot/dts/nuvoton/Makefile
> new file mode 100644
> index 000000000000..e1e0c466bf5e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nuvoton/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_NUVOTON) += ma35d1-evb.dtb
> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts
> new file mode 100644
> index 000000000000..95f0facb0476
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Device Tree Source for MA35D1 Evaluation Board (EVB)
> + *
> + * Copyright (C) 2022 Nuvoton Technology Corp.
> + */
> +
> +/dts-v1/;
> +#include "ma35d1.dtsi"
> +
> +/ {
> + model = "Nuvoton MA35D1-EVB";
> + compatible = "nuvoton,ma35d1-evb", "nuvoton,ma35d1";
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + memory@...00000 {
> + device_type = "memory";
> + reg = <0x0 0x80000000 0x0 0x10000000>;
> + };
> +};
> +
.git/rebase-apply/patch:60: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> new file mode 100644
> index 000000000000..7212f8de6906
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Copyright (c) 2022 Nuvoton Technology Corp.
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> +
> +/ {
> + compatible = "nuvoton,ma35d1";
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&cpu0>;
> + };
> + core1 {
> + cpu = <&cpu1>;
> + };
> + };
> + };
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a35";
> + reg = <0x0>;
> + enable-method = "psci";
> + next-level-cache = <&L2_0>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a35";
> + reg = <0x1>;
> + enable-method = "psci";
> + next-level-cache = <&L2_0>;
> + };
> +
> + L2_0: l2-cache0 {
> + compatible = "cache";
> + cache-level = <2>;
> + };
> + };
> +
> + psci {
> + compatible = "arm,psci-0.2";
> + method = "smc";
> + };
> +
> + hxt_24m: hxt_24mhz {
No underscores in node name. Generic node names, so "clock-X" or
"clock-some-suffix"
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <24000000>;
This does not look like property of SoC. Where is this clock defined? In
the SoC or on the board?
> + clock-output-names = "HXT_24MHz";
> + };
> +
> + 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)>;
> + clock-frequency = <12000000>;
> + };
> +
> + sys: system-controller@...60000 {
> + compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd";
Why is this a simple-mfd if there are no children here? What do you want
to instantiate here?
Where is the nuvoton,ma35d1-sys compatible documented?
> + reg = <0x0 0x40460000 0x0 0x400>;
> + };
> +
> + reset: reset-controller {
> + compatible = "nuvoton,ma35d1-reset";
Also not documented.
> + nuvoton,ma35d1-sys = <&sys>;
> + #reset-cells = <1>;
> + };
> +
> + clk: clock-controller@...60200 {
> + compatible = "nuvoton,ma35d1-clk";
> + reg = <0x0 0x40460200 0x0 0x100>;
> + #clock-cells = <1>;
> + clocks = <&hxt_24m>;
> + clock-names = "HXT_24MHz";
Please test your DTS with make dtbs_check.
Don't send DTS which does not pass the checks. It is unnecessary use of
reviewers time when the same job can be done by automated tools.
Best regards,
Krzysztof
Powered by blists - more mailing lists