[U-Boot] [PATCH 5/5] cm-t35: add support for loading splash image from NAND

[Nikita Kiryanov <nikita@compulab.co.il>]

Hi Albert,

On 01/22/2013 09:37 AM, Albert ARIBAUD wrote:
> Hi Nikita,
>

[...]

>> Barring that, we should at least protect lcd.c from this issue by
>> making some sort of check for affected targets, or maybe limit the
>> possible values for splashimage... This issue makes it way too easy
>> to accidentally break the boot process in a way that's hard to recover
>> from.
>
> I suggest a few solutions:
>
> 1) enforce given load address alignment so that BMP header fields are
> natively aligned, with a clear error message. Simple to implement,
> difficut for users to understand and accept.

Yes I agree that from a user point of view this looks terrible, which is
why I prefer not to do something like this.

>
> 2) once the address provided by the user is known, if it is not
> properly aligned, then the next properly aligned address should be
> used, and the byte at given address should contain the offset from the
> given address to the used address. This is a general solution that
> works for any given load address, odd ones included:
>
> Given address:  First bytes:    Used address:
> 10000000        2 x 'B' 'M'     10000002
> 10000001        1 'B' 'M'       10000002
> 10000002        'B' 'M'         10000002
> 10000003        3 x x 'B' 'M'   10000006
> 10000004        2 x 'B' 'M'     10000006
> ...
>
> Note that if the user address is constrained to be 4-byte-aligned,
> then only the "2 x 'B' 'M'" case would apply.

I think a simpler way to implement something like this is to just
use modulo 4 to check alignment and fix the address dynamically;
perhaps even fixing it in the environment.

This is a localized approach though. We will have to do this from
all the code paths that lead to a bmp header being probed in memory.
I would prefer a more localized solution.

>
> 3) define an internal 'BMP holder' structure which contains a
> two-byte prefix before the 'BMP file' header and data. This way, if
> the overall structure is aligned, then the fields in the BMP header are
> aligned too.
>
> 4) Build a time machine and tell the designers of the BMP header format,
> in great inventive and colorful detail, what horrible things will happen
> to them if they don't order and size their fields so that they naturally
> land on aligned offsets from the header start. This solution gives the
> best results IMO.

The problem with 3 (and 4) is that it still doesn't protect the user
from bricking their board by choosing a non-aligned address for their
BMP. This might happen because the user:
- is unaware of the dangers of choosing a non-aligned address
- made a typo
- relies on a script or program that runs under U-Boot to setup stuff
- I"m sure there are other possibilities

In terms of usability it's a *big* regression. If we do not actually
prevent the user from setting splashimage to an unaligned address we
should make sure that all usable addresses are safe.

>
> 5) if none the above (including 4) is feasible for some reason, then
> use unaligned accessors for this BMP fields, with a Big Fat Comment
> about why this is so.

I think, once this feature is merged, I'll try to see if something nice
can be done with an approach like this.
For now I'll add a suggestion-#2-style check in lcd.c

>
> Note that all solutions except 2 (and 4) depend on the given address
> being constrained in some way -- such a constraint does not seem
> excessive to me.
>
> Amicalement,
>


-- 
Regards,
Nikita.