Re: [PATCH v5 6/6] sysinfo: rcar3: Implement BOARD_ID and BOARD_REV_*

[Detlev Casanova <detlev.casanova@collabora.com>]

On Saturday, October 7, 2023 5:35:27 P.M. EDT Marek Vasut wrote:
> On 10/2/23 17:20, Detlev Casanova wrote:
> > Expose that information to the sysinfo command to let scripts make
> > decisions based on the board id and revision.
> > 
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > ---
> > 
> >   drivers/sysinfo/rcar3.c | 89 +++++++++++++++++++++++++++++------------
> >   1 file changed, 63 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c
> > index 633e80bc19b..a554323506a 100644
> > --- a/drivers/sysinfo/rcar3.c
> > +++ b/drivers/sysinfo/rcar3.c
> > @@ -32,6 +32,10 @@
> > 
> >    */
> >   
> >   struct sysinfo_rcar_priv {
> >   
> >   	char	boardmodel[64];
> > 
> > +	u8	id;
> > +	u8	rev_major;
> > +	u8	rev_minor;
> > +	bool	has_rev;
> > 
> >   	u8	val;
> >   
> >   };
> > 
> > @@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev,
> > int id, size_t size, char *> 
> >   	};
> >   
> >   }
> > 
> > +static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val)
> > +{
> > +	struct sysinfo_rcar_priv *priv = dev_get_priv(dev);
> > +
> > +	switch (id) {
> > +	case SYSINFO_ID_BOARD_ID:
> > +		*val = priv->id;
> > +		return 0;
> > +	case SYSINFO_ID_BOARD_REV_MAJOR:
> > +		if (!priv->has_rev)
> > +			return -EINVAL;
> > +		*val = priv->rev_major;
> > +		return 0;
> > +	case SYSINFO_ID_BOARD_REV_MINOR:
> > +		if (!priv->has_rev)
> > +			return -EINVAL;
> > +		*val = priv->rev_minor;
> > +		return 0;
> > +	default:
> > +		return -EINVAL;
> > +	};
> > +}
> > +
> > 
> >   static const struct sysinfo_ops sysinfo_rcar_ops = {
> >   
> >   	.detect = sysinfo_rcar_detect,
> >   	.get_str = sysinfo_rcar_get_str,
> > 
> > +	.get_int = sysinfo_rcar_get_int,
> > 
> >   };
> >   
> >   static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
> > 
> > @@ -69,8 +97,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv
> > *priv)> 
> >   	bool ebisu_4d = false;
> >   	bool condor_i = false;
> >   	char rev[4] = "?.?";
> > 
> > -	u8 rev_major = 0;
> > -	u8 rev_minor = 0;
> > +
> > +	priv->id = board_id;
> > +	priv->has_rev = false;
> 
> Would it make more sense to assign
> 
> priv->rev_major = 1;
> priv->rev_minor = 0;
> 
> And get rid of priv->has_rev entirely ?
> 
> Basically say that the default case is rev. 1.0 board.
> 
> [...]

I'm not really sure about this has it doesn't differentiate between rev 1.0 and 
unknown revision.