[PATCH 2/4] cpu: microblaze: remove unused ret variable

Michal Simek michal.simek at amd.com
Thu Aug 25 10:59:14 CEST 2022



On 8/25/22 08:41, Ovidiu Panait wrote:
> Drop the unused ret variable from microblaze_cpu_get_desc().
> 
> Signed-off-by: Ovidiu Panait <ovpanait at gmail.com>
> ---
> 
>   drivers/cpu/microblaze_cpu.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpu/microblaze_cpu.c b/drivers/cpu/microblaze_cpu.c
> index 969a1047e5..4eae06a8a6 100644
> --- a/drivers/cpu/microblaze_cpu.c
> +++ b/drivers/cpu/microblaze_cpu.c
> @@ -88,15 +88,14 @@ static int microblaze_cpu_get_desc(const struct udevice *dev, char *buf,
>   	struct microblaze_cpuinfo *ci = gd_cpuinfo();
>   	const char *cpu_ver, *fpga_family;
>   	u32 cpu_freq_mhz;
> -	int ret;
>   
>   	cpu_freq_mhz = ci->cpu_freq / 1000000;
>   	cpu_ver = microblaze_lookup_cpu_version_string(ci->ver_code);
>   	fpga_family = microblaze_lookup_fpga_family_string(ci->fpga_code);
>   
> -	ret = snprintf(buf, size,
> -		       "MicroBlaze @ %uMHz, Rev: %s, FPGA family: %s",
> -		       cpu_freq_mhz, cpu_ver, fpga_family);
> +	snprintf(buf, size,
> +		 "MicroBlaze @ %uMHz, Rev: %s, FPGA family: %s",
> +		 cpu_freq_mhz, cpu_ver, fpga_family);
>   
>   	return 0;
>   }

First of all I think you can remove DECLARE_GLOBAL_DATA_PTR from 
drivers/cpu/microblaze_cpu.c

I looked at the code and I think that there are some things what should happen.
First of all I would memset by 0 that buf which is passed and I think this 
should be done in uclass to make sure that you won't show anything what it is on 
the stack where buf is allocated in cmd/cpu.c for example.

The second if snprintf fails we shouldn't ignore that error because if you do 
that that means that buffer is valid and you can print content.

For cpu_freq_mhz I think that make sense to allocate some space in string. For 
example %4u gives you exact size GHz should be fine for now. cpu_ver has max 
size 6 now and family string has 18 chars now.


It means change string to this with some chars on the top should be fine for me.
	ret = snprintf(buf, size,
		 "MicroBlaze @ %5uMHz, Rev: %7s, FPGA family: %20s",
		 cpu_freq_mhz, cpu_ver, fpga_family);


And then check that ret is equal XX size would be easy for checking and 
returning 0 if they match.

Thanks,
Michal




More information about the U-Boot mailing list