[U-Boot] [PATCH 07/11] flash/cfi_flash: Use virtual sector start address, not phys

[Haavard Skinnemoen]

Kumar Gala wrote:
> 
> On Dec 3, 2008, at 11:04 PM, Becky Bruce wrote:
> 
> > include/flash.h was commented to say that the address in
> > flash_info->start was a physical address.  However, from u-boot's
> > point of view, and looking at most flash code, it makes more
> > sense for this to be a virtual address.  So I corrected the
> > comment to indicate that this was a virtual address.
> >
> > The only flash driver that was actually treating the address
> > as physical was the mtd/cfi_flash driver.  However, this code
> > was using it inconsistently as it actually directly dereferenced
> > the "start" element, while it used map_physmem to get a
> > virtual address in other places.  I changed this driver so
> > that the code which initializes the info->start field calls
> > map_physmem to get a virtual address, eliminating the need for
> > further map_physmem calls.  The code is now consistent.
> >
> > The *only* place a physical address should be used is when defining  
> > the
> > flash banks list that is used to initialize the flash_info struct.  I
> > have fixed the one platform that was impacted by this change
> > (MPC8641D).
> >
> > Signed-off-by: Becky Bruce <beckyb at kernel.crashing.org>
> > ---
> > drivers/mtd/cfi_flash.c       |   53 +++++++++++++++++ 
> > +----------------------
> > include/configs/MPC8641HPCN.h |    2 +-
> > include/flash.h               |    2 +-
> > 3 files changed, 26 insertions(+), 31 deletions(-)
> 
> Stefan,
> 
> Have you reviewed this.  I'm not sure if remvoing the map_physmem() is  
> the right answer because I'm assuming Haavard added them for a reason  
> (AVR32?). Should we instead change info->start to be a phys_addr_t?

There was definitely a reason: On AVR32, the caches are enabled by
default, and I prefer that they stay that way since it makes everything
faster. But when dealing with the flash, we must bypass the caches by
using a different virtual address which maps to the same physical
memory.

From what I can tell from a quick scan, this patch doesn't seem to
change that; all it does is push the map_physmem() call a bit up the
stack so it gets called less often. That seems like a good thing to me.

However, what I don't understand is if the 'start' array is really
supposed to hold virtual addresses, why isn't it an array of pointers?

Haavard