[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bb5b0d71-41b1-48a0-82fc-bdb362cc3db1@microchip.com>
Date: Mon, 1 Sep 2025 15:28:22 +0000
From: <Valentina.FernandezAlanis@...rochip.com>
To: <krzk@...nel.org>, <Conor.Dooley@...rochip.com>,
<Daire.McNamara@...rochip.com>, <paul.walmsley@...ive.com>,
<palmer@...belt.com>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<aou@...s.berkeley.edu>, <alex@...ti.fr>,
<Valentina.FernandezAlanis@...rochip.com>
CC: <linux-riscv@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<devicetree@...r.kernel.org>
Subject: Re: [PATCH v1 5/5] riscv: dts: microchip: add a device tree for
Discovery Kit
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@...00000 {
>> + 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@...00200 {
>> + 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@...00000 {
>> + 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@...00000 {
>> + 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@...00000 {
>
> Don't mix cases. Should be lowercase hex everywhere.
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists