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

[Haavard Skinnemoen]

Becky Bruce <becky.bruce at freescale.com> wrote:
> > Becky: the fact that Haavard's code is breaking is not considered an
> > indication of a deficiency in his code.  
> 
> I'm not calling the code deficient, just a bit inconsistent, and it  
> wasn't clear to me why.  When code uses the same value 1) by mapping  
> it and 2) by dereferencing it directly, that's a bit strange.  Why map  
> it in some cases and in not others?

I agree, that is inconsistent. However, you "fixed" the code which was
actually doing it correctly...we should instead fix the code which is
incorrectly using the physical address as a virtual one.

While doing that, we could also consider dropping that hideous start
array altogether and instead provide a handful of accessors for
locating sector start addresses on the flash.

>  How is that supposed to work when  
> VA!=PA or when the VA can change? This is one of the reasons that it  
> seemed to make sense to modify the driver as I did - it should now be  
> able to work when VA!=PA as well as when we're 1:1.  I could find no  
> users that seemed to need the dynamic mapping.  However, if anyone  
> does need to *dynamically* map the flash in and out with a different  
> VA each time, then we do need to do things a bit differently and we  
> should look into a solution for that.

I don't think anything needs a dynamic mapping right now, but if we're
going to _ever_ support such systems in the future (and your 36-bit PA
system is a likely candidate, isn't it?) we have to stop locking
ourselves into a static mapping.

>  Clearly, I'm not the expert on  
> the CFI code, so when I published that patch I expected someone to  
> smack me if I was being a moron :)

I apologize for not doing so earlier ;-)

Anyway, I have to take most of the blame for this situation...if I had
paid attention and flagged the problem earlier, much of this could have
been avoided.

I'm hoping we can avoid too much pointing of fingers and work towards a
reasonably future-proof solution which works well on all platforms.

> > If the CFI driver kind of allowed for VAs before (but incompletely /
> > incorrectly), then this dis not cause problems on any systems using a
> > strict 1:1 mapping.
> >
> > 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.  
> 
> Agreed, there shouldn't be any burden on those systems.

Agree too, as long as "those systems" does not include common code which
needs to run on all systems.

> >>> Everything is treated as virtual unless it's being used for hardware
> >>> setup.  
> >
> > Thisis NOT correct. U-Boot usually does NOT use virtual addresses.
> > Only very few systems do, and these must care not to disturb the
> > majority of systems which do no need to differentiate between
> > physical and virtual addresses.  
> 
> I'm not saying it *is* a VA as far as U-Boot knows, but that it is  
> *treated* as one, as mentioned above.   And this code was not expected  
> to disturb the 1:1 case.

Exposing VAs to the user interface and board code is IMO a very bad
idea, however. And that is what's happening now -- instead of
localizing the VAs to the CFI flash driver, it is now spread all over
the place.

> >>> A
> >>> lot of code had been just using the PA as a VA, because things were
> >>> always mapped 1-1.  
> >
> > Not only were, but _are_ and _will_be_.  
> 
> Of course - that should continue to be the default case unless it  
> needs to differ for some reason.

Yes, and that's why the external interface (at least the command line
and board configuration files) needs to use PA.

> > Indeed I'm deeply trouble when log standing rules get silently bent
> > and even broken.  
> 
> I agree 100% that 1:1 support should not be disturbed, since that's  
> the default case.   There was absolutely no intent here to bend any  
> "log<sic ;-)> standing rules", and nothing has been done here that  
> should have any impact on 1:1 as far as I'm aware.

Agree, 1:1 isn't the issue, it's two different systems with !1:1 which
have started making incompatible changes to the CFI driver.

Haavard