[U-Boot] [PATCH] atngw100: Use virtual address in CONFIG_ENV_ADDR

[Haavard Skinnemoen <haavard.skinnemoen@atmel.com>]

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