u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [RFC 00/10] Add support for StarFive VisionFive 2 Lite board
@ 2025-08-29  6:09 Hal Feng
  2025-08-29  6:09 ` [RFC 01/10] riscv: dts: starfive: jh7110-common: Move out some nodes to the board dts Hal Feng
                   ` (9 more replies)
  0 siblings, 10 replies; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

VisionFive 2 Lite is a mini SBC based on the StarFive JH7110S industrial
SoC which can run at -40~85 degrees centigrade and up to 1.25GHz.

Board features:
- JH7110S SoC
- 2/4/8 GiB LPDDR4 DRAM
- AXP15060 PMIC
- 40 pin GPIO header
- 1x USB 3.0 host port
- 3x USB 2.0 host port
- 1x M.2 M-Key (size: 2242)
- 1x MicroSD slot (optional non-removable eMMC)
- 1x QSPI Flash
- 1x I2C EEPROM
- 1x 1Gbps Ethernet port
- SDIO-based Wi-Fi & UART-based Bluetooth
- 1x HDMI port
- 1x 2-lane DSI
- 1x 2-lane CSI

For more details, please see [1].

Note: Patch 1 and 2 are the kernel device tree picked from [2]. If [2] is
merged, they will be no more needed. Please ignore these two patches.

[1] https://www.kickstarter.com/projects/starfive/visionfive-2-lite-unlock-risc-v-sbc-at-199
[2] https://lore.kernel.org/all/20250821100930.71404-1-hal.feng@starfivetech.com/

Hal Feng (10):
  riscv: dts: starfive: jh7110-common: Move out some nodes to the board
    dts
  riscv: dts: starfive: Add VisionFive 2 Lite board device tree
  eeprom: starfive: Simplify get_ddr_size_from_eeprom()
  eeprom: starfive: Correct get_pcb_revision_from_eeprom()
  eeprom: starfive: Update eeprom data format version to 3
  pcie: starfive: Add a optional power gpio support
  riscv: dts: jh7110: Add StarFive VisionFive 2 Lite u-boot device tree
  configs: visionfive2: Add VisionFive 2 Lite DT to OF_LIST
  board: starfive: spl: Support VisionFive 2 Lite
  board: starfive: visionfive2: Add VisionFive 2 Lite fdt selection

 arch/riscv/cpu/jh7110/spl.c                   |   2 +-
 ...10s-starfive-visionfive-2-lite-u-boot.dtsi |   7 +
 arch/riscv/include/asm/arch-jh7110/eeprom.h   |  13 +-
 board/starfive/visionfive2/spl.c              |  21 ++-
 .../visionfive2/starfive_visionfive2.c        |  22 +--
 .../visionfive2/visionfive2-i2c-eeprom.c      | 119 +++++++++-----
 configs/starfive_visionfive2_defconfig        |   2 +-
 drivers/pci/pcie_starfive_jh7110.c            |   8 +
 .../src/riscv/starfive/jh7110-common.dtsi     |  22 ---
 .../jh7110-deepcomputing-fml13v01.dts         |  49 ++++++
 .../src/riscv/starfive/jh7110-milkv-mars.dts  |  49 ++++++
 .../riscv/starfive/jh7110-pine64-star64.dts   |  49 ++++++
 .../jh7110-starfive-visionfive-2.dtsi         |  46 ++++++
 dts/upstream/src/riscv/starfive/jh7110.dtsi   |  16 --
 .../jh7110s-starfive-visionfive-2-lite.dts    | 152 ++++++++++++++++++
 15 files changed, 474 insertions(+), 103 deletions(-)
 create mode 100644 arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi
 create mode 100644 dts/upstream/src/riscv/starfive/jh7110s-starfive-visionfive-2-lite.dts


base-commit: 3dc5e9a0108bb114175b6362f9cb22367402f624
-- 
2.43.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFC 01/10] riscv: dts: starfive: jh7110-common: Move out some nodes to the board dts
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-08-29  7:19   ` Heinrich Schuchardt
  2025-08-29  6:09 ` [RFC 02/10] riscv: dts: starfive: Add VisionFive 2 Lite board device tree Hal Feng
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

Some node in this file are not used by the upcoming VisionFive 2 Lite
board. Move them to the board dts to prepare for adding the new
VisionFive 2 Lite device tree.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 .../src/riscv/starfive/jh7110-common.dtsi     | 22 ---------
 .../jh7110-deepcomputing-fml13v01.dts         | 49 +++++++++++++++++++
 .../src/riscv/starfive/jh7110-milkv-mars.dts  | 49 +++++++++++++++++++
 .../riscv/starfive/jh7110-pine64-star64.dts   | 49 +++++++++++++++++++
 .../jh7110-starfive-visionfive-2.dtsi         | 46 +++++++++++++++++
 dts/upstream/src/riscv/starfive/jh7110.dtsi   | 16 ------
 6 files changed, 193 insertions(+), 38 deletions(-)

diff --git a/dts/upstream/src/riscv/starfive/jh7110-common.dtsi b/dts/upstream/src/riscv/starfive/jh7110-common.dtsi
index 4baeb981d4d..9d3d03ad2ed 100644
--- a/dts/upstream/src/riscv/starfive/jh7110-common.dtsi
+++ b/dts/upstream/src/riscv/starfive/jh7110-common.dtsi
@@ -272,15 +272,9 @@
 	assigned-clock-rates = <50000000>;
 	bus-width = <8>;
 	bootph-pre-ram;
-	cap-mmc-highspeed;
-	mmc-ddr-1_8v;
-	mmc-hs200-1_8v;
-	cap-mmc-hw-reset;
 	post-power-on-delay-ms = <200>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins>;
-	vmmc-supply = <&vcc_3v3>;
-	vqmmc-supply = <&emmc_vdd>;
 	status = "okay";
 };
 
@@ -290,12 +284,7 @@
 	assigned-clock-rates = <50000000>;
 	bus-width = <4>;
 	bootph-pre-ram;
-	no-sdio;
-	no-mmc;
-	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
-	disable-wp;
 	cap-sd-highspeed;
-	post-power-on-delay-ms = <200>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc1_pins>;
 	status = "okay";
@@ -439,17 +428,6 @@
 	};
 
 	mmc0_pins: mmc0-0 {
-		 rst-pins {
-			pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
-					      GPOEN_ENABLE,
-					      GPI_NONE)>;
-			bias-pull-up;
-			drive-strength = <12>;
-			input-disable;
-			input-schmitt-disable;
-			slew-rate = <0>;
-		};
-
 		mmc-pins {
 			pinmux = <PINMUX(PAD_SD0_CLK, 0)>,
 				 <PINMUX(PAD_SD0_CMD, 0)>,
diff --git a/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts b/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts
index f2857d021d6..5a2a41a7e8c 100644
--- a/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts
+++ b/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts
@@ -11,6 +11,55 @@
 	compatible = "deepcomputing,fml13v01", "starfive,jh7110";
 };
 
+&cpu_opp {
+	opp-375000000 {
+		opp-hz = /bits/ 64 <375000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-500000000 {
+		opp-hz = /bits/ 64 <500000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-750000000 {
+		opp-hz = /bits/ 64 <750000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-1500000000 {
+		opp-hz = /bits/ 64 <1500000000>;
+		opp-microvolt = <1040000>;
+	};
+};
+
+&mmc0 {
+	cap-mmc-highspeed;
+	cap-mmc-hw-reset;
+	mmc-ddr-1_8v;
+	mmc-hs200-1_8v;
+	vmmc-supply = <&vcc_3v3>;
+	vqmmc-supply = <&emmc_vdd>;
+};
+
+&mmc0_pins {
+	rst-pins {
+		pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
+				      GPOEN_ENABLE,
+				      GPI_NONE)>;
+		bias-pull-up;
+		drive-strength = <12>;
+		input-disable;
+		input-schmitt-disable;
+		slew-rate = <0>;
+	};
+};
+
+&mmc1 {
+	no-sdio;
+	no-mmc;
+	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
+	disable-wp;
+	post-power-on-delay-ms = <200>;
+};
+
 &pcie1 {
 	perst-gpios = <&sysgpio 21 GPIO_ACTIVE_LOW>;
 	phys = <&pciephy1>;
diff --git a/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts b/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts
index 3bd62ab7852..0c90facc4ee 100644
--- a/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts
+++ b/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts
@@ -11,6 +11,25 @@
 	compatible = "milkv,mars", "starfive,jh7110";
 };
 
+&cpu_opp {
+	opp-375000000 {
+		opp-hz = /bits/ 64 <375000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-500000000 {
+		opp-hz = /bits/ 64 <500000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-750000000 {
+		opp-hz = /bits/ 64 <750000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-1500000000 {
+		opp-hz = /bits/ 64 <1500000000>;
+		opp-microvolt = <1040000>;
+	};
+};
+
 &gmac0 {
 	starfive,tx-use-rgmii-clk;
 	assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
@@ -22,6 +41,36 @@
 	status = "okay";
 };
 
+&mmc0 {
+	cap-mmc-highspeed;
+	cap-mmc-hw-reset;
+	mmc-ddr-1_8v;
+	mmc-hs200-1_8v;
+	vmmc-supply = <&vcc_3v3>;
+	vqmmc-supply = <&emmc_vdd>;
+};
+
+&mmc0_pins {
+	rst-pins {
+		pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
+				      GPOEN_ENABLE,
+				      GPI_NONE)>;
+		bias-pull-up;
+		drive-strength = <12>;
+		input-disable;
+		input-schmitt-disable;
+		slew-rate = <0>;
+	};
+};
+
+&mmc1 {
+	no-sdio;
+	no-mmc;
+	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
+	disable-wp;
+	post-power-on-delay-ms = <200>;
+};
+
 &pcie0 {
 	status = "okay";
 };
diff --git a/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts b/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
index 31e825be206..c9677aef9ff 100644
--- a/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
+++ b/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
@@ -14,6 +14,25 @@
 	};
 };
 
+&cpu_opp {
+	opp-375000000 {
+		opp-hz = /bits/ 64 <375000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-500000000 {
+		opp-hz = /bits/ 64 <500000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-750000000 {
+		opp-hz = /bits/ 64 <750000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-1500000000 {
+		opp-hz = /bits/ 64 <1500000000>;
+		opp-microvolt = <1040000>;
+	};
+};
+
 &gmac0 {
 	starfive,tx-use-rgmii-clk;
 	assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
@@ -44,6 +63,36 @@
 	status = "okay";
 };
 
+&mmc0 {
+	cap-mmc-highspeed;
+	cap-mmc-hw-reset;
+	mmc-ddr-1_8v;
+	mmc-hs200-1_8v;
+	vmmc-supply = <&vcc_3v3>;
+	vqmmc-supply = <&emmc_vdd>;
+};
+
+&mmc0_pins {
+	rst-pins {
+		pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
+				      GPOEN_ENABLE,
+				      GPI_NONE)>;
+		bias-pull-up;
+		drive-strength = <12>;
+		input-disable;
+		input-schmitt-disable;
+		slew-rate = <0>;
+	};
+};
+
+&mmc1 {
+	no-sdio;
+	no-mmc;
+	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
+	disable-wp;
+	post-power-on-delay-ms = <200>;
+};
+
 &pcie1 {
 	status = "okay";
 };
diff --git a/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi b/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi
index 5f14afb2c24..d1e4206f125 100644
--- a/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -13,6 +13,25 @@
 	};
 };
 
+&cpu_opp {
+	opp-375000000 {
+		opp-hz = /bits/ 64 <375000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-500000000 {
+		opp-hz = /bits/ 64 <500000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-750000000 {
+		opp-hz = /bits/ 64 <750000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-1500000000 {
+		opp-hz = /bits/ 64 <1500000000>;
+		opp-microvolt = <1040000>;
+	};
+};
+
 &gmac0 {
 	status = "okay";
 };
@@ -38,9 +57,36 @@
 };
 
 &mmc0 {
+	cap-mmc-highspeed;
+	cap-mmc-hw-reset;
+	mmc-ddr-1_8v;
+	mmc-hs200-1_8v;
+	vmmc-supply = <&vcc_3v3>;
+	vqmmc-supply = <&emmc_vdd>;
 	non-removable;
 };
 
+&mmc0_pins {
+	rst-pins {
+		pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
+				      GPOEN_ENABLE,
+				      GPI_NONE)>;
+		bias-pull-up;
+		drive-strength = <12>;
+		input-disable;
+		input-schmitt-disable;
+		slew-rate = <0>;
+	};
+};
+
+&mmc1 {
+	no-sdio;
+	no-mmc;
+	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
+	disable-wp;
+	post-power-on-delay-ms = <200>;
+};
+
 &pcie0 {
 	status = "okay";
 };
diff --git a/dts/upstream/src/riscv/starfive/jh7110.dtsi b/dts/upstream/src/riscv/starfive/jh7110.dtsi
index 0ba74ef0467..d2463399b95 100644
--- a/dts/upstream/src/riscv/starfive/jh7110.dtsi
+++ b/dts/upstream/src/riscv/starfive/jh7110.dtsi
@@ -200,22 +200,6 @@
 	cpu_opp: opp-table-0 {
 			compatible = "operating-points-v2";
 			opp-shared;
-			opp-375000000 {
-					opp-hz = /bits/ 64 <375000000>;
-					opp-microvolt = <800000>;
-			};
-			opp-500000000 {
-					opp-hz = /bits/ 64 <500000000>;
-					opp-microvolt = <800000>;
-			};
-			opp-750000000 {
-					opp-hz = /bits/ 64 <750000000>;
-					opp-microvolt = <800000>;
-			};
-			opp-1500000000 {
-					opp-hz = /bits/ 64 <1500000000>;
-					opp-microvolt = <1040000>;
-			};
 	};
 
 	thermal-zones {
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 02/10] riscv: dts: starfive: Add VisionFive 2 Lite board device tree
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
  2025-08-29  6:09 ` [RFC 01/10] riscv: dts: starfive: jh7110-common: Move out some nodes to the board dts Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-09-02 20:03   ` E Shattow
  2025-08-29  6:09 ` [RFC 03/10] eeprom: starfive: Simplify get_ddr_size_from_eeprom() Hal Feng
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

VisionFive 2 Lite is a mini SBC based on the StarFive JH7110S SoC.

Board features:
- JH7110S SoC
- 2/4/8 GiB LPDDR4 DRAM
- AXP15060 PMIC
- 40 pin GPIO header
- 1x USB 3.0 host port
- 3x USB 2.0 host port
- 1x M.2 M-Key (size: 2242)
- 1x MicroSD slot (optional non-removable eMMC)
- 1x QSPI Flash
- 1x I2C EEPROM
- 1x 1Gbps Ethernet port
- SDIO-based Wi-Fi & UART-based Bluetooth
- 1x HDMI port
- 1x 2-lane DSI
- 1x 2-lane CSI

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 .../jh7110s-starfive-visionfive-2-lite.dts    | 152 ++++++++++++++++++
 1 file changed, 152 insertions(+)
 create mode 100644 dts/upstream/src/riscv/starfive/jh7110s-starfive-visionfive-2-lite.dts

diff --git a/dts/upstream/src/riscv/starfive/jh7110s-starfive-visionfive-2-lite.dts b/dts/upstream/src/riscv/starfive/jh7110s-starfive-visionfive-2-lite.dts
new file mode 100644
index 00000000000..a0cb9912eb8
--- /dev/null
+++ b/dts/upstream/src/riscv/starfive/jh7110s-starfive-visionfive-2-lite.dts
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2025 StarFive Technology Co., Ltd.
+ * Copyright (C) 2025 Hal Feng <hal.feng@starfivetech.com>
+ */
+
+/dts-v1/;
+#include "jh7110-common.dtsi"
+
+/ {
+	model = "StarFive VisionFive 2 Lite";
+	compatible = "starfive,visionfive-2-lite", "starfive,jh7110s";
+};
+
+&cpu_opp {
+	opp-312500000 {
+		opp-hz = /bits/ 64 <312500000>;
+		opp-microvolt = <800000>;
+	};
+	opp-417000000 {
+		opp-hz = /bits/ 64 <417000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-625000000 {
+		opp-hz = /bits/ 64 <625000000>;
+		opp-microvolt = <800000>;
+	};
+	opp-1250000000 {
+		opp-hz = /bits/ 64 <1250000000>;
+		opp-microvolt = <1000000>;
+	};
+};
+
+&gmac0 {
+	starfive,tx-use-rgmii-clk;
+	assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
+	assigned-clock-parents = <&aoncrg JH7110_AONCLK_GMAC0_RMII_RTX>;
+	status = "okay";
+};
+
+&i2c0 {
+	status = "okay";
+};
+
+&mmc0 {
+	bus-width = <4>;
+	no-sdio;
+	no-mmc;
+	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_HIGH>;
+	disable-wp;
+	cap-sd-highspeed;
+};
+
+&mmc1 {
+	max-frequency = <50000000>;
+	keep-power-in-suspend;
+	non-removable;
+};
+
+&pcie1 {
+	enable-gpios = <&sysgpio 27 GPIO_ACTIVE_HIGH>;
+	status = "okay";
+};
+
+&phy0 {
+	motorcomm,tx-clk-adj-enabled;
+	motorcomm,tx-clk-100-inverted;
+	motorcomm,tx-clk-1000-inverted;
+	motorcomm,rx-clk-drv-microamp = <3970>;
+	motorcomm,rx-data-drv-microamp = <2910>;
+	rx-internal-delay-ps = <1500>;
+	tx-internal-delay-ps = <1500>;
+};
+
+&pwm {
+	status = "okay";
+};
+
+&spi0 {
+	status = "okay";
+};
+
+&syscrg {
+	assigned-clock-rates = <0>, <0>, <0>, <0>, <500000000>, <1250000000>;
+};
+
+&sysgpio {
+	uart1_pins: uart1-0 {
+		tx-pins {
+			pinmux = <GPIOMUX(22, GPOUT_SYS_UART1_TX,
+					      GPOEN_ENABLE,
+					      GPI_NONE)>;
+			bias-disable;
+			drive-strength = <12>;
+			input-disable;
+			input-schmitt-disable;
+			slew-rate = <0>;
+		};
+
+		rx-pins {
+			pinmux = <GPIOMUX(23, GPOUT_LOW,
+					      GPOEN_DISABLE,
+					      GPI_SYS_UART1_RX)>;
+			bias-pull-up;
+			drive-strength = <2>;
+			input-enable;
+			input-schmitt-enable;
+			slew-rate = <0>;
+		};
+
+		cts-pins {
+			pinmux = <GPIOMUX(24, GPOUT_LOW,
+					      GPOEN_DISABLE,
+					      GPI_SYS_UART1_CTS)>;
+			input-enable;
+		};
+
+		rts-pins {
+			pinmux = <GPIOMUX(25, GPOUT_SYS_UART1_RTS,
+					      GPOEN_ENABLE,
+					      GPI_NONE)>;
+			input-enable;
+		};
+	};
+
+	usb0_pins: usb0-0 {
+		power-pins {
+			pinmux = <GPIOMUX(26, GPOUT_HIGH,
+					      GPOEN_ENABLE,
+					      GPI_NONE)>;
+			input-disable;
+		};
+	};
+};
+
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>;
+	status = "okay";
+};
+
+&usb0 {
+	dr_mode = "host";
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb0_pins>;
+	status = "okay";
+};
+
+&usb_cdns3 {
+	phys = <&usbphy0>, <&pciephy0>;
+	phy-names = "cdns3,usb2-phy", "cdns3,usb3-phy";
+};
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 03/10] eeprom: starfive: Simplify get_ddr_size_from_eeprom()
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
  2025-08-29  6:09 ` [RFC 01/10] riscv: dts: starfive: jh7110-common: Move out some nodes to the board dts Hal Feng
  2025-08-29  6:09 ` [RFC 02/10] riscv: dts: starfive: Add VisionFive 2 Lite board device tree Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-08-29  7:33   ` Heinrich Schuchardt
  2025-08-29  6:09 ` [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom() Hal Feng
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

Directly return the DDR size instead of the field of 'DxxxExxx'.
Move the function description to the header file.

Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 arch/riscv/cpu/jh7110/spl.c                     |  2 +-
 arch/riscv/include/asm/arch-jh7110/eeprom.h     |  8 +++++++-
 .../visionfive2/visionfive2-i2c-eeprom.c        | 17 +++--------------
 3 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c
index 87aaf865246..3aece7d995b 100644
--- a/arch/riscv/cpu/jh7110/spl.c
+++ b/arch/riscv/cpu/jh7110/spl.c
@@ -41,7 +41,7 @@ int spl_dram_init(void)
 	/* Read the definition of the DDR size from eeprom, and if not,
 	 * use the definition in DT
 	 */
-	size = (get_ddr_size_from_eeprom() >> 16) & 0xFF;
+	size = get_ddr_size_from_eeprom();
 	if (check_ddr_size(size))
 		gd->ram_size = size << 30;
 
diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h b/arch/riscv/include/asm/arch-jh7110/eeprom.h
index 45ad2a5f7bc..6d0a0ba0c4a 100644
--- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
+++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
@@ -10,7 +10,13 @@
 #include <linux/types.h>
 
 u8 get_pcb_revision_from_eeprom(void);
-u32 get_ddr_size_from_eeprom(void);
+
+/**
+ * get_ddr_size_from_eeprom() - read DDR size from EEPROM
+ *
+ * @return: size in GiB or 0xFF on error.
+ */
+u8 get_ddr_size_from_eeprom(void);
 
 /**
  * get_mmc_size_from_eeprom() - read eMMC size from EEPROM
diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
index 17a44020bcf..3866d07f9d4 100644
--- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
+++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
@@ -550,23 +550,12 @@ u8 get_pcb_revision_from_eeprom(void)
 	return pbuf.eeprom.atom1.data.pstr[6];
 }
 
-/**
- * get_ddr_size_from_eeprom - get the DDR size
- * pstr:  VF7110A1-2228-D008E000-00000001
- * VF7110A1/VF7110B1 : VisionFive JH7110A /VisionFive JH7110B
- * D008: 8GB LPDDR4
- * E000: No emmc device, ECxx: include emmc device, xx: Capacity size[GB]
- * return: the field of 'D008E000'
- */
-
-u32 get_ddr_size_from_eeprom(void)
+u8 get_ddr_size_from_eeprom(void)
 {
-	u32 pv = 0xFFFFFFFF;
-
 	if (read_eeprom())
-		return pv;
+		return 0xFF;
 
-	return hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL);
+	return (hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL) >> 16) & 0xFF;
 }
 
 u32 get_mmc_size_from_eeprom(void)
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom()
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
                   ` (2 preceding siblings ...)
  2025-08-29  6:09 ` [RFC 03/10] eeprom: starfive: Simplify get_ddr_size_from_eeprom() Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-08-29  7:44   ` Heinrich Schuchardt
  2025-08-29  6:09 ` [RFC 05/10] eeprom: starfive: Update eeprom data format version to 3 Hal Feng
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

pcb_revision is stored in the pcb_revision field of ATOM4. Correct it.
Move the function description to the header file.
Remove the function calls in board/starfive/visionfive2/.

Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 arch/riscv/include/asm/arch-jh7110/eeprom.h   |  5 +++++
 board/starfive/visionfive2/spl.c              | 18 ++++++-----------
 .../visionfive2/starfive_visionfive2.c        | 20 ++++++-------------
 .../visionfive2/visionfive2-i2c-eeprom.c      | 11 ++--------
 4 files changed, 19 insertions(+), 35 deletions(-)

diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h b/arch/riscv/include/asm/arch-jh7110/eeprom.h
index 6d0a0ba0c4a..025f1d32c49 100644
--- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
+++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
@@ -9,6 +9,11 @@
 
 #include <linux/types.h>
 
+/**
+ * get_pcb_revision_from_eeprom() - get the PCB revision
+ *
+ * @return: the PCB revision or 0xFF on error.
+ */
 u8 get_pcb_revision_from_eeprom(void);
 
 /**
diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
index 3dfa931b655..901e7b58f36 100644
--- a/board/starfive/visionfive2/spl.c
+++ b/board/starfive/visionfive2/spl.c
@@ -126,19 +126,13 @@ int board_fit_config_name_match(const char *name)
 		    !strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
 		return 0;
 	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.2a") &&
-		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
-		switch (get_pcb_revision_from_eeprom()) {
-		case 'a':
-		case 'A':
-			return 0;
-		}
+		   (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7) ||
+		    !strncmp(get_product_id_from_eeprom(), "VF7110a", 7))) {
+		return 0;
 	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.3b") &&
-		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
-		switch (get_pcb_revision_from_eeprom()) {
-		case 'b':
-		case 'B':
-			return 0;
-		}
+		   (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
+		    !strncmp(get_product_id_from_eeprom(), "VF7110b", 7))) {
+		return 0;
 	}
 
 	return -EINVAL;
diff --git a/board/starfive/visionfive2/starfive_visionfive2.c b/board/starfive/visionfive2/starfive_visionfive2.c
index bfbb11a2ee7..f38433e94ac 100644
--- a/board/starfive/visionfive2/starfive_visionfive2.c
+++ b/board/starfive/visionfive2/starfive_visionfive2.c
@@ -59,20 +59,12 @@ static void set_fdtfile(void)
 		fdtfile = "starfive/jh7110-milkv-mars.dtb";
 	} else if (!strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
 		fdtfile = "starfive/jh7110-pine64-star64.dtb";
-	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
-		switch (get_pcb_revision_from_eeprom()) {
-		case 'a':
-		case 'A':
-			fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb";
-			break;
-		case 'b':
-		case 'B':
-			fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
-			break;
-		default:
-			log_err("Unknown revision\n");
-			return;
-		}
+	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7) ||
+		   !strncmp(get_product_id_from_eeprom(), "VF7110a", 7)) {
+		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb";
+	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
+		   !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)) {
+		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
 	} else {
 		log_err("Unknown product\n");
 		return;
diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
index 3866d07f9d4..43b8af4fc59 100644
--- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
+++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
@@ -535,19 +535,12 @@ int mac_read_from_eeprom(void)
 	return 0;
 }
 
-/**
- * get_pcb_revision_from_eeprom - get the PCB revision
- *
- * 1.2A return 'A'/'a', 1.3B return 'B'/'b',other values are illegal
- */
 u8 get_pcb_revision_from_eeprom(void)
 {
-	u8 pv = 0xFF;
-
 	if (read_eeprom())
-		return pv;
+		return 0xFF;
 
-	return pbuf.eeprom.atom1.data.pstr[6];
+	return pbuf.eeprom.atom4.data.pcb_revision;
 }
 
 u8 get_ddr_size_from_eeprom(void)
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 05/10] eeprom: starfive: Update eeprom data format version to 3
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
                   ` (3 preceding siblings ...)
  2025-08-29  6:09 ` [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom() Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-08-29  7:47   ` Heinrich Schuchardt
  2025-09-02 23:29   ` E Shattow
  2025-08-29  6:09 ` [RFC 06/10] pcie: starfive: Add a optional power gpio support Hal Feng
                   ` (4 subsequent siblings)
  9 siblings, 2 replies; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

Add eeprom data format v3 support.
Add wifi_bt field in ATOM4 and add "mac wifi_bt <?>" command to modify it.
Set or show eth0/1 MAC address only if the board has eth0/1.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 .../visionfive2/visionfive2-i2c-eeprom.c      | 91 ++++++++++++++++---
 1 file changed, 78 insertions(+), 13 deletions(-)

diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
index 43b8af4fc59..7352252c475 100644
--- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
+++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
@@ -11,7 +11,7 @@
 #include <u-boot/crc.h>
 #include <linux/delay.h>
 
-#define FORMAT_VERSION		0x2
+#define FORMAT_VERSION		0x3
 #define PCB_VERSION		0xB1
 #define BOM_VERSION		'A'
 /*
@@ -105,7 +105,8 @@ struct eeprom_atom4_data {
 	u8 bom_revision;		/* BOM version */
 	u8 mac0_addr[MAC_ADDR_BYTES];	/* Ethernet0 MAC */
 	u8 mac1_addr[MAC_ADDR_BYTES];	/* Ethernet1 MAC */
-	u8 reserved[2];
+	u8 wifi_bt;			/* WIFI/BT support flag */
+	u8 reserved;
 };
 
 struct starfive_eeprom_atom4 {
@@ -128,6 +129,35 @@ static union {
 /* Set to 1 if we've read EEPROM into memory */
 static int has_been_read __section(".data");
 
+static const char * const board_no_eth0_list[] = {
+	"FML13V01",
+};
+
+static const char * const board_no_eth1_list[] = {
+	"FML13V01",
+	"MARS",
+	"VF7110SL",
+};
+
+static bool board_have_eth(u8 eth)
+{
+	int i;
+
+	if (eth == 0) {
+		for (i = 0 ; i < ARRAY_SIZE(board_no_eth0_list); i++)
+			if (!strncmp(pbuf.eeprom.atom1.data.pstr, board_no_eth0_list[i],
+				     strlen(board_no_eth0_list[i])))
+				return false;
+	} else {
+		for (i = 0 ; i < ARRAY_SIZE(board_no_eth1_list); i++)
+			if (!strncmp(pbuf.eeprom.atom1.data.pstr, board_no_eth1_list[i],
+				     strlen(board_no_eth1_list[i])))
+				return false;
+	}
+
+	return true;
+}
+
 static inline int is_match_magic(void)
 {
 	return strncmp(pbuf.eeprom.header.signature, STARFIVE_EEPROM_HATS_SIG,
@@ -176,17 +206,22 @@ static void show_eeprom(void)
 	printf("Vendor : %s\n", pbuf.eeprom.atom1.data.vstr);
 	printf("Product full SN: %s\n", pbuf.eeprom.atom1.data.pstr);
 	printf("data version: 0x%x\n", pbuf.eeprom.atom4.data.version);
-	if (pbuf.eeprom.atom4.data.version == 2) {
+	if (pbuf.eeprom.atom4.data.version >= 2 && pbuf.eeprom.atom4.data.version <= 3) {
 		printf("PCB revision: 0x%x\n", pbuf.eeprom.atom4.data.pcb_revision);
 		printf("BOM revision: %c\n", pbuf.eeprom.atom4.data.bom_revision);
-		printf("Ethernet MAC0 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
-		       pbuf.eeprom.atom4.data.mac0_addr[0], pbuf.eeprom.atom4.data.mac0_addr[1],
-		       pbuf.eeprom.atom4.data.mac0_addr[2], pbuf.eeprom.atom4.data.mac0_addr[3],
-		       pbuf.eeprom.atom4.data.mac0_addr[4], pbuf.eeprom.atom4.data.mac0_addr[5]);
-		printf("Ethernet MAC1 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
-		       pbuf.eeprom.atom4.data.mac1_addr[0], pbuf.eeprom.atom4.data.mac1_addr[1],
-		       pbuf.eeprom.atom4.data.mac1_addr[2], pbuf.eeprom.atom4.data.mac1_addr[3],
-		       pbuf.eeprom.atom4.data.mac1_addr[4], pbuf.eeprom.atom4.data.mac1_addr[5]);
+		if (board_have_eth(0))
+			printf("Ethernet MAC0 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
+			pbuf.eeprom.atom4.data.mac0_addr[0], pbuf.eeprom.atom4.data.mac0_addr[1],
+			pbuf.eeprom.atom4.data.mac0_addr[2], pbuf.eeprom.atom4.data.mac0_addr[3],
+			pbuf.eeprom.atom4.data.mac0_addr[4], pbuf.eeprom.atom4.data.mac0_addr[5]);
+
+		if (board_have_eth(1))
+			printf("Ethernet MAC1 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
+			pbuf.eeprom.atom4.data.mac1_addr[0], pbuf.eeprom.atom4.data.mac1_addr[1],
+			pbuf.eeprom.atom4.data.mac1_addr[2], pbuf.eeprom.atom4.data.mac1_addr[3],
+			pbuf.eeprom.atom4.data.mac1_addr[4], pbuf.eeprom.atom4.data.mac1_addr[5]);
+		if (pbuf.eeprom.atom4.data.version >= 3)
+			printf("WIFI/BT support: %x\n", pbuf.eeprom.atom4.data.wifi_bt);
 	} else {
 		printf("Custom data v%d is not Supported\n", pbuf.eeprom.atom4.data.version);
 		dump_raw_eeprom();
@@ -260,6 +295,7 @@ static void init_local_copy(void)
 	pbuf.eeprom.atom4.data.bom_revision = BOM_VERSION;
 	set_mac_address(STARFIVE_DEFAULT_MAC0, 0);
 	set_mac_address(STARFIVE_DEFAULT_MAC1, 1);
+	pbuf.eeprom.atom4.data.wifi_bt = 0;
 }
 
 /**
@@ -385,6 +421,28 @@ static void set_bom_revision(char *string)
 	update_crc();
 }
 
+/**
+ * set_wifi_bt() - stores a StarFive WIFI/BT support flag into the local EEPROM copy
+ *
+ * Takes a pointer to a string representing the numeric WIFI/BT support flag in
+ * decimal ("0" or "1"), stores it in the wifi_bt field of the
+ * EEPROM local copy, and updates the CRC of the local copy.
+ */
+static void set_wifi_bt(char *string)
+{
+	u8 wifi_bt;
+
+	wifi_bt = simple_strtoul(string, &string, 16);
+	if (wifi_bt > 1) {
+		printf("WIFI/BT support flag must not be greater than 1");
+		return;
+	}
+
+	pbuf.eeprom.atom4.data.wifi_bt = wifi_bt;
+
+	update_crc();
+}
+
 /**
  * set_product_id() - stores a StarFive product ID into the local EEPROM copy
  *
@@ -478,6 +536,9 @@ int do_mac(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	} else if (!strcmp(cmd, "bom_revision")) {
 		set_bom_revision(argv[2]);
 		return 0;
+	} else if (!strcmp(cmd, "wifi_bt")) {
+		set_wifi_bt(argv[2]);
+		return 0;
 	} else if (!strcmp(cmd, "product_id")) {
 		set_product_id(argv[2]);
 		return 0;
@@ -513,8 +574,10 @@ int mac_read_from_eeprom(void)
 	}
 
 	// 1, setup ethaddr env
-	eth_env_set_enetaddr("ethaddr", pbuf.eeprom.atom4.data.mac0_addr);
-	eth_env_set_enetaddr("eth1addr", pbuf.eeprom.atom4.data.mac1_addr);
+	if (board_have_eth(0))
+		eth_env_set_enetaddr("ethaddr", pbuf.eeprom.atom4.data.mac0_addr);
+	if (board_have_eth(1))
+		eth_env_set_enetaddr("eth1addr", pbuf.eeprom.atom4.data.mac1_addr);
 
 	/**
 	 * 2, setup serial# env, reference to hifive-platform-i2c-eeprom.c,
@@ -585,6 +648,8 @@ U_BOOT_LONGHELP(mac,
 	"    - stores a StarFive PCB revision into the local EEPROM copy\n"
 	"mac bom_revision <A>\n"
 	"    - stores a StarFive BOM revision into the local EEPROM copy\n"
+	"mac wifi_bt <?>\n"
+	"    - stores a StarFive WIFI/BT support flag into the local EEPROM copy\n"
 	"mac product_id <VF7110A1-2228-D008E000-xxxxxxxx>\n"
 	"    - stores a StarFive product ID into the local EEPROM copy\n"
 	"mac vendor <Vendor Name>\n"
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 06/10] pcie: starfive: Add a optional power gpio support
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
                   ` (4 preceding siblings ...)
  2025-08-29  6:09 ` [RFC 05/10] eeprom: starfive: Update eeprom data format version to 3 Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-09-02 23:47   ` E Shattow
  2025-08-29  6:09 ` [RFC 07/10] riscv: dts: jh7110: Add StarFive VisionFive 2 Lite u-boot device tree Hal Feng
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

Get a optional power gpio and enable it if it is valid.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 drivers/pci/pcie_starfive_jh7110.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pcie_starfive_jh7110.c b/drivers/pci/pcie_starfive_jh7110.c
index 51aca7359ff..f6b50d12a90 100644
--- a/drivers/pci/pcie_starfive_jh7110.c
+++ b/drivers/pci/pcie_starfive_jh7110.c
@@ -45,6 +45,7 @@ struct starfive_pcie {
 	struct pcie_plda plda;
 	struct clk_bulk	clks;
 	struct reset_ctl_bulk	rsts;
+	struct gpio_desc	power_gpio;
 	struct gpio_desc	reset_gpio;
 	struct regmap *regmap;
 	unsigned int stg_pcie_base;
@@ -184,6 +185,10 @@ static int starfive_pcie_parse_dt(struct udevice *dev)
 		dev_err(dev, "reset-gpio is not valid\n");
 		return -EINVAL;
 	}
+
+	gpio_request_by_name(dev, "enable-gpios", 0, &priv->power_gpio,
+			     GPIOD_IS_OUT);
+
 	return 0;
 }
 
@@ -205,6 +210,9 @@ static int starfive_pcie_init_port(struct udevice *dev)
 		goto err_deassert_clk;
 	}
 
+	if (dm_gpio_is_valid(&priv->power_gpio))
+		dm_gpio_set_value(&priv->power_gpio, 1);
+
 	dm_gpio_set_value(&priv->reset_gpio, 1);
 	/* Disable physical functions except #0 */
 	for (i = 1; i < PLDA_FUNC_NUM; i++) {
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 07/10] riscv: dts: jh7110: Add StarFive VisionFive 2 Lite u-boot device tree
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
                   ` (5 preceding siblings ...)
  2025-08-29  6:09 ` [RFC 06/10] pcie: starfive: Add a optional power gpio support Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-09-03  0:00   ` E Shattow
  2025-08-29  6:09 ` [RFC 08/10] configs: visionfive2: Add VisionFive 2 Lite DT to OF_LIST Hal Feng
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

Add the u-boot device tree include needed to support the
StarFive VisionFive 2 Lite board.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 .../dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi     | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi

diff --git a/arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi b/arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi
new file mode 100644
index 00000000000..46ef07a99eb
--- /dev/null
+++ b/arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2025 StarFive Technology Co., Ltd.
+ */
+
+#include "jh7110-common-u-boot.dtsi"
+#include "starfive-visionfive2-binman.dtsi"
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 08/10] configs: visionfive2: Add VisionFive 2 Lite DT to OF_LIST
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
                   ` (6 preceding siblings ...)
  2025-08-29  6:09 ` [RFC 07/10] riscv: dts: jh7110: Add StarFive VisionFive 2 Lite u-boot device tree Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-09-03  0:15   ` E Shattow
  2025-08-29  6:09 ` [RFC 09/10] board: starfive: spl: Support VisionFive 2 Lite Hal Feng
  2025-08-29  6:09 ` [RFC 10/10] board: starfive: visionfive2: Add VisionFive 2 Lite fdt selection Hal Feng
  9 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

So the VisionFive 2 Lite DT will be built and merged into FIT.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 configs/starfive_visionfive2_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configs/starfive_visionfive2_defconfig b/configs/starfive_visionfive2_defconfig
index 544140c03f7..5a0e918c733 100644
--- a/configs/starfive_visionfive2_defconfig
+++ b/configs/starfive_visionfive2_defconfig
@@ -78,7 +78,7 @@ CONFIG_CMD_WDT=y
 CONFIG_CMD_WGET=y
 CONFIG_CMD_BOOTSTAGE=y
 CONFIG_OF_BOARD=y
-CONFIG_OF_LIST="starfive/jh7110-deepcomputing-fml13v01 starfive/jh7110-milkv-mars starfive/jh7110-pine64-star64 starfive/jh7110-starfive-visionfive-2-v1.2a starfive/jh7110-starfive-visionfive-2-v1.3b"
+CONFIG_OF_LIST="starfive/jh7110-deepcomputing-fml13v01 starfive/jh7110-milkv-mars starfive/jh7110-pine64-star64 starfive/jh7110-starfive-visionfive-2-v1.2a starfive/jh7110-starfive-visionfive-2-v1.3b starfive/jh7110s-starfive-visionfive-2-lite"
 CONFIG_MULTI_DTB_FIT=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 09/10] board: starfive: spl: Support VisionFive 2 Lite
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
                   ` (7 preceding siblings ...)
  2025-08-29  6:09 ` [RFC 08/10] configs: visionfive2: Add VisionFive 2 Lite DT to OF_LIST Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-09-03  0:21   ` E Shattow
  2025-08-29  6:09 ` [RFC 10/10] board: starfive: visionfive2: Add VisionFive 2 Lite fdt selection Hal Feng
  9 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

Choose the matching FIT config on the VisionFive 2 Lite board.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 board/starfive/visionfive2/spl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
index 901e7b58f36..18571b482f9 100644
--- a/board/starfive/visionfive2/spl.c
+++ b/board/starfive/visionfive2/spl.c
@@ -133,6 +133,9 @@ int board_fit_config_name_match(const char *name)
 		   (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
 		    !strncmp(get_product_id_from_eeprom(), "VF7110b", 7))) {
 		return 0;
+	} else if (!strcmp(name, "starfive/jh7110s-starfive-visionfive-2-lite") &&
+		    !strncmp(get_product_id_from_eeprom(), "VF7110SL", 8)) {
+		return 0;
 	}
 
 	return -EINVAL;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 10/10] board: starfive: visionfive2: Add VisionFive 2 Lite fdt selection
  2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
                   ` (8 preceding siblings ...)
  2025-08-29  6:09 ` [RFC 09/10] board: starfive: spl: Support VisionFive 2 Lite Hal Feng
@ 2025-08-29  6:09 ` Hal Feng
  2025-09-03  0:26   ` E Shattow
  9 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-08-29  6:09 UTC (permalink / raw)
  To: Leo, Tom Rini, Rick Chen, Sumit Garg, Emil Renner Berthing,
	Heinrich Schuchardt, E Shattow
  Cc: Hal Feng, u-boot

Set $fdtfile to the VisionFive 2 Lite DTB if the board is matched.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 board/starfive/visionfive2/starfive_visionfive2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/board/starfive/visionfive2/starfive_visionfive2.c b/board/starfive/visionfive2/starfive_visionfive2.c
index f38433e94ac..912e8cb967a 100644
--- a/board/starfive/visionfive2/starfive_visionfive2.c
+++ b/board/starfive/visionfive2/starfive_visionfive2.c
@@ -65,6 +65,8 @@ static void set_fdtfile(void)
 	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
 		   !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)) {
 		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
+	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110SL", 8)) {
+		fdtfile = "starfive/jh7110s-starfive-visionfive-2-lite.dtb";
 	} else {
 		log_err("Unknown product\n");
 		return;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [RFC 01/10] riscv: dts: starfive: jh7110-common: Move out some nodes to the board dts
  2025-08-29  6:09 ` [RFC 01/10] riscv: dts: starfive: jh7110-common: Move out some nodes to the board dts Hal Feng
@ 2025-08-29  7:19   ` Heinrich Schuchardt
  2025-09-02 16:17     ` E Shattow
  0 siblings, 1 reply; 30+ messages in thread
From: Heinrich Schuchardt @ 2025-08-29  7:19 UTC (permalink / raw)
  To: Hal Feng
  Cc: u-boot, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, E Shattow

On 29.08.25 08:09, Hal Feng wrote:
> Some node in this file are not used by the upcoming VisionFive 2 Lite
> board. Move them to the board dts to prepare for adding the new
> VisionFive 2 Lite device tree.
> 
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>

LGTM

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> ---
>   .../src/riscv/starfive/jh7110-common.dtsi     | 22 ---------
>   .../jh7110-deepcomputing-fml13v01.dts         | 49 +++++++++++++++++++
>   .../src/riscv/starfive/jh7110-milkv-mars.dts  | 49 +++++++++++++++++++
>   .../riscv/starfive/jh7110-pine64-star64.dts   | 49 +++++++++++++++++++
>   .../jh7110-starfive-visionfive-2.dtsi         | 46 +++++++++++++++++
>   dts/upstream/src/riscv/starfive/jh7110.dtsi   | 16 ------
>   6 files changed, 193 insertions(+), 38 deletions(-)
> 
> diff --git a/dts/upstream/src/riscv/starfive/jh7110-common.dtsi b/dts/upstream/src/riscv/starfive/jh7110-common.dtsi
> index 4baeb981d4d..9d3d03ad2ed 100644
> --- a/dts/upstream/src/riscv/starfive/jh7110-common.dtsi
> +++ b/dts/upstream/src/riscv/starfive/jh7110-common.dtsi
> @@ -272,15 +272,9 @@
>   	assigned-clock-rates = <50000000>;
>   	bus-width = <8>;
>   	bootph-pre-ram;
> -	cap-mmc-highspeed;
> -	mmc-ddr-1_8v;
> -	mmc-hs200-1_8v;
> -	cap-mmc-hw-reset;
>   	post-power-on-delay-ms = <200>;
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&mmc0_pins>;
> -	vmmc-supply = <&vcc_3v3>;
> -	vqmmc-supply = <&emmc_vdd>;
>   	status = "okay";
>   };
>   
> @@ -290,12 +284,7 @@
>   	assigned-clock-rates = <50000000>;
>   	bus-width = <4>;
>   	bootph-pre-ram;
> -	no-sdio;
> -	no-mmc;
> -	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
> -	disable-wp;
>   	cap-sd-highspeed;
> -	post-power-on-delay-ms = <200>;
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&mmc1_pins>;
>   	status = "okay";
> @@ -439,17 +428,6 @@
>   	};
>   
>   	mmc0_pins: mmc0-0 {
> -		 rst-pins {
> -			pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
> -					      GPOEN_ENABLE,
> -					      GPI_NONE)>;
> -			bias-pull-up;
> -			drive-strength = <12>;
> -			input-disable;
> -			input-schmitt-disable;
> -			slew-rate = <0>;
> -		};
> -
>   		mmc-pins {
>   			pinmux = <PINMUX(PAD_SD0_CLK, 0)>,
>   				 <PINMUX(PAD_SD0_CMD, 0)>,
> diff --git a/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts b/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts
> index f2857d021d6..5a2a41a7e8c 100644
> --- a/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts
> +++ b/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts
> @@ -11,6 +11,55 @@
>   	compatible = "deepcomputing,fml13v01", "starfive,jh7110";
>   };
>   
> +&cpu_opp {
> +	opp-375000000 {
> +		opp-hz = /bits/ 64 <375000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-500000000 {
> +		opp-hz = /bits/ 64 <500000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-750000000 {
> +		opp-hz = /bits/ 64 <750000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-1500000000 {
> +		opp-hz = /bits/ 64 <1500000000>;
> +		opp-microvolt = <1040000>;
> +	};
> +};
> +
> +&mmc0 {
> +	cap-mmc-highspeed;
> +	cap-mmc-hw-reset;
> +	mmc-ddr-1_8v;
> +	mmc-hs200-1_8v;
> +	vmmc-supply = <&vcc_3v3>;
> +	vqmmc-supply = <&emmc_vdd>;
> +};
> +
> +&mmc0_pins {
> +	rst-pins {
> +		pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
> +				      GPOEN_ENABLE,
> +				      GPI_NONE)>;
> +		bias-pull-up;
> +		drive-strength = <12>;
> +		input-disable;
> +		input-schmitt-disable;
> +		slew-rate = <0>;
> +	};
> +};
> +
> +&mmc1 {
> +	no-sdio;
> +	no-mmc;
> +	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
> +	disable-wp;
> +	post-power-on-delay-ms = <200>;
> +};
> +
>   &pcie1 {
>   	perst-gpios = <&sysgpio 21 GPIO_ACTIVE_LOW>;
>   	phys = <&pciephy1>;
> diff --git a/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts b/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts
> index 3bd62ab7852..0c90facc4ee 100644
> --- a/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts
> +++ b/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts
> @@ -11,6 +11,25 @@
>   	compatible = "milkv,mars", "starfive,jh7110";
>   };
>   
> +&cpu_opp {
> +	opp-375000000 {
> +		opp-hz = /bits/ 64 <375000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-500000000 {
> +		opp-hz = /bits/ 64 <500000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-750000000 {
> +		opp-hz = /bits/ 64 <750000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-1500000000 {
> +		opp-hz = /bits/ 64 <1500000000>;
> +		opp-microvolt = <1040000>;
> +	};
> +};
> +
>   &gmac0 {
>   	starfive,tx-use-rgmii-clk;
>   	assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
> @@ -22,6 +41,36 @@
>   	status = "okay";
>   };
>   
> +&mmc0 {
> +	cap-mmc-highspeed;
> +	cap-mmc-hw-reset;
> +	mmc-ddr-1_8v;
> +	mmc-hs200-1_8v;
> +	vmmc-supply = <&vcc_3v3>;
> +	vqmmc-supply = <&emmc_vdd>;
> +};
> +
> +&mmc0_pins {
> +	rst-pins {
> +		pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
> +				      GPOEN_ENABLE,
> +				      GPI_NONE)>;
> +		bias-pull-up;
> +		drive-strength = <12>;
> +		input-disable;
> +		input-schmitt-disable;
> +		slew-rate = <0>;
> +	};
> +};
> +
> +&mmc1 {
> +	no-sdio;
> +	no-mmc;
> +	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
> +	disable-wp;
> +	post-power-on-delay-ms = <200>;
> +};
> +
>   &pcie0 {
>   	status = "okay";
>   };
> diff --git a/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts b/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
> index 31e825be206..c9677aef9ff 100644
> --- a/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
> +++ b/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
> @@ -14,6 +14,25 @@
>   	};
>   };
>   
> +&cpu_opp {
> +	opp-375000000 {
> +		opp-hz = /bits/ 64 <375000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-500000000 {
> +		opp-hz = /bits/ 64 <500000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-750000000 {
> +		opp-hz = /bits/ 64 <750000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-1500000000 {
> +		opp-hz = /bits/ 64 <1500000000>;
> +		opp-microvolt = <1040000>;
> +	};
> +};
> +
>   &gmac0 {
>   	starfive,tx-use-rgmii-clk;
>   	assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
> @@ -44,6 +63,36 @@
>   	status = "okay";
>   };
>   
> +&mmc0 {
> +	cap-mmc-highspeed;
> +	cap-mmc-hw-reset;
> +	mmc-ddr-1_8v;
> +	mmc-hs200-1_8v;
> +	vmmc-supply = <&vcc_3v3>;
> +	vqmmc-supply = <&emmc_vdd>;
> +};
> +
> +&mmc0_pins {
> +	rst-pins {
> +		pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
> +				      GPOEN_ENABLE,
> +				      GPI_NONE)>;
> +		bias-pull-up;
> +		drive-strength = <12>;
> +		input-disable;
> +		input-schmitt-disable;
> +		slew-rate = <0>;
> +	};
> +};
> +
> +&mmc1 {
> +	no-sdio;
> +	no-mmc;
> +	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
> +	disable-wp;
> +	post-power-on-delay-ms = <200>;
> +};
> +
>   &pcie1 {
>   	status = "okay";
>   };
> diff --git a/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi b/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi
> index 5f14afb2c24..d1e4206f125 100644
> --- a/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -13,6 +13,25 @@
>   	};
>   };
>   
> +&cpu_opp {
> +	opp-375000000 {
> +		opp-hz = /bits/ 64 <375000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-500000000 {
> +		opp-hz = /bits/ 64 <500000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-750000000 {
> +		opp-hz = /bits/ 64 <750000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-1500000000 {
> +		opp-hz = /bits/ 64 <1500000000>;
> +		opp-microvolt = <1040000>;
> +	};
> +};
> +
>   &gmac0 {
>   	status = "okay";
>   };
> @@ -38,9 +57,36 @@
>   };
>   
>   &mmc0 {
> +	cap-mmc-highspeed;
> +	cap-mmc-hw-reset;
> +	mmc-ddr-1_8v;
> +	mmc-hs200-1_8v;
> +	vmmc-supply = <&vcc_3v3>;
> +	vqmmc-supply = <&emmc_vdd>;
>   	non-removable;
>   };
>   
> +&mmc0_pins {
> +	rst-pins {
> +		pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
> +				      GPOEN_ENABLE,
> +				      GPI_NONE)>;
> +		bias-pull-up;
> +		drive-strength = <12>;
> +		input-disable;
> +		input-schmitt-disable;
> +		slew-rate = <0>;
> +	};
> +};
> +
> +&mmc1 {
> +	no-sdio;
> +	no-mmc;
> +	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
> +	disable-wp;
> +	post-power-on-delay-ms = <200>;
> +};
> +
>   &pcie0 {
>   	status = "okay";
>   };
> diff --git a/dts/upstream/src/riscv/starfive/jh7110.dtsi b/dts/upstream/src/riscv/starfive/jh7110.dtsi
> index 0ba74ef0467..d2463399b95 100644
> --- a/dts/upstream/src/riscv/starfive/jh7110.dtsi
> +++ b/dts/upstream/src/riscv/starfive/jh7110.dtsi
> @@ -200,22 +200,6 @@
>   	cpu_opp: opp-table-0 {
>   			compatible = "operating-points-v2";
>   			opp-shared;
> -			opp-375000000 {
> -					opp-hz = /bits/ 64 <375000000>;
> -					opp-microvolt = <800000>;
> -			};
> -			opp-500000000 {
> -					opp-hz = /bits/ 64 <500000000>;
> -					opp-microvolt = <800000>;
> -			};
> -			opp-750000000 {
> -					opp-hz = /bits/ 64 <750000000>;
> -					opp-microvolt = <800000>;
> -			};
> -			opp-1500000000 {
> -					opp-hz = /bits/ 64 <1500000000>;
> -					opp-microvolt = <1040000>;
> -			};
>   	};
>   
>   	thermal-zones {


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 03/10] eeprom: starfive: Simplify get_ddr_size_from_eeprom()
  2025-08-29  6:09 ` [RFC 03/10] eeprom: starfive: Simplify get_ddr_size_from_eeprom() Hal Feng
@ 2025-08-29  7:33   ` Heinrich Schuchardt
  2025-09-02 21:28     ` E Shattow
  0 siblings, 1 reply; 30+ messages in thread
From: Heinrich Schuchardt @ 2025-08-29  7:33 UTC (permalink / raw)
  To: Hal Feng
  Cc: u-boot, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, E Shattow

On 29.08.25 08:09, Hal Feng wrote:
> Directly return the DDR size instead of the field of 'DxxxExxx'.
> Move the function description to the header file.
> 
> Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>   arch/riscv/cpu/jh7110/spl.c                     |  2 +-
>   arch/riscv/include/asm/arch-jh7110/eeprom.h     |  8 +++++++-
>   .../visionfive2/visionfive2-i2c-eeprom.c        | 17 +++--------------
>   3 files changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c
> index 87aaf865246..3aece7d995b 100644
> --- a/arch/riscv/cpu/jh7110/spl.c
> +++ b/arch/riscv/cpu/jh7110/spl.c
> @@ -41,7 +41,7 @@ int spl_dram_init(void)
>   	/* Read the definition of the DDR size from eeprom, and if not,
>   	 * use the definition in DT
>   	 */
> -	size = (get_ddr_size_from_eeprom() >> 16) & 0xFF;
> +	size = get_ddr_size_from_eeprom();
>   	if (check_ddr_size(size))
>   		gd->ram_size = size << 30;
>   
> diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> index 45ad2a5f7bc..6d0a0ba0c4a 100644
> --- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
> +++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> @@ -10,7 +10,13 @@
>   #include <linux/types.h>
>   
>   u8 get_pcb_revision_from_eeprom(void);
> -u32 get_ddr_size_from_eeprom(void);
> +
> +/**
> + * get_ddr_size_from_eeprom() - read DDR size from EEPROM
> + *
> + * @return: size in GiB or 0xFF on error.
> + */
> +u8 get_ddr_size_from_eeprom(void);
>   
>   /**
>    * get_mmc_size_from_eeprom() - read eMMC size from EEPROM
> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> index 17a44020bcf..3866d07f9d4 100644
> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> @@ -550,23 +550,12 @@ u8 get_pcb_revision_from_eeprom(void)
>   	return pbuf.eeprom.atom1.data.pstr[6];
>   }
>   
> -/**
> - * get_ddr_size_from_eeprom - get the DDR size
> - * pstr:  VF7110A1-2228-D008E000-00000001
> - * VF7110A1/VF7110B1 : VisionFive JH7110A /VisionFive JH7110B
> - * D008: 8GB LPDDR4
> - * E000: No emmc device, ECxx: include emmc device, xx: Capacity size[GB]
> - * return: the field of 'D008E000'
> - */
> -
> -u32 get_ddr_size_from_eeprom(void)
> +u8 get_ddr_size_from_eeprom(void)
>   {
> -	u32 pv = 0xFFFFFFFF;
> -
>   	if (read_eeprom())
> -		return pv;
> +		return 0xFF;

Let's assume somebody cleared the EEPROM and the SPI flash.

Would it make sense to assume the minimum encodable RAM size (1 GiB) in 
this case, just to let the board boot to the console to allow writing 
the EEPROM with valid data again?

The current patch to simplify the code looks correct.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

>   
> -	return hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL);
> +	return (hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL) >> 16) & 0xFF;
>   }
>   
>   u32 get_mmc_size_from_eeprom(void)


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom()
  2025-08-29  6:09 ` [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom() Hal Feng
@ 2025-08-29  7:44   ` Heinrich Schuchardt
  2025-09-02  7:44     ` Hal Feng
  0 siblings, 1 reply; 30+ messages in thread
From: Heinrich Schuchardt @ 2025-08-29  7:44 UTC (permalink / raw)
  To: Hal Feng
  Cc: u-boot, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, E Shattow

On 29.08.25 08:09, Hal Feng wrote:
> pcb_revision is stored in the pcb_revision field of ATOM4. Correct it.
> Move the function description to the header file.
> Remove the function calls in board/starfive/visionfive2/.
> 
> Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>   arch/riscv/include/asm/arch-jh7110/eeprom.h   |  5 +++++
>   board/starfive/visionfive2/spl.c              | 18 ++++++-----------
>   .../visionfive2/starfive_visionfive2.c        | 20 ++++++-------------
>   .../visionfive2/visionfive2-i2c-eeprom.c      | 11 ++--------
>   4 files changed, 19 insertions(+), 35 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> index 6d0a0ba0c4a..025f1d32c49 100644
> --- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
> +++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> @@ -9,6 +9,11 @@
>   
>   #include <linux/types.h>
>   
> +/**
> + * get_pcb_revision_from_eeprom() - get the PCB revision
> + *
> + * @return: the PCB revision or 0xFF on error.
> + */
>   u8 get_pcb_revision_from_eeprom(void);
>   
>   /**
> diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
> index 3dfa931b655..901e7b58f36 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -126,19 +126,13 @@ int board_fit_config_name_match(const char *name)
>   		    !strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
>   		return 0;
>   	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.2a") &&
> -		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
> -		switch (get_pcb_revision_from_eeprom()) {
> -		case 'a':
> -		case 'A':
> -			return 0;
> -		}
> +		   (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7) ||
> +		    !strncmp(get_product_id_from_eeprom(), "VF7110a", 7))) {
> +		return 0;
>   	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.3b") &&
> -		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
> -		switch (get_pcb_revision_from_eeprom()) {
> -		case 'b':
> -		case 'B':
> -			return 0;
> -		}
> +		   (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
> +		    !strncmp(get_product_id_from_eeprom(), "VF7110b", 7))) {
> +		return 0;
>   	}
>   
>   	return -EINVAL;
> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c b/board/starfive/visionfive2/starfive_visionfive2.c
> index bfbb11a2ee7..f38433e94ac 100644
> --- a/board/starfive/visionfive2/starfive_visionfive2.c
> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> @@ -59,20 +59,12 @@ static void set_fdtfile(void)
>   		fdtfile = "starfive/jh7110-milkv-mars.dtb";
>   	} else if (!strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
>   		fdtfile = "starfive/jh7110-pine64-star64.dtb";
> -	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
> -		switch (get_pcb_revision_from_eeprom()) {
> -		case 'a':
> -		case 'A':
> -			fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb";
> -			break;
> -		case 'b':
> -		case 'B':
> -			fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
> -			break;
> -		default:
> -			log_err("Unknown revision\n");
> -			return;
> -		}
> +	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7) ||
> +		   !strncmp(get_product_id_from_eeprom(), "VF7110a", 7)) {

Where boards both with 'a' and 'b' ever shipped?
I have only seen 'A' and 'B' in pbuf.eeprom.atom1.data.pstr[6].

The change looks technically correct.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> +		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb";
> +	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
> +		   !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)) {
> +		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
>   	} else {
>   		log_err("Unknown product\n");
>   		return;
> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> index 3866d07f9d4..43b8af4fc59 100644
> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> @@ -535,19 +535,12 @@ int mac_read_from_eeprom(void)
>   	return 0;
>   }
>   
> -/**
> - * get_pcb_revision_from_eeprom - get the PCB revision
> - *
> - * 1.2A return 'A'/'a', 1.3B return 'B'/'b',other values are illegal
> - */
>   u8 get_pcb_revision_from_eeprom(void)
>   {
> -	u8 pv = 0xFF;
> -
>   	if (read_eeprom())
> -		return pv;
> +		return 0xFF;
>   
> -	return pbuf.eeprom.atom1.data.pstr[6];
> +	return pbuf.eeprom.atom4.data.pcb_revision;
>   }
>   
>   u8 get_ddr_size_from_eeprom(void)


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 05/10] eeprom: starfive: Update eeprom data format version to 3
  2025-08-29  6:09 ` [RFC 05/10] eeprom: starfive: Update eeprom data format version to 3 Hal Feng
@ 2025-08-29  7:47   ` Heinrich Schuchardt
  2025-09-02  7:10     ` Hal Feng
  2025-09-02 23:29   ` E Shattow
  1 sibling, 1 reply; 30+ messages in thread
From: Heinrich Schuchardt @ 2025-08-29  7:47 UTC (permalink / raw)
  To: Hal Feng
  Cc: u-boot, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, E Shattow

On 29.08.25 08:09, Hal Feng wrote:
> Add eeprom data format v3 support.
> Add wifi_bt field in ATOM4 and add "mac wifi_bt <?>" command to modify it.
> Set or show eth0/1 MAC address only if the board has eth0/1.
> 
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>   .../visionfive2/visionfive2-i2c-eeprom.c      | 91 ++++++++++++++++---
>   1 file changed, 78 insertions(+), 13 deletions(-)
> 
> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> index 43b8af4fc59..7352252c475 100644
> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> @@ -11,7 +11,7 @@
>   #include <u-boot/crc.h>
>   #include <linux/delay.h>
>   
> -#define FORMAT_VERSION		0x2
> +#define FORMAT_VERSION		0x3
>   #define PCB_VERSION		0xB1
>   #define BOM_VERSION		'A'
>   /*
> @@ -105,7 +105,8 @@ struct eeprom_atom4_data {
>   	u8 bom_revision;		/* BOM version */
>   	u8 mac0_addr[MAC_ADDR_BYTES];	/* Ethernet0 MAC */
>   	u8 mac1_addr[MAC_ADDR_BYTES];	/* Ethernet1 MAC */
> -	u8 reserved[2];
> +	u8 wifi_bt;			/* WIFI/BT support flag */
> +	u8 reserved;
>   };
>   
>   struct starfive_eeprom_atom4 {
> @@ -128,6 +129,35 @@ static union {
>   /* Set to 1 if we've read EEPROM into memory */
>   static int has_been_read __section(".data");
>   
> +static const char * const board_no_eth0_list[] = {
> +	"FML13V01",
> +};
> +
> +static const char * const board_no_eth1_list[] = {
> +	"FML13V01",
> +	"MARS",
> +	"VF7110SL",
> +};

Do you know in which category the Milk-V Mars CM ("MARC") falls?

Best regards

Heinrich

> +
> +static bool board_have_eth(u8 eth)
> +{
> +	int i;
> +
> +	if (eth == 0) {
> +		for (i = 0 ; i < ARRAY_SIZE(board_no_eth0_list); i++)
> +			if (!strncmp(pbuf.eeprom.atom1.data.pstr, board_no_eth0_list[i],
> +				     strlen(board_no_eth0_list[i])))
> +				return false;
> +	} else {
> +		for (i = 0 ; i < ARRAY_SIZE(board_no_eth1_list); i++)
> +			if (!strncmp(pbuf.eeprom.atom1.data.pstr, board_no_eth1_list[i],
> +				     strlen(board_no_eth1_list[i])))
> +				return false;
> +	}
> +
> +	return true;
> +}
> +
>   static inline int is_match_magic(void)
>   {
>   	return strncmp(pbuf.eeprom.header.signature, STARFIVE_EEPROM_HATS_SIG,
> @@ -176,17 +206,22 @@ static void show_eeprom(void)
>   	printf("Vendor : %s\n", pbuf.eeprom.atom1.data.vstr);
>   	printf("Product full SN: %s\n", pbuf.eeprom.atom1.data.pstr);
>   	printf("data version: 0x%x\n", pbuf.eeprom.atom4.data.version);
> -	if (pbuf.eeprom.atom4.data.version == 2) {
> +	if (pbuf.eeprom.atom4.data.version >= 2 && pbuf.eeprom.atom4.data.version <= 3) {
>   		printf("PCB revision: 0x%x\n", pbuf.eeprom.atom4.data.pcb_revision);
>   		printf("BOM revision: %c\n", pbuf.eeprom.atom4.data.bom_revision);
> -		printf("Ethernet MAC0 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> -		       pbuf.eeprom.atom4.data.mac0_addr[0], pbuf.eeprom.atom4.data.mac0_addr[1],
> -		       pbuf.eeprom.atom4.data.mac0_addr[2], pbuf.eeprom.atom4.data.mac0_addr[3],
> -		       pbuf.eeprom.atom4.data.mac0_addr[4], pbuf.eeprom.atom4.data.mac0_addr[5]);
> -		printf("Ethernet MAC1 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> -		       pbuf.eeprom.atom4.data.mac1_addr[0], pbuf.eeprom.atom4.data.mac1_addr[1],
> -		       pbuf.eeprom.atom4.data.mac1_addr[2], pbuf.eeprom.atom4.data.mac1_addr[3],
> -		       pbuf.eeprom.atom4.data.mac1_addr[4], pbuf.eeprom.atom4.data.mac1_addr[5]);
> +		if (board_have_eth(0))
> +			printf("Ethernet MAC0 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> +			pbuf.eeprom.atom4.data.mac0_addr[0], pbuf.eeprom.atom4.data.mac0_addr[1],
> +			pbuf.eeprom.atom4.data.mac0_addr[2], pbuf.eeprom.atom4.data.mac0_addr[3],
> +			pbuf.eeprom.atom4.data.mac0_addr[4], pbuf.eeprom.atom4.data.mac0_addr[5]);
> +
> +		if (board_have_eth(1))
> +			printf("Ethernet MAC1 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> +			pbuf.eeprom.atom4.data.mac1_addr[0], pbuf.eeprom.atom4.data.mac1_addr[1],
> +			pbuf.eeprom.atom4.data.mac1_addr[2], pbuf.eeprom.atom4.data.mac1_addr[3],
> +			pbuf.eeprom.atom4.data.mac1_addr[4], pbuf.eeprom.atom4.data.mac1_addr[5]);
> +		if (pbuf.eeprom.atom4.data.version >= 3)
> +			printf("WIFI/BT support: %x\n", pbuf.eeprom.atom4.data.wifi_bt);
>   	} else {
>   		printf("Custom data v%d is not Supported\n", pbuf.eeprom.atom4.data.version);
>   		dump_raw_eeprom();
> @@ -260,6 +295,7 @@ static void init_local_copy(void)
>   	pbuf.eeprom.atom4.data.bom_revision = BOM_VERSION;
>   	set_mac_address(STARFIVE_DEFAULT_MAC0, 0);
>   	set_mac_address(STARFIVE_DEFAULT_MAC1, 1);
> +	pbuf.eeprom.atom4.data.wifi_bt = 0;
>   }
>   
>   /**
> @@ -385,6 +421,28 @@ static void set_bom_revision(char *string)
>   	update_crc();
>   }
>   
> +/**
> + * set_wifi_bt() - stores a StarFive WIFI/BT support flag into the local EEPROM copy
> + *
> + * Takes a pointer to a string representing the numeric WIFI/BT support flag in
> + * decimal ("0" or "1"), stores it in the wifi_bt field of the
> + * EEPROM local copy, and updates the CRC of the local copy.
> + */
> +static void set_wifi_bt(char *string)
> +{
> +	u8 wifi_bt;
> +
> +	wifi_bt = simple_strtoul(string, &string, 16);
> +	if (wifi_bt > 1) {
> +		printf("WIFI/BT support flag must not be greater than 1");
> +		return;
> +	}
> +
> +	pbuf.eeprom.atom4.data.wifi_bt = wifi_bt;
> +
> +	update_crc();
> +}
> +
>   /**
>    * set_product_id() - stores a StarFive product ID into the local EEPROM copy
>    *
> @@ -478,6 +536,9 @@ int do_mac(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   	} else if (!strcmp(cmd, "bom_revision")) {
>   		set_bom_revision(argv[2]);
>   		return 0;
> +	} else if (!strcmp(cmd, "wifi_bt")) {
> +		set_wifi_bt(argv[2]);
> +		return 0;
>   	} else if (!strcmp(cmd, "product_id")) {
>   		set_product_id(argv[2]);
>   		return 0;
> @@ -513,8 +574,10 @@ int mac_read_from_eeprom(void)
>   	}
>   
>   	// 1, setup ethaddr env
> -	eth_env_set_enetaddr("ethaddr", pbuf.eeprom.atom4.data.mac0_addr);
> -	eth_env_set_enetaddr("eth1addr", pbuf.eeprom.atom4.data.mac1_addr);
> +	if (board_have_eth(0))
> +		eth_env_set_enetaddr("ethaddr", pbuf.eeprom.atom4.data.mac0_addr);
> +	if (board_have_eth(1))
> +		eth_env_set_enetaddr("eth1addr", pbuf.eeprom.atom4.data.mac1_addr);
>   
>   	/**
>   	 * 2, setup serial# env, reference to hifive-platform-i2c-eeprom.c,
> @@ -585,6 +648,8 @@ U_BOOT_LONGHELP(mac,
>   	"    - stores a StarFive PCB revision into the local EEPROM copy\n"
>   	"mac bom_revision <A>\n"
>   	"    - stores a StarFive BOM revision into the local EEPROM copy\n"
> +	"mac wifi_bt <?>\n"
> +	"    - stores a StarFive WIFI/BT support flag into the local EEPROM copy\n"
>   	"mac product_id <VF7110A1-2228-D008E000-xxxxxxxx>\n"
>   	"    - stores a StarFive product ID into the local EEPROM copy\n"
>   	"mac vendor <Vendor Name>\n"


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [RFC 05/10] eeprom: starfive: Update eeprom data format version to 3
  2025-08-29  7:47   ` Heinrich Schuchardt
@ 2025-09-02  7:10     ` Hal Feng
  0 siblings, 0 replies; 30+ messages in thread
From: Hal Feng @ 2025-09-02  7:10 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: u-boot@lists.denx.de, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, E Shattow

> On 29.08.25 15:47, Heinrich Schuchardt wrote:
> On 29.08.25 08:09, Hal Feng wrote:
> > Add eeprom data format v3 support.
> > Add wifi_bt field in ATOM4 and add "mac wifi_bt <?>" command to modify
> it.
> > Set or show eth0/1 MAC address only if the board has eth0/1.
> >
> > Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> > ---
> >   .../visionfive2/visionfive2-i2c-eeprom.c      | 91 ++++++++++++++++---
> >   1 file changed, 78 insertions(+), 13 deletions(-)
> >
> > diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> > b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> > index 43b8af4fc59..7352252c475 100644
> > --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> > +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> > @@ -11,7 +11,7 @@
> >   #include <u-boot/crc.h>
> >   #include <linux/delay.h>
> >
> > -#define FORMAT_VERSION		0x2
> > +#define FORMAT_VERSION		0x3
> >   #define PCB_VERSION		0xB1
> >   #define BOM_VERSION		'A'
> >   /*
> > @@ -105,7 +105,8 @@ struct eeprom_atom4_data {
> >   	u8 bom_revision;		/* BOM version */
> >   	u8 mac0_addr[MAC_ADDR_BYTES];	/* Ethernet0 MAC */
> >   	u8 mac1_addr[MAC_ADDR_BYTES];	/* Ethernet1 MAC */
> > -	u8 reserved[2];
> > +	u8 wifi_bt;			/* WIFI/BT support flag */
> > +	u8 reserved;
> >   };
> >
> >   struct starfive_eeprom_atom4 {
> > @@ -128,6 +129,35 @@ static union {
> >   /* Set to 1 if we've read EEPROM into memory */
> >   static int has_been_read __section(".data");
> >
> > +static const char * const board_no_eth0_list[] = {
> > +	"FML13V01",
> > +};
> > +
> > +static const char * const board_no_eth1_list[] = {
> > +	"FML13V01",
> > +	"MARS",
> > +	"VF7110SL",
> > +};
> 
> Do you know in which category the Milk-V Mars CM ("MARC") falls?

Milk-V Mars CM board has eth0 but no eth1.
Will add "MARC" in board_no_eth1_list. Thanks.

Best regards,
Hal

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom()
  2025-08-29  7:44   ` Heinrich Schuchardt
@ 2025-09-02  7:44     ` Hal Feng
  2025-09-02 22:12       ` E Shattow
  0 siblings, 1 reply; 30+ messages in thread
From: Hal Feng @ 2025-09-02  7:44 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: u-boot@lists.denx.de, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, E Shattow

> On 29.08.25 15:44, Heinrich Schuchardt wrote:
> On 29.08.25 08:09, Hal Feng wrote:
> > pcb_revision is stored in the pcb_revision field of ATOM4. Correct it.
> > Move the function description to the header file.
> > Remove the function calls in board/starfive/visionfive2/.
> >
> > Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
> > Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> > ---
> >   arch/riscv/include/asm/arch-jh7110/eeprom.h   |  5 +++++
> >   board/starfive/visionfive2/spl.c              | 18 ++++++-----------
> >   .../visionfive2/starfive_visionfive2.c        | 20 ++++++-------------
> >   .../visionfive2/visionfive2-i2c-eeprom.c      | 11 ++--------
> >   4 files changed, 19 insertions(+), 35 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h
> b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> > index 6d0a0ba0c4a..025f1d32c49 100644
> > --- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
> > +++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> > @@ -9,6 +9,11 @@
> >
> >   #include <linux/types.h>
> >
> > +/**
> > + * get_pcb_revision_from_eeprom() - get the PCB revision
> > + *
> > + * @return: the PCB revision or 0xFF on error.
> > + */
> >   u8 get_pcb_revision_from_eeprom(void);
> >
> >   /**
> > diff --git a/board/starfive/visionfive2/spl.c
> b/board/starfive/visionfive2/spl.c
> > index 3dfa931b655..901e7b58f36 100644
> > --- a/board/starfive/visionfive2/spl.c
> > +++ b/board/starfive/visionfive2/spl.c
> > @@ -126,19 +126,13 @@ int board_fit_config_name_match(const char
> *name)
> >   		    !strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
> >   		return 0;
> >   	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.2a")
> &&
> > -		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
> > -		switch (get_pcb_revision_from_eeprom()) {
> > -		case 'a':
> > -		case 'A':
> > -			return 0;
> > -		}
> > +		   (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7)
> ||
> > +		    !strncmp(get_product_id_from_eeprom(), "VF7110a", 7)))
> {
> > +		return 0;
> >   	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.3b")
> &&
> > -		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
> > -		switch (get_pcb_revision_from_eeprom()) {
> > -		case 'b':
> > -		case 'B':
> > -			return 0;
> > -		}
> > +		   (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7)
> ||
> > +		    !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)))
> {
> > +		return 0;
> >   	}
> >
> >   	return -EINVAL;
> > diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
> b/board/starfive/visionfive2/starfive_visionfive2.c
> > index bfbb11a2ee7..f38433e94ac 100644
> > --- a/board/starfive/visionfive2/starfive_visionfive2.c
> > +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> > @@ -59,20 +59,12 @@ static void set_fdtfile(void)
> >   		fdtfile = "starfive/jh7110-milkv-mars.dtb";
> >   	} else if (!strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
> >   		fdtfile = "starfive/jh7110-pine64-star64.dtb";
> > -	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
> > -		switch (get_pcb_revision_from_eeprom()) {
> > -		case 'a':
> > -		case 'A':
> > -			fdtfile = "starfive/jh7110-starfive-visionfive-2-
> v1.2a.dtb";
> > -			break;
> > -		case 'b':
> > -		case 'B':
> > -			fdtfile = "starfive/jh7110-starfive-visionfive-2-
> v1.3b.dtb";
> > -			break;
> > -		default:
> > -			log_err("Unknown revision\n");
> > -			return;
> > -		}
> > +	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7) ||
> > +		   !strncmp(get_product_id_from_eeprom(), "VF7110a", 7)) {
> 
> Where boards both with 'a' and 'b' ever shipped?
> I have only seen 'A' and 'B' in pbuf.eeprom.atom1.data.pstr[6].

You're right. There are no boards marked "VF7110a" or "VF7110b". Let's remove
the comparison of "VF7110a" and "VF7110b".

Best regards,
Hal

> 
> The change looks technically correct.
> 
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> 
> > +		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb";
> > +	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
> > +		   !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)) {
> > +		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
> >   	} else {
> >   		log_err("Unknown product\n");
> >   		return;
> > diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> > index 3866d07f9d4..43b8af4fc59 100644
> > --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> > +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> > @@ -535,19 +535,12 @@ int mac_read_from_eeprom(void)
> >   	return 0;
> >   }
> >
> > -/**
> > - * get_pcb_revision_from_eeprom - get the PCB revision
> > - *
> > - * 1.2A return 'A'/'a', 1.3B return 'B'/'b',other values are illegal
> > - */
> >   u8 get_pcb_revision_from_eeprom(void)
> >   {
> > -	u8 pv = 0xFF;
> > -
> >   	if (read_eeprom())
> > -		return pv;
> > +		return 0xFF;
> >
> > -	return pbuf.eeprom.atom1.data.pstr[6];
> > +	return pbuf.eeprom.atom4.data.pcb_revision;
> >   }
> >
> >   u8 get_ddr_size_from_eeprom(void)


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 01/10] riscv: dts: starfive: jh7110-common: Move out some nodes to the board dts
  2025-08-29  7:19   ` Heinrich Schuchardt
@ 2025-09-02 16:17     ` E Shattow
  0 siblings, 0 replies; 30+ messages in thread
From: E Shattow @ 2025-09-02 16:17 UTC (permalink / raw)
  To: Heinrich Schuchardt, Hal Feng
  Cc: u-boot, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing

Hello Hal, and Heinrich,

On 8/29/25 00:19, Heinrich Schuchardt wrote:
> On 29.08.25 08:09, Hal Feng wrote:
>> Some node in this file are not used by the upcoming VisionFive 2 Lite
>> board. Move them to the board dts to prepare for adding the new
>> VisionFive 2 Lite device tree.
>>
>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> 
> LGTM
> 
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> 

NAK

Better to drop jh7110-common-u-boot.dtsi from U-Boot [1]. Also we can
drop jh7110-u-boot.dtsi file but that does require some additional work:

- upstream dt-binding memory-controllers/starfive,jh7110-dmc.yaml [2]
- upstream devicetree changes adding memory controller [3]
- upstream bootph-pre-ram hints [4]
- implement cpufreq driver
- refactor drivers/ram/starfive/starfive_ddr.c to use common clock
framework and delete 'clock-frequency = <2133>' in dts

I am not interested in writing U-Boot drivers. Hal, can you make the
needed improvements in U-Boot for cpufreq and memory controller drivers?

1: https://lore.kernel.org/u-boot/20250815050315.62956-1-e@freeshell.de/
2: https://lore.kernel.org/lkml/20250823100159.203925-2-e@freeshell.de/
3: https://lore.kernel.org/lkml/20250823100159.203925-3-e@freeshell.de/
4: https://lore.kernel.org/lkml/20250823100159.203925-4-e@freeshell.de/

Thanks,  -E Shattow

>> ---
>>   .../src/riscv/starfive/jh7110-common.dtsi     | 22 ---------
>>   .../jh7110-deepcomputing-fml13v01.dts         | 49 +++++++++++++++++++
>>   .../src/riscv/starfive/jh7110-milkv-mars.dts  | 49 +++++++++++++++++++
>>   .../riscv/starfive/jh7110-pine64-star64.dts   | 49 +++++++++++++++++++
>>   .../jh7110-starfive-visionfive-2.dtsi         | 46 +++++++++++++++++
>>   dts/upstream/src/riscv/starfive/jh7110.dtsi   | 16 ------
>>   6 files changed, 193 insertions(+), 38 deletions(-)
>>
>> diff --git a/dts/upstream/src/riscv/starfive/jh7110-common.dtsi b/dts/
>> upstream/src/riscv/starfive/jh7110-common.dtsi
>> index 4baeb981d4d..9d3d03ad2ed 100644
>> --- a/dts/upstream/src/riscv/starfive/jh7110-common.dtsi
>> +++ b/dts/upstream/src/riscv/starfive/jh7110-common.dtsi
>> @@ -272,15 +272,9 @@
>>       assigned-clock-rates = <50000000>;
>>       bus-width = <8>;
>>       bootph-pre-ram;
>> -    cap-mmc-highspeed;
>> -    mmc-ddr-1_8v;
>> -    mmc-hs200-1_8v;
>> -    cap-mmc-hw-reset;
>>       post-power-on-delay-ms = <200>;
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&mmc0_pins>;
>> -    vmmc-supply = <&vcc_3v3>;
>> -    vqmmc-supply = <&emmc_vdd>;
>>       status = "okay";
>>   };
>>   @@ -290,12 +284,7 @@
>>       assigned-clock-rates = <50000000>;
>>       bus-width = <4>;
>>       bootph-pre-ram;
>> -    no-sdio;
>> -    no-mmc;
>> -    cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
>> -    disable-wp;
>>       cap-sd-highspeed;
>> -    post-power-on-delay-ms = <200>;
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&mmc1_pins>;
>>       status = "okay";
>> @@ -439,17 +428,6 @@
>>       };
>>         mmc0_pins: mmc0-0 {
>> -         rst-pins {
>> -            pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
>> -                          GPOEN_ENABLE,
>> -                          GPI_NONE)>;
>> -            bias-pull-up;
>> -            drive-strength = <12>;
>> -            input-disable;
>> -            input-schmitt-disable;
>> -            slew-rate = <0>;
>> -        };
>> -
>>           mmc-pins {
>>               pinmux = <PINMUX(PAD_SD0_CLK, 0)>,
>>                    <PINMUX(PAD_SD0_CMD, 0)>,
>> diff --git a/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-
>> fml13v01.dts b/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-
>> fml13v01.dts
>> index f2857d021d6..5a2a41a7e8c 100644
>> --- a/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts
>> +++ b/dts/upstream/src/riscv/starfive/jh7110-deepcomputing-fml13v01.dts
>> @@ -11,6 +11,55 @@
>>       compatible = "deepcomputing,fml13v01", "starfive,jh7110";
>>   };
>>   +&cpu_opp {
>> +    opp-375000000 {
>> +        opp-hz = /bits/ 64 <375000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-500000000 {
>> +        opp-hz = /bits/ 64 <500000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-750000000 {
>> +        opp-hz = /bits/ 64 <750000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-1500000000 {
>> +        opp-hz = /bits/ 64 <1500000000>;
>> +        opp-microvolt = <1040000>;
>> +    };
>> +};
>> +
>> +&mmc0 {
>> +    cap-mmc-highspeed;
>> +    cap-mmc-hw-reset;
>> +    mmc-ddr-1_8v;
>> +    mmc-hs200-1_8v;
>> +    vmmc-supply = <&vcc_3v3>;
>> +    vqmmc-supply = <&emmc_vdd>;
>> +};
>> +
>> +&mmc0_pins {
>> +    rst-pins {
>> +        pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
>> +                      GPOEN_ENABLE,
>> +                      GPI_NONE)>;
>> +        bias-pull-up;
>> +        drive-strength = <12>;
>> +        input-disable;
>> +        input-schmitt-disable;
>> +        slew-rate = <0>;
>> +    };
>> +};
>> +
>> +&mmc1 {
>> +    no-sdio;
>> +    no-mmc;
>> +    cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
>> +    disable-wp;
>> +    post-power-on-delay-ms = <200>;
>> +};
>> +
>>   &pcie1 {
>>       perst-gpios = <&sysgpio 21 GPIO_ACTIVE_LOW>;
>>       phys = <&pciephy1>;
>> diff --git a/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts b/
>> dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts
>> index 3bd62ab7852..0c90facc4ee 100644
>> --- a/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts
>> +++ b/dts/upstream/src/riscv/starfive/jh7110-milkv-mars.dts
>> @@ -11,6 +11,25 @@
>>       compatible = "milkv,mars", "starfive,jh7110";
>>   };
>>   +&cpu_opp {
>> +    opp-375000000 {
>> +        opp-hz = /bits/ 64 <375000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-500000000 {
>> +        opp-hz = /bits/ 64 <500000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-750000000 {
>> +        opp-hz = /bits/ 64 <750000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-1500000000 {
>> +        opp-hz = /bits/ 64 <1500000000>;
>> +        opp-microvolt = <1040000>;
>> +    };
>> +};
>> +
>>   &gmac0 {
>>       starfive,tx-use-rgmii-clk;
>>       assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
>> @@ -22,6 +41,36 @@
>>       status = "okay";
>>   };
>>   +&mmc0 {
>> +    cap-mmc-highspeed;
>> +    cap-mmc-hw-reset;
>> +    mmc-ddr-1_8v;
>> +    mmc-hs200-1_8v;
>> +    vmmc-supply = <&vcc_3v3>;
>> +    vqmmc-supply = <&emmc_vdd>;
>> +};
>> +
>> +&mmc0_pins {
>> +    rst-pins {
>> +        pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
>> +                      GPOEN_ENABLE,
>> +                      GPI_NONE)>;
>> +        bias-pull-up;
>> +        drive-strength = <12>;
>> +        input-disable;
>> +        input-schmitt-disable;
>> +        slew-rate = <0>;
>> +    };
>> +};
>> +
>> +&mmc1 {
>> +    no-sdio;
>> +    no-mmc;
>> +    cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
>> +    disable-wp;
>> +    post-power-on-delay-ms = <200>;
>> +};
>> +
>>   &pcie0 {
>>       status = "okay";
>>   };
>> diff --git a/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
>> b/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
>> index 31e825be206..c9677aef9ff 100644
>> --- a/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
>> +++ b/dts/upstream/src/riscv/starfive/jh7110-pine64-star64.dts
>> @@ -14,6 +14,25 @@
>>       };
>>   };
>>   +&cpu_opp {
>> +    opp-375000000 {
>> +        opp-hz = /bits/ 64 <375000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-500000000 {
>> +        opp-hz = /bits/ 64 <500000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-750000000 {
>> +        opp-hz = /bits/ 64 <750000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-1500000000 {
>> +        opp-hz = /bits/ 64 <1500000000>;
>> +        opp-microvolt = <1040000>;
>> +    };
>> +};
>> +
>>   &gmac0 {
>>       starfive,tx-use-rgmii-clk;
>>       assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
>> @@ -44,6 +63,36 @@
>>       status = "okay";
>>   };
>>   +&mmc0 {
>> +    cap-mmc-highspeed;
>> +    cap-mmc-hw-reset;
>> +    mmc-ddr-1_8v;
>> +    mmc-hs200-1_8v;
>> +    vmmc-supply = <&vcc_3v3>;
>> +    vqmmc-supply = <&emmc_vdd>;
>> +};
>> +
>> +&mmc0_pins {
>> +    rst-pins {
>> +        pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
>> +                      GPOEN_ENABLE,
>> +                      GPI_NONE)>;
>> +        bias-pull-up;
>> +        drive-strength = <12>;
>> +        input-disable;
>> +        input-schmitt-disable;
>> +        slew-rate = <0>;
>> +    };
>> +};
>> +
>> +&mmc1 {
>> +    no-sdio;
>> +    no-mmc;
>> +    cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
>> +    disable-wp;
>> +    post-power-on-delay-ms = <200>;
>> +};
>> +
>>   &pcie1 {
>>       status = "okay";
>>   };
>> diff --git a/dts/upstream/src/riscv/starfive/jh7110-starfive-
>> visionfive-2.dtsi b/dts/upstream/src/riscv/starfive/jh7110-starfive-
>> visionfive-2.dtsi
>> index 5f14afb2c24..d1e4206f125 100644
>> --- a/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/dts/upstream/src/riscv/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -13,6 +13,25 @@
>>       };
>>   };
>>   +&cpu_opp {
>> +    opp-375000000 {
>> +        opp-hz = /bits/ 64 <375000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-500000000 {
>> +        opp-hz = /bits/ 64 <500000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-750000000 {
>> +        opp-hz = /bits/ 64 <750000000>;
>> +        opp-microvolt = <800000>;
>> +    };
>> +    opp-1500000000 {
>> +        opp-hz = /bits/ 64 <1500000000>;
>> +        opp-microvolt = <1040000>;
>> +    };
>> +};
>> +
>>   &gmac0 {
>>       status = "okay";
>>   };
>> @@ -38,9 +57,36 @@
>>   };
>>     &mmc0 {
>> +    cap-mmc-highspeed;
>> +    cap-mmc-hw-reset;
>> +    mmc-ddr-1_8v;
>> +    mmc-hs200-1_8v;
>> +    vmmc-supply = <&vcc_3v3>;
>> +    vqmmc-supply = <&emmc_vdd>;
>>       non-removable;
>>   };
>>   +&mmc0_pins {
>> +    rst-pins {
>> +        pinmux = <GPIOMUX(62, GPOUT_SYS_SDIO0_RST,
>> +                      GPOEN_ENABLE,
>> +                      GPI_NONE)>;
>> +        bias-pull-up;
>> +        drive-strength = <12>;
>> +        input-disable;
>> +        input-schmitt-disable;
>> +        slew-rate = <0>;
>> +    };
>> +};
>> +
>> +&mmc1 {
>> +    no-sdio;
>> +    no-mmc;
>> +    cd-gpios = <&sysgpio 41 GPIO_ACTIVE_LOW>;
>> +    disable-wp;
>> +    post-power-on-delay-ms = <200>;
>> +};
>> +
>>   &pcie0 {
>>       status = "okay";
>>   };
>> diff --git a/dts/upstream/src/riscv/starfive/jh7110.dtsi b/dts/
>> upstream/src/riscv/starfive/jh7110.dtsi
>> index 0ba74ef0467..d2463399b95 100644
>> --- a/dts/upstream/src/riscv/starfive/jh7110.dtsi
>> +++ b/dts/upstream/src/riscv/starfive/jh7110.dtsi
>> @@ -200,22 +200,6 @@
>>       cpu_opp: opp-table-0 {
>>               compatible = "operating-points-v2";
>>               opp-shared;
>> -            opp-375000000 {
>> -                    opp-hz = /bits/ 64 <375000000>;
>> -                    opp-microvolt = <800000>;
>> -            };
>> -            opp-500000000 {
>> -                    opp-hz = /bits/ 64 <500000000>;
>> -                    opp-microvolt = <800000>;
>> -            };
>> -            opp-750000000 {
>> -                    opp-hz = /bits/ 64 <750000000>;
>> -                    opp-microvolt = <800000>;
>> -            };
>> -            opp-1500000000 {
>> -                    opp-hz = /bits/ 64 <1500000000>;
>> -                    opp-microvolt = <1040000>;
>> -            };
>>       };
>>         thermal-zones {
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 02/10] riscv: dts: starfive: Add VisionFive 2 Lite board device tree
  2025-08-29  6:09 ` [RFC 02/10] riscv: dts: starfive: Add VisionFive 2 Lite board device tree Hal Feng
@ 2025-09-02 20:03   ` E Shattow
  0 siblings, 0 replies; 30+ messages in thread
From: E Shattow @ 2025-09-02 20:03 UTC (permalink / raw)
  To: Hal Feng, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, Heinrich Schuchardt
  Cc: u-boot

Hi Hal,

On 8/28/25 23:09, Hal Feng wrote:
> VisionFive 2 Lite is a mini SBC based on the StarFive JH7110S SoC.
> 
> Board features:
> - JH7110S SoC
> - 2/4/8 GiB LPDDR4 DRAM
> - AXP15060 PMIC
> - 40 pin GPIO header
> - 1x USB 3.0 host port
> - 3x USB 2.0 host port
> - 1x M.2 M-Key (size: 2242)
> - 1x MicroSD slot (optional non-removable eMMC)
> - 1x QSPI Flash
> - 1x I2C EEPROM
> - 1x 1Gbps Ethernet port
> - SDIO-based Wi-Fi & UART-based Bluetooth
> - 1x HDMI port
> - 1x 2-lane DSI
> - 1x 2-lane CSI
> 

Prefer to see subject line and URL of recent upstream discussion or git
web commit for the new or modified dts changes from upstream of
devicetree-rebasing in this commit and not just in the cover letter.

+ From upstream LKML discussion "[RFC 3/3] riscv: dts: starfive: Add
VisionFive 2 Lite board device tree" [1].
+ 1:
https://lore.kernel.org/all/20250821100930.71404-4-hal.feng@starfivetech.com/

> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  .../jh7110s-starfive-visionfive-2-lite.dts    | 152 ++++++++++++++++++
>  1 file changed, 152 insertions(+)
>  create mode 100644 dts/upstream/src/riscv/starfive/jh7110s-starfive-visionfive-2-lite.dts
> 
> diff --git a/dts/upstream/src/riscv/starfive/jh7110s-starfive-visionfive-2-lite.dts b/dts/upstream/src/riscv/starfive/jh7110s-starfive-visionfive-2-lite.dts
> new file mode 100644
> index 00000000000..a0cb9912eb8
> --- /dev/null
> +++ b/dts/upstream/src/riscv/starfive/jh7110s-starfive-visionfive-2-lite.dts

No, do not directly add or modify upstream dts in dts/upstream/src
subtree. We are not permitted in U-Boot to merge out-of-band devicetree
changes to the devicetree-rebasing subtree as this creates more work for
Tom, if I understand correctly?

Instead add or modify this dts as
arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi filename.

Note: Locally for development purpose I would like to extend U-Boot to
choose a Linux source tree path for location of devicetree. Today
OF_UPSTREAM support in U-Boot build system assumes to use the
devicetree-rebasing subtree and the path is not meant to be changed. The
devicetree-rebasing scripts assume only to merge from regular Linux
release version tags and not arbitrary commit IDs. As a workaround I do
something like git recursive remove the
dts/upstream/src/$(ARCH)/$(VENDOR) and replace with symbolic link to the
Linux source tree arch/$(ARCH)/boot/dts/$(VENDOR) for development. I
would like better if it was simply an environment flag for make system
where Linux or devicetree-rebasing source tree is located, but that is
for a different discussion not in this series.

> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2025 StarFive Technology Co., Ltd.
> + * Copyright (C) 2025 Hal Feng <hal.feng@starfivetech.com>
> + */
> +
> +/dts-v1/;
> +#include "jh7110-common.dtsi"
> +
> +/ {
> +	model = "StarFive VisionFive 2 Lite";
> +	compatible = "starfive,visionfive-2-lite", "starfive,jh7110s";
> +};
> +
> +&cpu_opp {
> +	opp-312500000 {
> +		opp-hz = /bits/ 64 <312500000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-417000000 {
> +		opp-hz = /bits/ 64 <417000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-625000000 {
> +		opp-hz = /bits/ 64 <625000000>;
> +		opp-microvolt = <800000>;
> +	};
> +	opp-1250000000 {
> +		opp-hz = /bits/ 64 <1250000000>;
> +		opp-microvolt = <1000000>;
> +	};
> +};
> +
> +&gmac0 {
> +	starfive,tx-use-rgmii-clk;
> +	assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>;
> +	assigned-clock-parents = <&aoncrg JH7110_AONCLK_GMAC0_RMII_RTX>;
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	status = "okay";
> +};
> +
> +&mmc0 {
> +	bus-width = <4>;
> +	no-sdio;
> +	no-mmc;
> +	cd-gpios = <&sysgpio 41 GPIO_ACTIVE_HIGH>;
> +	disable-wp;
> +	cap-sd-highspeed;
> +};
> +
> +&mmc1 {
> +	max-frequency = <50000000>;
> +	keep-power-in-suspend;
> +	non-removable;
> +};
> +
> +&pcie1 {
> +	enable-gpios = <&sysgpio 27 GPIO_ACTIVE_HIGH>;
> +	status = "okay";
> +};
> +
> +&phy0 {
> +	motorcomm,tx-clk-adj-enabled;
> +	motorcomm,tx-clk-100-inverted;
> +	motorcomm,tx-clk-1000-inverted;
> +	motorcomm,rx-clk-drv-microamp = <3970>;
> +	motorcomm,rx-data-drv-microamp = <2910>;
> +	rx-internal-delay-ps = <1500>;
> +	tx-internal-delay-ps = <1500>;
> +};
> +
> +&pwm {
> +	status = "okay";
> +};
> +
> +&spi0 {
> +	status = "okay";
> +};
> +
> +&syscrg {
> +	assigned-clock-rates = <0>, <0>, <0>, <0>, <500000000>, <1250000000>;
> +};
> +
> +&sysgpio {
> +	uart1_pins: uart1-0 {
> +		tx-pins {
> +			pinmux = <GPIOMUX(22, GPOUT_SYS_UART1_TX,
> +					      GPOEN_ENABLE,
> +					      GPI_NONE)>;
> +			bias-disable;
> +			drive-strength = <12>;
> +			input-disable;
> +			input-schmitt-disable;
> +			slew-rate = <0>;
> +		};
> +
> +		rx-pins {
> +			pinmux = <GPIOMUX(23, GPOUT_LOW,
> +					      GPOEN_DISABLE,
> +					      GPI_SYS_UART1_RX)>;
> +			bias-pull-up;
> +			drive-strength = <2>;
> +			input-enable;
> +			input-schmitt-enable;
> +			slew-rate = <0>;
> +		};
> +
> +		cts-pins {
> +			pinmux = <GPIOMUX(24, GPOUT_LOW,
> +					      GPOEN_DISABLE,
> +					      GPI_SYS_UART1_CTS)>;
> +			input-enable;
> +		};
> +
> +		rts-pins {
> +			pinmux = <GPIOMUX(25, GPOUT_SYS_UART1_RTS,
> +					      GPOEN_ENABLE,
> +					      GPI_NONE)>;
> +			input-enable;
> +		};
> +	};
> +
> +	usb0_pins: usb0-0 {
> +		power-pins {
> +			pinmux = <GPIOMUX(26, GPOUT_HIGH,
> +					      GPOEN_ENABLE,
> +					      GPI_NONE)>;
> +			input-disable;
> +		};
> +	};
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart1_pins>;
> +	status = "okay";
> +};
> +
> +&usb0 {
> +	dr_mode = "host";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&usb0_pins>;
> +	status = "okay";
> +};
> +
> +&usb_cdns3 {
> +	phys = <&usbphy0>, <&pciephy0>;
> +	phy-names = "cdns3,usb2-phy", "cdns3,usb3-phy";
> +};

Append the last line of this file an '#include' statement corresponding
with the next automatic dtsi inclusion order [1] as appropriate,
currently when we drop file jh7110-common-u-boot.dtsi from U-Boot:

+ #include "jh7110-u-boot.dtsi"

Or better approach in future for last line of the file would be if when
CONFIG_NAME is added to the search order [2], then this include would
instead be a hyphenated form of $(CONFIG_SYS_CONFIG_NAME)-u-boot.dtsi:

+ #include "starfive-visionfive2-u-boot.dtsi"

And drop all dts/riscv/jh7110*{.dts,.dtsi} not existing in
dts/upstream/src subtree.

Anyways I discover the build system is not working correctly for '$<'
symbol [3] and this breaks our assumption that
arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi would be
automatically included. It is needed to fix the situation separate from
this series.

1: https://docs.u-boot.org/en/latest/develop/devicetree/control.html
2: https://lore.kernel.org/u-boot/20250826214708.309271-1-e@freeshell.de/
3: https://lore.kernel.org/u-boot/20250901094506.647427-1-e@freeshell.de/

-E

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 03/10] eeprom: starfive: Simplify get_ddr_size_from_eeprom()
  2025-08-29  7:33   ` Heinrich Schuchardt
@ 2025-09-02 21:28     ` E Shattow
  2025-09-02 22:18       ` Heinrich Schuchardt
  0 siblings, 1 reply; 30+ messages in thread
From: E Shattow @ 2025-09-02 21:28 UTC (permalink / raw)
  To: Heinrich Schuchardt, Hal Feng
  Cc: u-boot, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing


On 8/29/25 00:33, Heinrich Schuchardt wrote:
> On 29.08.25 08:09, Hal Feng wrote:
>> Directly return the DDR size instead of the field of 'DxxxExxx'.
>> Move the function description to the header file.
>>
>> Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
>> ---
>>   arch/riscv/cpu/jh7110/spl.c                     |  2 +-
>>   arch/riscv/include/asm/arch-jh7110/eeprom.h     |  8 +++++++-
>>   .../visionfive2/visionfive2-i2c-eeprom.c        | 17 +++--------------
>>   3 files changed, 11 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c
>> index 87aaf865246..3aece7d995b 100644
>> --- a/arch/riscv/cpu/jh7110/spl.c
>> +++ b/arch/riscv/cpu/jh7110/spl.c
>> @@ -41,7 +41,7 @@ int spl_dram_init(void)
>>       /* Read the definition of the DDR size from eeprom, and if not,
>>        * use the definition in DT
>>        */
>> -    size = (get_ddr_size_from_eeprom() >> 16) & 0xFF;
>> +    size = get_ddr_size_from_eeprom();
>>       if (check_ddr_size(size))
>>           gd->ram_size = size << 30;
>>   diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h b/arch/
>> riscv/include/asm/arch-jh7110/eeprom.h
>> index 45ad2a5f7bc..6d0a0ba0c4a 100644
>> --- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
>> +++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
>> @@ -10,7 +10,13 @@
>>   #include <linux/types.h>
>>     u8 get_pcb_revision_from_eeprom(void);
>> -u32 get_ddr_size_from_eeprom(void);
>> +
>> +/**
>> + * get_ddr_size_from_eeprom() - read DDR size from EEPROM
>> + *
>> + * @return: size in GiB or 0xFF on error.

No. This is not consistent with get_mmc_size_from_eeprom() return value
which uses 0 as short-circuit return value if read_eeprom() fails.

It is not important to make the distinction here if eeprom read failed
or the data was not a valid size (i.e. zero from successful eeprom read).

>> + */
>> +u8 get_ddr_size_from_eeprom(void);
>>     /**
>>    * get_mmc_size_from_eeprom() - read eMMC size from EEPROM
>> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c b/
>> board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>> index 17a44020bcf..3866d07f9d4 100644
>> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>> @@ -550,23 +550,12 @@ u8 get_pcb_revision_from_eeprom(void)
>>       return pbuf.eeprom.atom1.data.pstr[6];
>>   }
>>   -/**
>> - * get_ddr_size_from_eeprom - get the DDR size
>> - * pstr:  VF7110A1-2228-D008E000-00000001
>> - * VF7110A1/VF7110B1 : VisionFive JH7110A /VisionFive JH7110B
>> - * D008: 8GB LPDDR4
>> - * E000: No emmc device, ECxx: include emmc device, xx: Capacity
>> size[GB]
>> - * return: the field of 'D008E000'
>> - */
>> -
>> -u32 get_ddr_size_from_eeprom(void)
>> +u8 get_ddr_size_from_eeprom(void)
>>   {
>> -    u32 pv = 0xFFFFFFFF;
>> -
>>       if (read_eeprom())
>> -        return pv;
>> +        return 0xFF;

No, inverted logic, see above return of 0xFF as an error condition is
suspect.

> 
> Let's assume somebody cleared the EEPROM and the SPI flash.
> 
> Would it make sense to assume the minimum encodable RAM size (1 GiB) in
> this case, just to let the board boot to the console to allow writing
> the EEPROM with valid data again?
> 
> The current patch to simplify the code looks correct.
> 
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> 
>>   -    return hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL);
>> +    return (hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL) >> 16) &
>> 0xFF;
>>   }
>>     u32 get_mmc_size_from_eeprom(void)
> 

Yes, later fallback to 2G size configuration makes sense. Compatible
products with less than 2G do not exist and probably never will; there
may be some developer boards with 1G but none I have ever heard of
directly in conversations since 2023. The other common situation with
zero or uninitialized EEPROM would be a failed read of eeprom i.e. RTC
module at same i2c address or missing devicetree nodes at SPL phase.

1. DDR size detection heuristic should happen first without need of
eeprom, if possible.

2. Else warn DDR detection failed and read size from eeprom config

3. Else warn eeprom memory config invalid and set size set to default 2G.

-E

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom()
  2025-09-02  7:44     ` Hal Feng
@ 2025-09-02 22:12       ` E Shattow
  2025-09-02 22:32         ` Heinrich Schuchardt
  0 siblings, 1 reply; 30+ messages in thread
From: E Shattow @ 2025-09-02 22:12 UTC (permalink / raw)
  To: Hal Feng, Heinrich Schuchardt
  Cc: u-boot@lists.denx.de, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing


On 9/2/25 00:44, Hal Feng wrote:
>> On 29.08.25 15:44, Heinrich Schuchardt wrote:
>> On 29.08.25 08:09, Hal Feng wrote:
>>> pcb_revision is stored in the pcb_revision field of ATOM4. Correct it.
>>> Move the function description to the header file.
>>> Remove the function calls in board/starfive/visionfive2/.
>>>
>>> Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
>>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
>>> ---
>>>   arch/riscv/include/asm/arch-jh7110/eeprom.h   |  5 +++++
>>>   board/starfive/visionfive2/spl.c              | 18 ++++++-----------
>>>   .../visionfive2/starfive_visionfive2.c        | 20 ++++++-------------
>>>   .../visionfive2/visionfive2-i2c-eeprom.c      | 11 ++--------
>>>   4 files changed, 19 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h
>> b/arch/riscv/include/asm/arch-jh7110/eeprom.h
>>> index 6d0a0ba0c4a..025f1d32c49 100644
>>> --- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
>>> +++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
>>> @@ -9,6 +9,11 @@
>>>
>>>   #include <linux/types.h>
>>>
>>> +/**
>>> + * get_pcb_revision_from_eeprom() - get the PCB revision
>>> + *
>>> + * @return: the PCB revision or 0xFF on error.
>>> + */

No. Inverted logic. See below comment about body of function.

>>>   u8 get_pcb_revision_from_eeprom(void);
>>>
>>>   /**
>>> diff --git a/board/starfive/visionfive2/spl.c
>> b/board/starfive/visionfive2/spl.c
>>> index 3dfa931b655..901e7b58f36 100644
>>> --- a/board/starfive/visionfive2/spl.c
>>> +++ b/board/starfive/visionfive2/spl.c
>>> @@ -126,19 +126,13 @@ int board_fit_config_name_match(const char
>> *name)
>>>   		    !strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
>>>   		return 0;
>>>   	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.2a")
>> &&
>>> -		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
>>> -		switch (get_pcb_revision_from_eeprom()) {
>>> -		case 'a':
>>> -		case 'A':
>>> -			return 0;
>>> -		}
>>> +		   (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7)
>> ||
>>> +		    !strncmp(get_product_id_from_eeprom(), "VF7110a", 7)))
>> {
>>> +		return 0;
>>>   	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.3b")
>> &&
>>> -		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
>>> -		switch (get_pcb_revision_from_eeprom()) {
>>> -		case 'b':
>>> -		case 'B':
>>> -			return 0;
>>> -		}
>>> +		   (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7)
>> ||
>>> +		    !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)))
>> {
>>> +		return 0;
>>>   	}
>>>
>>>   	return -EINVAL;

I agree it is good to delete the case statement and drop lowercase
compare as suggested, also...

>>> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
>> b/board/starfive/visionfive2/starfive_visionfive2.c
>>> index bfbb11a2ee7..f38433e94ac 100644
>>> --- a/board/starfive/visionfive2/starfive_visionfive2.c
>>> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
>>> @@ -59,20 +59,12 @@ static void set_fdtfile(void)
>>>   		fdtfile = "starfive/jh7110-milkv-mars.dtb";
>>>   	} else if (!strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
>>>   		fdtfile = "starfive/jh7110-pine64-star64.dtb";
>>> -	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
>>> -		switch (get_pcb_revision_from_eeprom()) {
>>> -		case 'a':
>>> -		case 'A':
>>> -			fdtfile = "starfive/jh7110-starfive-visionfive-2-
>> v1.2a.dtb";
>>> -			break;
>>> -		case 'b':
>>> -		case 'B':
>>> -			fdtfile = "starfive/jh7110-starfive-visionfive-2-
>> v1.3b.dtb";
>>> -			break;
>>> -		default:
>>> -			log_err("Unknown revision\n");
>>> -			return;
>>> -		}
>>> +	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7) ||
>>> +		   !strncmp(get_product_id_from_eeprom(), "VF7110a", 7)) {
>>
>> Where boards both with 'a' and 'b' ever shipped?
>> I have only seen 'A' and 'B' in pbuf.eeprom.atom1.data.pstr[6].
> 
> You're right. There are no boards marked "VF7110a" or "VF7110b". Let's remove
> the comparison of "VF7110a" and "VF7110b".
> 
> Best regards,
> Hal

... again here too delete the case statement and drop lowercase compare.
Thanks.

> 
>>
>> The change looks technically correct.
>>
>> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>
>>> +		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb";
>>> +	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
>>> +		   !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)) {
>>> +		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
>>>   	} else {
>>>   		log_err("Unknown product\n");
>>>   		return;

>>> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>> b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>> index 3866d07f9d4..43b8af4fc59 100644
>>> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>> @@ -535,19 +535,12 @@ int mac_read_from_eeprom(void)
>>>   	return 0;
>>>   }
>>>
>>> -/**
>>> - * get_pcb_revision_from_eeprom - get the PCB revision
>>> - *
>>> - * 1.2A return 'A'/'a', 1.3B return 'B'/'b',other values are illegal
>>> - */
>>>   u8 get_pcb_revision_from_eeprom(void)
>>>   {
>>> -	u8 pv = 0xFF;
>>> -
>>>   	if (read_eeprom())
>>> -		return pv;
>>> +		return 0xFF;
>>>
>>> -	return pbuf.eeprom.atom1.data.pstr[6];
>>> +	return pbuf.eeprom.atom4.data.pcb_revision;
>>>   }
>>>
>>>   u8 get_ddr_size_from_eeprom(void)
> 

No. I do not want to have to guess what error does "0xFF" have the
meaning of, it makes the code annoying to read every function header
documentation to learn this information.

Since we remove all users of mac_read_from_eeprom() it is certainly not
a problem we care about now anymore. Default PCB revision in case of
error may be zero. It is not important to make distinction of additional
failure detection, this is already resolved by read_eeprom() where any
error message may be presented to the user.

I appreciate the effort to be more technically correct and have a unique
error value but with u8 return value 0xFF it is an additional layer of
complication not needed. If you insist on this approach then the return
value should be arbitrarily defined as a compiler macro, say
CMD_EEPROM_FAIL (255) and consistently used. In fact I would not
complain if it improved code readability.

-E

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 03/10] eeprom: starfive: Simplify get_ddr_size_from_eeprom()
  2025-09-02 21:28     ` E Shattow
@ 2025-09-02 22:18       ` Heinrich Schuchardt
  0 siblings, 0 replies; 30+ messages in thread
From: Heinrich Schuchardt @ 2025-09-02 22:18 UTC (permalink / raw)
  To: E Shattow
  Cc: u-boot, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, Hal Feng

On 9/2/25 23:28, E Shattow wrote:
> 
> On 8/29/25 00:33, Heinrich Schuchardt wrote:
>> On 29.08.25 08:09, Hal Feng wrote:
>>> Directly return the DDR size instead of the field of 'DxxxExxx'.
>>> Move the function description to the header file.
>>>
>>> Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
>>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
>>> ---
>>>    arch/riscv/cpu/jh7110/spl.c                     |  2 +-
>>>    arch/riscv/include/asm/arch-jh7110/eeprom.h     |  8 +++++++-
>>>    .../visionfive2/visionfive2-i2c-eeprom.c        | 17 +++--------------
>>>    3 files changed, 11 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c
>>> index 87aaf865246..3aece7d995b 100644
>>> --- a/arch/riscv/cpu/jh7110/spl.c
>>> +++ b/arch/riscv/cpu/jh7110/spl.c
>>> @@ -41,7 +41,7 @@ int spl_dram_init(void)
>>>        /* Read the definition of the DDR size from eeprom, and if not,
>>>         * use the definition in DT
>>>         */
>>> -    size = (get_ddr_size_from_eeprom() >> 16) & 0xFF;
>>> +    size = get_ddr_size_from_eeprom();
>>>        if (check_ddr_size(size))
>>>            gd->ram_size = size << 30;
>>>    diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h b/arch/
>>> riscv/include/asm/arch-jh7110/eeprom.h
>>> index 45ad2a5f7bc..6d0a0ba0c4a 100644
>>> --- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
>>> +++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
>>> @@ -10,7 +10,13 @@
>>>    #include <linux/types.h>
>>>      u8 get_pcb_revision_from_eeprom(void);
>>> -u32 get_ddr_size_from_eeprom(void);
>>> +
>>> +/**
>>> + * get_ddr_size_from_eeprom() - read DDR size from EEPROM
>>> + *
>>> + * @return: size in GiB or 0xFF on error.
> 
> No. This is not consistent with get_mmc_size_from_eeprom() return value
> which uses 0 as short-circuit return value if read_eeprom() fails.
> 
> It is not important to make the distinction here if eeprom read failed
> or the data was not a valid size (i.e. zero from successful eeprom read).
> 
>>> + */
>>> +u8 get_ddr_size_from_eeprom(void);
>>>      /**
>>>     * get_mmc_size_from_eeprom() - read eMMC size from EEPROM
>>> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c b/
>>> board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>> index 17a44020bcf..3866d07f9d4 100644
>>> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>> @@ -550,23 +550,12 @@ u8 get_pcb_revision_from_eeprom(void)
>>>        return pbuf.eeprom.atom1.data.pstr[6];
>>>    }
>>>    -/**
>>> - * get_ddr_size_from_eeprom - get the DDR size
>>> - * pstr:  VF7110A1-2228-D008E000-00000001
>>> - * VF7110A1/VF7110B1 : VisionFive JH7110A /VisionFive JH7110B
>>> - * D008: 8GB LPDDR4
>>> - * E000: No emmc device, ECxx: include emmc device, xx: Capacity
>>> size[GB]
>>> - * return: the field of 'D008E000'
>>> - */
>>> -
>>> -u32 get_ddr_size_from_eeprom(void)
>>> +u8 get_ddr_size_from_eeprom(void)
>>>    {
>>> -    u32 pv = 0xFFFFFFFF;
>>> -
>>>        if (read_eeprom())
>>> -        return pv;
>>> +        return 0xFF;
> 
> No, inverted logic, see above return of 0xFF as an error condition is
> suspect.
> 
>>
>> Let's assume somebody cleared the EEPROM and the SPI flash.
>>
>> Would it make sense to assume the minimum encodable RAM size (1 GiB) in
>> this case, just to let the board boot to the console to allow writing
>> the EEPROM with valid data again?
>>
>> The current patch to simplify the code looks correct.
>>
>> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>
>>>    -    return hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL);
>>> +    return (hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL) >> 16) &
>>> 0xFF;
>>>    }
>>>      u32 get_mmc_size_from_eeprom(void)
>>
> 
> Yes, later fallback to 2G size configuration makes sense. Compatible
> products with less than 2G do not exist and probably never will; there

I wrote 1 GiB because this is the smallest size that can be encoded in 
the current EEPROM scheme. 1 GiB is big enough for reaching the U-Boot 
console and reprogramming the EEPROM or even booting into Linux.

> may be some developer boards with 1G but none I have ever heard of
> directly in conversations since 2023. The other common situation with
> zero or uninitialized EEPROM would be a failed read of eeprom i.e. RTC
> module at same i2c address or missing devicetree nodes at SPL phase.
> 
> 1. DDR size detection heuristic should happen first without need of
> eeprom, if possible.

We have get_ram_size(). According to a comment 
arch/arm/mach-stm32mp/stm32mp1/spl.c that function has a problem with 
speculative access.

I don't see an urgent necessity to move away from EEPROM as the only 
method to determine the RAM size.

Best regards

Heinrich

> 
> 2. Else warn DDR detection failed and read size from eeprom config
> 
> 3. Else warn eeprom memory config invalid and set size set to default 2G.
> 
> -E


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom()
  2025-09-02 22:12       ` E Shattow
@ 2025-09-02 22:32         ` Heinrich Schuchardt
  0 siblings, 0 replies; 30+ messages in thread
From: Heinrich Schuchardt @ 2025-09-02 22:32 UTC (permalink / raw)
  To: E Shattow
  Cc: u-boot@lists.denx.de, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, Hal Feng

On 9/3/25 00:12, E Shattow wrote:
> 
> On 9/2/25 00:44, Hal Feng wrote:
>>> On 29.08.25 15:44, Heinrich Schuchardt wrote:
>>> On 29.08.25 08:09, Hal Feng wrote:
>>>> pcb_revision is stored in the pcb_revision field of ATOM4. Correct it.
>>>> Move the function description to the header file.
>>>> Remove the function calls in board/starfive/visionfive2/.
>>>>
>>>> Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
>>>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
>>>> ---
>>>>    arch/riscv/include/asm/arch-jh7110/eeprom.h   |  5 +++++
>>>>    board/starfive/visionfive2/spl.c              | 18 ++++++-----------
>>>>    .../visionfive2/starfive_visionfive2.c        | 20 ++++++-------------
>>>>    .../visionfive2/visionfive2-i2c-eeprom.c      | 11 ++--------
>>>>    4 files changed, 19 insertions(+), 35 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h
>>> b/arch/riscv/include/asm/arch-jh7110/eeprom.h
>>>> index 6d0a0ba0c4a..025f1d32c49 100644
>>>> --- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
>>>> +++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
>>>> @@ -9,6 +9,11 @@
>>>>
>>>>    #include <linux/types.h>
>>>>
>>>> +/**
>>>> + * get_pcb_revision_from_eeprom() - get the PCB revision
>>>> + *
>>>> + * @return: the PCB revision or 0xFF on error.
>>>> + */
> 
> No. Inverted logic. See below comment about body of function.
> 
>>>>    u8 get_pcb_revision_from_eeprom(void);
>>>>
>>>>    /**
>>>> diff --git a/board/starfive/visionfive2/spl.c
>>> b/board/starfive/visionfive2/spl.c
>>>> index 3dfa931b655..901e7b58f36 100644
>>>> --- a/board/starfive/visionfive2/spl.c
>>>> +++ b/board/starfive/visionfive2/spl.c
>>>> @@ -126,19 +126,13 @@ int board_fit_config_name_match(const char
>>> *name)
>>>>    		    !strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
>>>>    		return 0;
>>>>    	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.2a")
>>> &&
>>>> -		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
>>>> -		switch (get_pcb_revision_from_eeprom()) {
>>>> -		case 'a':
>>>> -		case 'A':
>>>> -			return 0;
>>>> -		}
>>>> +		   (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7)
>>> ||
>>>> +		    !strncmp(get_product_id_from_eeprom(), "VF7110a", 7)))
>>> {
>>>> +		return 0;
>>>>    	} else if (!strcmp(name, "starfive/jh7110-starfive-visionfive-2-v1.3b")
>>> &&
>>>> -		    !strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
>>>> -		switch (get_pcb_revision_from_eeprom()) {
>>>> -		case 'b':
>>>> -		case 'B':
>>>> -			return 0;
>>>> -		}
>>>> +		   (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7)
>>> ||
>>>> +		    !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)))
>>> {
>>>> +		return 0;
>>>>    	}
>>>>
>>>>    	return -EINVAL;
> 
> I agree it is good to delete the case statement and drop lowercase
> compare as suggested, also...
> 
>>>> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
>>> b/board/starfive/visionfive2/starfive_visionfive2.c
>>>> index bfbb11a2ee7..f38433e94ac 100644
>>>> --- a/board/starfive/visionfive2/starfive_visionfive2.c
>>>> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
>>>> @@ -59,20 +59,12 @@ static void set_fdtfile(void)
>>>>    		fdtfile = "starfive/jh7110-milkv-mars.dtb";
>>>>    	} else if (!strncmp(get_product_id_from_eeprom(), "STAR64", 6)) {
>>>>    		fdtfile = "starfive/jh7110-pine64-star64.dtb";
>>>> -	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110", 6)) {
>>>> -		switch (get_pcb_revision_from_eeprom()) {
>>>> -		case 'a':
>>>> -		case 'A':
>>>> -			fdtfile = "starfive/jh7110-starfive-visionfive-2-
>>> v1.2a.dtb";
>>>> -			break;
>>>> -		case 'b':
>>>> -		case 'B':
>>>> -			fdtfile = "starfive/jh7110-starfive-visionfive-2-
>>> v1.3b.dtb";
>>>> -			break;
>>>> -		default:
>>>> -			log_err("Unknown revision\n");
>>>> -			return;
>>>> -		}
>>>> +	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110A", 7) ||
>>>> +		   !strncmp(get_product_id_from_eeprom(), "VF7110a", 7)) {
>>>
>>> Where boards both with 'a' and 'b' ever shipped?
>>> I have only seen 'A' and 'B' in pbuf.eeprom.atom1.data.pstr[6].
>>
>> You're right. There are no boards marked "VF7110a" or "VF7110b". Let's remove
>> the comparison of "VF7110a" and "VF7110b".
>>
>> Best regards,
>> Hal
> 
> ... again here too delete the case statement and drop lowercase compare.
> Thanks.
> 
>>
>>>
>>> The change looks technically correct.
>>>
>>> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>>
>>>> +		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb";
>>>> +	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
>>>> +		   !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)) {
>>>> +		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
>>>>    	} else {
>>>>    		log_err("Unknown product\n");
>>>>    		return;
> 
>>>> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>> b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>>> index 3866d07f9d4..43b8af4fc59 100644
>>>> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>>> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
>>>> @@ -535,19 +535,12 @@ int mac_read_from_eeprom(void)
>>>>    	return 0;
>>>>    }
>>>>
>>>> -/**
>>>> - * get_pcb_revision_from_eeprom - get the PCB revision
>>>> - *
>>>> - * 1.2A return 'A'/'a', 1.3B return 'B'/'b',other values are illegal
>>>> - */
>>>>    u8 get_pcb_revision_from_eeprom(void)
>>>>    {
>>>> -	u8 pv = 0xFF;
>>>> -
>>>>    	if (read_eeprom())
>>>> -		return pv;
>>>> +		return 0xFF;
>>>>
>>>> -	return pbuf.eeprom.atom1.data.pstr[6];
>>>> +	return pbuf.eeprom.atom4.data.pcb_revision;
>>>>    }
>>>>
>>>>    u8 get_ddr_size_from_eeprom(void)
>>
> 
> No. I do not want to have to guess what error does "0xFF" have the
> meaning of, it makes the code annoying to read every function header
> documentation to learn this information.
> 
> Since we remove all users of mac_read_from_eeprom() it is certainly not
> a problem we care about now anymore. Default PCB revision in case of
> error may be zero. It is not important to make distinction of additional
> failure detection, this is already resolved by read_eeprom() where any
> error message may be presented to the user.
> 
> I appreciate the effort to be more technically correct and have a unique
> error value but with u8 return value 0xFF it is an additional layer of
> complication not needed. If you insist on this approach then the return
> value should be arbitrarily defined as a compiler macro, say
> CMD_EEPROM_FAIL (255) and consistently used. In fact I would not
> complain if it improved code readability.

There is no necessity to use u8 as the return value. We could also use 
int and return a negative error code (-EIO) on failure.

Best regards

Heinrich

> 
> -E


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 05/10] eeprom: starfive: Update eeprom data format version to 3
  2025-08-29  6:09 ` [RFC 05/10] eeprom: starfive: Update eeprom data format version to 3 Hal Feng
  2025-08-29  7:47   ` Heinrich Schuchardt
@ 2025-09-02 23:29   ` E Shattow
  1 sibling, 0 replies; 30+ messages in thread
From: E Shattow @ 2025-09-02 23:29 UTC (permalink / raw)
  To: Hal Feng, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, Heinrich Schuchardt
  Cc: u-boot


On 8/28/25 23:09, Hal Feng wrote:
> Add eeprom data format v3 support.
> Add wifi_bt field in ATOM4 and add "mac wifi_bt <?>" command to modify it.
> Set or show eth0/1 MAC address only if the board has eth0/1.
> 
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  .../visionfive2/visionfive2-i2c-eeprom.c      | 91 ++++++++++++++++---
>  1 file changed, 78 insertions(+), 13 deletions(-)
> 
> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> index 43b8af4fc59..7352252c475 100644
> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> @@ -11,7 +11,7 @@
>  #include <u-boot/crc.h>
>  #include <linux/delay.h>
>  
> -#define FORMAT_VERSION		0x2
> +#define FORMAT_VERSION		0x3

No. This stays FORMAT_VERSION 0x2 for compatibility, allow to write
EEPROM in the "old" format on boards. We must allow compatible action
for vendor BSP firmware and weird old software that we do not want
responsibility for updating.

Update to 0x3 only if some data field is expressed that needs 0x3 format.

>  #define PCB_VERSION		0xB1
>  #define BOM_VERSION		'A'
>  /*
> @@ -105,7 +105,8 @@ struct eeprom_atom4_data {
>  	u8 bom_revision;		/* BOM version */
>  	u8 mac0_addr[MAC_ADDR_BYTES];	/* Ethernet0 MAC */
>  	u8 mac1_addr[MAC_ADDR_BYTES];	/* Ethernet1 MAC */
> -	u8 reserved[2];
> +	u8 wifi_bt;			/* WIFI/BT support flag */

Are you sure a whole u8 for a one-bit flag?

> +	u8 reserved;
>  };
>  
>  struct starfive_eeprom_atom4 {
> @@ -128,6 +129,35 @@ static union {
>  /* Set to 1 if we've read EEPROM into memory */
>  static int has_been_read __section(".data");
>  
> +static const char * const board_no_eth0_list[] = {
> +	"FML13V01",
> +};
> +
> +static const char * const board_no_eth1_list[] = {
> +	"FML13V01",
> +	"MARS",
> +	"VF7110SL",
> +};
> +
> +static bool board_have_eth(u8 eth)
> +{
> +	int i;
> +
> +	if (eth == 0) {
> +		for (i = 0 ; i < ARRAY_SIZE(board_no_eth0_list); i++)
> +			if (!strncmp(pbuf.eeprom.atom1.data.pstr, board_no_eth0_list[i],
> +				     strlen(board_no_eth0_list[i])))
> +				return false;
> +	} else {
> +		for (i = 0 ; i < ARRAY_SIZE(board_no_eth1_list); i++)
> +			if (!strncmp(pbuf.eeprom.atom1.data.pstr, board_no_eth1_list[i],
> +				     strlen(board_no_eth1_list[i])))
> +				return false;
> +	}
> +
> +	return true;
> +}
> +

No. Drop all this no_eth0 no_eth1 configuration, the string arrays, it
exists only to show or hide some EEPROM configuration text and can be
deleted.

>  static inline int is_match_magic(void)
>  {
>  	return strncmp(pbuf.eeprom.header.signature, STARFIVE_EEPROM_HATS_SIG,
> @@ -176,17 +206,22 @@ static void show_eeprom(void)
>  	printf("Vendor : %s\n", pbuf.eeprom.atom1.data.vstr);
>  	printf("Product full SN: %s\n", pbuf.eeprom.atom1.data.pstr);
>  	printf("data version: 0x%x\n", pbuf.eeprom.atom4.data.version);
> -	if (pbuf.eeprom.atom4.data.version == 2) {

> +	if (pbuf.eeprom.atom4.data.version >= 2 && pbuf.eeprom.atom4.data.version <= 3) {

This is not some floating point value. Compare with discrete values:

+ if (pbuf.eeprom.atom4.data.version == 2 ||
+     pbuf.eeprom.atom4.data.version == 3) {

or else I think more readable is convert to a switch case conditional:

+ switch (pbuf.eeprom.atom4.data.version) {
+ case 2:
+ case 3:
		...

>  		printf("PCB revision: 0x%x\n", pbuf.eeprom.atom4.data.pcb_revision);
>  		printf("BOM revision: %c\n", pbuf.eeprom.atom4.data.bom_revision);
> -		printf("Ethernet MAC0 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> -		       pbuf.eeprom.atom4.data.mac0_addr[0], pbuf.eeprom.atom4.data.mac0_addr[1],
> -		       pbuf.eeprom.atom4.data.mac0_addr[2], pbuf.eeprom.atom4.data.mac0_addr[3],
> -		       pbuf.eeprom.atom4.data.mac0_addr[4], pbuf.eeprom.atom4.data.mac0_addr[5]);
> -		printf("Ethernet MAC1 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> -		       pbuf.eeprom.atom4.data.mac1_addr[0], pbuf.eeprom.atom4.data.mac1_addr[1],
> -		       pbuf.eeprom.atom4.data.mac1_addr[2], pbuf.eeprom.atom4.data.mac1_addr[3],
> -		       pbuf.eeprom.atom4.data.mac1_addr[4], pbuf.eeprom.atom4.data.mac1_addr[5]);
> +		if (board_have_eth(0))
> +			printf("Ethernet MAC0 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> +			pbuf.eeprom.atom4.data.mac0_addr[0], pbuf.eeprom.atom4.data.mac0_addr[1],
> +			pbuf.eeprom.atom4.data.mac0_addr[2], pbuf.eeprom.atom4.data.mac0_addr[3],
> +			pbuf.eeprom.atom4.data.mac0_addr[4], pbuf.eeprom.atom4.data.mac0_addr[5]);
> +
> +		if (board_have_eth(1))

No. There is not any purpose to hide this data from the user. In fact it
is useful when Mars CM Lite SoM installed to carrier board that has its
own additional network interface on PCIe bus, setting eth2addr
environment variable to be the same as (disabled) MAC1 from EEPROM print
output.

> +			printf("Ethernet MAC1 address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> +			pbuf.eeprom.atom4.data.mac1_addr[0], pbuf.eeprom.atom4.data.mac1_addr[1],
> +			pbuf.eeprom.atom4.data.mac1_addr[2], pbuf.eeprom.atom4.data.mac1_addr[3],
> +			pbuf.eeprom.atom4.data.mac1_addr[4], pbuf.eeprom.atom4.data.mac1_addr[5]);
> +		if (pbuf.eeprom.atom4.data.version >= 3)
> +			printf("WIFI/BT support: %x\n", pbuf.eeprom.atom4.data.wifi_bt);
+break;

+ default:

>  	} else {
>  		printf("Custom data v%d is not Supported\n", pbuf.eeprom.atom4.data.version);
>  		dump_raw_eeprom();
> @@ -260,6 +295,7 @@ static void init_local_copy(void)
>  	pbuf.eeprom.atom4.data.bom_revision = BOM_VERSION;
>  	set_mac_address(STARFIVE_DEFAULT_MAC0, 0);
>  	set_mac_address(STARFIVE_DEFAULT_MAC1, 1);
> +	pbuf.eeprom.atom4.data.wifi_bt = 0;
+ break;
>  }
>  
>  /**
> @@ -385,6 +421,28 @@ static void set_bom_revision(char *string)
>  	update_crc();
>  }
>  
> +/**
> + * set_wifi_bt() - stores a StarFive WIFI/BT support flag into the local EEPROM copy
> + *
> + * Takes a pointer to a string representing the numeric WIFI/BT support flag in
> + * decimal ("0" or "1"), stores it in the wifi_bt field of the
> + * EEPROM local copy, and updates the CRC of the local copy.
> + */
> +static void set_wifi_bt(char *string)
> +{
> +	u8 wifi_bt;
> +
> +	wifi_bt = simple_strtoul(string, &string, 16);
> +	if (wifi_bt > 1) {
> +		printf("WIFI/BT support flag must not be greater than 1");
> +		return;
> +	}
> +
> +	pbuf.eeprom.atom4.data.wifi_bt = wifi_bt;
> +
> +	update_crc();
> +}
> +

I am surprised this wifi_bt u8 is a 1-bit flag and does not contain any
mac address. What is the SDIO module present on VisionFive2 Lite and
does it contain memory for WiFi MAC address and Bluetooth MAC address?

>  /**
>   * set_product_id() - stores a StarFive product ID into the local EEPROM copy
>   *
> @@ -478,6 +536,9 @@ int do_mac(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  	} else if (!strcmp(cmd, "bom_revision")) {
>  		set_bom_revision(argv[2]);
>  		return 0;
> +	} else if (!strcmp(cmd, "wifi_bt")) {
> +		set_wifi_bt(argv[2]);
> +		return 0;
>  	} else if (!strcmp(cmd, "product_id")) {
>  		set_product_id(argv[2]);
>  		return 0;
> @@ -513,8 +574,10 @@ int mac_read_from_eeprom(void)
>  	}
>  
>  	// 1, setup ethaddr env
> -	eth_env_set_enetaddr("ethaddr", pbuf.eeprom.atom4.data.mac0_addr);
> -	eth_env_set_enetaddr("eth1addr", pbuf.eeprom.atom4.data.mac1_addr);
> +	if (board_have_eth(0))
> +		eth_env_set_enetaddr("ethaddr", pbuf.eeprom.atom4.data.mac0_addr);
> +	if (board_have_eth(1))
> +		eth_env_set_enetaddr("eth1addr", pbuf.eeprom.atom4.data.mac1_addr);
>  

No. Unnecessary complication to have a list of products and do all this
extra work. We should only update fields that the user has set.

>  	/**
>  	 * 2, setup serial# env, reference to hifive-platform-i2c-eeprom.c,
> @@ -585,6 +648,8 @@ U_BOOT_LONGHELP(mac,
>  	"    - stores a StarFive PCB revision into the local EEPROM copy\n"
>  	"mac bom_revision <A>\n"
>  	"    - stores a StarFive BOM revision into the local EEPROM copy\n"
> +	"mac wifi_bt <?>\n"
> +	"    - stores a StarFive WIFI/BT support flag into the local EEPROM copy\n"
>  	"mac product_id <VF7110A1-2228-D008E000-xxxxxxxx>\n"
>  	"    - stores a StarFive product ID into the local EEPROM copy\n"
>  	"mac vendor <Vendor Name>\n"

NAK to this patch.

Add the support for the new EEPROM format and keep it simple:

- It is okay if the user sees print of all valid fields of EEPROM, even
when some printed data does not have any importance for them. The format
version will hide or show some data fields - okay.

- For compatibility keep FORMAT_VERSION (0x2) default and each field
being set should do a check with pbuf.eeprom.header.version ; for
example if there is wifi_bt being set then it is appropriate to promote
format version (0x3) and print info that the format version has changed.

- For explicit write operation to EEPROM of format version 0x3 but user
did not set any data fields that need more than format version (0x2),
then the user must set format version to 0x3. Make it possible to set
the format version directly.

If you want to get complicated and add conditional for promotion of
minimum format version required by each data field, okay then, I would
like to see that.

-E

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 06/10] pcie: starfive: Add a optional power gpio support
  2025-08-29  6:09 ` [RFC 06/10] pcie: starfive: Add a optional power gpio support Hal Feng
@ 2025-09-02 23:47   ` E Shattow
  0 siblings, 0 replies; 30+ messages in thread
From: E Shattow @ 2025-09-02 23:47 UTC (permalink / raw)
  To: Hal Feng, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, Heinrich Schuchardt
  Cc: u-boot



On 8/28/25 23:09, Hal Feng wrote:
> Get a optional power gpio and enable it if it is valid.
> 

I see this appears to be a normal thing that needs implemented in U-Boot
to have parity with Linux kernel, but the commit message should help me
understand why so I don't have to think so hard.

Expand why this is useful and how this is implemented in Linux kernel,
what is the API reference or something to give reason why U-Boot can
expect the devicetree property to be present from upstream
devicetree-rebasing.

> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  drivers/pci/pcie_starfive_jh7110.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/pcie_starfive_jh7110.c b/drivers/pci/pcie_starfive_jh7110.c
> index 51aca7359ff..f6b50d12a90 100644
> --- a/drivers/pci/pcie_starfive_jh7110.c
> +++ b/drivers/pci/pcie_starfive_jh7110.c
> @@ -45,6 +45,7 @@ struct starfive_pcie {
>  	struct pcie_plda plda;
>  	struct clk_bulk	clks;
>  	struct reset_ctl_bulk	rsts;
> +	struct gpio_desc	power_gpio;
>  	struct gpio_desc	reset_gpio;
>  	struct regmap *regmap;
>  	unsigned int stg_pcie_base;
> @@ -184,6 +185,10 @@ static int starfive_pcie_parse_dt(struct udevice *dev)
>  		dev_err(dev, "reset-gpio is not valid\n");
>  		return -EINVAL;
>  	}
> +
> +	gpio_request_by_name(dev, "enable-gpios", 0, &priv->power_gpio,
> +			     GPIOD_IS_OUT);
> +
>  	return 0;
>  }
>  
> @@ -205,6 +210,9 @@ static int starfive_pcie_init_port(struct udevice *dev)
>  		goto err_deassert_clk;
>  	}
>  
> +	if (dm_gpio_is_valid(&priv->power_gpio))
> +		dm_gpio_set_value(&priv->power_gpio, 1);
> +
>  	dm_gpio_set_value(&priv->reset_gpio, 1);
>  	/* Disable physical functions except #0 */
>  	for (i = 1; i < PLDA_FUNC_NUM; i++) {

-E

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 07/10] riscv: dts: jh7110: Add StarFive VisionFive 2 Lite u-boot device tree
  2025-08-29  6:09 ` [RFC 07/10] riscv: dts: jh7110: Add StarFive VisionFive 2 Lite u-boot device tree Hal Feng
@ 2025-09-03  0:00   ` E Shattow
  0 siblings, 0 replies; 30+ messages in thread
From: E Shattow @ 2025-09-03  0:00 UTC (permalink / raw)
  To: Hal Feng, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, Heinrich Schuchardt
  Cc: u-boot


On 8/28/25 23:09, Hal Feng wrote:
> Add the u-boot device tree include needed to support the
> StarFive VisionFive 2 Lite board.
> 
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  .../dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi     | 7 +++++++
>  1 file changed, 7 insertions(+)
>  create mode 100644 arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi
> 
> diff --git a/arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi b/arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi
> new file mode 100644
> index 00000000000..46ef07a99eb
> --- /dev/null
> +++ b/arch/riscv/dts/jh7110s-starfive-visionfive-2-lite-u-boot.dtsi
> @@ -0,0 +1,7 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2025 StarFive Technology Co., Ltd.
> + */
> +
> +#include "jh7110-common-u-boot.dtsi"
> +#include "starfive-visionfive2-binman.dtsi"

NAK. Not needed, with some expected changes to U-Boot. See comments
about "[RFC 02/10] riscv: dts: starfive: Add VisionFive 2 Lite board
device tree" [1]
1:
https://lore.kernel.org/u-boot/3b75e20f-261e-4a23-b2b6-d4f4596b5fb3@freeshell.de/

-E

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 08/10] configs: visionfive2: Add VisionFive 2 Lite DT to OF_LIST
  2025-08-29  6:09 ` [RFC 08/10] configs: visionfive2: Add VisionFive 2 Lite DT to OF_LIST Hal Feng
@ 2025-09-03  0:15   ` E Shattow
  0 siblings, 0 replies; 30+ messages in thread
From: E Shattow @ 2025-09-03  0:15 UTC (permalink / raw)
  To: Hal Feng, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, Heinrich Schuchardt
  Cc: u-boot


On 8/28/25 23:09, Hal Feng wrote:
> So the VisionFive 2 Lite DT will be built and merged into FIT.
> 
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  configs/starfive_visionfive2_defconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configs/starfive_visionfive2_defconfig b/configs/starfive_visionfive2_defconfig
> index 544140c03f7..5a0e918c733 100644
> --- a/configs/starfive_visionfive2_defconfig
> +++ b/configs/starfive_visionfive2_defconfig
> @@ -78,7 +78,7 @@ CONFIG_CMD_WDT=y
>  CONFIG_CMD_WGET=y
>  CONFIG_CMD_BOOTSTAGE=y
>  CONFIG_OF_BOARD=y
> -CONFIG_OF_LIST="starfive/jh7110-deepcomputing-fml13v01 starfive/jh7110-milkv-mars starfive/jh7110-pine64-star64 starfive/jh7110-starfive-visionfive-2-v1.2a starfive/jh7110-starfive-visionfive-2-v1.3b"
> +CONFIG_OF_LIST="starfive/jh7110-deepcomputing-fml13v01 starfive/jh7110-milkv-mars starfive/jh7110-pine64-star64 starfive/jh7110-starfive-visionfive-2-v1.2a starfive/jh7110-starfive-visionfive-2-v1.3b starfive/jh7110s-starfive-visionfive-2-lite"
>  CONFIG_MULTI_DTB_FIT=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_IS_IN_SPI_FLASH=y

Okay. While we are adding a target with a different (yet similar) CPU
JH7110S, I have questions about if we can also make JH7100 a variant
here too. How much difference is allowed for one U-Boot board target?

It would be great to have a single U-Boot target
starfive_jh71xx_defconfig for example. I am aware that mainline Linux
kernel defconfig can boot on JH7100 but some peripheral drivers do not
work correctly, and so there is a requirement to compile Linux kernel as
non-portable [1]. However I want to know what prevents U-Boot from
supporting all of JH7100, JH7110, JH7110S, with the same build target?

1: https://wiki.debian.org/InstallingDebianOn/StarFive/VisionFiveV1

Anyways, this change to CONFIG_OF_LIST is sorted correctly and looks
good to me.

Reviewed-by: E Shattow <e@freeshell.de>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 09/10] board: starfive: spl: Support VisionFive 2 Lite
  2025-08-29  6:09 ` [RFC 09/10] board: starfive: spl: Support VisionFive 2 Lite Hal Feng
@ 2025-09-03  0:21   ` E Shattow
  0 siblings, 0 replies; 30+ messages in thread
From: E Shattow @ 2025-09-03  0:21 UTC (permalink / raw)
  To: Hal Feng, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, Heinrich Schuchardt
  Cc: u-boot



On 8/28/25 23:09, Hal Feng wrote:
> Choose the matching FIT config on the VisionFive 2 Lite board.
> 
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  board/starfive/visionfive2/spl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
> index 901e7b58f36..18571b482f9 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -133,6 +133,9 @@ int board_fit_config_name_match(const char *name)
>  		   (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
>  		    !strncmp(get_product_id_from_eeprom(), "VF7110b", 7))) {

Deprecation of "VF7110b" and "VF7110a" mentioned previously should be in
its own patch.

>  		return 0;
> +	} else if (!strcmp(name, "starfive/jh7110s-starfive-visionfive-2-lite") &&
> +		    !strncmp(get_product_id_from_eeprom(), "VF7110SL", 8)) {
> +		return 0;
>  	}
>  
>  	return -EINVAL;

This is sorted correctly and looks good to me. With that,

Reviewed-by: E Shattow <e@freeshell.de>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 10/10] board: starfive: visionfive2: Add VisionFive 2 Lite fdt selection
  2025-08-29  6:09 ` [RFC 10/10] board: starfive: visionfive2: Add VisionFive 2 Lite fdt selection Hal Feng
@ 2025-09-03  0:26   ` E Shattow
  2025-09-03  4:34     ` Heinrich Schuchardt
  0 siblings, 1 reply; 30+ messages in thread
From: E Shattow @ 2025-09-03  0:26 UTC (permalink / raw)
  To: Hal Feng, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, Heinrich Schuchardt
  Cc: u-boot



On 8/28/25 23:09, Hal Feng wrote:
> Set $fdtfile to the VisionFive 2 Lite DTB if the board is matched.
> 
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  board/starfive/visionfive2/starfive_visionfive2.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c b/board/starfive/visionfive2/starfive_visionfive2.c
> index f38433e94ac..912e8cb967a 100644
> --- a/board/starfive/visionfive2/starfive_visionfive2.c
> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> @@ -65,6 +65,8 @@ static void set_fdtfile(void)
>  	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
>  		   !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)) {
>  		fdtfile = "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
> +	} else if (!strncmp(get_product_id_from_eeprom(), "VF7110SL", 8)) {
> +		fdtfile = "starfive/jh7110s-starfive-visionfive-2-lite.dtb";
>  	} else {
>  		log_err("Unknown product\n");
>  		return;

I continue to think it is silly we are setting $fdtfile env variable
past the moment since adopting OF_UPSTREAM. That is a discussion for
somewhere else, however.

Anyhow the sorting is correct and new code follows the existing code
pattern.

Reviewed-by: E Shattow <e@freeshell.de>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 10/10] board: starfive: visionfive2: Add VisionFive 2 Lite fdt selection
  2025-09-03  0:26   ` E Shattow
@ 2025-09-03  4:34     ` Heinrich Schuchardt
  0 siblings, 0 replies; 30+ messages in thread
From: Heinrich Schuchardt @ 2025-09-03  4:34 UTC (permalink / raw)
  To: E Shattow
  Cc: Hal Feng, Leo, Tom Rini, Rick Chen, Sumit Garg,
	Emil Renner Berthing, U-Boot Mailing List

E Shattow <e@freeshell.de> schrieb am Mi., 3. Sept. 2025, 02:26:

>
>
> On 8/28/25 23:09, Hal Feng wrote:
> > Set $fdtfile to the VisionFive 2 Lite DTB if the board is matched.
> >
> > Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> > ---
> >  board/starfive/visionfive2/starfive_visionfive2.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
> b/board/starfive/visionfive2/starfive_visionfive2.c
> > index f38433e94ac..912e8cb967a 100644
> > --- a/board/starfive/visionfive2/starfive_visionfive2.c
> > +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> > @@ -65,6 +65,8 @@ static void set_fdtfile(void)
> >       } else if (!strncmp(get_product_id_from_eeprom(), "VF7110B", 7) ||
> >                  !strncmp(get_product_id_from_eeprom(), "VF7110b", 7)) {
> >               fdtfile =
> "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb";
> > +     } else if (!strncmp(get_product_id_from_eeprom(), "VF7110SL", 8)) {
> > +             fdtfile =
> "starfive/jh7110s-starfive-visionfive-2-lite.dtb";
> >       } else {
> >               log_err("Unknown product\n");
> >               return;
>
> I continue to think it is silly we are setting $fdtfile env variable
> past the moment since adopting OF_UPSTREAM. That is a discussion for
> somewhere else, however.
>

Do not expect an old kernel to boot with a newer dtb. Linux broke such
expectations repeatedly.

Do not assume that users upgrade U-Boot to keep it in sync with the kernel
they use.

Debian/Ubuntu packages flash-kernel and u-boot-menu rely on $fdtfile being
present.

Best regards

Heinrich


> Anyhow the sorting is correct and new code follows the existing code
> pattern.
>
> Reviewed-by: E Shattow <e@freeshell.de>
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2025-09-03  4:34 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29  6:09 [RFC 00/10] Add support for StarFive VisionFive 2 Lite board Hal Feng
2025-08-29  6:09 ` [RFC 01/10] riscv: dts: starfive: jh7110-common: Move out some nodes to the board dts Hal Feng
2025-08-29  7:19   ` Heinrich Schuchardt
2025-09-02 16:17     ` E Shattow
2025-08-29  6:09 ` [RFC 02/10] riscv: dts: starfive: Add VisionFive 2 Lite board device tree Hal Feng
2025-09-02 20:03   ` E Shattow
2025-08-29  6:09 ` [RFC 03/10] eeprom: starfive: Simplify get_ddr_size_from_eeprom() Hal Feng
2025-08-29  7:33   ` Heinrich Schuchardt
2025-09-02 21:28     ` E Shattow
2025-09-02 22:18       ` Heinrich Schuchardt
2025-08-29  6:09 ` [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom() Hal Feng
2025-08-29  7:44   ` Heinrich Schuchardt
2025-09-02  7:44     ` Hal Feng
2025-09-02 22:12       ` E Shattow
2025-09-02 22:32         ` Heinrich Schuchardt
2025-08-29  6:09 ` [RFC 05/10] eeprom: starfive: Update eeprom data format version to 3 Hal Feng
2025-08-29  7:47   ` Heinrich Schuchardt
2025-09-02  7:10     ` Hal Feng
2025-09-02 23:29   ` E Shattow
2025-08-29  6:09 ` [RFC 06/10] pcie: starfive: Add a optional power gpio support Hal Feng
2025-09-02 23:47   ` E Shattow
2025-08-29  6:09 ` [RFC 07/10] riscv: dts: jh7110: Add StarFive VisionFive 2 Lite u-boot device tree Hal Feng
2025-09-03  0:00   ` E Shattow
2025-08-29  6:09 ` [RFC 08/10] configs: visionfive2: Add VisionFive 2 Lite DT to OF_LIST Hal Feng
2025-09-03  0:15   ` E Shattow
2025-08-29  6:09 ` [RFC 09/10] board: starfive: spl: Support VisionFive 2 Lite Hal Feng
2025-09-03  0:21   ` E Shattow
2025-08-29  6:09 ` [RFC 10/10] board: starfive: visionfive2: Add VisionFive 2 Lite fdt selection Hal Feng
2025-09-03  0:26   ` E Shattow
2025-09-03  4:34     ` Heinrich Schuchardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).