Add a minimal device tree for the Microchip PolarFire SoC Discovery Kit.
The Discovery Kit is a cost-optimized board based on PolarFire SoC
MPFS095T and features:
- 1 GB DDR4x16
- 1x Gigabit Ethernet
- 3x UARTs
- Raspberry Pi connector
- mikroBus connector
- microSD card connector
Link: https://www.microchip.com/en-us/development-tool/mpfs-disco-kit
Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com>
---
arch/riscv/boot/dts/microchip/Makefile | 1 +
.../dts/microchip/mpfs-disco-kit-fabric.dtsi | 58 ++++++
.../boot/dts/microchip/mpfs-disco-kit.dts | 191 ++++++++++++++++++
3 files changed, 250 insertions(+)
create mode 100644 arch/riscv/boot/dts/microchip/mpfs-disco-kit-fabric.dtsi
create mode 100644 arch/riscv/boot/dts/microchip/mpfs-disco-kit.dts
diff --git a/arch/riscv/boot/dts/microchip/Makefile b/arch/riscv/boot/dts/microchip/Makefile
index 1e2f4e41bf0d..345ed7a48cc1 100644
--- a/arch/riscv/boot/dts/microchip/Makefile
+++ b/arch/riscv/boot/dts/microchip/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-beaglev-fire.dtb
+dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-disco-kit.dtb
dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-icicle-kit.dtb
dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-icicle-kit-prod.dtb
dtb-$(CONFIG_ARCH_MICROCHIP_POLARFIRE) += mpfs-m100pfsevp.dtb
diff --git a/arch/riscv/boot/dts/microchip/mpfs-disco-kit-fabric.dtsi b/arch/riscv/boot/dts/microchip/mpfs-disco-kit-fabric.dtsi
new file mode 100644
index 000000000000..f9b94b5ead96
--- /dev/null
+++ 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>;
+ 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 {
+ 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";
+ };
+
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ hss_payload: region@BFC00000 {
+ reg = <0x0 0xBFC00000 0x0 0x400000>;
+ no-map;
+ };
+ };
+};
+
+&core_pwm0 {
+ status = "okay";
+};
+
+&gpio1 {
+ interrupts = <27>, <28>, <29>, <30>,
+ <31>, <32>, <33>, <47>,
+ <35>, <36>, <37>, <38>,
+ <39>, <40>, <41>, <42>,
+ <43>, <44>, <45>, <46>,
+ <47>, <48>, <49>, <50>;
+ status = "okay";
+};
+
+&gpio2 {
+ interrupts = <53>, <53>, <53>, <53>,
+ <53>, <53>, <53>, <53>,
+ <53>, <53>, <53>, <53>,
+ <53>, <53>, <53>, <53>,
+ <53>, <53>, <53>, <53>,
+ <53>, <53>, <53>, <53>,
+ <53>, <53>, <53>, <53>,
+ <53>, <53>, <53>, <53>;
+ status = "okay";
+};
+
+&i2c0 {
+ status = "okay";
+};
+
+&i2c2 {
+ status = "okay";
+};
+
+&ihc {
+ status = "okay";
+};
+
+&mac0 {
+ phy-mode = "sgmii";
+ phy-handle = <&phy0>;
+ status = "okay";
+
+ phy0: ethernet-phy@b {
+ reg = <0xb>;
+ };
+};
+
+&mbox {
+ status = "okay";
+};
+
+&mmc {
+ bus-width = <4>;
+ disable-wp;
+ cap-sd-highspeed;
+ cap-mmc-highspeed;
+ sd-uhs-sdr12;
+ sd-uhs-sdr25;
+ sd-uhs-sdr50;
+ sd-uhs-sdr104;
+ no-1-8-v;
+ status = "okay";
+};
+
+&mmuart1 {
+ status = "okay";
+};
+
+&mmuart4 {
+ status = "okay";
+};
+
+&refclk {
+ clock-frequency = <125000000>;
+};
+
+&refclk_ccc {
+ clock-frequency = <50000000>;
+};
+
+&rtc {
+ status = "okay";
+};
+
+&spi0 {
+ status = "okay";
+};
+
+&spi1 {
+ status = "okay";
+};
+
+&syscontroller {
+ status = "okay";
+};
--
2.34.1
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
On 28/08/2025 18:46, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > 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 The fabric dtsi describes elements configured by the FPGA bitstream. This node is named as such because the Clock Conditioner Circuit CCC's reference clock source is set by the FPGA bitstream, while its frequency is determined by an on-board oscillator. Hope this clarifies the rationale behind the node name. Thanks, Valentina > > 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
On 01/09/2025 17:28, Valentina.FernandezAlanis@microchip.com wrote: > On 28/08/2025 18:46, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> 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 > The fabric dtsi describes elements configured by the FPGA bitstream. > This node is named as such because the Clock Conditioner Circuit CCC's > reference clock source is set by the FPGA bitstream, while its frequency > is determined by an on-board oscillator. > > Hope this clarifies the rationale behind the node name. No, because there is no style naming clocks like this. Neither proper suffix, nor prefix. Use standard naming. And all other comments you ignored? Best regards, Krzysztof
On 02/09/2025 07:22, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 01/09/2025 17:28, Valentina.FernandezAlanis@microchip.com wrote: >> On 28/08/2025 18:46, Krzysztof Kozlowski wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> 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 >> The fabric dtsi describes elements configured by the FPGA bitstream. >> This node is named as such because the Clock Conditioner Circuit CCC's >> reference clock source is set by the FPGA bitstream, while its frequency >> is determined by an on-board oscillator. >> >> Hope this clarifies the rationale behind the node name. > No, because there is no style naming clocks like this. Neither proper > suffix, nor prefix. Use standard naming. > > And all other comments you ignored? I sent a v2 with the rest of the comments addressed. I didn't notice you were still not happy with the clock node name, please ignore the v2. > > Best regards, > Krzysztof
On Tue, Sep 02, 2025 at 08:22:02AM +0200, Krzysztof Kozlowski wrote: > >>> + 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 > > The fabric dtsi describes elements configured by the FPGA bitstream. > > This node is named as such because the Clock Conditioner Circuit CCC's > > reference clock source is set by the FPGA bitstream, while its frequency > > is determined by an on-board oscillator. > > > > Hope this clarifies the rationale behind the node name. > No, because there is no style naming clocks like this. Neither proper > suffix, nor prefix. Use standard naming. So you want all fixed frequency clocks to be named "clk-foo" when "clk-<freq>" is not suitable? Fine if you do, but I didn't realise that it was required and haven't been keeping an eye out for it.
On 02/09/2025 10:31, Conor Dooley wrote: > On Tue, Sep 02, 2025 at 08:22:02AM +0200, Krzysztof Kozlowski wrote: > >>>>> + 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 >>> The fabric dtsi describes elements configured by the FPGA bitstream. >>> This node is named as such because the Clock Conditioner Circuit CCC's >>> reference clock source is set by the FPGA bitstream, while its frequency >>> is determined by an on-board oscillator. >>> >>> Hope this clarifies the rationale behind the node name. >> No, because there is no style naming clocks like this. Neither proper >> suffix, nor prefix. Use standard naming. > > So you want all fixed frequency clocks to be named "clk-foo" when > "clk-<freq>" is not suitable? Fine if you do, but I didn't realise that > it was required and haven't been keeping an eye out for it. Recommended is to just use consistent suffixes or prefixes. Binding asks for "clock-" so that's what I propose to use here. Best regards, Krzysztof
On Tue, Sep 02, 2025 at 03:47:56PM +0200, Krzysztof Kozlowski wrote: > On 02/09/2025 10:31, Conor Dooley wrote: > > On Tue, Sep 02, 2025 at 08:22:02AM +0200, Krzysztof Kozlowski wrote: > > > >>>>> + 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 > >>> The fabric dtsi describes elements configured by the FPGA bitstream. > >>> This node is named as such because the Clock Conditioner Circuit CCC's > >>> reference clock source is set by the FPGA bitstream, while its frequency > >>> is determined by an on-board oscillator. > >>> > >>> Hope this clarifies the rationale behind the node name. > >> No, because there is no style naming clocks like this. Neither proper > >> suffix, nor prefix. Use standard naming. > > > > So you want all fixed frequency clocks to be named "clk-foo" when > > "clk-<freq>" is not suitable? Fine if you do, but I didn't realise that > > it was required and haven't been keeping an eye out for it. > > Recommended is to just use consistent suffixes or prefixes. Binding asks > for "clock-" so that's what I propose to use here. Okay, I'll keep that in mind.
© 2016 - 2025 Red Hat, Inc.