Skip to contentSkip to conversation

Basic device tree support for StarFive JH7110 RISC-V SoC

PATCH Applied Hal Feng 7 participants 56 messages Dec 20, 2022 – Feb 16, 2023

Subsystems (6)

v3 of 7 Applied Latest is v7 →

lore
Share & tools
lore

Hal Feng

author
#0 lore raw

v3 cover Basic device tree support for StarFive JH7110 RISC-V SoC

This patch series adds basic device tree support for StarFive JH7110 SoC. This patch series depends on series [1] and [2]. You can simply get or review the patches at the link [3].

[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]/
[3]: https://github.com/hal-feng/linux/commits/visionfive2-minimal

Changes since v2:
- Rebased on tag v6.1.
- Dropped patch 8 because it was merged.
Patch 1:
- Made the links into "Link:" tags. (by Conor)
- Corrected the board name to "VisionFive 2" instead of "VisionFive V2" and added compatibles for version A and version B of VisionFive 2. (by Emil)
Patch 4:
- Used "sifive,ccache0" compatible string to match. (by Conor) Patch 5:
- Dropped "select SIFIVE_CCACHE" in config SOC_STARFIVE. (by Conor)
- Dropped "starfive,jh7110-ccache" compatible in drivers/soc/sifive/sifive_ccache.c.
Patch 6:
- Removed all "clock-frequency = <0>". (by Conor)
- Sorted the nodes after their addresses. (by Emil)
- Renamed "clk_rtc" to "rtc_osc".
- Added "sifive,ccache0" compatible in the cache-controller node.
- Renamed "JH7110_SYSCLK_APB_BUS_FUNC" to "JH7110_SYSCLK_APB_BUS" and renamed "apb_bus_func" to "apb_bus".
Renamed "JH7110_SYSCLK_IOMUX" to "JH7110_SYSCLK_IOMUX_APB". Renamed "JH7110_SYSRST_IOMUX" to "JH7110_SYSRST_IOMUX_APB". Renamed "JH7110_AONRST_AON_IOMUX" to "JH7110_AONRST_IOMUX".
- Removed "reg-names" in gpio nodes.
Patch 7:
- Corrected the board name to "VisionFive 2" instead of "VisionFive V2".
- Renamed jh7110-starfive-visionfive-v2.dts to jh7110-starfive-visionfive-2.dtsi.
- Added dts for VisionFive 2 version A and version B boards.
- In the chosen node, deleted "linux,initrd-start" and "linux,initrd-end" and changed the value of "stdout-path" to "serial0:115200n8".
- Changed the bias of uart0 "rx-pins" to
"bias-disable; /* external pull-up */".
- Renamed "clk_rtc" to "rtc_osc".
- Moved the gpio node behind the uart0 node.

v2: https://lore.kernel.org/all/[email protected]/

Changes since v1:
- Rebased on tag v6.1-rc5.
- Added blank line in patch 1. (by Krzysztof)
- Rebased patch 4 and 6 on the newest code. (by Conor)
- Dropped patch 5. (by Conor)
- Removed the quirk of JH7100 in patch 6, considering this patch series should only add support for JH7110.
- For patch 27, added Co-developed-by tag for Jianlong and me. Renamed cpu labels to "S76_0", "U74_*" instead of "cpu*" following the style of jh7100.dtsi. Moved all "clock-frequency" properties to the board dts. Rewrote clock-controller nodes and deleted reset-controller nodes for using auxiliary bus. Rewrote gpio nodes following generic pinctrl bindings. Removed the redundant second reset entry of uart nodes.
- For patch 28, added Co-developed-by tag for Jianlong and me. Added a chosen node. Removed reserved-memory node. Added fixed frequency clock nodes for overriding the "clock-frequency" properties. Rewrote the gpio nodes following generic pinctrl bindings.
- Dropped patch 30. (by Conor)
- Reworded the commit messages.

v1: https://lore.kernel.org/all/[email protected]/

Emil Renner Berthing (7):
dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board
dt-bindings: timer: Add StarFive JH7110 clint
dt-bindings: interrupt-controller: Add StarFive JH7110 plic
dt-bindings: sifive,ccache0: Support StarFive JH7110 SoC
soc: sifive: ccache: Add StarFive JH7110 support
riscv: dts: starfive: Add initial StarFive JH7110 device tree
riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device
tree

 .../sifive,plic-1.0.0.yaml                    |   1 +
 .../bindings/riscv/sifive,ccache0.yaml        |   9 +-
 .../devicetree/bindings/riscv/starfive.yaml   |   6 +
 .../bindings/timer/sifive,clint.yaml          |   1 +
 arch/riscv/boot/dts/starfive/Makefile         |   1 +
 .../jh7110-starfive-visionfive-2-va.dts       |  13 +
 .../jh7110-starfive-visionfive-2-vb.dts       |  13 +
 .../jh7110-starfive-visionfive-2.dtsi         | 111 +++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 411 ++++++++++++++++++
 drivers/soc/Makefile                          |   2 +-
 drivers/soc/sifive/Kconfig                    |   2 +-
 11 files changed, 567 insertions(+), 3 deletions(-)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi

base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476
prerequisite-patch-id: 4dc515731ce237184553c1606ffb3afaeb51c3d8
prerequisite-patch-id: 09c98554df52d17ba5fd604125f8cdd62cbe80d1
prerequisite-patch-id: a798370d170dc2bcc79ed86f741c21c1e6d87c78
prerequisite-patch-id: bd9fd8b5cb2376dc7a5e08e1a1fbb969cf475926
prerequisite-patch-id: c57ebb83bc43ccd2a8366ff166eb499da1e1d2cf
prerequisite-patch-id: a1673a9e9f19d6fab5a51abb721e54e36636f067
prerequisite-patch-id: 94860423c7acc9025249d4bb36652a585bd0a797
prerequisite-patch-id: b5084253283929d9a6d0e66c350400c7c85d034d
prerequisite-patch-id: 6e369dbe9dca2785e4ea7d0b80e525e227a90a6e
prerequisite-patch-id: e08806183c152714c563f3a21c6d7b2f539c4d6e
prerequisite-patch-id: 79db8036abdc48fd36da227652ec62627a6b548b
prerequisite-patch-id: 06971b8e6bddc0e87e63bfdb0ce8bfb653bd73aa
prerequisite-patch-id: 16309a0e23811a2c55d2e56886de3e8eccc51554
prerequisite-patch-id: bf4f7ab0b6cfa90b6e49e66c7d75ed2eaaebbe78
prerequisite-patch-id: 38468d532e87867990055d3320679f18c5f52278
prerequisite-patch-id: 4710f2ac22dca0bdd9ff5d744d2c37cab3c74515

Signature · 1 line
2.38.1
author
#1 lore raw

PATCH v3 1/7 dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board

From: Emil Renner Berthing <[email protected]>

Add device tree bindings for the StarFive JH7110 RISC-V SoC and the VisionFive 2 board equipped with it.

VisionFive 2 board has version A and version B, which are different in gmac and phy chip. The version A board has one 1000Mbps and one 100Mbps Ethernet ports while the version B board has two 1000Mbps Ethernet ports.

---
 Documentation/devicetree/bindings/riscv/starfive.yaml | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/riscv/starfive.yaml +6 -0
index 5b36243fd674..fbe3c012dadf 100644
@@ -22,6 +22,12 @@ properties:
           - const: beagle,beaglev-starlight-jh7100-r0
           - const: starfive,jh7100
 
+      - items:
+          - enum:
+              - starfive,visionfive-2-va
+              - starfive,visionfive-2-vb
+          - const: starfive,jh7110
+
 additionalProperties: true
 
 ...
Signature · 1 line
2.38.1
Acked-by on v3 1/7
#2 lore raw

Re: [PATCH v3 1/7] dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board

Quoted 10 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add device tree bindings for the StarFive JH7110 RISC-V SoC and the VisionFive 2 board equipped with it.

VisionFive 2 board has version A and version B, which are different in gmac and phy chip. The version A board has one 1000Mbps and one 100Mbps Ethernet ports while the version B board has two 1000Mbps Ethernet ports.

Quoted 4 lines

I assume you did not add my tag because of changes?

Best regards,
Krzysztof

author
#3 lore raw

Re: [PATCH v3 1/7] dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board

Quoted 11 lines · Krzysztof Kozlowski
Quoted 10 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add device tree bindings for the StarFive JH7110 RISC-V SoC and the VisionFive 2 board equipped with it.

VisionFive 2 board has version A and version B, which are different in gmac and phy chip. The version A board has one 1000Mbps and one 100Mbps Ethernet ports while the version B board has two 1000Mbps Ethernet ports.

Quoted 6 lines
Quoted 4 lines

I assume you did not add my tag because of changes?

Yes, it might not the original one to which you wanna add the Acked-by tag.

Best regards,
Hal

Quoted 2 lines
maintainer
#4 lore raw

Re: [PATCH v3 1/7] dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board

In reply to Hal Feng
Quoted 10 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add device tree bindings for the StarFive JH7110 RISC-V SoC and the VisionFive 2 board equipped with it.

VisionFive 2 board has version A and version B, which are different in gmac and phy chip. The version A board has one 1000Mbps and one 100Mbps Ethernet ports while the version B board has two 1000Mbps Ethernet ports.

Quoted 4 lines

Unless Emil objects, I'll queue this for 6.3 once -rc1 has been tagged.

Thanks,
Conor.

author
#5 lore raw

Re: [PATCH v3 1/7] dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board

Quoted 11 lines · Conor Dooley
Quoted 10 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add device tree bindings for the StarFive JH7110 RISC-V SoC and the VisionFive 2 board equipped with it.

VisionFive 2 board has version A and version B, which are different in gmac and phy chip. The version A board has one 1000Mbps and one 100Mbps Ethernet ports while the version B board has two 1000Mbps Ethernet ports.

Quoted 6 lines
Quoted 4 lines

Unless Emil objects, I'll queue this for 6.3 once -rc1 has been tagged.

Thank you so much.

Best regards,
Hal

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author
#6 lore raw

PATCH v3 2/7 dt-bindings: timer: Add StarFive JH7110 clint

In reply to Hal Feng

From: Emil Renner Berthing <[email protected]>

Add compatible string for the StarFive JH7110 clint.

---
 Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/timer/sifive,clint.yaml +1 -0
index bbad24165837..a9580191f78b 100644
@@ -27,6 +27,7 @@ properties:
           - enum:
               - sifive,fu540-c000-clint
               - starfive,jh7100-clint
+              - starfive,jh7110-clint
               - canaan,k210-clint
           - const: sifive,clint0
       - items:
Signature · 1 line
2.38.1
author
#7 lore raw

PATCH v3 3/7 dt-bindings: interrupt-controller: Add StarFive JH7110 plic

In reply to Hal Feng

From: Emil Renner Berthing <[email protected]>

Add compatible string for StarFive JH7110 plic.

---
 .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml         | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml +1 -0
index 99e01f4d0a69..571700d5cb9e 100644
@@ -60,6 +60,7 @@ properties:
           - enum:
               - sifive,fu540-c000-plic
               - starfive,jh7100-plic
+              - starfive,jh7110-plic
               - canaan,k210-plic
           - const: sifive,plic-1.0.0
       - items:
Signature · 1 line
2.38.1
author
#8 lore raw

PATCH v3 4/7 dt-bindings: sifive,ccache0: Support StarFive JH7110 SoC

In reply to Hal Feng

From: Emil Renner Berthing <[email protected]>

This cache controller is also used on the StarFive JH7110 SoC.

---
 .../devicetree/bindings/riscv/sifive,ccache0.yaml        | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml +8 -1
index bf3f07421f7e..31d20efaa6d3 100644
@@ -38,6 +38,10 @@ properties:
               - sifive,fu540-c000-ccache
               - sifive,fu740-c000-ccache
           - const: cache
+      - items:
+          - const: starfive,jh7110-ccache
+          - const: sifive,ccache0
+          - const: cache
       - items:
           - const: microchip,mpfs-ccache
           - const: sifive,fu540-c000-ccache
@@ -85,6 +89,7 @@ allOf:
           contains:
             enum:
               - sifive,fu740-c000-ccache
+              - starfive,jh7110-ccache
               - microchip,mpfs-ccache
 
     then:
@@ -105,7 +110,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: sifive,fu740-c000-ccache
+            enum:
+              - sifive,fu740-c000-ccache
+              - starfive,jh7110-ccache
 
     then:
       properties:
Signature · 1 line
2.38.1
maintainer Acked-by on v3 4/7
#9 lore raw

Re: [PATCH v3 4/7] dt-bindings: sifive,ccache0: Support StarFive JH7110 SoC

Quoted 10 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

This cache controller is also used on the StarFive JH7110 SoC.

---
 .../devicetree/bindings/riscv/sifive,ccache0.yaml        | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author
#10 lore raw

PATCH v3 5/7 soc: sifive: ccache: Add StarFive JH7110 support

In reply to Hal Feng

From: Emil Renner Berthing <[email protected]>

This adds support for the StarFive JH7110 SoC which also features this SiFive cache controller.

---
 drivers/soc/Makefile       | 2 +-
 drivers/soc/sifive/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/soc/Makefile +1 -1
index 69ba6508cf2c..534669840858 100644
@@ -26,7 +26,7 @@ obj-y				+= qcom/
 obj-y				+= renesas/
 obj-y				+= rockchip/
 obj-$(CONFIG_SOC_SAMSUNG)	+= samsung/
-obj-$(CONFIG_SOC_SIFIVE)	+= sifive/
+obj-y				+= sifive/
 obj-y				+= sunxi/
 obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
 obj-y				+= ti/
drivers/soc/sifive/Kconfig +1 -1
index ed4c571f8771..e86870be34c9 100644
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-if SOC_SIFIVE
+if SOC_SIFIVE || SOC_STARFIVE
 
 config SIFIVE_CCACHE
 	bool "Sifive Composable Cache controller"
Signature · 1 line
2.38.1
maintainer
#11 lore raw

Re: [PATCH v3 5/7] soc: sifive: ccache: Add StarFive JH7110 support

Quoted 7 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

This adds support for the StarFive JH7110 SoC which also features this SiFive cache controller.

I'll queue this and the ccache binding for v6.3 once -rc1 is tagged, once again with the caveat that screaming from Emil with dissuade me. Ordinarily, I'd wait for his R-b, but he is the author..

Thanks,
Conor.

Quoted 34 lines
---
 drivers/soc/Makefile       | 2 +-
 drivers/soc/sifive/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/soc/Makefile +1 -1
index 69ba6508cf2c..534669840858 100644
@@ -26,7 +26,7 @@ obj-y				+= qcom/
 obj-y				+= renesas/
 obj-y				+= rockchip/
 obj-$(CONFIG_SOC_SAMSUNG)	+= samsung/
-obj-$(CONFIG_SOC_SIFIVE)	+= sifive/
+obj-y				+= sifive/
 obj-y				+= sunxi/
 obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
 obj-y				+= ti/
drivers/soc/sifive/Kconfig +1 -1
index ed4c571f8771..e86870be34c9 100644
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-if SOC_SIFIVE
+if SOC_SIFIVE || SOC_STARFIVE
 
 config SIFIVE_CCACHE
 	bool "Sifive Composable Cache controller"
Signature · 1 line
2.38.1
author
#12 lore raw

PATCH v3 6/7 riscv: dts: starfive: Add initial StarFive JH7110 device tree

In reply to Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411 +++++++++++++++++++++++
 1 file changed, 411 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +411 -0
new file mode 100644
index 000000000000..64d260ea1f29
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+#include <dt-bindings/reset/starfive,jh7110-crg.h>
+
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <0>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <8192>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <16384>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imac";
+			tlb-split;
+			status = "disabled";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_1: cpu@1 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <1>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu1_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_2: cpu@2 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <2>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu2_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_3: cpu@3 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <3>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu3_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_4: cpu@4 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <4>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu4_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&S76_0>;
+				};
+
+				core1 {
+					cpu = <&U74_1>;
+				};
+
+				core2 {
+					cpu = <&U74_2>;
+				};
+
+				core3 {
+					cpu = <&U74_3>;
+				};
+
+				core4 {
+					cpu = <&U74_4>;
+				};
+			};
+		};
+	};
+
+	osc: osc {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	rtc_osc: rtc_osc {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac0_rmii_refin: gmac0_rmii_refin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac0_rgmii_rxin: gmac0_rgmii_rxin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac1_rmii_refin: gmac1_rmii_refin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac1_rgmii_rxin: gmac1_rgmii_rxin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2stx_bclk_ext: i2stx_bclk_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2stx_lrck_ext: i2stx_lrck_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2srx_bclk_ext: i2srx_bclk_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2srx_lrck_ext: i2srx_lrck_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	tdm_ext: tdm_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	mclk_ext: mclk_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		interrupt-parent = <&plic>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		clint: clint@2000000 {
+			compatible = "starfive,jh7110-clint", "sifive,clint0";
+			reg = <0x0 0x2000000 0x0 0x10000>;
+			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>,
+					      <&cpu1_intc 3>, <&cpu1_intc 7>,
+					      <&cpu2_intc 3>, <&cpu2_intc 7>,
+					      <&cpu3_intc 3>, <&cpu3_intc 7>,
+					      <&cpu4_intc 3>, <&cpu4_intc 7>;
+		};
+
+		plic: plic@c000000 {
+			compatible = "starfive,jh7110-plic", "sifive,plic-1.0.0";
+			reg = <0x0 0xc000000 0x0 0x4000000>;
+			interrupts-extended = <&cpu0_intc 11>,
+					      <&cpu1_intc 11>, <&cpu1_intc 9>,
+					      <&cpu2_intc 11>, <&cpu2_intc 9>,
+					      <&cpu3_intc 11>, <&cpu3_intc 9>,
+					      <&cpu4_intc 11>, <&cpu4_intc 9>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			#address-cells = <0>;
+			riscv,ndev = <136>;
+		};
+
+		ccache: cache-controller@2010000 {
+			compatible = "starfive,jh7110-ccache", "sifive,ccache0", "cache";
+			reg = <0x0 0x2010000 0x0 0x4000>;
+			interrupts = <1>, <3>, <4>, <2>;
+			cache-block-size = <64>;
+			cache-level = <2>;
+			cache-sets = <2048>;
+			cache-size = <2097152>;
+			cache-unified;
+		};
+
+		uart0: serial@10000000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x10000000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_UART0_CORE>,
+				 <&syscrg JH7110_SYSCLK_UART0_APB>;
+			clock-names = "baudclk", "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_UART0_APB>;
+			interrupts = <32>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		uart1: serial@10010000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x10010000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_UART1_CORE>,
+				 <&syscrg JH7110_SYSCLK_UART1_APB>;
+			clock-names = "baudclk", "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_UART1_APB>;
+			interrupts = <33>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		uart2: serial@10020000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x10020000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_UART2_CORE>,
+				 <&syscrg JH7110_SYSCLK_UART2_APB>;
+			clock-names = "baudclk", "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_UART2_APB>;
+			interrupts = <34>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		uart3: serial@12000000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x12000000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_UART3_CORE>,
+				 <&syscrg JH7110_SYSCLK_UART3_APB>;
+			clock-names = "baudclk", "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_UART3_APB>;
+			interrupts = <45>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		uart4: serial@12010000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x12010000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_UART4_CORE>,
+				 <&syscrg JH7110_SYSCLK_UART4_APB>;
+			clock-names = "baudclk", "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_UART4_APB>;
+			interrupts = <46>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		uart5: serial@12020000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x12020000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_UART5_CORE>,
+				 <&syscrg JH7110_SYSCLK_UART5_APB>;
+			clock-names = "baudclk", "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_UART5_APB>;
+			interrupts = <47>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		syscrg: clock-controller@13020000 {
+			compatible = "starfive,jh7110-syscrg";
+			reg = <0x0 0x13020000 0x0 0x10000>;
+			clocks = <&osc>, <&gmac1_rmii_refin>,
+				 <&gmac1_rgmii_rxin>,
+				 <&i2stx_bclk_ext>, <&i2stx_lrck_ext>,
+				 <&i2srx_bclk_ext>, <&i2srx_lrck_ext>,
+				 <&tdm_ext>, <&mclk_ext>;
+			clock-names = "osc", "gmac1_rmii_refin",
+				      "gmac1_rgmii_rxin",
+				      "i2stx_bclk_ext", "i2stx_lrck_ext",
+				      "i2srx_bclk_ext", "i2srx_lrck_ext",
+				      "tdm_ext", "mclk_ext";
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+		};
+
+		gpio: gpio@13040000 {
+			compatible = "starfive,jh7110-sys-pinctrl";
+			reg = <0x0 0x13040000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_IOMUX_APB>;
+			resets = <&syscrg JH7110_SYSRST_IOMUX_APB>;
+			interrupts = <86>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		aoncrg: clock-controller@17000000 {
+			compatible = "starfive,jh7110-aoncrg";
+			reg = <0x0 0x17000000 0x0 0x10000>;
+			clocks = <&osc>, <&rtc_osc>,
+				 <&gmac0_rmii_refin>, <&gmac0_rgmii_rxin>,
+				 <&syscrg JH7110_SYSCLK_STG_AXIAHB>,
+				 <&syscrg JH7110_SYSCLK_APB_BUS>,
+				 <&syscrg JH7110_SYSCLK_GMAC0_GTXCLK>;
+			clock-names = "osc", "rtc_osc", "gmac0_rmii_refin",
+				      "gmac0_rgmii_rxin", "stg_axiahb",
+				      "apb_bus", "gmac0_gtxclk";
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+		};
+
+		gpioa: gpio@17020000 {
+			compatible = "starfive,jh7110-aon-pinctrl";
+			reg = <0x0 0x17020000 0x0 0x10000>;
+			resets = <&aoncrg JH7110_AONRST_IOMUX>;
+			interrupts = <85>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+	};
+};
Signature · 1 line
2.38.1
#13 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 187 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411 +++++++++++++++++++++++
 1 file changed, 411 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +166 -0
new file mode 100644
index 000000000000..64d260ea1f29
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+#include <dt-bindings/reset/starfive,jh7110-crg.h>
+
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <0>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <8192>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <16384>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imac";
+			tlb-split;
+			status = "disabled";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_1: cpu@1 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <1>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu1_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_2: cpu@2 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <2>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu2_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_3: cpu@3 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <3>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu3_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_4: cpu@4 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <4>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu4_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&S76_0>;
+				};
+
+				core1 {
+					cpu = <&U74_1>;
+				};
+
+				core2 {
+					cpu = <&U74_2>;
+				};
+
+				core3 {
+					cpu = <&U74_3>;

arch/riscv/boot/dts/starfive/jh7110.dtsi line 176

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+				};
+
+				core4 {
+					cpu = <&U74_4>;
+				};
+			};
+		};
+	};
+
+	osc: osc {

Node names should be generic, so why this is "osc" and other oscillators are not "osc"?

arch/riscv/boot/dts/starfive/jh7110.dtsi line 181

Quoted 5 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	rtc_osc: rtc_osc {

No underscores in node names. Generic node names (so each of them starting or ending with clock).

arch/riscv/boot/dts/starfive/jh7110.dtsi line 186

Quoted 5 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac0_rmii_refin: gmac0_rmii_refin {

Same problem... and actually you have way too many fixed clocks which do nothing. It looks like you avoid to define proper clock controller. What's the point for all these clocks? These are no-op.

Quoted 47 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +47 -0
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac0_rgmii_rxin: gmac0_rgmii_rxin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac1_rmii_refin: gmac1_rmii_refin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac1_rgmii_rxin: gmac1_rgmii_rxin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2stx_bclk_ext: i2stx_bclk_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2stx_lrck_ext: i2stx_lrck_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2srx_bclk_ext: i2srx_bclk_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2srx_lrck_ext: i2srx_lrck_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	tdm_ext: tdm_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	mclk_ext: mclk_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;

arch/riscv/boot/dts/starfive/jh7110.dtsi line 243

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+	};
+
+	soc {
+		compatible = "simple-bus";
+		interrupt-parent = <&plic>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		clint: clint@2000000 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

arch/riscv/boot/dts/starfive/jh7110.dtsi line 253

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+			compatible = "starfive,jh7110-clint", "sifive,clint0";
+			reg = <0x0 0x2000000 0x0 0x10000>;
+			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>,
+					      <&cpu1_intc 3>, <&cpu1_intc 7>,
+					      <&cpu2_intc 3>, <&cpu2_intc 7>,
+					      <&cpu3_intc 3>, <&cpu3_intc 7>,
+					      <&cpu4_intc 3>, <&cpu4_intc 7>;
+		};
+
+		plic: plic@c000000 {

Node names should be generic, so interrupt-controller
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Quoted 27 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +27 -0
+			compatible = "starfive,jh7110-plic", "sifive,plic-1.0.0";
+			reg = <0x0 0xc000000 0x0 0x4000000>;
+			interrupts-extended = <&cpu0_intc 11>,
+					      <&cpu1_intc 11>, <&cpu1_intc 9>,
+					      <&cpu2_intc 11>, <&cpu2_intc 9>,
+					      <&cpu3_intc 11>, <&cpu3_intc 9>,
+					      <&cpu4_intc 11>, <&cpu4_intc 9>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			#address-cells = <0>;
+			riscv,ndev = <136>;
+		};
+
+		ccache: cache-controller@2010000 {
+			compatible = "starfive,jh7110-ccache", "sifive,ccache0", "cache";
+			reg = <0x0 0x2010000 0x0 0x4000>;
+			interrupts = <1>, <3>, <4>, <2>;
+			cache-block-size = <64>;
+			cache-level = <2>;
+			cache-sets = <2048>;
+			cache-size = <2097152>;
+			cache-unified;
+		};
+
+		uart0: serial@10000000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x10000000 0x0 0x10000>;

arch/riscv/boot/dts/starfive/jh7110.dtsi line 290

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+			clocks = <&syscrg JH7110_SYSCLK_UART0_CORE>,
+				 <&syscrg JH7110_SYSCLK_UART0_APB>;
+			clock-names = "baudclk", "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_UART0_APB>;
+			interrupts = <32>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+

Best regards,
Krzysztof

author
#14 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 193 lines · Krzysztof Kozlowski
Quoted 192 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411 +++++++++++++++++++++++
 1 file changed, 411 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +171 -0
new file mode 100644
index 000000000000..64d260ea1f29
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+#include <dt-bindings/reset/starfive,jh7110-crg.h>
+
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <0>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <8192>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <16384>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imac";
+			tlb-split;
+			status = "disabled";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_1: cpu@1 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <1>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu1_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_2: cpu@2 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <2>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu2_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_3: cpu@3 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <3>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu3_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_4: cpu@4 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <4>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";
+			tlb-split;
+
+			cpu4_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&S76_0>;
+				};
+
+				core1 {
+					cpu = <&U74_1>;
+				};
+
+				core2 {
+					cpu = <&U74_2>;
+				};
+
+				core3 {
+					cpu = <&U74_3>;
+				};
+
+				core4 {
+					cpu = <&U74_4>;
+				};
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 176

Quoted 5 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+			};
+		};
+	};
+
+	osc: osc {

Node names should be generic, so why this is "osc" and other oscillators are not "osc"?

Only "osc" and "rtc_osc" are oscillators, the rest are clock sources provided through gpio. I will modify the node names according to your link below. So

	osc: oscillator {
Quoted 10 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 181

Quoted 7 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	rtc_osc: rtc_osc {

No underscores in node names. Generic node names (so each of them starting or ending with clock).

Will change this line to

	rtc_osc: oscillator {
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 186

Quoted 6 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac0_rmii_refin: gmac0_rmii_refin {

Same problem... and actually you have way too many fixed clocks which do

Will change the node names as follows.

	gmac0_rmii_refin: clock {
		...
	};

	gmac0_rgmii_rxin: clock {
		...
	};
	...
Quoted 2 lines

nothing. It looks like you avoid to define proper clock controller. What's the point for all these clocks? These are no-op.

These are all external fixed rate clocks inputted to the SoC. They are the root clocks of the clock tree made by clock drivers. Their ops are provided in drivers/clk/clk-fixed-rate.c.

Quoted 53 lines
Quoted 53 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +52 -0
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac0_rgmii_rxin: gmac0_rgmii_rxin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac1_rmii_refin: gmac1_rmii_refin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	gmac1_rgmii_rxin: gmac1_rgmii_rxin {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2stx_bclk_ext: i2stx_bclk_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2stx_lrck_ext: i2stx_lrck_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2srx_bclk_ext: i2srx_bclk_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	i2srx_lrck_ext: i2srx_lrck_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	tdm_ext: tdm_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	mclk_ext: mclk_ext {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		interrupt-parent = <&plic>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 243

Quoted 5 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		clint: clint@2000000 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Change it to

		clint: timer@2000000 {
Quoted 14 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 253

Quoted 11 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+			compatible = "starfive,jh7110-clint", "sifive,clint0";
+			reg = <0x0 0x2000000 0x0 0x10000>;
+			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>,
+					      <&cpu1_intc 3>, <&cpu1_intc 7>,
+					      <&cpu2_intc 3>, <&cpu2_intc 7>,
+					      <&cpu3_intc 3>, <&cpu3_intc 7>,
+					      <&cpu4_intc 3>, <&cpu4_intc 7>;
+		};
+
+		plic: plic@c000000 {

Node names should be generic, so interrupt-controller
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Change it to

		plic: interrupt-controller@c000000 {

Best regards,
Hal

Quoted 13 lines
Quoted 13 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +12 -0
+			compatible = "starfive,jh7110-plic", "sifive,plic-1.0.0";
+			reg = <0x0 0xc000000 0x0 0x4000000>;
+			interrupts-extended = <&cpu0_intc 11>,
+					      <&cpu1_intc 11>, <&cpu1_intc 9>,
+					      <&cpu2_intc 11>, <&cpu2_intc 9>,
+					      <&cpu3_intc 11>, <&cpu3_intc 9>,
+					      <&cpu4_intc 11>, <&cpu4_intc 9>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			#address-cells = <0>;
+			riscv,ndev = <136>;
+		};
#15 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 10 lines · Hal Feng
	gmac0_rmii_refin: clock {
		...
	};

	gmac0_rgmii_rxin: clock {
		...
	};
	...
Quoted 4 lines
Quoted 2 lines

nothing. It looks like you avoid to define proper clock controller. What's the point for all these clocks? These are no-op.

These are all external fixed rate clocks inputted to the SoC. They are the root

If they are all external, then it is fine.

Quoted 5 lines
Quoted 5 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 179

Quoted 5 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +3 -0
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};

Best regards,
Krzysztof

maintainer
#16 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

In reply to Hal Feng
Quoted 11 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---

FWIW, this cpu-map is now the default in linux, so you no longer *need* to add it for that purpose - but there's obviously no harm in being explicit for other operating systems etc. (IOW, don't remove it!)

Quoted 16 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +16 -0
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&S76_0>;
+				};
+
+				core1 {
+					cpu = <&U74_1>;
+				};
+
+				core2 {
+					cpu = <&U74_2>;
+				};
+
+				core3 {
+					cpu = <&U74_3>;

arch/riscv/boot/dts/starfive/jh7110.dtsi

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +9 -0
+				};
+
+				core4 {
+					cpu = <&U74_4>;
+				};
+			};
+		};
+	};
 
+		syscrg: clock-controller@13020000 {

For obvious reasons, I cannot apply this until both the clock & pinctrl bindings are in my tree - but you know that already.

arch/riscv/boot/dts/starfive/jh7110.dtsi line 363

Quoted 7 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +7 -0
+			compatible = "starfive,jh7110-syscrg";
+			reg = <0x0 0x13020000 0x0 0x10000>;
+			clocks = <&osc>, <&gmac1_rmii_refin>,
+				 <&gmac1_rgmii_rxin>,
+				 <&i2stx_bclk_ext>, <&i2stx_lrck_ext>,
+				 <&i2srx_bclk_ext>, <&i2srx_lrck_ext>,
+				 <&tdm_ext>, <&mclk_ext>;

As Krzk asked - are these clocks really all inputs to the SoC?

arch/riscv/boot/dts/starfive/jh7110.dtsi line 400

Quoted 12 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +11 -0
+			clock-names = "osc", "gmac1_rmii_refin",
+				      "gmac1_rgmii_rxin",
+				      "i2stx_bclk_ext", "i2stx_lrck_ext",
+				      "i2srx_bclk_ext", "i2srx_lrck_ext",
+				      "tdm_ext", "mclk_ext";
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+		};
+
+		gpio: gpio@13040000 {
 
+		gpioa: gpio@17020000 {

Out of curiousity, why gpio & gpioa?

Thanks,
Conor.

author
#17 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 47 lines · Conor Dooley
Quoted 11 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---

FWIW, this cpu-map is now the default in linux, so you no longer *need* to add it for that purpose - but there's obviously no harm in being explicit for other operating systems etc. (IOW, don't remove it!)

Quoted 16 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +16 -0
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&S76_0>;
+				};
+
+				core1 {
+					cpu = <&U74_1>;
+				};
+
+				core2 {
+					cpu = <&U74_2>;
+				};
+
+				core3 {
+					cpu = <&U74_3>;

arch/riscv/boot/dts/starfive/jh7110.dtsi

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +9 -0
+				};
+
+				core4 {
+					cpu = <&U74_4>;
+				};
+			};
+		};
+	};
 
+		syscrg: clock-controller@13020000 {

For obvious reasons, I cannot apply this until both the clock & pinctrl bindings are in my tree - but you know that already.

Quoted 9 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 363

Quoted 7 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +7 -0
+			compatible = "starfive,jh7110-syscrg";
+			reg = <0x0 0x13020000 0x0 0x10000>;
+			clocks = <&osc>, <&gmac1_rmii_refin>,
+				 <&gmac1_rgmii_rxin>,
+				 <&i2stx_bclk_ext>, <&i2stx_lrck_ext>,
+				 <&i2srx_bclk_ext>, <&i2srx_lrck_ext>,
+				 <&tdm_ext>, <&mclk_ext>;

As Krzk asked - are these clocks really all inputs to the SoC?

Yes, they are all external clock sources inputted to the SoC. They are used as root clocks or optional parent clocks in clock tree.

Quoted 12 lines
Quoted 11 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+			clock-names = "osc", "gmac1_rmii_refin",
+				      "gmac1_rgmii_rxin",
+				      "i2stx_bclk_ext", "i2stx_lrck_ext",
+				      "i2srx_bclk_ext", "i2srx_lrck_ext",
+				      "tdm_ext", "mclk_ext";
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+		};
+
+		gpio: gpio@13040000 {
Quoted 3 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 400

Quoted 1 line
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +1 -0
+		gpioa: gpio@17020000 {

Out of curiousity, why gpio & gpioa?

Oh, is it easier to read if I change "gpio" and "gpioa" to "sysgpio" and "aongpio"? Thanks.

Best regards,
Hal

maintainer
#18 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 14 lines · Hal Feng
Quoted 12 lines · Conor Dooley
Quoted 11 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---

Quoted 8 lines
Quoted 5 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 400

Quoted 3 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +2 -0
+		gpio: gpio@13040000 {
 
+		gpioa: gpio@17020000 {

Out of curiousity, why gpio & gpioa?

Oh, is it easier to read if I change "gpio" and "gpioa" to "sysgpio" and "aongpio"? Thanks.

I think those would be more reader friendly, that's for sure.

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

maintainer
#19 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

In reply to Hal Feng

Hey,

Quoted 32 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411 +++++++++++++++++++++++
 1 file changed, 411 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +11 -0
new file mode 100644
index 000000000000..64d260ea1f29
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+#include <dt-bindings/reset/starfive,jh7110-crg.h>
+
+/ {

arch/riscv/boot/dts/starfive/jh7110.dtsi line 21

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc. Which is correct? Your docs say S7 and S76, so I would imagine that it is actually an S76?

Quoted 5 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+			reg = <0>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <8192>;
+			d-tlb-sets = <1>;

arch/riscv/boot/dts/starfive/jh7110.dtsi line 36

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <16384>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imac";

While I was poking around trying to see if there was some logic behind that compatible, I noticed that SiFive's docs for the S76 say it is RV64GBC *but* the docs for the u74-mc say "4xRV64GBC and 1xRV64IMAC". I assume that rv64imac is the correct one here?

Quoted 17 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +17 -0
+			tlb-split;
+			status = "disabled";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_1: cpu@1 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <1>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;

arch/riscv/boot/dts/starfive/jh7110.dtsi line 63

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

Thanks,
Conor.

#20 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 98 lines · 在 2022-12-28星期三的 22:48 +0000,Conor Dooley写道:

Hey,

Quoted 34 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411
+++++++++++++++++++++++

 1 file changed, 411 insertions(+)

 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi

arch/riscv/boot/dts/starfive/jh7110.dtsi new file +11 -0
new file mode 100644
index 000000000000..64d260ea1f29
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+#include <dt-bindings/reset/starfive,jh7110-crg.h>
+
+/ {

arch/riscv/boot/dts/starfive/jh7110.dtsi line 21

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+       compatible = "starfive,jh7110";
+       #address-cells = <2>;
+       #size-cells = <2>;
+
+       cpus {
+               #address-cells = <1>;
+               #size-cells = <0>;
+
+               S76_0: cpu@0 {
+                       compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc. Which is correct? Your docs say S7 and S76, so I would imagine that it
is actually an S76?

Quoted 5 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+                       reg = <0>;
+                       d-cache-block-size = <64>;
+                       d-cache-sets = <64>;
+                       d-cache-size = <8192>;
+                       d-tlb-sets = <1>;

arch/riscv/boot/dts/starfive/jh7110.dtsi line 36

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+                       d-tlb-size = <40>;
+                       device_type = "cpu";
+                       i-cache-block-size = <64>;
+                       i-cache-sets = <64>;
+                       i-cache-size = <16384>;
+                       i-tlb-sets = <1>;
+                       i-tlb-size = <40>;
+                       mmu-type = "riscv,sv39";
+                       next-level-cache = <&ccache>;
+                       riscv,isa = "rv64imac";

While I was poking around trying to see if there was some logic behind
that compatible, I noticed that SiFive's docs for the S76 say it is RV64GBC *but* the docs for the u74-mc say "4xRV64GBC and 1xRV64IMAC". I assume that rv64imac is the correct one here?

Quoted 23 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +23 -0
+                       tlb-split;
+                       status = "disabled";
+
+                       cpu0_intc: interrupt-controller {
+                               compatible = "riscv,cpu-intc";
+                               interrupt-controller;
+                               #interrupt-cells = <1>;
+                       };
+               };
+
+               U74_1: cpu@1 {
+                       compatible = "sifive,u74-mc", "riscv";
+                       reg = <1>;
+                       d-cache-block-size = <64>;
+                       d-cache-sets = <64>;
+                       d-cache-size = <32768>;
+                       d-tlb-sets = <1>;
+                       d-tlb-size = <40>;
+                       device_type = "cpu";
+                       i-cache-block-size = <64>;
+                       i-cache-sets = <64>;
+                       i-cache-size = <32768>;
+                       i-tlb-sets = <1>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 63

Quoted 4 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +4 -0
+                       i-tlb-size = <40>;
+                       mmu-type = "riscv,sv39";
+                       next-level-cache = <&ccache>;
+                       riscv,isa = "rv64imafdc";

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

It supports RV64GCZbaZbb.

B is not a well-defined thing by specifications, so it should be prevented here.

Quoted 8 lines

Thanks,
Conor.

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

maintainer
#21 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Hey Icenowy, Hal

Quoted 108 lines · Icenowy Zheng
Quoted 98 lines · 在 2022-12-28星期三的 22:48 +0000,Conor Dooley写道:

Hey,

Quoted 34 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411
+++++++++++++++++++++++

 1 file changed, 411 insertions(+)

 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi

arch/riscv/boot/dts/starfive/jh7110.dtsi new file +11 -0
new file mode 100644
index 000000000000..64d260ea1f29
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+#include <dt-bindings/reset/starfive,jh7110-crg.h>
+
+/ {

arch/riscv/boot/dts/starfive/jh7110.dtsi line 21

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+       compatible = "starfive,jh7110";
+       #address-cells = <2>;
+       #size-cells = <2>;
+
+       cpus {
+               #address-cells = <1>;
+               #size-cells = <0>;
+
+               S76_0: cpu@0 {
+                       compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc. Which is correct? Your docs say S7 and S76, so I would imagine that it
is actually an S76?

Quoted 5 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+                       reg = <0>;
+                       d-cache-block-size = <64>;
+                       d-cache-sets = <64>;
+                       d-cache-size = <8192>;
+                       d-tlb-sets = <1>;

arch/riscv/boot/dts/starfive/jh7110.dtsi line 36

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+                       d-tlb-size = <40>;
+                       device_type = "cpu";
+                       i-cache-block-size = <64>;
+                       i-cache-sets = <64>;
+                       i-cache-size = <16384>;
+                       i-tlb-sets = <1>;
+                       i-tlb-size = <40>;
+                       mmu-type = "riscv,sv39";
+                       next-level-cache = <&ccache>;
+                       riscv,isa = "rv64imac";

While I was poking around trying to see if there was some logic behind
that compatible, I noticed that SiFive's docs for the S76 say it is RV64GBC *but* the docs for the u74-mc say "4xRV64GBC and 1xRV64IMAC". I assume that rv64imac is the correct one here?

Quoted 23 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +23 -0
+                       tlb-split;
+                       status = "disabled";
+
+                       cpu0_intc: interrupt-controller {
+                               compatible = "riscv,cpu-intc";
+                               interrupt-controller;
+                               #interrupt-cells = <1>;
+                       };
+               };
+
+               U74_1: cpu@1 {
+                       compatible = "sifive,u74-mc", "riscv";
+                       reg = <1>;
+                       d-cache-block-size = <64>;
+                       d-cache-sets = <64>;
+                       d-cache-size = <32768>;
+                       d-tlb-sets = <1>;
+                       d-tlb-size = <40>;
+                       device_type = "cpu";
+                       i-cache-block-size = <64>;
+                       i-cache-sets = <64>;
+                       i-cache-size = <32768>;
+                       i-tlb-sets = <1>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 63

Quoted 4 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +4 -0
+                       i-tlb-size = <40>;
+                       mmu-type = "riscv,sv39";
+                       next-level-cache = <&ccache>;
+                       riscv,isa = "rv64imafdc";

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

Quoted 1 line

It supports RV64GCZbaZbb.

Sweet, thanks.

Quoted 2 lines

B is not a well-defined thing by specifications, so it should be prevented here.

Yah, don't worry - my next question was going to be which bits were supported :)

Hal, can you update the isa string in the next version please?

Thanks,
Conor.

author
#22 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 7 lines · Conor Dooley

Hey Icenowy, Hal

Quoted 4 lines · Icenowy Zheng
Quoted 3 lines · 在 2022-12-28星期三的 22:48 +0000,Conor Dooley写道:

Hey,

On Tue, Dec 20, 2022 at 09:12:46AM +0800, Hal Feng wrote:

[...]

Quoted 24 lines
Quoted 18 lines
Quoted 18 lines
Quoted 17 lines

+               U74_1: cpu@1 {
+                       compatible = "sifive,u74-mc", "riscv"; +                       reg = <1>;
+                       d-cache-block-size = <64>;
+                       d-cache-sets = <64>;
+                       d-cache-size = <32768>;
+                       d-tlb-sets = <1>;
+                       d-tlb-size = <40>;
+                       device_type = "cpu";
+                       i-cache-block-size = <64>;
+                       i-cache-sets = <64>;
+                       i-cache-size = <32768>;
+                       i-tlb-sets = <1>;
+                       i-tlb-size = <40>;
+                       mmu-type = "riscv,sv39";
+                       next-level-cache = <&ccache>; +                       riscv,isa = "rv64imafdc";

Quoted 5 lines
Quoted 3 lines

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

It supports RV64GCZbaZbb.

Quoted 8 lines

Sweet, thanks.

Quoted 2 lines

B is not a well-defined thing by specifications, so it should be prevented here.

Yah, don't worry - my next question was going to be which bits were supported :)

Hal, can you update the isa string in the next version please?

The current isa description is correct. Please see my reply [1]. Thank you.

[1] https://lore.kernel.org/all/[email protected]/

Best regards,
Hal

author
#23 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

In reply to Icenowy Zheng
Quoted 4 lines · Icenowy Zheng
Quoted 3 lines · 在 2022-12-28星期三的 22:48 +0000,Conor Dooley写道:

Hey,

On Tue, Dec 20, 2022 at 09:12:46AM +0800, Hal Feng wrote:

[...]

Quoted 18 lines
Quoted 18 lines
Quoted 17 lines

+               U74_1: cpu@1 {
+                       compatible = "sifive,u74-mc", "riscv"; +                       reg = <1>;
+                       d-cache-block-size = <64>;
+                       d-cache-sets = <64>;
+                       d-cache-size = <32768>;
+                       d-tlb-sets = <1>;
+                       d-tlb-size = <40>;
+                       device_type = "cpu";
+                       i-cache-block-size = <64>;
+                       i-cache-sets = <64>;
+                       i-cache-size = <32768>;
+                       i-tlb-sets = <1>;
+                       i-tlb-size = <40>;
+                       mmu-type = "riscv,sv39";
+                       next-level-cache = <&ccache>; +                       riscv,isa = "rv64imafdc";

Quoted 8 lines
Quoted 3 lines

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

It supports RV64GCZbaZbb.

B is not a well-defined thing by specifications, so it should be prevented here.

Thank you for your kindly reply.

Best regards,
Hal

author
#24 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

In reply to Conor Dooley
Quoted 39 lines · Conor Dooley

Hey,

Quoted 36 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411 +++++++++++++++++++++++
 1 file changed, 411 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +15 -0
new file mode 100644
index 000000000000..64d260ea1f29
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+#include <dt-bindings/reset/starfive,jh7110-crg.h>
+
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 21

Quoted 6 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +6 -0
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc.

U74-MC has 5 cores including 1 * S7 core and 4 * U74 cores.

Quoted 2 lines

Which is correct? Your docs say S7 and S76, so I would imagine that it is actually an S76?

I found SiFive website [1] call it S76, but call it S7 in other places. So I misunderstood this. Considering the ISA difference you described as below, I think it's proper to change the label to "S7_0".

[1] https://www.sifive.com/cores/essential

Quoted 13 lines
Quoted 13 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +12 -0
+			reg = <0>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <8192>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <16384>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 36

Quoted 3 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +3 -0
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imac";

While I was poking around trying to see if there was some logic behind that compatible, I noticed that SiFive's docs for the S76 say it is RV64GBC *but* the docs for the u74-mc say "4xRV64GBC and 1xRV64IMAC". I assume that rv64imac is the correct one here?

Yes, "RV64IMAC" is correct. The monitor core in U74-MC is a S7-series core, not S76.

Quoted 24 lines
Quoted 24 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +23 -0
+			tlb-split;
+			status = "disabled";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_1: cpu@1 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <1>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 63

Quoted 4 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +4 -0
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

Actually, U74 doesn't support the full B extension, and the SiFive doc [1] describes the ISA of U74 is "RV64GC_Zba_Zbb_Sscofpmf" which "G" includes "IMAFD". "_Zba_Zbb_Sscofpmf" is not shown in other device trees such as jh7100.dtsi and fu740-c000.dtsi, so I didn't show them here.

[1] https://sifive.cdn.prismic.io/sifive/2dd11994-693c-4360-8aea-5453d8642c42_u74mc_core_complex_manual_21G3.pdf

Best regards,
Hal

#25 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 17 lines · Hal Feng
Quoted 15 lines · Conor Dooley

arch/riscv/boot/dts/starfive/jh7110.dtsi line 21

Quoted 11 lines · Hal Feng
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +11 -0
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc.

Quoted 8 lines

U74-MC has 5 cores including 1 * S7 core and 4 * U74 cores.

Quoted 2 lines

Which is correct? Your docs say S7 and S76, so I would imagine that it is actually an S76?

I found SiFive website [1] call it S76, but call it S7 in other places. So I misunderstood this. Considering the ISA difference you described as below, I think it's proper to change the label to "S7_0".

I'm less worried about the label & more interested in the compatible. hart0 is, as you say, not a u74. Should we not be adding a "sifive,s7" compatible string to Documentation/devicetree/bindings/riscv/cpus.yaml and using that here instead?

Quoted 25 lines

[1] https://www.sifive.com/cores/essential

Quoted 13 lines
Quoted 13 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +12 -0
+			reg = <0>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <8192>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <16384>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 36

Quoted 3 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +3 -0
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imac";

While I was poking around trying to see if there was some logic behind that compatible, I noticed that SiFive's docs for the S76 say it is RV64GBC *but* the docs for the u74-mc say "4xRV64GBC and 1xRV64IMAC". I assume that rv64imac is the correct one here?

Quoted 2 lines

Yes, "RV64IMAC" is correct. The monitor core in U74-MC is a S7-series core, not S76.

Cool, thanks.

Quoted 32 lines
Quoted 23 lines
Quoted 23 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +23 -0
+			tlb-split;
+			status = "disabled";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_1: cpu@1 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <1>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 63

Quoted 4 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +4 -0
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

Quoted 1 line

Actually, U74 doesn't support the full B extension, and the SiFive doc [1]

Yeah, I knew asking that question that the "RV64GBC" in SiFive's online documentation was using outdated terminology. Also, that is not the doc for your core complex as far as I can tell. That is the document for impid 0x0621_1222, whereas (IIRC) your core is 0x0421_0427. Jess and I had a look one evening but could not find the 21G1.02.00 revision of this document, which is the one corresponding to 0x421_0427. See Table 92 for more details.

Quoted 2 lines

describes the ISA of U74 is "RV64GC_Zba_Zbb_Sscofpmf" which "G" includes "IMAFD".

I could not find the 21G1.02.00 version of this document online, but I was able to find the 21G1.01.00 version of it & that version does not support the Sscofpmf extension (but does have Zba/Zbb support).

Quoted 2 lines

"_Zba_Zbb_Sscofpmf" is not shown in other device trees such as jh7100.dtsi and fu740-c000.dtsi, so I didn't show them here.

Just because other devicetrees omit them, doesn't mean that you should too!
This compatible should be an accurate description of your hardware, so you should add what you actually have.
If you have Zba and Zbb, then add them.
I would double check against your internal documentation for 21G2.02.00 as to whether you do have Sscofpmf, and if you do, add that too!

That way, whenever support for those extensions lands, the jh7110 will automatically pick it up, rather than needing to have them retrofitted.

Quoted 1 line

[1] https://sifive.cdn.prismic.io/sifive/2dd11994-693c-4360-8aea-5453d8642c42_u74mc_core_complex_manual_21G3.pdf

Thanks,
Conor.

author
#26 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 23 lines · Conor Dooley
Quoted 22 lines · Hal Feng
Quoted 15 lines · Conor Dooley

arch/riscv/boot/dts/starfive/jh7110.dtsi line 21

Quoted 11 lines · Hal Feng
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +11 -0
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc.

U74-MC has 5 cores including 1 * S7 core and 4 * U74 cores.

Quoted 2 lines

Which is correct? Your docs say S7 and S76, so I would imagine that it is actually an S76?

Quoted 8 lines
Quoted 3 lines

I found SiFive website [1] call it S76, but call it S7 in other places. So I misunderstood this. Considering the ISA difference you described as below, I think it's proper to change the label to "S7_0".

I'm less worried about the label & more interested in the compatible. hart0 is, as you say, not a u74. Should we not be adding a "sifive,s7" compatible string to Documentation/devicetree/bindings/riscv/cpus.yaml and using that here instead?

First of all, it's my fault that I didn't check the revision of U74-MC manual, so most of my previous replies might not make sense.

If we add a new compatible string for S7, should we change the compatibles of hart1~3 to "sifive,u74" also? And then, there may be no point keeping some compatible strings of core complex like "sifive,u74-mc" and "sifive,u54-mc". I'm not sure about this.

Quoted 26 lines
Quoted 26 lines

[1] https://www.sifive.com/cores/essential

Quoted 21 lines
Quoted 6 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +5 -0
+			reg = <0>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <8192>;
+			d-tlb-sets = <1>;

arch/riscv/boot/dts/starfive/jh7110.dtsi line 36

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +10 -0
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <16384>;
+			i-tlb-sets = <1>;
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imac";

While I was poking around trying to see if there was some logic behind that compatible, I noticed that SiFive's docs for the S76 say it is RV64GBC *but* the docs for the u74-mc say "4xRV64GBC and 1xRV64IMAC". I assume that rv64imac is the correct one here?

Quoted 4 lines
Quoted 2 lines

Yes, "RV64IMAC" is correct. The monitor core in U74-MC is a S7-series core, not S76.

Cool, thanks.

Now I think it might be another version of S76.

Quoted 35 lines
Quoted 33 lines
Quoted 24 lines
Quoted 24 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +23 -0
+			tlb-split;
+			status = "disabled";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_1: cpu@1 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <1>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 63

Quoted 4 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +4 -0
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

Quoted 1 line

Actually, U74 doesn't support the full B extension, and the SiFive doc [1]

Quoted 7 lines

Yeah, I knew asking that question that the "RV64GBC" in SiFive's online documentation was using outdated terminology. Also, that is not the doc for your core complex as far as I can tell. That is the document for impid 0x0621_1222, whereas (IIRC) your core is 0x0421_0427. Jess and I had a look one evening but could not find the 21G1.02.00 revision of this document, which is the one corresponding to 0x421_0427. See Table 92 for more details.

I found the 21G1.02.00 revision on StarFive internal net, but I'm not sure whether I can make it public and I am checking this. This revision records that the ISA of 21G1.02.00 U74 is "RV64GCB" and ISA of 21G1.02.00 S7 is "RV64IMACB". I am asking someone to check with SiFive whether both 21G1.02.00 U74 and S7 support the full B extension.

Quoted 8 lines
Quoted 3 lines

describes the ISA of U74 is "RV64GC_Zba_Zbb_Sscofpmf" which "G" includes "IMAFD".

I could not find the 21G1.02.00 version of this document online, but I was able to find the 21G1.01.00 version of it & that version does not support the Sscofpmf extension (but does have Zba/Zbb support).

Quoted 7 lines
Quoted 2 lines

"_Zba_Zbb_Sscofpmf" is not shown in other device trees such as jh7100.dtsi and fu740-c000.dtsi, so I didn't show them here.

Just because other devicetrees omit them, doesn't mean that you should too!
This compatible should be an accurate description of your hardware, so you should add what you actually have.

Will keep it in mind. Thank you.

Best regards,
Hal

Quoted 8 lines

If you have Zba and Zbb, then add them.
I would double check against your internal documentation for 21G2.02.00 as to whether you do have Sscofpmf, and if you do, add that too!

That way, whenever support for those extensions lands, the jh7110 will automatically pick it up, rather than needing to have them retrofitted.

Quoted 1 line

[1] https://sifive.cdn.prismic.io/sifive/2dd11994-693c-4360-8aea-5453d8642c42_u74mc_core_complex_manual_21G3.pdf

maintainer
#27 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 28 lines · Hal Feng
Quoted 27 lines · Conor Dooley
Quoted 25 lines · Hal Feng
Quoted 15 lines · Conor Dooley

arch/riscv/boot/dts/starfive/jh7110.dtsi line 21

Quoted 11 lines · Hal Feng
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +11 -0
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc.

U74-MC has 5 cores including 1 * S7 core and 4 * U74 cores.

Quoted 2 lines

Which is correct? Your docs say S7 and S76, so I would imagine that it is actually an S76?

I found SiFive website [1] call it S76, but call it S7 in other places. So I misunderstood this. Considering the ISA difference you described as below, I think it's proper to change the label to "S7_0".

Quoted 7 lines
Quoted 4 lines

I'm less worried about the label & more interested in the compatible. hart0 is, as you say, not a u74. Should we not be adding a "sifive,s7" compatible string to Documentation/devicetree/bindings/riscv/cpus.yaml and using that here instead?

First of all, it's my fault that I didn't check the revision of U74-MC manual, so most of my previous replies might not make sense.

No that's fine. The manual stuff confused me too when I went looking initially, and I still get get mixed up by the fact that there are core-complex manuals but not core manuals.

Quoted 4 lines

If we add a new compatible string for S7, should we change the compatibles of hart1~3 to "sifive,u74" also? And then, there may be no point keeping some compatible strings of core complex like "sifive,u74-mc" and "sifive,u54-mc". I'm not sure about this.

[...]

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Yes, "RV64IMAC" is correct. The monitor core in U74-MC is a S7-series core, not S76.

Cool, thanks.

Now I think it might be another version of S76.

The SiFive docs describe the u74-mc core complex, which AFAIU you have, as being 1x S7 & 4x U7.

I'd be happy with new binding for "sifive,s7" & then we use that here. If you're sure it's S76, we can also use that. S76 is described, in what docs I can see, as a core complex containing an S7, so S7 seems likely to be correct?

u7, u74 & u74-mc are valid compatibles, added by SiFive, in commit 75e6d7248efc ("dt-bindings: riscv: Update DT binding docs to support SiFive FU740 SoC"). Unfortunately, they never actually *used* those compatibles for anything, and just used "sifive,bullet0" for the fu740.

I'll accept any of u7, u74 or u74-mc for those harts.

Quoted 42 lines
Quoted 34 lines
Quoted 32 lines
Quoted 23 lines
Quoted 23 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +23 -0
+			tlb-split;
+			status = "disabled";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_1: cpu@1 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <1>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 63

Quoted 4 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +4 -0
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

Quoted 1 line

Actually, U74 doesn't support the full B extension, and the SiFive doc [1]

Quoted 7 lines

Yeah, I knew asking that question that the "RV64GBC" in SiFive's online documentation was using outdated terminology. Also, that is not the doc for your core complex as far as I can tell. That is the document for impid 0x0621_1222, whereas (IIRC) your core is 0x0421_0427. Jess and I had a look one evening but could not find the 21G1.02.00 revision of this document, which is the one corresponding to 0x421_0427. See Table 92 for more details.

Quoted 2 lines

I found the 21G1.02.00 revision on StarFive internal net, but I'm not sure whether I can make it public and I am checking this.

Yeah, certainly don't do anything without cross-checking!

Quoted 4 lines

This revision records
that the ISA of 21G1.02.00 U74 is "RV64GCB" and ISA of 21G1.02.00 S7 is "RV64IMACB". I am asking someone to check with SiFive whether both 21G1.02.00 U74 and S7 support the full B extension.

Having cross-checked a 21G1.01.00 document against a 21G2.01.00 one, I'm 99% sure that you have _Zba_Zbb.
The G2.01 document says _Zba_Zbb & has the same instructions listed as supported as the G1.02 one.
I've also tried the Zbb support patches posted by Heiko [2] on a VisionFive V2 and had them work - which is why a definitive statement on the version of Zbb supported would be really great to have!

[2] - https://lore.kernel.org/linux-riscv/[email protected]/

Quoted 10 lines
Quoted 10 lines
Quoted 2 lines

describes the ISA of U74 is "RV64GC_Zba_Zbb_Sscofpmf" which "G" includes "IMAFD".

I could not find the 21G1.02.00 version of this document online, but I was able to find the 21G1.01.00 version of it & that version does not support the Sscofpmf extension (but does have Zba/Zbb support).

Quoted 2 lines

"_Zba_Zbb_Sscofpmf" is not shown in other device trees such as jh7100.dtsi and fu740-c000.dtsi, so I didn't show them here.

Quoted 6 lines
Quoted 4 lines

Just because other devicetrees omit them, doesn't mean that you should too!
This compatible should be an accurate description of your hardware, so you should add what you actually have.

Will keep it in mind. Thank you.

FWIW, the deadline for getting material in for v6.3 has already passed, so you can send the next version of this series without waiting for clarification on the compatibles & ISA string. We should have plenty of time to get those fixed up before the series gets applied.

Thanks,
Conor.

Quoted 8 lines
Quoted 8 lines

If you have Zba and Zbb, then add them.
I would double check against your internal documentation for 21G2.02.00 as to whether you do have Sscofpmf, and if you do, add that too!

That way, whenever support for those extensions lands, the jh7110 will automatically pick it up, rather than needing to have them retrofitted.

Quoted 1 line

[1] https://sifive.cdn.prismic.io/sifive/2dd11994-693c-4360-8aea-5453d8642c42_u74mc_core_complex_manual_21G3.pdf

#28 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 10 lines · Conor Dooley
Quoted 4 lines · Hal Feng
Quoted 3 lines · Conor Dooley
Quoted 2 lines · Hal Feng
Quoted 1 line · Conor Dooley

On Tue, Dec 20, 2022 at 09:12:46AM +0800, Hal Feng wrote:

FWIW, the deadline for getting material in for v6.3 has already passed, so you can send the next version of this series without waiting for clarification on the compatibles & ISA string. We should have plenty of time to get those fixed up before the series gets applied.

Also, as it looks like the pinctrl driver is going to land in time for v6.3, that leaves just this series and the clock driver required for base support.

In the original submission, you sent the clock driver and dt in the same series & I think it might make the process a bit faster if you sent them both together for the next version again.

That way, both the drivers and dts can go together as their have an inter dependence.

That's my opinion anyway, will make trying to sequence things between trees easier.

Cheers,
Conor.

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author
#29 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 20 lines · Conor Dooley
Quoted 10 lines · Conor Dooley
Quoted 4 lines · Hal Feng
Quoted 3 lines · Conor Dooley
Quoted 2 lines · Hal Feng
Quoted 1 line · Conor Dooley

On Tue, Dec 20, 2022 at 09:12:46AM +0800, Hal Feng wrote:

FWIW, the deadline for getting material in for v6.3 has already passed, so you can send the next version of this series without waiting for clarification on the compatibles & ISA string. We should have plenty of time to get those fixed up before the series gets applied.

Also, as it looks like the pinctrl driver is going to land in time for v6.3, that leaves just this series and the clock driver required for base support.

In the original submission, you sent the clock driver and dt in the same series & I think it might make the process a bit faster if you sent them both together for the next version again.

Quoted 5 lines

That way, both the drivers and dts can go together as their have an inter dependence.

That's my opinion anyway, will make trying to sequence things between trees easier.

Good idea. But how can I write the change log if we do so? Will it make the history confused? Thanks.

Best regards,
Hal

#30 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 21 lines · Hal Feng
Quoted 20 lines · Conor Dooley
Quoted 10 lines · Conor Dooley
Quoted 4 lines · Hal Feng
Quoted 3 lines · Conor Dooley
Quoted 2 lines · Hal Feng
Quoted 1 line · Conor Dooley

On Tue, Dec 20, 2022 at 09:12:46AM +0800, Hal Feng wrote:

FWIW, the deadline for getting material in for v6.3 has already passed, so you can send the next version of this series without waiting for clarification on the compatibles & ISA string. We should have plenty of time to get those fixed up before the series gets applied.

Also, as it looks like the pinctrl driver is going to land in time for v6.3, that leaves just this series and the clock driver required for base support.

In the original submission, you sent the clock driver and dt in the same series & I think it might make the process a bit faster if you sent them both together for the next version again.

Quoted 8 lines
Quoted 5 lines

That way, both the drivers and dts can go together as their have an inter dependence.

That's my opinion anyway, will make trying to sequence things between trees easier.

Good idea. But how can I write the change log if we do so? Will it make the history confused? Thanks.

I'm not sure what you mean w.r.t. history.
Both series are on V3 I think, so just make the next version v4 title it something like "Basic clock, reset & dt support..."
For the changelogs, just mention you merged the two series again in the cover letter & add the changelogs that you would have made for each series to the cover as a single changelog.
Say somewhere in the cover that I suggested merging the series together so that they could go via the same tree as the dt-binding headers are required by both driver & devicetree.

Cheers,
Conor.

author
#31 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 31 lines · Conor Dooley
Quoted 21 lines · Hal Feng
Quoted 20 lines · Conor Dooley
Quoted 10 lines · Conor Dooley
Quoted 4 lines · Hal Feng
Quoted 3 lines · Conor Dooley
Quoted 2 lines · Hal Feng
Quoted 1 line · Conor Dooley

On Tue, Dec 20, 2022 at 09:12:46AM +0800, Hal Feng wrote:

FWIW, the deadline for getting material in for v6.3 has already passed, so you can send the next version of this series without waiting for clarification on the compatibles & ISA string. We should have plenty of time to get those fixed up before the series gets applied.

Also, as it looks like the pinctrl driver is going to land in time for v6.3, that leaves just this series and the clock driver required for base support.

In the original submission, you sent the clock driver and dt in the same series & I think it might make the process a bit faster if you sent them both together for the next version again.

Quoted 8 lines
Quoted 5 lines

That way, both the drivers and dts can go together as their have an inter dependence.

That's my opinion anyway, will make trying to sequence things between trees easier.

Good idea. But how can I write the change log if we do so? Will it make the history confused? Thanks.

Quoted 9 lines

I'm not sure what you mean w.r.t. history.
Both series are on V3 I think, so just make the next version v4 title it something like "Basic clock, reset & dt support..."
For the changelogs, just mention you merged the two series again in the cover letter & add the changelogs that you would have made for each series to the cover as a single changelog.
Say somewhere in the cover that I suggested merging the series together so that they could go via the same tree as the dt-binding headers are required by both driver & devicetree.

OK, I see. I will merge the clock patch series [1] and this DT patch series in v4. Thanks for your suggestions.

[1] https://lore.kernel.org/all/[email protected]/

Best regards,
Hal

author
#32 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

In reply to Conor Dooley
Quoted 55 lines · Conor Dooley
Quoted 28 lines · Hal Feng
Quoted 27 lines · Conor Dooley
Quoted 25 lines · Hal Feng
Quoted 15 lines · Conor Dooley

arch/riscv/boot/dts/starfive/jh7110.dtsi line 21

Quoted 11 lines · Hal Feng
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +11 -0
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc.

U74-MC has 5 cores including 1 * S7 core and 4 * U74 cores.

Quoted 2 lines

Which is correct? Your docs say S7 and S76, so I would imagine that it is actually an S76?

I found SiFive website [1] call it S76, but call it S7 in other places. So I misunderstood this. Considering the ISA difference you described as below, I think it's proper to change the label to "S7_0".

Quoted 7 lines
Quoted 4 lines

I'm less worried about the label & more interested in the compatible. hart0 is, as you say, not a u74. Should we not be adding a "sifive,s7" compatible string to Documentation/devicetree/bindings/riscv/cpus.yaml and using that here instead?

First of all, it's my fault that I didn't check the revision of U74-MC manual, so most of my previous replies might not make sense.

No that's fine. The manual stuff confused me too when I went looking initially, and I still get get mixed up by the fact that there are core-complex manuals but not core manuals.

Quoted 4 lines

If we add a new compatible string for S7, should we change the compatibles of hart1~3 to "sifive,u74" also? And then, there may be no point keeping some compatible strings of core complex like "sifive,u74-mc" and "sifive,u54-mc". I'm not sure about this.

[...]

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Yes, "RV64IMAC" is correct. The monitor core in U74-MC is a S7-series core, not S76.

Cool, thanks.

Now I think it might be another version of S76.

Quoted 7 lines

The SiFive docs describe the u74-mc core complex, which AFAIU you have, as being 1x S7 & 4x U7.

I'd be happy with new binding for "sifive,s7" & then we use that here. If you're sure it's S76, we can also use that. S76 is described, in what docs I can see, as a core complex containing an S7, so S7 seems likely to be correct?

I will add a new binding for "sifive,s7" and modify the code as follows.

	S7_0: cpu@0 {
		compatible = "sifive,s7", "riscv";
		...
		riscv,isa = "rv64imac_zicsr_zba_zbb";
		...
	};
Quoted 68 lines

u7, u74 & u74-mc are valid compatibles, added by SiFive, in commit 75e6d7248efc ("dt-bindings: riscv: Update DT binding docs to support SiFive FU740 SoC"). Unfortunately, they never actually *used* those compatibles for anything, and just used "sifive,bullet0" for the fu740.

I'll accept any of u7, u74 or u74-mc for those harts.

Quoted 42 lines
Quoted 34 lines
Quoted 32 lines
Quoted 23 lines
Quoted 23 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +23 -0
+			tlb-split;
+			status = "disabled";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		U74_1: cpu@1 {
+			compatible = "sifive,u74-mc", "riscv";
+			reg = <1>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <64>;
+			d-cache-size = <32768>;
+			d-tlb-sets = <1>;
+			d-tlb-size = <40>;
+			device_type = "cpu";
+			i-cache-block-size = <64>;
+			i-cache-sets = <64>;
+			i-cache-size = <32768>;
+			i-tlb-sets = <1>;
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110.dtsi line 63

Quoted 4 lines
arch/riscv/boot/dts/starfive/jh7110.dtsi new file +4 -0
+			i-tlb-size = <40>;
+			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
+			riscv,isa = "rv64imafdc";

That also begs the question:
Do your u74s support RV64GBC, as the (current) SiFive documentation suggests?

Quoted 1 line

Actually, U74 doesn't support the full B extension, and the SiFive doc [1]

Quoted 7 lines

Yeah, I knew asking that question that the "RV64GBC" in SiFive's online documentation was using outdated terminology. Also, that is not the doc for your core complex as far as I can tell. That is the document for impid 0x0621_1222, whereas (IIRC) your core is 0x0421_0427. Jess and I had a look one evening but could not find the 21G1.02.00 revision of this document, which is the one corresponding to 0x421_0427. See Table 92 for more details.

Quoted 2 lines

I found the 21G1.02.00 revision on StarFive internal net, but I'm not sure whether I can make it public and I am checking this.

Yeah, certainly don't do anything without cross-checking!

Quoted 4 lines

This revision records
that the ISA of 21G1.02.00 U74 is "RV64GCB" and ISA of 21G1.02.00 S7 is "RV64IMACB". I am asking someone to check with SiFive whether both 21G1.02.00 U74 and S7 support the full B extension.

Having cross-checked a 21G1.01.00 document against a 21G2.01.00 one, I'm 99% sure that you have _Zba_Zbb.
The G2.01 document says _Zba_Zbb & has the same instructions listed as supported as the G1.02 one.
I've also tried the Zbb support patches posted by Heiko [2] on a VisionFive V2 and had them work - which is why a definitive statement on the version of Zbb supported would be really great to have!

Quoted 1 line

[2] - https://lore.kernel.org/linux-riscv/[email protected]/

The 21G1.02.00 document is still not allowed to be public so far. By comparing with instructions included in b extensions [1], I can confirm that the 21G1.02.00 only supports Zba and Zbb.

[1] https://github.com/riscv/riscv-bitmanip/blob/main/bitmanip/overview.adoc#

Zicsr is also supported as described in 21G1.02.00 document. So I will modify as follows.

	U74_1: cpu@1 {
		compatible = "sifive,u74-mc", "riscv";
		...
		riscv,isa = "rv64imafdc_zicsr_zba_zbb";
		...
	};

Best regards,
Hal

Quoted 34 lines
Quoted 11 lines
Quoted 11 lines
Quoted 3 lines

describes the ISA of U74 is "RV64GC_Zba_Zbb_Sscofpmf" which "G" includes "IMAFD".

I could not find the 21G1.02.00 version of this document online, but I was able to find the 21G1.01.00 version of it & that version does not support the Sscofpmf extension (but does have Zba/Zbb support).

Quoted 2 lines

"_Zba_Zbb_Sscofpmf" is not shown in other device trees such as jh7100.dtsi and fu740-c000.dtsi, so I didn't show them here.

Quoted 6 lines
Quoted 4 lines

Just because other devicetrees omit them, doesn't mean that you should too!
This compatible should be an accurate description of your hardware, so you should add what you actually have.

Will keep it in mind. Thank you.

FWIW, the deadline for getting material in for v6.3 has already passed, so you can send the next version of this series without waiting for clarification on the compatibles & ISA string. We should have plenty of time to get those fixed up before the series gets applied.

Thanks,
Conor.

Quoted 8 lines
Quoted 8 lines

If you have Zba and Zbb, then add them.
I would double check against your internal documentation for 21G2.02.00 as to whether you do have Sscofpmf, and if you do, add that too!

That way, whenever support for those extensions lands, the jh7110 will automatically pick it up, rather than needing to have them retrofitted.

Quoted 1 line

[1] https://sifive.cdn.prismic.io/sifive/2dd11994-693c-4360-8aea-5453d8642c42_u74mc_core_complex_manual_21G3.pdf

maintainer
#33 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Hey Hal!

Quoted 64 lines · Hal Feng
Quoted 55 lines · Conor Dooley
Quoted 28 lines · Hal Feng
Quoted 27 lines · Conor Dooley
Quoted 25 lines · Hal Feng
Quoted 15 lines · Conor Dooley

Review

Quoted 11 lines · Hal Feng
(quoted diff) +11 -0
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc.

U74-MC has 5 cores including 1 * S7 core and 4 * U74 cores.

Quoted 2 lines

Which is correct? Your docs say S7 and S76, so I would imagine that it is actually an S76?

I found SiFive website [1] call it S76, but call it S7 in other places. So I misunderstood this. Considering the ISA difference you described as below, I think it's proper to change the label to "S7_0".

Quoted 7 lines
Quoted 4 lines

I'm less worried about the label & more interested in the compatible. hart0 is, as you say, not a u74. Should we not be adding a "sifive,s7" compatible string to Documentation/devicetree/bindings/riscv/cpus.yaml and using that here instead?

First of all, it's my fault that I didn't check the revision of U74-MC manual, so most of my previous replies might not make sense.

No that's fine. The manual stuff confused me too when I went looking initially, and I still get get mixed up by the fact that there are core-complex manuals but not core manuals.

Quoted 4 lines

If we add a new compatible string for S7, should we change the compatibles of hart1~3 to "sifive,u74" also? And then, there may be no point keeping some compatible strings of core complex like "sifive,u74-mc" and "sifive,u54-mc". I'm not sure about this.

[...]

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Yes, "RV64IMAC" is correct. The monitor core in U74-MC is a S7-series core, not S76.

Cool, thanks.

Now I think it might be another version of S76.

Quoted 7 lines

The SiFive docs describe the u74-mc core complex, which AFAIU you have, as being 1x S7 & 4x U7.

I'd be happy with new binding for "sifive,s7" & then we use that here. If you're sure it's S76, we can also use that. S76 is described, in what docs I can see, as a core complex containing an S7, so S7 seems likely to be correct?

Quoted 6 lines

I will add a new binding for "sifive,s7" and modify the code as follows.

	S7_0: cpu@0 {
		compatible = "sifive,s7", "riscv";
		...
		riscv,isa = "rv64imac_zicsr_zba_zbb";

I'm not sure that I'd bother with the zicsr, it gets added automagically by the Makefile if needed:

| # Newer binutils versions default to ISA spec version 20191213 which moves some
| # instructions from the I extension to the Zicsr and Zifencei extensions.
| toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
| riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei

Otherwise, thanks for the actual confirmation of zba/zbb!

Thanks,
Conor.

maintainer
#34 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 69 lines · Conor Dooley

Hey Hal!

Quoted 66 lines · Hal Feng
Quoted 55 lines · Conor Dooley
Quoted 28 lines · Hal Feng
Quoted 27 lines · Conor Dooley
Quoted 25 lines · Hal Feng
Quoted 15 lines · Conor Dooley

Review

Quoted 11 lines · Hal Feng
(quoted diff) +11 -0
+/ {
+	compatible = "starfive,jh7110";
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		S76_0: cpu@0 {
+			compatible = "sifive,u74-mc", "riscv";

The label here says S76 but the compatible says u74-mc.

U74-MC has 5 cores including 1 * S7 core and 4 * U74 cores.

Quoted 2 lines

Which is correct? Your docs say S7 and S76, so I would imagine that it is actually an S76?

I found SiFive website [1] call it S76, but call it S7 in other places. So I misunderstood this. Considering the ISA difference you described as below, I think it's proper to change the label to "S7_0".

Quoted 7 lines
Quoted 4 lines

I'm less worried about the label & more interested in the compatible. hart0 is, as you say, not a u74. Should we not be adding a "sifive,s7" compatible string to Documentation/devicetree/bindings/riscv/cpus.yaml and using that here instead?

First of all, it's my fault that I didn't check the revision of U74-MC manual, so most of my previous replies might not make sense.

No that's fine. The manual stuff confused me too when I went looking initially, and I still get get mixed up by the fact that there are core-complex manuals but not core manuals.

Quoted 4 lines

If we add a new compatible string for S7, should we change the compatibles of hart1~3 to "sifive,u74" also? And then, there may be no point keeping some compatible strings of core complex like "sifive,u74-mc" and "sifive,u54-mc". I'm not sure about this.

[...]

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Yes, "RV64IMAC" is correct. The monitor core in U74-MC is a S7-series core, not S76.

Cool, thanks.

Now I think it might be another version of S76.

Quoted 7 lines

The SiFive docs describe the u74-mc core complex, which AFAIU you have, as being 1x S7 & 4x U7.

I'd be happy with new binding for "sifive,s7" & then we use that here. If you're sure it's S76, we can also use that. S76 is described, in what docs I can see, as a core complex containing an S7, so S7 seems likely to be correct?

I will add a new binding for "sifive,s7" and modify the code as follows.

Quoted 7 lines
Quoted 4 lines
	S7_0: cpu@0 {
		compatible = "sifive,s7", "riscv";
		...
		riscv,isa = "rv64imac_zicsr_zba_zbb";

I'm not sure that I'd bother with the zicsr, it gets added automagically by the Makefile if needed:

Meh, I probably shouldn't have replied to this first thing in the morning as this comment of mine doesn't really make sense. I skipped the middle part of my point here...
What I meant was that you can avoid zicsr & zifencei because when the binding was defined they were included in i. I meant to use the following as a kinda explanation of it depending on the version of the ISA spec & that we just assume that zicsr & zifencei are present. I suppose you can add them to the isa string if you like, dtbs_check shouldn't complain!

Quoted 10 lines
| # Newer binutils versions default to ISA spec version 20191213 which moves some
| # instructions from the I extension to the Zicsr and Zifencei extensions.
| toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
| riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei

Otherwise, thanks for the actual confirmation of zba/zbb!

Thanks,
Conor.

author
#35 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

In reply to Hal Feng
Quoted 10 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

Quoted 4 lines
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411 +++++++++++++++++++++++
 1 file changed, 411 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi

I wanna add i2c nodes (i2c0-6) in the next version, so someone else can use them when they submit i2c driver patches.

Best regards,
Hal

maintainer
#36 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 16 lines · Hal Feng
Quoted 10 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

Quoted 4 lines
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411 +++++++++++++++++++++++
 1 file changed, 411 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi
Quoted 2 lines

I wanna add i2c nodes (i2c0-6) in the next version, so someone else can use them when they submit i2c driver patches.

All of the other series depend on this one for enablement,
so unless the binding for i2c is already upstream I'd advise keeping it separate.

Cheers,
Conor.

author
#37 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 17 lines · Conor Dooley
Quoted 16 lines · Hal Feng
Quoted 14 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411 +++++++++++++++++++++++
 1 file changed, 411 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi
Quoted 5 lines
Quoted 2 lines

I wanna add i2c nodes (i2c0-6) in the next version, so someone else can use them when they submit i2c driver patches.

All of the other series depend on this one for enablement,
so unless the binding for i2c is already upstream I'd advise keeping it separate.

The i2c IP of JH7110 is from Synopsys and the same as the i2c IP in JH7100. The binding and driver for i2c are already upstream. It works as long as we add the i2c nodes and configure pins for i2c in device tree. It will simplify the dependency if we do that.

By the way, I am checking the ISA of U74-MC on JH7110 with someone else. I will reply you today.

Best regards,
Hal

maintainer
#38 lore raw

Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

Quoted 21 lines · Hal Feng
Quoted 20 lines · Conor Dooley
Quoted 18 lines · Hal Feng
Quoted 10 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add initial device tree for the JH7110 RISC-V SoC by StarFive Technology Ltd.

Quoted 4 lines
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 411 +++++++++++++++++++++++
 1 file changed, 411 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi

I wanna add i2c nodes (i2c0-6) in the next version, so someone else can use them when they submit i2c driver patches.

Quoted 7 lines
Quoted 2 lines

All of the other series depend on this one for enablement,
so unless the binding for i2c is already upstream I'd advise keeping it separate.

The i2c IP of JH7110 is from Synopsys and the same as the i2c IP in JH7100. The binding and driver for i2c are already upstream. It works as long as we add the i2c nodes and configure pins for i2c in device tree. It will simplify the dependency if we do that.

Please make sure that you add a device specific compatible for jh7110 then, thanks.

Quoted 2 lines

By the way, I am checking the ISA of U74-MC on JH7110 with someone else. I will reply you today.

Cool!

author
#39 lore raw

PATCH v3 7/7 riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

In reply to Hal Feng

From: Emil Renner Berthing <[email protected]>

Add a minimal device tree for StarFive JH7110 VisionFive 2 board which has version A and version B. Support booting and basic clock/reset/pinctrl/uart drivers.

---
 arch/riscv/boot/dts/starfive/Makefile         |   1 +
 .../jh7110-starfive-visionfive-2-va.dts       |  13 ++
 .../jh7110-starfive-visionfive-2-vb.dts       |  13 ++
 .../jh7110-starfive-visionfive-2.dtsi         | 111 ++++++++++++++++++
 4 files changed, 138 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
arch/riscv/boot/dts/starfive/Makefile +1 -0
index 0ea1bc15ab30..79e925a4a227 100644
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb jh7110-starfive-visionfive-2-vb.dtb
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts new file +13 -0
new file mode 100644
index 000000000000..188d3fddbe88
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110-starfive-visionfive-2.dtsi"
+
+/ {
+	model = "StarFive VisionFive 2 VA";
+	compatible = "starfive,visionfive-2-va", "starfive,jh7110";
+};
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts new file +13 -0
new file mode 100644
index 000000000000..f75c10536f84
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110-starfive-visionfive-2.dtsi"
+
+/ {
+	model = "StarFive VisionFive 2 VB";
+	compatible = "starfive,visionfive-2-vb", "starfive,jh7110";
+};
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +111 -0

111 lines not on this page · raw message

Signature · 1 line
2.38.1
maintainer Acked-by on v3 7/7
#40 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 26 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add a minimal device tree for StarFive JH7110 VisionFive 2 board which has version A and version B. Support booting and basic clock/reset/pinctrl/uart drivers.

---
 arch/riscv/boot/dts/starfive/Makefile         |   1 +
 .../jh7110-starfive-visionfive-2-va.dts       |  13 ++
 .../jh7110-starfive-visionfive-2-vb.dts       |  13 ++
 .../jh7110-starfive-visionfive-2.dtsi         | 111 ++++++++++++++++++
 4 files changed, 138 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
arch/riscv/boot/dts/starfive/Makefile +0 -0
index 0ea1bc15ab30..79e925a4a227 100644

arch/riscv/boot/dts/starfive/Makefile line 3

Quoted 3 lines
arch/riscv/boot/dts/starfive/Makefile +1 -0
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb jh7110-starfive-visionfive-2-vb.dtb

Could you rebase on top of v6.2-rc1 when you submit your next version squash this in please (unless Emil hates it):

arch/riscv/boot/dts/starfive/Makefile +4 -2
index c38a9ade7f48..b3744420253a 100644
@@ -1,3 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb jh7100-starfive-visionfive-v1.dtb
-dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb jh7110-starfive-visionfive-2-vb.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7100-starfive-visionfive-v1.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-vb.dtb

I'd rather have more, but easier to read lines than long ones.

Quoted 23 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +17 -0
new file mode 100644
index 000000000000..c60280b89c73
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110.dtsi"
+#include "jh7110-pinfunc.h"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi line 27

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +10 -0
+		stdout-path = "serial0:115200n8";
+	};
+
+	cpus {
+		timebase-frequency = <4000000>;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x0 0x40000000 0x1 0x0>;

Is this a good idea when you have SKUs with 2, 4 & 8 GiB of DDR?

Anyways, I can't review this as I've got neither board nor documentation, so with the above stuff sorted out:

I'll not apply it until the clock binding header is in my tree.

Thanks,
Conor.

author
#41 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 22 lines · Conor Dooley
Quoted 21 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add a minimal device tree for StarFive JH7110 VisionFive 2 board which has version A and version B. Support booting and basic clock/reset/pinctrl/uart drivers.

---
 arch/riscv/boot/dts/starfive/Makefile         |   1 +
 .../jh7110-starfive-visionfive-2-va.dts       |  13 ++
 .../jh7110-starfive-visionfive-2-vb.dts       |  13 ++
 .../jh7110-starfive-visionfive-2.dtsi         | 111 ++++++++++++++++++
 4 files changed, 138 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
Quoted 10 lines

arch/riscv/boot/dts/starfive/Makefile line 3

Quoted 8 lines
arch/riscv/boot/dts/starfive/Makefile +1 -0
index 0ea1bc15ab30..79e925a4a227 100644
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb jh7110-starfive-visionfive-2-vb.dtb

Could you rebase on top of v6.2-rc1 when you submit your next version

Sure.

Quoted 14 lines

squash this in please (unless Emil hates it):

arch/riscv/boot/dts/starfive/Makefile +4 -2
index c38a9ade7f48..b3744420253a 100644
@@ -1,3 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb jh7100-starfive-visionfive-v1.dtb
-dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb jh7110-starfive-visionfive-2-vb.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7100-starfive-visionfive-v1.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-vb.dtb
Quoted 1 line

I'd rather have more, but easier to read lines than long ones.

I'm fine with this change. If no objection, I will modify it.

Best regards,
Hal

Quoted 28 lines
Quoted 28 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +21 -0
new file mode 100644
index 000000000000..c60280b89c73
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110.dtsi"
+#include "jh7110-pinfunc.h"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	cpus {
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi line 27

Quoted 6 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +6 -0
+		timebase-frequency = <4000000>;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x0 0x40000000 0x1 0x0>;

Is this a good idea when you have SKUs with 2, 4 & 8 GiB of DDR?

The VisionFive 2 board only has 4GB version and 8GB version now. Before linux startup, we will change this property in dtb through u-boot to make sure the board can boot up with the correct memory size.

Best regards,
Hal

Quoted 5 lines

Anyways, I can't review this as I've got neither board nor documentation, so with the above stuff sorted out:

I'll not apply it until the clock binding header is in my tree.

maintainer
#42 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 51 lines · Hal Feng
Quoted 41 lines · Conor Dooley
Quoted 40 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add a minimal device tree for StarFive JH7110 VisionFive 2 board which has version A and version B. Support booting and basic clock/reset/pinctrl/uart drivers.

---

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +21 -0
new file mode 100644
index 000000000000..c60280b89c73
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110.dtsi"
+#include "jh7110-pinfunc.h"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	cpus {
Quoted 8 lines

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi line 27

Quoted 6 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +6 -0
+		timebase-frequency = <4000000>;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x0 0x40000000 0x1 0x0>;

Is this a good idea when you have SKUs with 2, 4 & 8 GiB of DDR?

Quoted 3 lines

The VisionFive 2 board only has 4GB version and 8GB version now. Before linux startup, we will change this property in dtb through u-boot to make sure the board can boot up with the correct memory size.

Ah I see. Thanks.

maintainer
#43 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

In reply to Hal Feng

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi line 19

Quoted 7 lines · Hal Feng
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +7 -0
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};

So I think this is wrong, and the stdout-path should be uart3 instead. Per the QSG [0], GPIO5/6 are the suggested UART Tx/Rx to use. This appears to map to uart3 rather than uart0.
FWIW, uart3 is also the stdout-path for the v1, see:
arch/riscv/boot/dts/starfive/jh7100-common.dtsi

At least, that change is what I needed to do in order to use the JH7110_VisionFive2_upstream branch, AFAICT matches what you've got in this series.

Thanks,
Conor.

0 - https://doc-en.rvspace.org/VisionFive2/PDF/VisionFive2_QSG.pdf

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

maintainer
#44 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 16 lines · Conor Dooley

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi line 19

Quoted 7 lines · Hal Feng
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +7 -0
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};

So I think this is wrong, and the stdout-path should be uart3 instead. Per the QSG [0], GPIO5/6 are the suggested UART Tx/Rx to use. This appears to map to uart3 rather than uart0.
FWIW, uart3 is also the stdout-path for the v1, see:
arch/riscv/boot/dts/starfive/jh7100-common.dtsi

Quoted 3 lines

At least, that change is what I needed to do in order to use the JH7110_VisionFive2_upstream branch, AFAICT matches what you've got in this series.

I went and re-tried things again today, but with the clk & pinctrl series from the ML instead of that branch.
I'd gone and used that branch instead of the patches, as there were some build issues with the pinctrl driver from the patches.
I fixed the build issues in the pinctrl driver and now uart0 works.

Looking at it again, it makes sense for uart0 to be serial0, given there's pinconf stuff being done in this DT for pins 5 & 6 for uart0.

Perhaps by using that branch, I was using some older drivers etc given it seems to mostly contain commits dating from November? I'm still a bit confused about the whole thing, given I tried out a whole load of branches to get something booting with ethernet support that day!

Since this seems to work with the fixed pinctrl driver, I don't think you need to change anything here.

Apologies for the noise!
Conor.

#45 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

In reply to Hal Feng
Quoted 57 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add a minimal device tree for StarFive JH7110 VisionFive 2 board which has version A and version B. Support booting and basic clock/reset/pinctrl/uart drivers.

---
 arch/riscv/boot/dts/starfive/Makefile         |   1 +
 .../jh7110-starfive-visionfive-2-va.dts       |  13 ++
 .../jh7110-starfive-visionfive-2-vb.dts       |  13 ++
 .../jh7110-starfive-visionfive-2.dtsi         | 111 ++++++++++++++++++
 4 files changed, 138 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
arch/riscv/boot/dts/starfive/Makefile +1 -0
index 0ea1bc15ab30..79e925a4a227 100644
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb jh7110-starfive-visionfive-2-vb.dtb
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts new file +13 -0
new file mode 100644
index 000000000000..188d3fddbe88
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110-starfive-visionfive-2.dtsi"
+
+/ {
+       model = "StarFive VisionFive 2 VA";
+       compatible = "starfive,visionfive-2-va", "starfive,jh7110";
+};
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts new file +2 -0
new file mode 100644
index 000000000000..f75c10536f84
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts line 12

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts new file +10 -0
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110-starfive-visionfive-2.dtsi"
+
+/ {
+       model = "StarFive VisionFive 2 VB";
+       compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Hi Hal,

I just want to double check, is "VisionFive 2 VA" / "visoinfive-2-va" and "VisionFive 2 VB" / "visionfive-2-vb" really what you want? I still think having these names match what is printed on the silkscreen makes it a lot easier for everybody. Even your own releases calls the boards "v1.2A" and "v1.3B":
https://github.com/starfive-tech/VisionFive2/releases/

So I'd suggest
model = "StarFive VisionFive 2 v1.3B";
compatible = "starfive,visionfive-2-v1.3b", "starfive,jh7110";

I haven't seen these "VA" and "VB" anywhere else, so if you don't want the version numbers and can promise that there will be no incompatible future revisions of the boards then maybe just drop the "V". Eg. model = "StarFive VisionFive 2 B";
compatible = "starfive,visionfive-2-b", "starfive,jh7110";

/Emil

Quoted 121 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts new file +1 -0
+};
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi new file +111 -1
new file mode 100644
index 000000000000..c60280b89c73
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110.dtsi"
+#include "jh7110-pinfunc.h"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+       aliases {
+               serial0 = &uart0;
+       };
+
+       chosen {
+               stdout-path = "serial0:115200n8";
+       };
+
+       cpus {
+               timebase-frequency = <4000000>;
+       };
+
+       memory@40000000 {
+               device_type = "memory";
+               reg = <0x0 0x40000000 0x1 0x0>;
+       };
+
+       gpio-restart {
+               compatible = "gpio-restart";
+               gpios = <&gpio 35 GPIO_ACTIVE_HIGH>;
+               priority = <224>;
+       };
+};
+
+&osc {
+       clock-frequency = <24000000>;
+};
+
+&rtc_osc {
+       clock-frequency = <32768>;
+};
+
+&gmac0_rmii_refin {
+       clock-frequency = <50000000>;
+};
+
+&gmac0_rgmii_rxin {
+       clock-frequency = <125000000>;
+};
+
+&gmac1_rmii_refin {
+       clock-frequency = <50000000>;
+};
+
+&gmac1_rgmii_rxin {
+       clock-frequency = <125000000>;
+};
+
+&i2stx_bclk_ext {
+       clock-frequency = <12288000>;
+};
+
+&i2stx_lrck_ext {
+       clock-frequency = <192000>;
+};
+
+&i2srx_bclk_ext {
+       clock-frequency = <12288000>;
+};
+
+&i2srx_lrck_ext {
+       clock-frequency = <192000>;
+};
+
+&tdm_ext {
+       clock-frequency = <49152000>;
+};
+
+&mclk_ext {
+       clock-frequency = <12288000>;
+};
+
+&uart0 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&uart0_pins>;
+       status = "okay";
+};
+
+&gpio {
+       uart0_pins: uart0-0 {
+               tx-pins {
+                       pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>;
+                       bias-disable;
+                       drive-strength = <12>;
+                       input-disable;
+                       input-schmitt-disable;
+                       slew-rate = <0>;
+               };
+
+               rx-pins {
+                       pinmux = <GPIOMUX(6, GPOUT_LOW, GPOEN_DISABLE, GPI_SYS_UART0_RX)>;
+                       bias-disable; /* external pull-up */
+                       drive-strength = <2>;
+                       input-enable;
+                       input-schmitt-enable;
+                       slew-rate = <0>;
+               };
+       };
+};
--

2.38.1

author
#46 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 82 lines · Emil Renner Berthing
Quoted 57 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add a minimal device tree for StarFive JH7110 VisionFive 2 board which has version A and version B. Support booting and basic clock/reset/pinctrl/uart drivers.

---
 arch/riscv/boot/dts/starfive/Makefile         |   1 +
 .../jh7110-starfive-visionfive-2-va.dts       |  13 ++
 .../jh7110-starfive-visionfive-2-vb.dts       |  13 ++
 .../jh7110-starfive-visionfive-2.dtsi         | 111 ++++++++++++++++++
 4 files changed, 138 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
arch/riscv/boot/dts/starfive/Makefile +1 -0
index 0ea1bc15ab30..79e925a4a227 100644
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb jh7110-starfive-visionfive-2-vb.dtb
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts new file +13 -0
new file mode 100644
index 000000000000..188d3fddbe88
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110-starfive-visionfive-2.dtsi"
+
+/ {
+       model = "StarFive VisionFive 2 VA";
+       compatible = "starfive,visionfive-2-va", "starfive,jh7110";
+};
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts new file +2 -0
new file mode 100644
index 000000000000..f75c10536f84
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts line 12

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts new file +10 -0
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110-starfive-visionfive-2.dtsi"
+
+/ {
+       model = "StarFive VisionFive 2 VB";
+       compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Hi Hal,

I just want to double check, is "VisionFive 2 VA" / "visoinfive-2-va" and "VisionFive 2 VB" / "visionfive-2-vb" really what you want? I still think having these names match what is printed on the silkscreen makes it a lot easier for everybody. Even your own releases calls the boards "v1.2A" and "v1.3B":
https://github.com/starfive-tech/VisionFive2/releases/

So I'd suggest
model = "StarFive VisionFive 2 v1.3B";
compatible = "starfive,visionfive-2-v1.3b", "starfive,jh7110";

Quoted 5 lines

I haven't seen these "VA" and "VB" anywhere else, so if you don't want the version numbers and can promise that there will be no incompatible future revisions of the boards then maybe just drop the "V". Eg. model = "StarFive VisionFive 2 B";
compatible = "starfive,visionfive-2-b", "starfive,jh7110";

The version A board has reached the end of life. As far as I know, the version B board will not update also unless there are some important requirements and StarFive decides to update. Furthermore, it's too late to change the compatible as patch 1 was already accepted. Will it be easier to read if I modify it as below?

	model = "StarFive VisionFive 2 vB";
	compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Best regards,
Hal

#47 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 90 lines · Hal Feng
Quoted 82 lines · Emil Renner Berthing
Quoted 57 lines · Hal Feng

From: Emil Renner Berthing <[email protected]>

Add a minimal device tree for StarFive JH7110 VisionFive 2 board which has version A and version B. Support booting and basic clock/reset/pinctrl/uart drivers.

---
 arch/riscv/boot/dts/starfive/Makefile         |   1 +
 .../jh7110-starfive-visionfive-2-va.dts       |  13 ++
 .../jh7110-starfive-visionfive-2-vb.dts       |  13 ++
 .../jh7110-starfive-visionfive-2.dtsi         | 111 ++++++++++++++++++
 4 files changed, 138 insertions(+)
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts
 create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
arch/riscv/boot/dts/starfive/Makefile +1 -0
index 0ea1bc15ab30..79e925a4a227 100644
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb
+dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-2-va.dtb jh7110-starfive-visionfive-2-vb.dtb
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-va.dts new file +13 -0
new file mode 100644
index 000000000000..188d3fddbe88
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110-starfive-visionfive-2.dtsi"
+
+/ {
+       model = "StarFive VisionFive 2 VA";
+       compatible = "starfive,visionfive-2-va", "starfive,jh7110";
+};
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts new file +2 -0
new file mode 100644
index 000000000000..f75c10536f84
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts line 12

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts new file +10 -0
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+ */
+
+/dts-v1/;
+#include "jh7110-starfive-visionfive-2.dtsi"
+
+/ {
+       model = "StarFive VisionFive 2 VB";
+       compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Hi Hal,

I just want to double check, is "VisionFive 2 VA" / "visoinfive-2-va" and "VisionFive 2 VB" / "visionfive-2-vb" really what you want? I still think having these names match what is printed on the silkscreen makes it a lot easier for everybody. Even your own releases calls the boards "v1.2A" and "v1.3B":
https://github.com/starfive-tech/VisionFive2/releases/

So I'd suggest
model = "StarFive VisionFive 2 v1.3B";
compatible = "starfive,visionfive-2-v1.3b", "starfive,jh7110";

Quoted 5 lines

I haven't seen these "VA" and "VB" anywhere else, so if you don't want the version numbers and can promise that there will be no incompatible future revisions of the boards then maybe just drop the "V". Eg. model = "StarFive VisionFive 2 B";
compatible = "starfive,visionfive-2-b", "starfive,jh7110";

Quoted 8 lines

The version A board has reached the end of life. As far as I know, the version B board will not update also unless there are some important requirements and StarFive decides to update. Furthermore, it's too late to change the compatible as patch 1 was already accepted. Will it be easier to read if I modify it as below?

        model = "StarFive VisionFive 2 vB";
        compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Oh, that's sad that the kernel will now end up calling the boards something that's used nowhere else, even by StarFive :/ But yeah, I guess vA and vB are a little easier to read.

Quoted 2 lines

Best regards,
Hal

#48 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 32 lines · Emil Renner Berthing
Quoted 31 lines · Hal Feng
Quoted 18 lines · Emil Renner Berthing

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts line 12

Quoted 2 lines · Hal Feng
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-vb.dts new file +2 -0
+       model = "StarFive VisionFive 2 VB";
+       compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Hi Hal,

I just want to double check, is "VisionFive 2 VA" / "visoinfive-2-va" and "VisionFive 2 VB" / "visionfive-2-vb" really what you want? I still think having these names match what is printed on the silkscreen makes it a lot easier for everybody. Even your own releases calls the boards "v1.2A" and "v1.3B":
https://github.com/starfive-tech/VisionFive2/releases/

So I'd suggest
model = "StarFive VisionFive 2 v1.3B";
compatible = "starfive,visionfive-2-v1.3b", "starfive,jh7110";

Quoted 5 lines

I haven't seen these "VA" and "VB" anywhere else, so if you don't want the version numbers and can promise that there will be no incompatible future revisions of the boards then maybe just drop the "V". Eg. model = "StarFive VisionFive 2 B";
compatible = "starfive,visionfive-2-b", "starfive,jh7110";

The version A board has reached the end of life. As far as I know, the version B board will not update also unless there are some important requirements and StarFive decides to update. Furthermore, it's too late to change the compatible as patch 1 was already accepted. Will it be easier to read if I modify it as below?

Quoted 6 lines
Quoted 2 lines
        model = "StarFive VisionFive 2 vB";
        compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Oh, that's sad that the kernel will now end up calling the boards something that's used nowhere else, even by StarFive :/ But yeah, I guess vA and vB are a little easier to read.

Nothing has been released with that name so AFAIU we can change it still. If you sort it out today/tomorrow I'll try get it to Arnd before the merge window opens...

I might've jumped the gun a bit here, I thought that it'd been changed to what you (Emil) had suggested.

#49 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 40 lines · Conor Dooley
Quoted 32 lines · Emil Renner Berthing
Quoted 31 lines · Hal Feng
Quoted 18 lines · Emil Renner Berthing

Review

Quoted 2 lines · Hal Feng
(quoted diff) +2 -0
+       model = "StarFive VisionFive 2 VB";
+       compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Hi Hal,

I just want to double check, is "VisionFive 2 VA" / "visoinfive-2-va" and "VisionFive 2 VB" / "visionfive-2-vb" really what you want? I still think having these names match what is printed on the silkscreen makes it a lot easier for everybody. Even your own releases calls the boards "v1.2A" and "v1.3B":
https://github.com/starfive-tech/VisionFive2/releases/

So I'd suggest
model = "StarFive VisionFive 2 v1.3B";
compatible = "starfive,visionfive-2-v1.3b", "starfive,jh7110";

Quoted 5 lines

I haven't seen these "VA" and "VB" anywhere else, so if you don't want the version numbers and can promise that there will be no incompatible future revisions of the boards then maybe just drop the "V". Eg. model = "StarFive VisionFive 2 B";
compatible = "starfive,visionfive-2-b", "starfive,jh7110";

The version A board has reached the end of life. As far as I know, the version B board will not update also unless there are some important requirements and StarFive decides to update. Furthermore, it's too late to change the compatible as patch 1 was already accepted. Will it be easier to read if I modify it as below?

Quoted 6 lines
Quoted 2 lines
        model = "StarFive VisionFive 2 vB";
        compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Oh, that's sad that the kernel will now end up calling the boards something that's used nowhere else, even by StarFive :/ But yeah, I guess vA and vB are a little easier to read.

Quoted 6 lines

Nothing has been released with that name so AFAIU we can change it still. If you sort it out today/tomorrow I'll try get it to Arnd before the merge window opens...

I might've jumped the gun a bit here, I thought that it'd been changed to what you (Emil) had suggested.

?

-- >8 --
From 4d44e8a83716d1caa314f25a95bd21ac8904909e Mon Sep 17 00:00:00 2001
From: Conor Dooley <[email protected]>
Date: Thu, 16 Feb 2023 09:58:22 +0000
Subject: [PATCH] dt-bindings: riscv: correct starfive visionfive 2 compatibles

Using "va" and "vb" doesn't match what's written on the board, or the communications from StarFive.
Switching to using the silkscreened version number will ease confusion & the risk of another spin of the board containing a "conflicting" version identifier.

Fixes 97b7ed072784 ("dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board")
---
 Documentation/devicetree/bindings/riscv/starfive.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/riscv/starfive.yaml +2 -2
index 60c7c03fcdce..cc4d92f0a1bf 100644
@@ -26,8 +26,8 @@ properties:
 
       - items:
           - enum:
-              - starfive,visionfive-2-va
-              - starfive,visionfive-2-vb
+              - starfive,visionfive-2-v1.2a
+              - starfive,visionfive-2-v1.3b
           - const: starfive,jh7110
 
 additionalProperties: true
Signature · 1 line
2.39.0
#50 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 42 lines · Conor Dooley
Quoted 40 lines · Conor Dooley
Quoted 38 lines · Emil Renner Berthing
Quoted 25 lines · Hal Feng
Quoted 18 lines · Emil Renner Berthing

Review

Quoted 2 lines · Hal Feng
(quoted diff) +2 -0
+       model = "StarFive VisionFive 2 VB";
+       compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Hi Hal,

I just want to double check, is "VisionFive 2 VA" / "visoinfive-2-va" and "VisionFive 2 VB" / "visionfive-2-vb" really what you want? I still think having these names match what is printed on the silkscreen makes it a lot easier for everybody. Even your own releases calls the boards "v1.2A" and "v1.3B":
https://github.com/starfive-tech/VisionFive2/releases/

So I'd suggest
model = "StarFive VisionFive 2 v1.3B";
compatible = "starfive,visionfive-2-v1.3b", "starfive,jh7110";

Quoted 5 lines

I haven't seen these "VA" and "VB" anywhere else, so if you don't want the version numbers and can promise that there will be no incompatible future revisions of the boards then maybe just drop the "V". Eg. model = "StarFive VisionFive 2 B";
compatible = "starfive,visionfive-2-b", "starfive,jh7110";

Quoted 8 lines

The version A board has reached the end of life. As far as I know, the version B board will not update also unless there are some important requirements and StarFive decides to update. Furthermore, it's too late to change the compatible as patch 1 was already accepted. Will it be easier to read if I modify it as below?

        model = "StarFive VisionFive 2 vB";
        compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Oh, that's sad that the kernel will now end up calling the boards something that's used nowhere else, even by StarFive :/ But yeah, I guess vA and vB are a little easier to read.

Quoted 6 lines
Quoted 6 lines

Nothing has been released with that name so AFAIU we can change it still. If you sort it out today/tomorrow I'll try get it to Arnd before the merge window opens...

I might've jumped the gun a bit here, I thought that it'd been changed to what you (Emil) had suggested.

No, I'm sorry for being late here. The below definitely looks better to me.

Hal, would you be fine with this change?

/Emil

Quoted 39 lines

-- >8 --
From 4d44e8a83716d1caa314f25a95bd21ac8904909e Mon Sep 17 00:00:00 2001
From: Conor Dooley <[email protected]>
Date: Thu, 16 Feb 2023 09:58:22 +0000
Subject: [PATCH] dt-bindings: riscv: correct starfive visionfive 2 compatibles

Using "va" and "vb" doesn't match what's written on the board, or the communications from StarFive.
Switching to using the silkscreened version number will ease confusion & the risk of another spin of the board containing a "conflicting" version identifier.

Fixes 97b7ed072784 ("dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board")
---
 Documentation/devicetree/bindings/riscv/starfive.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/riscv/starfive.yaml +2 -3
index 60c7c03fcdce..cc4d92f0a1bf 100644
@@ -26,8 +26,8 @@ properties:
 
       - items:
           - enum:
-              - starfive,visionfive-2-va
-              - starfive,visionfive-2-vb
+              - starfive,visionfive-2-v1.2a
+              - starfive,visionfive-2-v1.3b
           - const: starfive,jh7110
 
 additionalProperties: true
--

2.39.0

author
#51 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 47 lines · Emil Renner Berthing
Quoted 46 lines · Conor Dooley
Quoted 44 lines · Conor Dooley
Quoted 32 lines · Emil Renner Berthing
Quoted 31 lines · Hal Feng
Quoted 18 lines · Emil Renner Berthing

Review

Quoted 2 lines · Hal Feng
(quoted diff) +2 -0
+       model = "StarFive VisionFive 2 VB";
+       compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Hi Hal,

I just want to double check, is "VisionFive 2 VA" / "visoinfive-2-va" and "VisionFive 2 VB" / "visionfive-2-vb" really what you want? I still think having these names match what is printed on the silkscreen makes it a lot easier for everybody. Even your own releases calls the boards "v1.2A" and "v1.3B":
https://github.com/starfive-tech/VisionFive2/releases/

So I'd suggest
model = "StarFive VisionFive 2 v1.3B";
compatible = "starfive,visionfive-2-v1.3b", "starfive,jh7110";

Quoted 5 lines

I haven't seen these "VA" and "VB" anywhere else, so if you don't want the version numbers and can promise that there will be no incompatible future revisions of the boards then maybe just drop the "V". Eg. model = "StarFive VisionFive 2 B";
compatible = "starfive,visionfive-2-b", "starfive,jh7110";

The version A board has reached the end of life. As far as I know, the version B board will not update also unless there are some important requirements and StarFive decides to update. Furthermore, it's too late to change the compatible as patch 1 was already accepted. Will it be easier to read if I modify it as below?

Quoted 6 lines
Quoted 2 lines
        model = "StarFive VisionFive 2 vB";
        compatible = "starfive,visionfive-2-vb", "starfive,jh7110";

Oh, that's sad that the kernel will now end up calling the boards something that's used nowhere else, even by StarFive :/ But yeah, I guess vA and vB are a little easier to read.

Nothing has been released with that name so AFAIU we can change it still. If you sort it out today/tomorrow I'll try get it to Arnd before the merge window opens...

Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

I might've jumped the gun a bit here, I thought that it'd been changed to what you (Emil) had suggested.

No, I'm sorry for being late here. The below definitely looks better to me.

Hal, would you be fine with this change?

I'm fine with this. It will be more exact. Thanks.

Best regards,
Hal

Quoted 40 lines
Quoted 40 lines

-- >8 --
From 4d44e8a83716d1caa314f25a95bd21ac8904909e Mon Sep 17 00:00:00 2001
From: Conor Dooley <[email protected]>
Date: Thu, 16 Feb 2023 09:58:22 +0000
Subject: [PATCH] dt-bindings: riscv: correct starfive visionfive 2 compatibles

Using "va" and "vb" doesn't match what's written on the board, or the communications from StarFive.
Switching to using the silkscreened version number will ease confusion & the risk of another spin of the board containing a "conflicting" version identifier.

Fixes 97b7ed072784 ("dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board")
---
 Documentation/devicetree/bindings/riscv/starfive.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/riscv/starfive.yaml +2 -3
index 60c7c03fcdce..cc4d92f0a1bf 100644
@@ -26,8 +26,8 @@ properties:
 
       - items:
           - enum:
-              - starfive,visionfive-2-va
-              - starfive,visionfive-2-vb
+              - starfive,visionfive-2-v1.2a
+              - starfive,visionfive-2-v1.3b
           - const: starfive,jh7110
 
 additionalProperties: true
--

2.39.0

#52 lore raw

Re: [PATCH v3 7/7] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree

Quoted 9 lines · Hal Feng
Quoted 6 lines · Emil Renner Berthing

On Thu, 16 Feb 2023 at 11:09, Conor Dooley <[email protected]> wrote:

No, I'm sorry for being late here. The below definitely looks better to me.

Hal, would you be fine with this change?

I'm fine with this. It will be more exact. Thanks.

I'll convert these both to Acked-by & send this as a real patch then. Thanks!

Quoted 40 lines
Quoted 38 lines
Quoted 38 lines

-- >8 --
From 4d44e8a83716d1caa314f25a95bd21ac8904909e Mon Sep 17 00:00:00 2001
From: Conor Dooley <[email protected]>
Date: Thu, 16 Feb 2023 09:58:22 +0000
Subject: [PATCH] dt-bindings: riscv: correct starfive visionfive 2 compatibles

Using "va" and "vb" doesn't match what's written on the board, or the communications from StarFive.
Switching to using the silkscreened version number will ease confusion & the risk of another spin of the board containing a "conflicting" version identifier.

Fixes 97b7ed072784 ("dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board")
---
 Documentation/devicetree/bindings/riscv/starfive.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/riscv/starfive.yaml +2 -3
index 60c7c03fcdce..cc4d92f0a1bf 100644
@@ -26,8 +26,8 @@ properties:
 
       - items:
           - enum:
-              - starfive,visionfive-2-va
-              - starfive,visionfive-2-vb
+              - starfive,visionfive-2-v1.2a
+              - starfive,visionfive-2-v1.3b
           - const: starfive,jh7110
 
 additionalProperties: true
--

2.39.0

maintainer Applied
#53 lore raw
In reply to Hal Feng
Quoted 5 lines · Hal Feng

This patch series adds basic device tree support for StarFive JH7110 SoC. This patch series depends on series [1] and [2]. You can simply get or review the patches at the link [3].
Emil Renner Berthing (7):
dt-bindings: riscv: Add StarFive JH7110 SoC and VisionFive 2 board

I've applied this one to riscv-dt-for-next
https://git.kernel.org/conor/c/97b7ed072784

Quoted 5 lines

dt-bindings: timer: Add StarFive JH7110 clint
dt-bindings: interrupt-controller: Add StarFive JH7110 plic

dt-bindings: sifive,ccache0: Support StarFive JH7110 SoC
soc: sifive: ccache: Add StarFive JH7110 support

And these two to riscv-soc-for-next
https://git.kernel.org/conor/c/1caf002efa22
https://git.kernel.org/conor/c/6635e91648ce

Quoted 3 lines

riscv: dts: starfive: Add initial StarFive JH7110 device tree
riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device
tree

The rest I can take with maintainer Acks alongside the DT, or they can go via subsystem trees.
The DT is waiting for the clock/reset & pinctrl binding headers anyway, so IDC which it is.

Thanks,
Conor.

#54 lore raw
Quoted 2 lines · Conor Dooley

On Tue, Dec 20, 2022 at 09:12:40AM +0800, Hal Feng wrote:
The DT is waiting for the clock/reset & pinctrl binding headers anyway,

Which were sent with build warnings/errors...

Best regards,
Krzysztof

maintainer
#55 lore raw
Quoted 5 lines · Krzysztof Kozlowski
Quoted 2 lines · Conor Dooley

On Tue, Dec 20, 2022 at 09:12:40AM +0800, Hal Feng wrote:
The DT is waiting for the clock/reset & pinctrl binding headers anyway,

Which were sent with build warnings/errors...

Yah, DW chief - I'm not in a rush. Figure it'll take a few more iterations.

Errors/warnings aside, responding to the clock series again is in my todo list, not convinced by the clock binding itself.