[PATCH v3 5/6] sysinfo: rcar3: Use int instead of char for revision

[Marek Vasut]

On 7/14/23 20:27, Detlev Casanova wrote:
> On Friday, July 14, 2023 1:33:01 P.M. EDT Marek Vasut wrote:
>> On 7/14/23 18:43, Detlev Casanova wrote:
>>> To be used with the sysinfo command, revision values must be considered
>>> as integers, not chars as some boards will implement BOARD_REVISION_*
>>> and might use numbers greater than 9.
>>>
>>> Signed-off-by: Detlev Casanova <detlev.casanova at collabora.com>
>>> ---
>>>
>>>    drivers/sysinfo/rcar3.c | 104 ++++++++++++++++++++++++----------------
>>>    1 file changed, 62 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c
>>> index 7b127986da7..450a4c26773 100644
>>> --- a/drivers/sysinfo/rcar3.c
>>> +++ b/drivers/sysinfo/rcar3.c
>>> @@ -68,90 +68,110 @@ static void sysinfo_rcar_parse(struct
>>> sysinfo_rcar_priv *priv)>
>>>    	bool salvator_xs = false;
>>>    	bool ebisu_4d = false;
>>>    	bool condor_i = false;
>>>
>>> -	char rev_major = '?';
>>> -	char rev_minor = '?';
>>> +	char model[64];
>>> +	char rev[4] = "?.?";
>>> +	u8 rev_major = 0;
>>> +	u8 rev_minor = 0;
>>>
>>>    	switch (board_id) {
>>>    	
>>>    	case BOARD_SALVATOR_XS:
>>>    		salvator_xs = true;
>>>    		fallthrough;
>>>    	
>>>    	case BOARD_SALVATOR_X:
>>> +		snprintf(model, sizeof(model),
>>> +			 "Renesas Salvator-X%s board", salvator_xs ?
> "S" : "");
>>>
>>>    		if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid
> */
>>>
>>> -			rev_major = '1';
>>> -			rev_minor = '0' + (board_rev & BIT(0));
>>> +			rev_major = 1;
>>> +			rev_minor = board_rev & BIT(0);
>>> +			snprintf(rev, sizeof(rev), "%u.%u",
> rev_major, rev_minor);
>>>
>>>    		}
>>>
>>> -		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
>>> -			 "Renesas Salvator-X%s board rev %c.%c",
>>> -			 salvator_xs ? "S" : "", rev_major,
> rev_minor);
>>> +
>>> +		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
> "%s rev %s",
>>> +			 model, rev);
>>
>> Is there really no way to do this with single snprintf() call instead of
>> two snprintf() calls ?
> 
> I find it more readable like this, as opposed to have an snprintf in the if and
> one in an else block for each switch case.

I think I don't get it here -- why can't this be done with single 
snprintf() ? I mean, the revision is always set anyway.