[K42-discussion] G5 thermal management patches for kitchsrc and linux-029

[Amos Waterland]

On Fri, Feb 03, 2006 at 05:59:38PM -0800, Patrick Bozeman wrote:
> k42-g5-interrupts.patch:
> Fixes up the i2c interrupts on newer G5s during the prom init code in
> K42.  Without either this or the g5-interrupts.patch for linux-029,
> newer G5's will crash at bootup.

I went through this and committed it.  Please see my questions below though.

> k42-g5-therm.patch:
> Enables therm_pm72 in powerpc/ConfigureArch.c  Someone had previously
> commented out the therm_pm72_init call though.  Does anyone know why?

Hopefully Michal can comment on this:

 1.5          (mostrows 07-Apr-04): //       INITCALL(therm_pm72_init);

> g5-therm.patch:
> Updates linux-029 to use the linux-2.6.12 therm_pm72 thermal management
> code.

Unless Michal has objections, I will go through this patch tomorrow and
commit it if I don't see anything wrong.  Have you stress tested it
(hopefully sitting in front of the machine with the ability to yank the
power)?  I'd hate to melt your or somebody else's procesor ...

> g5-interrupts.patch: (optional)
> Fixes up the i2c interrupts in linux-029's prom init code.  When running
> under K42, this isn't needed since K42 is responsible for reading the OF
> device tree.  However, if anyone were to build and boot the linux-029
> kernel directly on a newer G5, they will need this patch.  This patch
> also places a guard in i2c-keywest.c that avoids dereferencing null
> pointers on buggy OF tress that haven't been fixed up.  Since
> k42-g5-interrupts.patch has already fixed the tree up, this is for
> future safety only.

I think I will hold off on this patch, unless you really think it should
go in.  We don't really worry about booting linux-029 as Linux.

I will add the guard though at some point.

> diff -ur kitchsrc.orig/os/boot/arch/powerpc/prom.c kitchsrc/os/boot/arch/powerpc/prom.c
> --- kitchsrc.orig/os/boot/arch/powerpc/prom.c	2004-10-27 06:56:42.000000000 -0700
> +++ kitchsrc/os/boot/arch/powerpc/prom.c	2006-02-03 16:33:33.000000000 -0800
> @@ -30,6 +30,17 @@
>  
>  #define reloc_offset() 0
>  
> +/*
> + * Error results ... some OF calls will return "-1" on error, some
> + * will return 0, some will return either. To simplify, here are
> + * macros to use with any ihandle or phandle return value to check if
> + * it is valid
> + */
> +#define PROM_ERROR              (-1u)
> +#define PHANDLE_VALID(p)        ((p) != 0 && (p) != PROM_ERROR)
> +#define IHANDLE_VALID(i)        ((i) != 0 && (i) != PROM_ERROR)

Did you pull this from a public patch somewhere, or just reason from
Arnd's patch that it is dangerous to check for less than zero and one
must explicitly compare against -1?

> +
> +
>  typedef void (*prom_entry)(struct prom_args *);
>  struct prom_t prom;
>  
> @@ -341,6 +352,63 @@
>  	return PLATFORM_PSERIES;
>  }
>  
> +/*
> + *
> + * Fixup interrupts on some G5s.
> + *
> + * See: the following thread for more info:
> + *     http://ozlabs.org/pipermail/linuxppc64-dev/2005-May/004135.html
> + *
> + */
> +static void
> +fixup_device_tree(void)
> +{
> +        unsigned long offset = reloc_offset();
> +        phandle u3, i2c, mpic;
> +        u32 u3_rev;
> +        u32 interrupts[2];
> +        u32 parent;
> +
> +        /* Some G5s have a missing interrupt definition, fix it up here */
> +        u3 = call_prom(RELOC("finddevice"), 1, 1,
> +                       RELOC("/u3 at 0,f8000000"));
> +        if (!PHANDLE_VALID(u3))
> +                return;
> +        i2c = call_prom(RELOC("finddevice"), 1, 1,
> +                        RELOC("/u3 at 0,f8000000/i2c at f8001000"));
> +        if (!PHANDLE_VALID(i2c))
> +                return;
> +        mpic = call_prom(RELOC("finddevice"), 1, 1,
> +                         RELOC("/u3 at 0,f8000000/mpic at f8040000"));
> +        if (!PHANDLE_VALID(mpic))
> +                return;
> +
> +        /* check if proper rev of u3 */
> +        if (call_prom(RELOC("getprop"), 4, 1, u3,
> +                      RELOC("device-rev"), &u3_rev, sizeof(u3_rev))
> +            == PROM_ERROR)
> +                return;
> +
> +        if (u3_rev != 0x35 && u3_rev != 0x37)
> +                return;

How do you know that revision 0x37 has this problem too?  I talked to
Ben and he said that he had reports that it might, but he wasn't
completely sure.

> +        /* does it need fixup ? */
> +        if ((int) call_prom(RELOC("getproplen"), 2, 1, i2c,
> +                            RELOC("interrupts")) > 0) {
> +                return;
> +        }
> +
> +        prom_print(RELOC("fixing up bogus interrupts for u3 i2c\n"));
> +
> +        /* interrupt on this revision of u3 is number 0 and level */
> +        interrupts[0] = 0;
> +        interrupts[1] = 1;
> +        call_prom(RELOC("setprop"), 4, 1, i2c,
> +                  RELOC("interrupts"), &interrupts, sizeof(interrupts));
> +        parent = (u32)mpic;
> +        call_prom(RELOC("setprop"), 4, 1, i2c,
> +                  RELOC("interrupt-parent"), &parent, sizeof(parent));
> +}
> +
>  void
>  linux_prom_init(unsigned long r3, unsigned long r4, unsigned long pp,
>  		unsigned long r6, unsigned long r7)
> @@ -398,6 +465,7 @@
>  		&getprop_rval, sizeof(getprop_rval));
>  	_prom->cpu = (int)(unsigned long)getprop_rval;
>  
> +        fixup_device_tree();
>  }