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

[Hal Feng]

> 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 at 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 at 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)