Hi Haavard,
> Detlev Zundel <d...@denx.de> wrote:
>> > Problem is that in order to make the CFI driver work on avr32, we need
>> > to change the map_physmem() macro to return the physical address
>> > unchanged.
>>
>> I see. And I presume you cannot tell the situation apart inside
>> map_physmem?
>
> I don't think so. How do you propose we do that?
I don't know, that's why I'm asking.
Let's take a step back and please excuse my ignorant question - why
exactly does the CFI driver need the physical address unchanged? Isn't
the real constraint that the address requested by CFI is uncached? Why
can't this be done by map_physmem()?
>> > The map_physmem() macro currently does exactly the same thing as the
>> > uncached() macro, and the unmap is a noop, but the next patch changes
>> > it in order to fix the CFI driver. If the next patch is applied without
>> > this patch being applied first, the SDRAM driver will do cached
>> > accesses during initialization, and that may cause the initialization
>> > to fail.
>>
>> Why not include a note to this extent into the git commit message? This
>> would be a great help for other people to later understand why this
>> change has been done the "backward way" that it was.
>
> The commit message already contains this:
>
> The paging system which is required to set up caching properties has not
> yet been initialized when the SDRAM is initialized. So when the
> map_physmem() function is converted to return the physical address
> unchanged, the SDRAM initialization will break on some boards.
>
> which is essentially the same thing, isn't it?
For me this is not the same - it does not include a word why the change
which you agree "looks backward" is something that we want to do. For
me something like this would give me more information:
Unfortunately we cannot make "map_physmem()/unmap_physmem()" on the
AVR32 platform work with the CFI driver as it works on other
platforms. [I don't understand why this is the case, so your
explanation would go here ;) ] To still fix the issue we deliberately
replace these generic routines by AVR32 specific routines. If someone
can fix this using the generic patterns, patches are welcome.
I believe that good docmumentation should include such pro- and con
reasoning so that code changes can be comprehended even after the fact.
Cheers
Detlev
--
Emacs seems a more likely candidate to contain a mail system than the
mail system to contain an Emacs, so this is the way it was done.
-- Bernard S. Greenberg
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
