Re: [RFC 04/10] eeprom: starfive: Correct get_pcb_revision_from_eeprom()

Heinrich Schuchardt
2025-08-29T07:44:27Z

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)