On 6 July 2012 20:26, Dave Martin dave.martin@linaro.org wrote:
On Fri, Jul 06, 2012 at 07:07:35PM +0100, Peter Maydell wrote:
On 6 July 2012 00:27, Rob Herring robherring2@gmail.com wrote:
I would just change arm_add_memory to use phys_addr_t for the size param. This ultimately calls memblock functions which use phys_addr_t for sizes.
So I have a patch that does this which basically works. However there is a bit I'm not sure about. arm_add_memory() does this: bank->size = size & PAGE_MASK;
in an attempt to clear the bottom bits of the size. However, since PAGE_MASK is defined as: #define PAGE_SIZE (_AC(1,UL) << PAGE_SHIFT) #define PAGE_MASK (~(PAGE_SIZE-1))
PAGE_MASK is a 32 bit unsigned constant and so this & will clear the top 32 bits of bank->size.
I'm really not sure what the best way to fix this is; suggestions?
Maybe something like
~(phys_addr_t)~PAGE_MASKor
~(phys_addr_t)(PAGE_SIZE - 1)would work.
Mmm. It feels a bit unsatisfactory that an "obviously correct" line of code like "size &= ~PAGE_MASK" could actually be subtly wrong (seems like the kind of thing that could easily slip through code review), so I was wondering if maybe redefining PAGE_MASK so it didn't do the wrong thing on 64 bit types would be better. (That's definitely way out of my depth though.)
-- PMM