On 20.12.2016 18:30, Wolfgang Denk wrote:
> Dear Michal...
>
> In message
> <1e029d3a67722c20d8b6194d3388efe5ca92f5bf.1481881501.git.michal.si...@xilinx.com>
> you wrote:
>> Stop boot process if fpga programming fails.
>>
>> Signed-off-by: Michal Simek <michal.si...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
