Paul Mackerras wrote:
Mohan Kumar M writes:
Hi Paul,
Thanks for your comments.
I will update the patches as per your comment and will give a detailed
description for each patch.
Regards,
Mohan.
static inline int in_kernel_text(unsigned long addr)
{
- if (addr >= (unsigned long)_stext && addr < (unsigned long)__init_end)
+ if (addr >= (unsigned long)_stext && addr < (unsigned long)__init_end
+ + kernel_base)
Your patch adds kernel_base to some addresses but not to all of them,
so your patch description should have told us why you added it in the
those places and not others. If you tell us the general principle
you're following (even if it seems obvious to you) it will be useful
to people chasing bugs or adding new code later on, or even just
trying to understand what the code does.
- RELOC(alloc_bottom) = PAGE_ALIGN((unsigned long)&RELOC(_end) + 0x4000);
+#ifndef CONFIG_RELOCATABLE_PPC64
+ RELOC(alloc_bottom) = PAGE_ALIGN((unsigned long)&RELOC(_end) + 0x4000);
+#else
+ RELOC(alloc_bottom) = PAGE_ALIGN((unsigned long)&RELOC(_end) + 0x4000 +
+ RELOC(reloc_delta));
+#endif
Ifdefs in code inside a function are frowned upon in the Linux
kernel. Try to find an alternative way to do this, such as ensuring
that reloc_delta is 0 when CONFIG_RELOCATABLE_PPC64 is not set.
Also it's not clear (to me at least) why you need to add reloc_data in
the relocatable case.
+#ifndef CONFIG_RELOCATABLE_PPC64
unsigned long *spinloop
= (void *) LOW_ADDR(__secondary_hold_spinloop);
unsigned long *acknowledge
= (void *) LOW_ADDR(__secondary_hold_acknowledge);
+#else
+ unsigned long *spinloop
+ = (void *) &__secondary_hold_spinloop;
+ unsigned long *acknowledge
+ = (void *) &__secondary_hold_acknowledge;
+#endif
This also needs some explanation. (Put it in the patch description or
in a comment in the code, not in a reply to this mail. :)
+#ifndef CONFIG_RELOCATABLE_PPC64
ld r4,[EMAIL PROTECTED](2)
+#else
+ LOAD_REG_IMMEDIATE(r4,htab_hash_mask)
+#endif
ld r27,0(r4) /* htab_hash_mask -> r27 */
Here and in the other similar places, I would prefer you just changed
it to LOAD_REG_ADDR and not have any ifdef.
Paul.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
