Re: [PATCH v2 15/17] riscv: dts: microchip: refactor icicle kit device tree

On 17/12/2021 15:04, Krzysztof Kozlowski wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On 17/12/2021 10:33, conor.dooley@microchip.com wrote:

quoted

From: Conor Dooley <conor.dooley@microchip.com>

Assorted minor changes to the MPFS/Icicle kit device tree:

- rename serial to mmuart to match microchip documentation
- enable mmuart4 instead of mmuart0

This is not refactoring. Refactoring could include renames,
hierarchy/layout differences, naming, coding convention. You are
changing features, e.g. using different UART. Please split the changes.

will do :)

quoted

- move stdout path to serial1 to avoid collision with
       bootloader running on the e51
- split memory node to match updated fpga design
- move phy0 inside mac1 node to match phy configuration
- add labels where missing (cpus, cache controller)
- add missing address cells & interrupts to MACs

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
  .../microchip/microchip-mpfs-icicle-kit.dts   | 52 ++++++++------
  .../boot/dts/microchip/microchip-mpfs.dtsi    | 70 ++++++++++---------
  2 files changed, 68 insertions(+), 54 deletions(-)

diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
index 174f977c164b..f6542ef76046 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts

@@ -1,5 +1,5 @@
  // SPDX-License-Identifier: (GPL-2.0 OR MIT)
-/* Copyright (c) 2020 Microchip Technology Inc */
+/* Copyright (c) 2020-2021 Microchip Technology Inc */

  /dts-v1/;

@@ -13,25 +13,34 @@ / {
       compatible = "microchip,mpfs-icicle-kit", "microchip,mpfs";

       aliases {
-             ethernet0 = &emac1;
-             serial0 = &serial0;
-             serial1 = &serial1;
-             serial2 = &serial2;
-             serial3 = &serial3;
+             ethernet0 = &mac1;
+             serial0 = &mmuart0;
+             serial1 = &mmuart1;
+             serial2 = &mmuart2;
+             serial3 = &mmuart3;
+             serial4 = &mmuart4;
       };

       chosen {
-             stdout-path = "serial0:115200n8";
+             stdout-path = "serial1:115200n8";
       };

       cpus {
               timebase-frequency = <RTCCLK_FREQ>;
       };

-     memory@80000000 {
+     ddrc_cache_lo: memory@80000000 {
               device_type = "memory";
-             reg = <0x0 0x80000000 0x0 0x40000000>;
+             reg = <0x0 0x80000000 0x0 0x2e000000>;
               clocks = <&clkcfg CLK_DDRC>;
+             status = "okay";
+     };
+
+     ddrc_cache_hi: memory@1000000000 {

This looks unrelated to refactoring - split of memory - and needs
separate change.

quoted

+             device_type = "memory";
+             reg = <0x10 0x0 0x0 0x40000000>;
+             clocks = <&clkcfg CLK_DDRC>;
+             status = "okay";
       };
  };

@@ -39,19 +48,19 @@ &refclk {
       clock-frequency = <600000000>;
  };

-&serial0 {
+&mmuart1 {
       status = "okay";
  };

-&serial1 {
+&mmuart2 {
       status = "okay";
  };

-&serial2 {
+&mmuart3 {
       status = "okay";
  };

-&serial3 {
+&mmuart4 {
       status = "okay";
  };

@@ -61,29 +70,32 @@ &mmc {
       bus-width = <4>;
       disable-wp;
       cap-sd-highspeed;
+     cap-mmc-highspeed;
       card-detect-delay = <200>;
+     mmc-ddr-1_8v;
+     mmc-hs200-1_8v;

This looks unrelated to refactoring - new modes for MMC - and needs
separate change.

quoted

       sd-uhs-sdr12;
       sd-uhs-sdr25;
       sd-uhs-sdr50;
       sd-uhs-sdr104;
  };

-&emac0 {
+&mac0 {
       phy-mode = "sgmii";
       phy-handle = <&phy0>;
-     phy0: ethernet-phy@8 {
-             reg = <8>;
-             ti,fifo-depth = <0x01>;
-     };
  };

-&emac1 {
+&mac1 {
       status = "okay";
       phy-mode = "sgmii";
       phy-handle = <&phy1>;
       phy1: ethernet-phy@9 {
               reg = <9>;
-             ti,fifo-depth = <0x01>;
+             ti,fifo-depth = <0x1>;
+     };
+     phy0: ethernet-phy@8 {
+             reg = <8>;
+             ti,fifo-depth = <0x1>;
       };
  };

diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
index 808500be26c3..d311c5ea27c9 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi

@@ -1,5 +1,5 @@
  // SPDX-License-Identifier: (GPL-2.0 OR MIT)
-/* Copyright (c) 2020 Microchip Technology Inc */
+/* Copyright (c) 2020-2021 Microchip Technology Inc */

  /dts-v1/;
  #include "dt-bindings/clock/microchip,mpfs-clock.h"

@@ -16,7 +16,7 @@ cpus {
               #address-cells = <1>;
               #size-cells = <0>;

-             cpu@0 {
+             cpu0: cpu@0 {
                       compatible = "sifive,e51", "sifive,rocket0", "riscv";
                       device_type = "cpu";
                       i-cache-block-size = <64>;

@@ -34,7 +34,7 @@ cpu0_intc: interrupt-controller {
                       };
               };

-             cpu@1 {
+             cpu1: cpu@1 {
                       compatible = "sifive,u54-mc", "sifive,rocket0", "riscv";
                       d-cache-block-size = <64>;
                       d-cache-sets = <64>;

@@ -61,7 +61,7 @@ cpu1_intc: interrupt-controller {
                       };
               };

-             cpu@2 {
+             cpu2: cpu@2 {
                       compatible = "sifive,u54-mc", "sifive,rocket0", "riscv";
                       d-cache-block-size = <64>;
                       d-cache-sets = <64>;

@@ -88,7 +88,7 @@ cpu2_intc: interrupt-controller {
                       };
               };

-             cpu@3 {
+             cpu3: cpu@3 {
                       compatible = "sifive,u54-mc", "sifive,rocket0", "riscv";
                       d-cache-block-size = <64>;
                       d-cache-sets = <64>;

@@ -115,7 +115,7 @@ cpu3_intc: interrupt-controller {
                       };
               };

-             cpu@4 {
+             cpu4: cpu@4 {
                       compatible = "sifive,u54-mc", "sifive,rocket0", "riscv";
                       d-cache-block-size = <64>;
                       d-cache-sets = <64>;

@@ -153,8 +153,9 @@ soc {
               compatible = "simple-bus";
               ranges;

-             cache-controller@2010000 {
+             cctrllr: cache-controller@2010000 {
                       compatible = "sifive,fu540-c000-ccache", "cache";
+                     reg = <0x0 0x2010000 0x0 0x1000>;
                       cache-block-size = <64>;
                       cache-level = <2>;
                       cache-sets = <1024>;

@@ -162,10 +163,9 @@ cache-controller@2010000 {
                       cache-unified;
                       interrupt-parent = <&plic>;
                       interrupts = <1>, <2>, <3>;
-                     reg = <0x0 0x2010000 0x0 0x1000>;
               };

-             clint@2000000 {
+             clint: clint@2000000 {
                       compatible = "sifive,fu540-c000-clint", "sifive,clint0";
                       reg = <0x0 0x2000000 0x0 0xC000>;
                       interrupts-extended = <&cpu0_intc HART_INT_M_SOFT>,

@@ -198,15 +198,6 @@ plic: interrupt-controller@c000000 {
                       riscv,ndev = <186>;
               };

-             dma@3000000 {
-                     compatible = "sifive,fu540-c000-pdma";

Removal of nodes does not look like refactoring.

quoted

-                     reg = <0x0 0x3000000 0x0 0x8000>;
-                     interrupt-parent = <&plic>;
-                     interrupts = <23>, <24>, <25>, <26>, <27>, <28>, <29>,
-                                  <30>;
-                     #dma-cells = <1>;
-             };
-

Best regards,
Krzysztof