Re: Redefining GRUB_MOD_GAP in init.c

[Robert Millan]

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 /.)