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

Patrick Bozeman PEBozeman at lbl.gov
Tue Feb 7 07:37:31 EST 2006


fyi, that part about the testing should have read, *I* should probably
do more.

I'm doing more extensive cpu testing now, and am going to see if I can
get a patch to read the sensor data in /proc to monitor the cpu info and
fan speeds to make sure.

Patrick Bozeman wrote:

>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
>> 
>>
>>    
>>
>
>_______________________________________________
>K42-discussion mailing list
>K42-discussion at ozlabs.org
>https://ozlabs.org/mailman/listinfo/k42-discussion
>  
>




More information about the K42-discussion mailing list