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

[Haavard Skinnemoen]

Wolfgang Denk <wd at denx.de> wrote:
> Dear Haavard Skinnemoen,
> > Like I explained in an earlier mail, the default setup includes a 1:1
> > mapping of the lowest addresses, but it's cacheable. The default setup
> > also includes an uncached mapping of the same memory at a higher
> > virtual address.
> 
> You mean you want to have the same memory area mapped _twice_, once
> cached and once (at another address) uncached?

Yes.

> Well, this obviously cannot be done with a plain 1:1 mapping. But
> then: isn't that asking for trouble anyway? Or is there anything that
> prevents you for example reading stale cached data after the memory
> content has changed by accesses through the uncached mapping?

There's nothing which prevents me from accessing a completely different
address either -- I just need to make sure that I access the correct
address, or things will blow up one way or another.

The default virtual memory model makes it very easy to do uncached
access to certain types of memory (i.e. flash) and cached access to
others (i.e. SDRAM). It also makes it easy to run from cached flash
memory at startup and switch to uncached access later (after flushing
the cache, of course).

> > Yes, it is much easier (and smaller) to use the default virtual memory
> > layout than setting up paging (which involves several exception
> > handlers).
> 
> I don't see how paging comes into play here?

If I can't use the default scheme described above (and you're pretty
much saying I can't), I can define caching properties on a page-by-page
basis, but that obviously requires paging to be enabled.

> > Ok, so the code is broken and nobody else cares?
> 
> Broken? You might call it a design decision. This is a boot loader,
> and simplicity of design and easy porting and board bring up are
> usually higher rated that sequeezing out the last few percent of
> performance.

And that is EXACTLY why I'm trying to advocate: Keep the additional
complexity (which can be kept to a minimum) localized to a handful of
drivers, and don't change the command line interface or the board
configuration interface.

> IIRC, on PowerPC the difference of running with DC
> enabled or not is only in the 10% range or so.

But as a matter of principle, I don't want to reduce the performance
_and_ increase the code size just because a driver that used to work
got broken. I want to fix the driver instead!

> > I suppose I could disable the DC (which is a bit complicated, but
> > possible), but that would just add to the already high cost (in terms
> > of both code size and performance) of using common code (i.e. the CFI
> > driver), so I'm leaning towards a custom flash driver instead.
> 
> If you want to run with data caches enabled by default, then it would
> probably make more sense if you invested efforts in extending the CFI
> driver to provide hooks / callbacks to (temporarily) switch of data
> cache for the memory range in question. This wouls allow you to run
> with DC enabled on the flash area, and still use the CFI driver.

But that is exactly how map_physmem() works -- it allows the CFI driver
(or any other driver) to request the caches to be bypassed for a given
physical address range, possibly resulting in a different virtual
address (though for backwards compatibility, all platforms except AVR32
simply return the physical address unchanged.)

The problem is that this virtual address (which currently isn't
dynamic, but it's easy to imagine a platform where it could be) is
exposed to the rest of the world, thus breaking both existing board
configurations and potentially any command-line scripts (and,
apparently, the jffs2 driver though I'm not entirely sure how that
happened.)

> Such changes will have it much easier to find their way into mainline
> than adding a proprietary flash driver.

It certainly won't be a proprietary driver; if anything, it would be a
variant of the driver currently used by ATSTK1000.

Haavard