Re: [PATCH v1 3/9] eeprom: starfive: Simplify get_ddr_size_from_eeprom()

E Shattow
2025-10-24T11:24:14Z

On 10/24/25 01:59, Hal Feng wrote:
> Directly return the DDR size instead of the field of 'DxxxExxx'.
> Move the function description to the header file.
> Return 0 instead of 0xFF if read_eeprom() fails.
>
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> 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..1ae9f2b840a 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 0 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..ca5039ee433 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 0;
>
> - 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)

LGTM

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