On 12/05/2022 08:31, Qin Jian wrote:
> Add the basic support for Sunplus SP7021-Demo-V3 board.
>
> Signed-off-by: Qin Jian <[email protected]>
> ---
> 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@osc0 {
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@A_pll0 {
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
> > 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.
>
> > +++ 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.
I did passed the make dtbs_check.
compatible string: "sunplus,sp7021", "sunplus,sp7021-achip", "sunplus,sp7021-demo-v3"
all defined @ Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml [1]
[1] https://lore.kernel.org/lkml/6107a68008f2d71d2c7868d4d94cb66c5b5fc134.1652329411.git.qinjian@cqplus1.com/
On 13/05/2022 09:44, qinjian[覃健] wrote:
>>> 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.
>>
>
>>> +++ 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.
>
> I did passed the make dtbs_check.
> compatible string: "sunplus,sp7021", "sunplus,sp7021-achip", "sunplus,sp7021-demo-v3"
> all defined @ Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml [1]
How this can pass the check if it is entirely different compatible and
does not match schema? The code is not correct. If you test your DTS
with dtbs_check you will see:
sunplus-sp7021-demo-v3.dtb: /: compatible: ['sunplus,sp7021-demo-v3']
is too short
Additionally:
1. Your DTBs do not compile, missing Makefile entry.
2. You have to fix whitespace issues in your patches:
.git/rebase-apply/patch:48: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
.git/rebase-apply/patch:97: new blank line at EOF.
+
warning: 1
Best regards,
Krzysztof