On Sun, Jan 27, 2008 at 10:25:32PM -0500, Pavel Roskin wrote: > Hi Robert, > > I'm concerned about your latest commit: > > * kern/powerpc/ieee1275/init.c (grub_arch_modules_addr): Skip > `GRUB_MOD_GAP' for platforms in which it's not defined. > > The algorithm for calculating the start of the modules should be exactly > the same in grub-mkimage and in the core. Any fallbacks should be > coherent if possible. Therefore, I think it would be better to define > GRUB_MOD_GAP only in header files.
Sorry, I should've really sent this for review; it wasn't such an obvious fix as you pointed out. I'll get that moved to headers as you suggest. > Also, I don't see how GRUB_MOD_GAP would not be defined to 0x8000 for > i386-ieee1275 platform considering that > include/grub/i386/ieee1275/kernel.h simply includes > include/grub/powerpc/ieee1275/kernel.h Sounds strange.. I'm wondering that myself. Will check.. > It's hard for me to understand why kern/powerpc/ieee1275/init.c can be > used on any platform other than PowerPC. I was assuming that my changes > would not affect other platforms. > > [...] > I suggest that i386-ieee1275 stops using any files for PowerPC. > kernel.h is not big, so it shouldn't be a problem to copy it. As for > init.c, it should be either copied or moved to a more suitable place. I think it's safe to say that the only part of kern/powerpc/ieee1275/init.c that only works on PowerPC is its name ;-) Maybe it's time we move out those generic files under powerpc directory. I propose moving the following: kern/powerpc/ieee1275/init.c kern/powerpc/ieee1275/cmain.c kern/powerpc/ieee1275/openfw.c loader/powerpc/ieee1275/multiboot2.c Does that seem fine? -- Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call… if you are unable to speak? (as seen on /.) _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
