[K42-discussion] G5 thermal management patches for kitchsrc and linux-029
Patrick Bozeman
PEBozeman at lbl.gov
Tue Feb 7 07:31:14 EST 2006
Responses inline...
Amos Waterland wrote:
>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 ...
>
>
I've tested it some and initial tests seemed ok but you should probably
do more before a commit is made since I haven't put it under as much
stress as it deserves. I am probably too trusting of the fact that that
the therm code is a direct copy of the linux code. And since it isn't
really running under a full linux kernel, this trust may be unfounded.
>
>
>>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.
>
>
Sounds good.
>
>
>>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?
>
>
This is the error handing that went in when the original patch was
committed to the linux 2.6.12. The xHANDLE_VALID macros were added at
the same time as the device tree fixup code. I know any other details
other than what is included in the comment I brought over withthe macro
definitions.
>
>
>>+
>>+
>> 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.
>
>
There were some complaints about it on the ppc64 mailing list. The
check was changed to include 0x37 in the 2.6.13 linux kernel. I don't
have a 0x37 to verify this personally though.
>
>
>>+ /* 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();
>> }
>>
>>
>_______________________________________________
>K42-discussion mailing list
>K42-discussion at ozlabs.org
>https://ozlabs.org/mailman/listinfo/k42-discussion
>
>
More information about the K42-discussion
mailing list