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

[Wolfgang Denk]

Dear Haavard Skinnemoen,

In message <20090901123829.55fcb24e at hskinnemoen-d830> you wrote:
>
> 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.

We're not doing this. At least not intentionally.

The discussion we had was based on our knowledge about existing
systems, and needs to also handle more complex situations like for
example 32 bit systems with 36 bit physical addresses.

> 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!

Agreed - assuming it is possible without hurting the majority of
other existing configurations.

> > 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.)

I have to admit that I have no idea how map_physmem() used to work or
how it works now; at this point, I don;t care about implementation
details, I try to focus only on the overall design.


I think  your  double  mapping  is  a  hightly  architecture-specific
feature  which I do not like at all, but if you are happy with it and
it works for you  I  cannot  and  will  not  prevent  it.  But  after
discussions  with  Stefan Roese and Detlev Zundel it seems to me that
map_physmem() is probably not the right approach (judging  only  from
the  name).  We  should  not try to fix cache attributes by modifying
addresses / address maps

Instead, we should really extend the CFI driver such that it does not
matter if the addresses you are passing refer to cached or uncached
memory, and then provide hooks in the CFI driver to allow for testing
if cache is enabled, and switching cache off if it is.

Detlev Zundel suggested to use this as  a  chance  to  come  up  with
something  like a cache API which then could be used by other drivers
as well.

To me such an approach makes much more sense, as it  is  generic  and
can be used by other drivers and other architectures - even if it may
come at the cost of more effort on your systems.

> > 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.

Well, but board/atmel/atstk1000/flash.c _is_ a proprietary driver for
some flash chips that seem to be CFI conformant at first glance.  You
would not get such a driver into mailine any more. 

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
On my planet, to rest is to rest -- to cease using energy. To me,  it
is  quite  illogical to run up and down on green grass, using energy,
instead of saving it.
	-- Spock, "Shore Leave", stardate 3025.2