On 6 March 2014 20:42, Stephen Boyd <sb...@codeaurora.org> wrote: > On 03/05, Sebastian Capella wrote: >> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h >> index 8756e4b..d32adbb 100644 >> --- a/arch/arm/include/asm/memory.h >> +++ b/arch/arm/include/asm/memory.h >> @@ -291,6 +291,7 @@ static inline void *phys_to_virt(phys_addr_t x) >> */ >> #define __pa(x) __virt_to_phys((unsigned long)(x)) >> #define __va(x) ((void >> *)__phys_to_virt((phys_addr_t)(x))) >> +#define __pa_symbol(x) __pa((unsigned long)(x)) > Looking at this definition now it doesn't look right. Isn't > &__nosave_begin a virtual address? Casting it to an unsigned long > isn't going to give you a physical address. Why can't we use > __pa()?
You're correct, sorry, I missed removing the cast, it's not needed. I've removed the __pa_symbol define as I'm switching to the __pa implementation you suggested below. >> +extern const void __nosave_begin, __nosave_end; >> + >> +int pfn_is_nosave(unsigned long pfn) >> +{ >> + unsigned long nosave_begin_pfn = >> + __pa_symbol(&__nosave_begin) >> PAGE_SHIFT; >> + unsigned long nosave_end_pfn = >> + PAGE_ALIGN(__pa_symbol(&__nosave_end)) >> PAGE_SHIFT; >> + >> + return (pfn >= nosave_begin_pfn) && (pfn < nosave_end_pfn); >> +} > > Perhaps this code could be: > > unsigned long nosave_begin_pfn = virt_to_pfn(&__nosave_begin); > unsigned long nosave_end_pfn = virt_to_pfn(&__nosave_end); > > return (pfn >= nosave_begin_pfn) && (pfn <= nosave_end_pfn); > > or if virt_to_pfn() doesn't exist on ARM and we can't add it for > some reason: > > unsigned long nosave_begin_pfn = __phys_to_pfn(__pa(&__nosave_begin)); > unsigned long nosave_end_pfn = __phys_to_pfn(__pa(&__nosave_end)); > > return (pfn >= nosave_begin_pfn) && (pfn <= nosave_end_pfn); I've moved to this second implementation. I don't think I understand the ramifications enough to add the virt_to_pfn call for arm. I've changed one thing. __nosave_end is pointing at the first byte not included in the nosave region. I subtracted one from it so that we make sure we're referring to the last pfn, and left the pfn comparison as you'd suggested. int pfn_is_nosave(unsigned long pfn) { unsigned long nosave_begin_pfn = __phys_to_pfn(__pa(&__nosave_begin)); unsigned long nosave_end_pfn = __phys_to_pfn(__pa(&__nosave_end - 1)); return (pfn >= nosave_begin_pfn) && (pfn <= nosave_end_pfn); } I'll test this and post it if everyone's ok with it. Thanks for your review and comments Stephen! Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/