Re: [PATCH v1 7/8] mips: Add EcoNet MIPS platform support

[Krzysztof Kozlowski <krzk@kernel.org>]

On 21/03/2025 14:46, Caleb James DeLisle wrote:
> Add platform support for EcoNet MIPS SoCs.
> 
> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> ---
> As is typical of embedded platforms, it's not realistic to imagine building
> a fully functioning system from kernel sources alone. In the interest of
> providing something without external dependencies, I have included build and
> device tree for a minimal testing / PoC image that can be booted from memory
> on these devices.
> ---
>  arch/mips/Kbuild.platforms                    |  1 +
>  arch/mips/Kconfig                             | 25 ++++++
>  arch/mips/boot/compressed/uart-16550.c        |  5 ++
>  arch/mips/boot/dts/Makefile                   |  1 +
>  arch/mips/boot/dts/econet/Makefile            |  2 +
>  arch/mips/boot/dts/econet/en751221.dtsi       | 62 +++++++++++++++
>  .../boot/dts/econet/en751221_test_image.dts   | 19 +++++
>  arch/mips/econet/Kconfig                      | 42 ++++++++++
>  arch/mips/econet/Makefile                     |  2 +
>  arch/mips/econet/Platform                     |  5 ++
>  arch/mips/econet/init.c                       | 78 +++++++++++++++++++
>  11 files changed, 242 insertions(+)
>  create mode 100644 arch/mips/boot/dts/econet/Makefile
>  create mode 100644 arch/mips/boot/dts/econet/en751221.dtsi
>  create mode 100644 arch/mips/boot/dts/econet/en751221_test_image.dts

DTS are always, always separate patches. See also DTS coding style and
submitting patches in the bindings, which already covers this.



...


> new file mode 100644
> index 000000000000..e4404aed5705
> --- /dev/null
> +++ b/arch/mips/boot/dts/econet/en751221.dtsi
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +
> +/ {
> +	compatible = "econet,en751221";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	hpt_clock: hpt_clock {

No underscores in node names.

Follow DTS coding style.

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <200000000>;  /* 200 MHz */
> +	};
> +
> +	cpus: cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "mips,mips24KEc";
> +			reg = <0>;
> +		};
> +	};
> +
> +	cpuintc: interrupt-controller {
> +		#address-cells = <0>;
> +		#interrupt-cells = <1>;
> +		interrupt-controller;
> +		compatible = "mti,cpu-interrupt-controller";
> +	};
> +
> +	intc: interrupt-controller@1fb40000 {
> +		compatible = "econet,en751221-intc";
> +		reg = <0x1fb40000 0x100>;
> +		interrupt-parent = <&cpuintc>;
> +		interrupts = <2>;
> +
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		econet,shadow-interrupts = <7 2>, <8 3>, <13 12>, <30 29>;
> +	};
> +
> +	uart: serial@1fbf0000 {
> +		compatible = "ns16550";
> +		reg = <0x1fbf0000 0x30>;
> +		reg-io-width = <4>;
> +		reg-shift = <2>;
> +		interrupt-parent = <&intc>;
> +		interrupts = <0>;
> +		clock-frequency = <1843200>;
> +	};
> +
> +	timer_hpt: timer_hpt@1fbf0400 {

Same problem as with binding.

> +		compatible = "econet,timer-hpt";
> +		reg = <0x1fbf0400 0x100>;
> +
> +		interrupt-parent = <&intc>;
> +		interrupts = <30>;
> +		clocks = <&hpt_clock>;
> +	};
> +};
> diff --git a/arch/mips/boot/dts/econet/en751221_test_image.dts b/arch/mips/boot/dts/econet/en751221_test_image.dts
> new file mode 100644
> index 000000000000..bc140c4043b2
> --- /dev/null
> +++ b/arch/mips/boot/dts/econet/en751221_test_image.dts

Does not look like a board. We do not take some testing/debugging
thingies. Please upstream *real* board.

> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +
> +#include "en751221.dtsi"
> +
> +/ {
> +	model = "Generic EN751221";

Missing compatible.

> +
> +	memory@0 {
> +		/* We hope at least 64MB will be available wherever we are run */
> +		device_type = "memory";
> +		reg = <0x00000000 0x4000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200";

Drop bootargs and use standard property - stdout.

See how all other platforms are doing it (and not some ancient MIPS, but
the most recent arm64 or riscv).


> +		linux,usable-memory-range = <0x00020000 0x3fe0000>;
> +	};
> +};
> diff --git a/arch/mips/econet/Kconfig b/arch/mips/econet/Kconfig
> new file mode 100644
> index 000000000000..12f85d638e47
> --- /dev/null
> +++ b/arch/mips/econet/Kconfig
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: GPL-2.0
> +if ECONET
> +
> +config SOC_ECONET_EN751221
> +	bool
> +	select COMMON_CLK
> +	select ECONET_EN751221_INTC
> +	select IRQ_MIPS_CPU
> +	select SMP
> +	select SMP_UP
> +	select SYS_SUPPORTS_SMP
> +
> +choice
> +	prompt "EcoNet SoC selection"
> +	default SOC_ECONET_EN751221_FAMILY
> +	help
> +	  Select EcoNet MIPS SoC type. Individual SoCs within a family are
> +	  similar enough that is it enough to select the right family, and
> +	  then customize to the specific SoC using the device tree only.
> +
> +	config SOC_ECONET_EN751221_FAMILY
> +		bool "EN751221 family"
> +		select SOC_ECONET_EN751221
> +		help
> +		  The EN751221 family includes EN7512, RN7513, EN7521, EN7526.
> +		  They are based on single core MIPS 34Kc processors. To boot
> +		  this kernel, you will need a device tree such as
> +		  MIPS_RAW_APPENDED_DTB=y, and a root filesystem.
> +
> +	config SOC_ECONET_EN751221_TEST_IMAGE
> +		bool "EN751221 test image"
> +		select SOC_ECONET_EN751221
> +		select BUILTIN_DTB
> +		help
> +		  Build a minimal kernel that will boot on any EN751221 board
> +		  with at least 64MB of memory. This has a builtin device tree
> +		  so it can boot with nothing more than an appended initramfs.
> +		  This is good for validating that a given SoC is EN751221
> +		  compatible, or for regression testing.
> +endchoice
> +
> +endif
> diff --git a/arch/mips/econet/Makefile b/arch/mips/econet/Makefile
> new file mode 100644
> index 000000000000..7e4529e7d3d7
> --- /dev/null
> +++ b/arch/mips/econet/Makefile
> @@ -0,0 +1,2 @@
> +
> +obj-y := init.o
> diff --git a/arch/mips/econet/Platform b/arch/mips/econet/Platform
> new file mode 100644
> index 000000000000..bb659876d855
> --- /dev/null
> +++ b/arch/mips/econet/Platform
> @@ -0,0 +1,5 @@
> +# To address a 7.2MB kernel size limit in the EcoNet SDK bootloader,
> +# we put the load address well above where the bootloader loads and then use
> +# zboot. So please set CONFIG_ZBOOT_LOAD_ADDRESS to the address where your
> +# bootloader actually places the kernel.
> +load-$(CONFIG_ECONET)	+= 0xffffffff81000000
> \ No newline at end of file

You have patch warnings

Best regards,
Krzysztof