Re: [PATCH v1 5/5] riscv: dts: microchip: add a device tree for Discovery Kit

[Krzysztof Kozlowski <krzk@kernel.org>]

On 25/08/2025 18:19, Valentina Fernandez wrote:
> +++ b/arch/riscv/boot/dts/microchip/mpfs-disco-kit-fabric.dtsi
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/* Copyright (c) 2020-2025 Microchip Technology Inc */
> +
> +/ {
> +	core_pwm0: pwm@40000000 {
> +		compatible = "microchip,corepwm-rtl-v4";
> +		reg = <0x0 0x40000000 0x0 0xF0>;
> +		microchip,sync-update-mask = /bits/ 32 <0>;
> +		#pwm-cells = <3>;
> +		clocks = <&ccc_sw CLK_CCC_PLL0_OUT3>;
> +		status = "disabled";
> +	};
> +
> +	i2c2: i2c@40000200 {
> +		compatible = "microchip,corei2c-rtl-v7";
> +		reg = <0x0 0x40000200 0x0 0x100>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clocks = <&ccc_sw CLK_CCC_PLL0_OUT3>;
> +		interrupt-parent = <&plic>;
> +		interrupts = <122>;
> +		clock-frequency = <100000>;
> +		status = "disabled";
> +	};
> +
> +	ihc: mailbox {
> +		compatible = "microchip,sbi-ipc";
> +		interrupt-parent = <&plic>;
> +		interrupts = <180>, <179>, <178>, <177>;
> +		interrupt-names = "hart-1", "hart-2", "hart-3", "hart-4";
> +		#mbox-cells = <1>;
> +		status = "disabled";
> +	};
> +
> +	mailbox@50000000 {
> +		compatible = "microchip,miv-ihc-rtl-v2";
> +		microchip,ihc-chan-disabled-mask = /bits/ 16 <0>;

Does not look like following DTS coding style - order of properties.

> +		reg = <0x0 0x50000000 0x0 0x1c000>;
> +		interrupt-parent = <&plic>;
> +		interrupts = <180>, <179>, <178>, <177>;
> +		interrupt-names = "hart-1", "hart-2", "hart-3", "hart-4";
> +		#mbox-cells = <1>;
> +		status = "disabled";
> +	};
> +
> +	refclk_ccc: cccrefclk {

Please use name for all fixed clocks which matches current format
recommendation: 'clock-<freq>' (see also the pattern in the binding for
any other options).

https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml

Or anything more reasonable than just bunch of letters.

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;


> +	};
> +};
> +
> +&ccc_sw {
> +	clocks = <&refclk_ccc>, <&refclk_ccc>, <&refclk_ccc>, <&refclk_ccc>,
> +		 <&refclk_ccc>, <&refclk_ccc>;
> +	clock-names = "pll0_ref0", "pll0_ref1", "pll1_ref0", "pll1_ref1",
> +		      "dll0_ref", "dll1_ref";
> +	status = "okay";
> +};
> diff --git a/arch/riscv/boot/dts/microchip/mpfs-disco-kit.dts b/arch/riscv/boot/dts/microchip/mpfs-disco-kit.dts
> new file mode 100644
> index 000000000000..742369470ab0
> --- /dev/null
> +++ b/arch/riscv/boot/dts/microchip/mpfs-disco-kit.dts
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/* Copyright (c) 2020-2025 Microchip Technology Inc */
> +
> +/dts-v1/;
> +
> +#include "mpfs.dtsi"
> +#include "mpfs-disco-kit-fabric.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	model = "Microchip PolarFire-SoC Discovery Kit";
> +	compatible = "microchip,mpfs-disco-kit-reference-rtl-v2507",
> +		     "microchip,mpfs-disco-kit",
> +		     "microchip,mpfs";
> +
> +	aliases {
> +		ethernet0 = &mac0;
> +		serial4 = &mmuart4;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial4:115200n8";
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		led-1 {
> +			gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>;
> +			color = <LED_COLOR_ID_AMBER>;
> +			label = "led1";
> +		};
> +
> +		led-2 {
> +			gpios = <&gpio2 18 GPIO_ACTIVE_HIGH>;
> +			color = <LED_COLOR_ID_RED>;
> +			label = "led2";
> +		};
> +
> +		led-3 {
> +			gpios = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> +			color = <LED_COLOR_ID_AMBER>;
> +			label = "led3";
> +		};
> +
> +		led-4 {
> +			gpios = <&gpio2 20 GPIO_ACTIVE_HIGH>;
> +			color = <LED_COLOR_ID_RED>;
> +			label = "led4";
> +		};
> +
> +		led-5 {
> +			gpios = <&gpio2 21 GPIO_ACTIVE_HIGH>;
> +			color = <LED_COLOR_ID_AMBER>;
> +			label = "led5";
> +		};
> +
> +		led-6 {
> +			gpios = <&gpio2 22 GPIO_ACTIVE_HIGH>;
> +			color = <LED_COLOR_ID_RED>;
> +			label = "led6";
> +		};
> +
> +		led-7 {
> +			gpios = <&gpio2 23 GPIO_ACTIVE_HIGH>;
> +			color = <LED_COLOR_ID_AMBER>;
> +			label = "led7";
> +		};
> +
> +		led-8 {
> +			gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> +			color = <LED_COLOR_ID_RED>;
> +			label = "led8";
> +		};
> +	};
> +
> +	ddrc_cache_lo: memory@80000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x80000000 0x0 0x40000000>;
> +		status = "okay";

Why? Did you disable it anywhere?

> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		hss_payload: region@BFC00000 {

Don't mix cases. Should be lowercase hex everywhere.

Best regards,
Krzysztof