Hi Albert ARIBAUD,
On 02/04/2013 05:25 PM, Albert ARIBAUD wrote:
Hi Nikita,
On Mon, 04 Feb 2013 17:07:05 +0200, Nikita Kiryanov
<[email protected]> wrote:
On 02/04/2013 04:19 PM, Albert ARIBAUD wrote:
Hi Nikita,
On Mon, 04 Feb 2013 14:37:06 +0200, Nikita Kiryanov
<[email protected]> wrote:
Hi Albert,
On 02/04/2013 01:57 PM, Albert ARIBAUD wrote:
IIRC, you has submitted a fix so that BMP loads would result in
correctly aligned fields and thus no need for accessors. Why this
change of mind?
>
I did mention that I consider that fix as temporary. I believe this fix
is better because it addresses the issue everywhere and does so in a
more maintainable way (does not require the address fix to be duplicated
everywhere there is a problem).
I actually like the new fix less, as it adds an API where there is no
need of one -- it's not like the implementation of BMP is at risk of
changing. What is the problem in fixing the load address at load time
and then passing this fixed address around?
The problem is that it makes the U-Boot display subsystems broken in
terms of what kind of programming interface they provide.
I'm not sure I understand what this means. Once all places that
construct BMPs in memory are fixed so that they don't misalign, and
documentation is there to tell about the issue, what exactly remains
broken?
See below...
The load address fix has to be applied in every code path that leads
to a BMP being loaded and read in memory. This means that anyone who
wants to implement some BMP related functionality needs to care about
this architecture/memory issue, and actively circumvent it.
Yes; but it is only a matter of getting the properly aligned address
from the non-aligned one, which a simple macro is enough to provide;
I would've been in favour of the address fix solution if it was confined
to lcd.c, because then it would've been an implementation detail of
U-Boot's display subsystem, but it's not going to be necessarily
confined there. We have display_draw_bitmap() in the API, so users can
write standalone programs that display BMPs. Boards are required to
load the BMP to memory on their own, so that's another place where
there might be probing of the BMP header. So new code paths that lead
to BMP header probing are to be expected, and this means that we would
be adding a new requirement to anyone who wants to use BMPs.
the rest, namely accessing fields, does not have to be turned into an API,
nor does it benefit from it.
I think an API has benefits, namely that it simplifies the code a bit,
and it separates the alignment concern (handled with the compilation
flag) from the reading-BMP-data concern (handled by using the API).
Besides, switching to an API requires
searching every place where BMPs are used, just like fixing addressing
does.
This isn't good design. If I'm manipulating BMPs I should only care
about my BMPs, not hardware issues.
Except that you have to, because you're not simply manipulating BMPs,
you're manipulating them in a context where alignment is required.
That's true, but it's not an argument against simplifying the code.
Amicalement,
--
Regards,
Nikita.
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
