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

[Becky Bruce]

On Aug 30, 2009, at 1:11 PM, Wolfgang Denk wrote:

> Dear Haavard & Becky,
>
> In message <20090830173640.2af9ce3c at 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.

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

There have always been VAs in U-boot, whether the programmer was aware  
of it or not.  They just happened to always be the same as a PA, and  
we never needed a separate "U-boot concept" for them.  However,  for  
any system with an MMU that has been turned on by u-boot, the notion  
of VAs exists.  You can't dereference a PA directly in a system with  
translation (even though the MMU is going to translate it to 1:1), and  
that's what all of the flash drivers were doing.  You can *treat* a PA  
as a VA and dereference it when you're 1:1, but at that point, you are  
in fact using it as a VA (nevermind its value).

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

Agreed.

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

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

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

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

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

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

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.  It's the !1:1 case  
that becomes more interesting and could have problems if you mix up  
your VAs and PAs, or you actually need dynamic mappings.

In any event, in this case, the problem that provoked this thread is  
not with 1:1, but with !1:1......

Cheers,
b