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

[Marek Vasut]

On 10/16/23 19:03, Detlev Casanova wrote:
> 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 at 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.

Good point, please document this in the commit message.