Re: [PATCH] Running GRUB under qemu on PowerPC

[Robert Millan]

On Wed, Jan 23, 2008 at 12:07:57PM +0100, Marco Gerards wrote:
> Pavel Roskin <address@hidden> writes:
> 
> Hi,
> 
> [...]
> 
> > ChangeLog:
> >
> >         * include/grub/ieee1275/ieee1275.h: Introduce a flag for
> >         broken color support, needed for Open Hack'Ware.
> >         * kern/powerpc/ieee1275/cmain.c (grub_ieee1275_find_options):
> >         Recognize Open Hack'Ware.
> >         * term/ieee1275/ofconsole.c (grub_ofconsole_init): Skip color
> >         initialization for Open Hack'Ware.
> 
> This looks fine to me.  Robert, what do you think about this patch
> specifically?  I see no reason why this can't be committed, but I am
> not working on the PPC port lately.

It looks mostly fine to me.  I have 3 suggestions:

> +  if (! grub_ieee1275_finddevice ("/rom/boot-rom", &bootrom)) {
> +    rc = grub_ieee1275_get_property (bootrom, "model",
> +                                  tmp, sizeof (tmp), 0);
> +#define OHW "PPC Open Hack'Ware"
> +    if (rc >= 0 && !grub_strncmp (tmp, OHW, sizeof (OHW) - 1)) {

I think it's better to #undef OHW after using it, just in case.

> -  /* Set the right fg and bg colors.  */
> -  grub_ofconsole_setcolorstate (GRUB_TERM_COLOR_NORMAL);
> +  if (! grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_BROKEN_COLORS)) {

Perhaps GRUB_IEEE1275_FLAG_BROKEN_COLORS could be made more descriptive
(in the future we might find other, new, amusing ways in which colors can
break! :-)).  How about GRUB_IEEE1275_FLAG_CANNOT_SET_COLORS ?

Also, GRUB code style puts a newline before opening brackets.

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