[U-Boot] [PATCH] common: Fix logic in fpga programming

[Michal Simek]

On 20.12.2016 18:30, Wolfgang Denk wrote:
> Dear Michal...
> 
> In message <1e029d3a67722c20d8b6194d3388efe5ca92f5bf.1481881501.git.michal.simek at xilinx.com> you wrote:
>> Stop boot process if fpga programming fails.
>>
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>>  common/image.c | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/common/image.c b/common/image.c
>> index bd07e86701a4..e3486e46a459 100644
>> --- a/common/image.c
>> +++ b/common/image.c
>> @@ -1375,11 +1375,10 @@ int boot_get_fpga(int argc, char * const argv[], bootm_headers_t *images,
>>  						img_len, BIT_PARTIAL);
>>  		}
>>  
>> -		printf("   Programming %s bitstream... ", name);
>>  		if (err)
>> -			printf("failed\n");
>> -		else
>> -			printf("OK\n");
>> +			return 1;
>> +
>> +		printf("   Programming %s bitstream... OK", name);
> 
> Good old U-Boot style says we print a message when we start an
> operation, and then an OK / failed when done.  When the system crashes
> in this command, you can see at last where it crashed, i. e. what the
> last running command was.

Based on this style the first part of this message should be called at
the beginning of this function not in this possition.

> 
> Your change breaks this - in the error path nothing gets printed.

If you look at the code
253         ret = boot_get_fpga(argc, argv, &images, IH_ARCH_DEFAULT,
254                             NULL, NULL);
255         if (ret) {
256                 printf("FPGA image is corrupted or invalid\n");
257                 return 1;
258         }

There is already printf for error.


> 
> This is even worse, as even though the system keeps running the user
> has no indication of what failed...

Isn't it message above enough?

Thanks,
Michal