linux-kernel - Re: [PATCH v15 10/10] ARM: dts: Add Sunplus SP7021-Demo-V3 board device tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 12 May 2022 12:28:00 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Qin Jian <qinjian@...lus1.com>, sboyd@...nel.org
Cc:     robh+dt@...nel.org, mturquette@...libre.com, tglx@...utronix.de,
        maz@...nel.org, p.zabel@...gutronix.de, linux@...linux.org.uk,
        arnd@...db.de, linux-arm-kernel@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-clk@...r.kernel.org
Subject: Re: [PATCH v15 10/10] ARM: dts: Add Sunplus SP7021-Demo-V3 board
 device tree

On 12/05/2022 08:31, Qin Jian wrote:
> Add the basic support for Sunplus SP7021-Demo-V3 board.
> 
> Signed-off-by: Qin Jian <qinjian@...lus1.com>
> ---
>  MAINTAINERS                                  |   1 +
>  arch/arm/boot/dts/sunplus-sp7021-achip.dtsi  |  85 +++++
>  arch/arm/boot/dts/sunplus-sp7021-demo-v3.dts |  27 ++
>  arch/arm/boot/dts/sunplus-sp7021.dtsi        | 369 +++++++++++++++++++
>  4 files changed, 482 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sunplus-sp7021-achip.dtsi
>  create mode 100644 arch/arm/boot/dts/sunplus-sp7021-demo-v3.dts
>  create mode 100644 arch/arm/boot/dts/sunplus-sp7021.dtsi
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9cf30e776..b55ec0768 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2747,6 +2747,7 @@ F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
>  F:	Documentation/devicetree/bindings/clock/sunplus,sp7021-clkc.yaml
>  F:	Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
>  F:	Documentation/devicetree/bindings/reset/sunplus,reset.yaml
> +F:	arch/arm/boot/dts/sunplus-sp7021*.dts*
>  F:	arch/arm/configs/sp7021_*defconfig
>  F:	arch/arm/mach-sunplus/
>  F:	drivers/clk/clk-sp7021.c
> diff --git a/arch/arm/boot/dts/sunplus-sp7021-achip.dtsi b/arch/arm/boot/dts/sunplus-sp7021-achip.dtsi
> new file mode 100644
> index 000000000..1560c95d9
> --- /dev/null
> +++ b/arch/arm/boot/dts/sunplus-sp7021-achip.dtsi
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for Sunplus SP7021
> + *
> + * Copyright (C) 2021 Sunplus Technology Co.
> + */
> +
> +#include "sunplus-sp7021.dtsi"
> +
> +/ {
> +	compatible = "sunplus,sp7021-achip";

This does not match your bindings.

> +	model = "Sunplus SP7021 (CA7)";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	interrupt-parent = <&gic>;
> +
> +	clocks {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		extclk: clk@...0 {

This is not a valid device tree. Please run make dtbs_check and compile
dtbs with W=1.

> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <27000000>;
> +			clock-output-names = "extclk";
> +		};
> +
> +		divextclk: clk@0 {

How is it suppose to pass any automated checks if there is no unit address?

> +			compatible = "fixed-factor-clock";
> +			#clock-cells = <0>;
> +			clocks  = <&extclk>;
> +			clock-mult = <1>;
> +			clock-div = <2>;
> +			clock-output-names = "extdivclk";
> +		};
> +
> +		A_pll0: clk@...ll0 {

This is not a valid device tree.

> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <2000000000>;
> +			clock-output-names = "A_pll0";
> +		};
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu0: cpu@0 {
> +			compatible = "arm,cortex-a7";
> +			device_type = "cpu";
> +			reg = <0>;
> +			clock-frequency = <931000000>;
> +		};
> +		cpu1: cpu@1 {
> +			compatible = "arm,cortex-a7";
> +			device_type = "cpu";
> +			reg = <1>;
> +			clock-frequency = <931000000>;
> +		};
> +		cpu2: cpu@2 {
> +			compatible = "arm,cortex-a7";
> +			device_type = "cpu";
> +			reg = <2>;
> +			clock-frequency = <931000000>;
> +		};
> +		cpu3: cpu@3 {
> +			compatible = "arm,cortex-a7";
> +			device_type = "cpu";
> +			reg = <3>;
> +			clock-frequency = <931000000>;
> +		};
> +	};
> +
> +	arm-pmu {
> +		compatible = "arm,cortex-a7-pmu";
> +		interrupts = <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> +	};
> +
> +};
> diff --git a/arch/arm/boot/dts/sunplus-sp7021-demo-v3.dts b/arch/arm/boot/dts/sunplus-sp7021-demo-v3.dts
> new file mode 100644
> index 000000000..05e164115
> --- /dev/null
> +++ b/arch/arm/boot/dts/sunplus-sp7021-demo-v3.dts
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for Sunplus SP7021 Demo V3 SBC board
> + *
> + * Copyright (C) Sunplus Technology Co.
> + */
> +
> +/dts-v1/;
> +
> +#include "sunplus-sp7021-achip.dtsi"
> +
> +/ {
> +	compatible = "sunplus,sp7021-demo-v3";

This does not match your bindings.

Please run make dtbs_check.

> +	model = "SP7021/CA7/Demo_V3";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200 loglevel=8 earlycon";

No bootargs.

I'll stop reviewing. This either does not compile, does not work or does
not pass automated checks. There is no point to use reviewers time if
the tools are doing the same job, so use the tools and then submit DTS.

Best regards,
Krzysztof