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 <[email protected]> > > --- > > 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 _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
