On Sun, 30 Aug 2009 20:11:01 +0200 Wolfgang Denk wd@denx.de wrote:
Dear Haavard & Becky,
In message 20090830173640.2af9ce3c@siona you wrote:
The only way for that to work is when VA=PA (or, depending on what you were doing with the address,
Well, VA==PA is the default setting for U-Boot that shall be used for all systems, unless it is really impossible on a board to implement an exception from that rule.
While not impossible, following that rule on the NGW100 would require that I either disable the caches (which would result in a massive slowdown) or start using the MMU more actively to get the caching properties right.
you just got lucky). The CFI driver was the outlier - all the other flash code was treating the start field as a VA already. So I don't think just reverting the patch is the answer.
We did not even have any notion of VA's in U-Boot until very recently, and I call on everybody not to make U-Boot more complicated than necessary.
I don't think any boards with PA==VA are affected. This issue is about two platforms with VA!=PA following different rules...
In almost all cases RAM and NOR flash fit easily in the physical address space of the CPUs, and for the sake of this majority of systems it must be possible to access memory on such systems assuming a simple 1:1 mapping.
You're ignoring cache issues.
Any changes to the code to correctly support other mappings must be done in a way that they (1) do not break and (2) do not add additional burdon on systems with a simple 1:1 mapping.
That was the idea when I introduced map_physmem() to the CFI driver a while back: the externally visible addresses were kept unchanged, while the remapping was done internally. Also, map_physmem() is a no-op on platforms with a simple 1:1 mapping.
If you use something to do memory accesses, it's virtual.
Unless you have a very special system you can rely on a strict 1:1 mapping.
Technically, the addresses seen by the CPU are virtual. And I don't think systems with a cache should be considered 'very special' these days...
There are basically two ways to fix it: Either go back to using physical addresses in the flash API, or make CONFIG_ENV_ADDR virtual
I understand we cannot do that, because some systems need to map (NOR) flash to virtual addresses outside the physical address space. Ok, so the CFI driver shall consistently be able to use VAs - but on simple systems with a 1:1 mapping there shall be no need in the system configuration to spend any thoughts on this.
But exactly what's wrong with hiding all this complexity inside map_physmem()? It was designed _exactly_ for this purpose -- to allow platforms with non-trivial mappings between VA and PA to do the necessary mapping when the driver requests it.
(and from what I hear, the jffs2 code needs a similar fix.) This patch does the latter, but it seems like it doesn't fix things completely, and Wolfgang didn't appear very happy about it.
Indeed I'm deeply trouble when log standing rules get silently bent and even broken.
And I deeply regret using the CFI driver on NGW100...it took a lot of effort to get it mostly limping along, and then it got broken at the first opportunity. I should have stuck with the much smaller and more efficient board-specific driver I had to begin with.
That PA==VA rule is pretty much the reason we're in this mess -- if more platforms had broken this rule, perhaps more of the code would have been written properly without lots of implicit conversion from PA to VA via ugly casts between unsigned long and all sorts of pointers.
Haavard