Hi Igor,
On 11 December 2016 at 10:37, Igor Grinberg <grinb...@compulab.co.il> wrote:
> Hi Tomas, Simon,
>
> Sorry, to break in that late...
> I have a quick question below.
>
> On 12/05/16 09:36, Tomas Melin wrote:
>> Enable support for loading a splash image from within a FIT image.
>> The image is assumed to be generated with mkimage -E flag to hold
>> the data external to the FIT.
>>
>> Signed-off-by: Tomas Melin <tomas.me...@vaisala.com>
>
> [...]
>
>> diff --git a/common/splash_source.c b/common/splash_source.c
>> index 70d724f..94b46d3 100644
>> --- a/common/splash_source.c
>> +++ b/common/splash_source.c
>
> [...]
>
>> +#ifdef CONFIG_FIT
>> +static int splash_load_fit(struct splash_location *location, u32
>> bmp_load_addr)
>> +{
>> + int res;
>> + int node_offset;
>> + int splash_offset;
>> + int splash_size;
>> + struct image_header *img_header;
>> + const u32 *fit_header;
>> + u32 fit_size;
>> + const size_t header_size = sizeof(struct image_header);
>> +
>> + /* Read in image header */
>> + res = splash_storage_read_raw(location, bmp_load_addr, header_size);
>> + if (res < 0)
>> + return res;
>> +
>> + img_header = (struct image_header *)bmp_load_addr;
>> + fit_size = fdt_totalsize(img_header);
>> +
>> + /* Read in entire FIT */
>> + fit_header = (const u32 *)(bmp_load_addr + header_size);
>> + res = splash_storage_read_raw(location, (u32)fit_header, fit_size);
>> + if (res < 0)
>> + return res;
>> +
>> + res = fit_check_format(fit_header);
>> + if (!res) {
>> + debug("Could not find valid FIT image\n");
>> + return -EINVAL;
>> + }
>> +
>> + node_offset = fit_image_get_node(fit_header, location->name);
>> + if (node_offset < 0) {
>> + debug("Could not find splash image '%s' in FIT\n",
>> + location->name);
>> + return -ENOENT;
>> + }
>> +
>
> I think two above debug() are very legitimate - no need to shout if no FIT
> image
> or no splash in it...
>
>> + res = fit_image_get_data_offset(fit_header, node_offset,
>> + &splash_offset);
>> + if (res < 0) {
>> + debug("Could not find 'data-offset' property in FIT\n");
>> + return res;
>> + }
>> +
>> + res = fit_image_get_data_size(fit_header, node_offset, &splash_size);
>> + if (res < 0) {
>> + debug("Could not find 'data-size' property in FIT\n");
>> + return res;
>> + }
>
> Now regarding these two, I'm not sure.
> Since we have found a valid FIT and also a node with a correct splash name,
> probably the intent is that we show the splash, right?
> But in the two above checks, we find inconsistencies that do not allow us to
> show the splash - meaning the FIT is not actually good (am I right here?).
> So may be we should report it to the 'user' and allow correcting the FIT?
> Otherwise, it is impossible to debug the image w/o a debug version of
> U-Boot...
> Do I make sense, or do I miss something?
Yes that makes some sense, but the problem is that then you are
including error messages always which would never happen in a working
system (i.e. it just bloats the code).
So long as the error is reported (even if it is not a very specific
error), people can add DEBUG and track it down.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
