[PATCH 1/4] board: starfive: function to read eMMC size

[Heinrich Schuchardt]

On 4/16/24 06:09, E Shattow wrote:
> On Mon, Apr 15, 2024 at 4:50 AM Heinrich Schuchardt
> <heinrich.schuchardt at 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.schuchardt at 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