Re: [PATCH 2/2] cmd/sbi: add missing SBI information

[Sean Anderson <seanga2@gmail.com>]

On 7/19/21 4:28 PM, Heinrich Schuchardt wrote:
> Let the sbi command display:
> 
> * SBI implementation version
> * machine vendor ID
> * machine architecture ID
> * machine implementation ID
> 
> With this patch the output for the HiFive Unmatched looks like
> 
>      => sbi
>      SBI 0.3
>      OpenSBI 0.9
>      Machine:
>        Vendor ID 489
>        Architecture ID 8000000000000007
>        Implementation ID 20181004
>      Extensions:
>        sbi_set_timer
>        sbi_console_putchar
>        sbi_console_getchar
>        sbi_clear_ipi
>        sbi_send_ipi
>        sbi_remote_fence_i
>        sbi_remote_sfence_vma
>        sbi_remote_sfence_vma_asid
>        sbi_shutdown
>        SBI Base Functionality
>        Timer Extension
>        IPI Extension
>        RFENCE Extension
>        Hart State Management Extension
>        System Reset Extension
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>   cmd/riscv/sbi.c | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
> index 90c0811e14..c0db763ba7 100644
> --- a/cmd/riscv/sbi.c
> +++ b/cmd/riscv/sbi.c
> @@ -59,13 +59,30 @@ static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc,
>   	if (ret >= 0) {
>   		for (i = 0; i < ARRAY_SIZE(implementations); ++i) {
>   			if (ret == implementations[i].id) {
> -				printf("%s\n", implementations[i].name);
> +				printf("%s", implementations[i].name);
> +				ret = sbi_get_impl_version();
> +				if (ret > 0) {

Shouldn't this have to check to ensure that i == 1?

> +					/* OpenSBI specific version encoding */
> +					printf(" %ld", ret >> 16);
> +					printf(".%ld", ret & 0xffff);
> +				}
> +				printf("\n");
>   				break;
>   			}
>   		}
>   		if (i == ARRAY_SIZE(implementations))
>   			printf("Unknown implementation ID %ld\n", ret);
>   	}
> +	printf("Machine:\n");
> +	ret = sbi_get_mvendorid();
> +	if (ret != -ENOTSUPP)
> +		printf("  Vendor ID %lx\n", ret);

perhaps %0.8lx? And should we decode the JEDEC id?

> +	ret = sbi_get_marchid();
> +	if (ret != -ENOTSUPP)
> +		printf("  Architecture ID %lx\n", ret);
> +	ret = sbi_get_mimpid();
> +	if (ret != -ENOTSUPP)
> +		printf("  Implementation ID %lx\n", ret);
>   	printf("Extensions:\n");
>   	for (i = 0; i < ARRAY_SIZE(extensions); ++i) {
>   		ret = sbi_probe_extension(extensions[i].id);
> --
> 2.30.2
>