Becky Bruce <becky.br...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
