On 4/16/24 06:09, E Shattow wrote:
On Mon, Apr 15, 2024 at 4:50 AM Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:

The EEPROM provides information about the size of the EEPROM.

"The EEPROM provides information about the size of the eMMC."

Thanks for catching this.


Provide a new function get_mmc_size_from_eeprom() to read it.

Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
  arch/riscv/include/asm/arch-jh7110/eeprom.h    |  7 +++++++
  board/starfive/visionfive2/Kconfig             |  9 +++++++++
  .../visionfive2/visionfive2-i2c-eeprom.c       | 18 ++++++++++++++++++
  3 files changed, 34 insertions(+)

diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h 
b/arch/riscv/include/asm/arch-jh7110/eeprom.h
index 62d184aeb57..17395d4269e 100644
--- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
+++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
@@ -12,6 +12,13 @@
  u8 get_pcb_revision_from_eeprom(void);
  u32 get_ddr_size_from_eeprom(void);

+/**
+ * get_mmc_size_from_eeprom() - read MMC size form EEPROM
+ *
+ * @return: size in GiB or 0 on error.
+ */
+u32 get_mmc_size_from_eeprom(void);
+
  /**
   * get_product_id_from_eeprom - get product ID string
   *
diff --git a/board/starfive/visionfive2/Kconfig 
b/board/starfive/visionfive2/Kconfig
index 2186a939646..d7e8a7a7d78 100644
--- a/board/starfive/visionfive2/Kconfig
+++ b/board/starfive/visionfive2/Kconfig
@@ -50,4 +50,13 @@ config BOARD_SPECIFIC_OPTIONS # dummy
         imply PHY_LIB
         imply PHY_MSCC

+config STARFIVE_NO_EMMC
+       bool "Report eMMC size as zero"
+       help
+         The serial number string in the EEPROM is meant to report the
+         size of onboard eMMC. Unfortunately some Milk-V Mars CM Lite
+         modules without eMMC show a non-zero size here.
+
+         Set to 'Y' if you have a Mars CM Lite module.
+
  endif
diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c 
b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
index ddef7d61235..cd3d8bd51a6 100644
--- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
+++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
@@ -548,6 +548,24 @@ u32 get_ddr_size_from_eeprom(void)
         return hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL);
  }

+u32 get_mmc_size_from_eeprom(void)
+{
+       u32 size;
+
+       if (IS_ENABLED(CONFIG_STARFIVE_NO_EMMC))
+               return 0;
+
+       if (read_eeprom())
+               return 0;
+
+       size = dectoul(&pbuf.eeprom.atom1.data.pstr[19], NULL);
+
+       if (pbuf.eeprom.atom1.data.pstr[21] == 'T')
+               size <<= 10;
+
+       return size;
+}
+
  U_BOOT_LONGHELP(mac,
         "\n"
         "    - display EEPROM content\n"
--
2.43.0


Fixed-position parsing on a data format of ordered variable length
hyphen-delimited fields. Notable is that some Pine64 Star64 and Milk-V
Mars CM Lite boards shipped with uninitialized or wrong EEPROM data;
further the EEPROM Write Protect can be trivially disabled and
arbitrary data written i.e. with a paperclip then `mac` command. Could
this code be generalized to split fields on hyphen character better
expressing the expected data format or is that unwanted complexity and
code size?

The boards that I have seen up to now are consistent in the string layout of the boards that I have received up to now:

VisionFive 2 1.2B   VF7110A1-2239-D004E000-xxxxxxxx
VisionFive 2 1.3B   VF7110B1-2253-D004E000-xxxxxxxx
VisionFive 2 1.3B   VF7110B1-2253-D008E000-xxxxxxxx
Pine64 Pinetab V    VF7110B1-2230-D008E000-xxxxxxxx
Pine64 Star64 v1    STAR64V1-2310-D008E000-xxxxxxxx
Milk-V Mars         MARS-V11-2340-D008E000-xxxxxxxx
Milk-V Mars CM Lite MARC-V10-2340-D002E016-xxxxxxxx

The example of the Pinetab V and of the first batch of the Milk V CM Lite shows that the trustworthiness of the EEPROM data is limited.

As you mentioned such deviations can be fixed by rewriting the EEPROM. I don't think that this is problematic for the parsing of the string. It might be an issue for warranty claims.

Retrieving the RAM size already relies on positional data. The eMMC size is just a sub-field of what is read in get_ddr_size_from_eeprom().

Changes would have to start there if wanted by the StarFive maintainer.

Best regards

Heinrich

Reply via email to