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

[Haavard Skinnemoen]

Becky Bruce <becky.bruce at freescale.com> wrote:
> Sorry, guys, I'm still not clear on what's going on. Haavard, did you  
> expect each call to flash_map to return a different VA each time (i.e.  
> you're trying to do some sort of dynamic mapping), or are you just  
> calling it to get the VA that corresponds to some PA, since VA != PA  
> on these 2 boards?

I'm calling it to get some VA which corresponds to a given PA with
given caching properties. I don't think it's good design if the board
code for every single board needs to know exactly which VA is going to
be returned.

> >> 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.
> >
> > Sorry, I don't know that code and it's users well enough to coment.
> > Maybe Becky and/or Stefan can shed some light on this...
> 
> The problem is that would mean the CFI driver would be treating start  
> as a PA, while every other flash driver that I looked at treats it as  
> a VA.  That kind of inconsistency would be bad.

Except that it's even more inconsistent now since lots of code
elsewhere treats start as a PA, and the CFI driver was the only driver
which did it mostly correctly.

> Plus, Wolfgang,  
> Stefan, Kumar, and I discussed this on the list/IRC last november and  
> agreed that it made sense for command-line foo (including the flash  
> commands)  to take a virtual address as the argument.  Platforms that  
> have a non-fixed memory map would need to provide a command-line  
> interface to get a VA to use (since that's highly unusual and expected  
> to remain so).

I think that's an incredibly stupid idea, and it's really a shame that
I didn't participate in the IRC meeting.

It would have been so much easier if the command line only had to deal
with PA and the platform had to remap things transparently if
necessary. Now, if you need to access an arbitrary physical address
through a script, you need to first call this "set up a mapping"
command, record the value it prints out (presumably) and make sure not
to interpret any error message as a virtual address, use it to perform
the command, and then unmap it afterwards.

So if anything is breaking the u-boot philosophy of simplicity it is
this, and it does it in the worst possible way: it exposes all the
complexity through the user interface!

> >> That PA==VA rule is pretty much the reason we're in this mess -- if
> >
> > Let's say, the fact that this rule has not been stricter enforced has
> > caused that teh appearant problems were not discovered earlier.
> >
> >> more platforms had broken this rule, perhaps more of the code would
> >
> > Heh. If more  platforms had broken this rule we would probably have
> > become aware of these violations earlier, and stopped them doing such
> > naughty things ;-)
> 
> Well, u-boot was supposed to be simple, and a VA=PA assumption ended  
> up built into a lot of the code.  Which isn't a problem until you need  
> something different....  I had a lot of fun standing on my head trying  
> to get 36-bit physical addressing on PowerPC working as a result of  
> this.   I suspect the next big u-boot problem will be the need for  
> dynamic mappings, because we're assuming for the most part now that  
> the board memory map is static.

Exactly, and with the current approach, dynamic mappings will _never_
be possible because all the VA-to-PA mapping assumptions will be built
into both the board code and the user interface!

So I ask again: Why don't we just fix the broken code instead? We have
the mechanisms available to make this work via the map_physmem()
functions, we just need to use them.

Haavard