Hi Haavard,
Detlev Zundel dzu@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