Re: [PATCH 6/6] mips: dts: realtek: Add RTL9302C board

[Krzysztof Kozlowski <krzk@kernel.org>]

On 21/06/2024 06:27, Chris Packham wrote:
> Add support for the RTL930x SoC and the RTL9302C reference board.
> 
> The RTL930x family of SoCs are Realtek switches with an embedded MIPS
> core (800MHz 34Kc). Most of the peripherals are similar to the RTL838x
> SoC and can make use of many existing drivers.
> 
> Add in full DSA switch support is still a work in progress.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  arch/mips/boot/dts/realtek/Makefile     |  1 +
>  arch/mips/boot/dts/realtek/RTL9302C.dts | 74 +++++++++++++++++++++++
>  arch/mips/boot/dts/realtek/rtl930x.dtsi | 78 +++++++++++++++++++++++++
>  3 files changed, 153 insertions(+)
>  create mode 100644 arch/mips/boot/dts/realtek/RTL9302C.dts
>  create mode 100644 arch/mips/boot/dts/realtek/rtl930x.dtsi
> 
> diff --git a/arch/mips/boot/dts/realtek/Makefile b/arch/mips/boot/dts/realtek/Makefile
> index fba4e93187a6..54dc2d280cd5 100644
> --- a/arch/mips/boot/dts/realtek/Makefile
> +++ b/arch/mips/boot/dts/realtek/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
>  dtb-y	+= cisco_sg220-26.dtb
> +dtb-y	+= RTL9302C.dtb
> diff --git a/arch/mips/boot/dts/realtek/RTL9302C.dts b/arch/mips/boot/dts/realtek/RTL9302C.dts
> new file mode 100644
> index 000000000000..d921067d5006
> --- /dev/null
> +++ b/arch/mips/boot/dts/realtek/RTL9302C.dts
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/dts-v1/;
> +
> +#include "rtl930x.dtsi"
> +
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/thermal/thermal.h>
> +
> +/ {
> +	compatible = "realtek,RTL9302C", "realtek,rtl930x-soc";

Really, this wasn't ever tested.

> +	model = "RTL9302C Development Board";
> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x0 0x8000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "earlycon";

Drop. earlycon is debugging tool, not a wide-mainline usage configuration.

> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +
> +&uart0 {
> +	status = "okay";
> +};
> +
> +&spi0 {
> +	status = "okay";
> +	flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <10000000>;
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			partition@0 {
> +				label = "u-boot";
> +				reg = <0x0 0xe0000>;
> +				read-only;
> +			};
> +			partition@e0000 {
> +				label = "u-boot-env";
> +				reg = <0xe0000 0x10000>;
> +			};
> +			partition@f0000 {
> +				label = "u-boot-env2";
> +				reg = <0xf0000 0x10000>;
> +				read-only;
> +			};
> +			partition@100000 {
> +				label = "jffs";
> +				reg = <0x100000 0x100000>;
> +			};
> +			partition@200000 {
> +				label = "jffs2";
> +				reg = <0x200000 0x100000>;
> +			};
> +			partition@300000 {
> +				label = "runtime";
> +				reg = <0x300000 0xe80000>;
> +			};
> +			partition@1180000 {
> +				label = "runtime2";
> +				reg = <0x1180000 0xe80000>;
> +			};
> +		};
> +	};
> +};
> diff --git a/arch/mips/boot/dts/realtek/rtl930x.dtsi b/arch/mips/boot/dts/realtek/rtl930x.dtsi
> new file mode 100644
> index 000000000000..5e088c90d2ee
> --- /dev/null
> +++ b/arch/mips/boot/dts/realtek/rtl930x.dtsi
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR BSD-2-Clause
> +
> +#include "rtl83xx.dtsi"
> +
> +/ {
> +	compatible = "realtek,rtl930x-soc";
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			compatible = "mips,mips34Kc";
> +			reg = <0>;
> +			clocks = <&baseclk 0>;
> +			clock-names = "cpu";
> +		};
> +	};
> +
> +	baseclk: baseclk {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <800000000>;
> +	};
> +
> +	lx_clk: lx_clk {

No underscors in node names.

Use recommended clock names, see:
Documentation/devicetree/bindings/clock/fixed-clock.yaml


> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency  = <175000000>;
> +	};
> +};
> +
> +&soc {
> +	intc: interrupt-controller@3000 {
> +		compatible = "realtek,rtl9300-intc", "realtek,rtl-intc";
> +		reg = <0x3000 0x18>, <0x3018 0x18>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +
> +		interrupt-parent = <&cpuintc>;
> +		interrupts = <2>, <3>, <4>, <5>, <6>, <7>;
> +	};
> +
> +	spi0: spi@1200 {
> +		compatible = "realtek,rtl8380-spi";
> +		reg = <0x1200 0x100>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> +
> +	timer0: timer@3200 {
> +		compatible = "realtek,rtl930x-timer", "realtek,otto-timer";
> +		reg = <0x3200 0x10>, <0x3210 0x10>, <0x3220 0x10>,
> +		    <0x3230 0x10>, <0x3240 0x10>;
> +
> +		interrupt-parent = <&intc>;
> +		interrupts = <7 4>, <8 4>, <9 4>, <10 4>, <11 4>;

Are you open-coding IRQ flags?

> +		clocks = <&lx_clk>;
> +	};
> +};
> +
> +&uart0 {
> +	/delete-property/ clock-frequency;
> +	clocks = <&lx_clk>;
> +
> +	interrupt-parent = <&intc>;
> +	interrupts = <30 1>;

Are you open-coding IRQ flags?


> +};
> +
> +&uart1 {
> +	/delete-property/ clock-frequency;
> +	clocks = <&lx_clk>;
> +
> +	interrupt-parent = <&intc>;
> +	interrupts = <31 0>;

Are you open-coding IRQ flags?

> +};
> +

Best regards,
Krzysztof