On Thu, May 03, 2007 at 12:15:54PM -0600, Marc Jones wrote: > This patch adds support for the AMD Geode LX CPU.
Sorry, I would still reject it because of all the whitespace breakage. > - * CPUbug784 > - * Bugtool #784 + #792 > - * Fix CPUID instructions for < 3.0 CPUs > - > -void bug784(void) And what about all these bug functions? > -/* FooGlue Setup*/ Hahaha, "FooGlue" ?! :) > Index: LinuxBIOSv2/src/cpu/amd/model_lx/vsmsetup.c > =================================================================== > --- LinuxBIOSv2.orig/src/cpu/amd/model_lx/vsmsetup.c 2007-05-03 > 11:20:48.000000000 -0600 > +++ LinuxBIOSv2/src/cpu/amd/model_lx/vsmsetup.c 2007-05-03 > 11:22:07.000000000 -0600 > @@ -17,62 +17,65 @@ > /* vsmsetup.c derived from vgabios.c. Derived from: */ > > /*------------------------------------------------------------ -*- C -*- > - * 2 Kernel Monte a.k.a. Linux loading Linux on x86 > + * 2 Kernel Monte a.k.a. Linux loading Linux on x86 At least 90% of the changes in this file is whitespace. That's just not OK IMHO. There's more in other files and the other patches but this may be the worst. > @@ -85,7 +88,7 @@ > - " .long gdt \n" > + " .long gdt \n" > @@ -105,7 +108,7 @@ > - " .byte 0x00, 0x9b, 0xcf, 0x00 \n" > + " .byte 0x00, 0x9b, 0xcf, 0x00 \n" > @@ -115,7 +118,7 @@ > - /* selgdt 0x28 16-bit 64k code at 0x00000000 */ > + /* selgdt 0x28 16-bit 64k code at 0x00000000 */ ..in the GDT too.. :\ > /* the VSA starts at the base of rom - 64 */ > - //rom = ((unsigned long) 0) - (ROM_SIZE + 64*1024); > - > - rom = 0xfffc8000; > + //rom = ((unsigned long) 0) - (ROM_SIZE + 64*1024); > + > + //rom = 0xfffc8000; > > + //VSA is cat onto the end after LB builds > + rom = ((unsigned long) 0) - (ROM_SIZE + 36 * 1024); This hardcoded size doesn't look so nice, especially if the VSA blob will get hacked on. And, if code is dead then please just delete it instead of commenting out the old assignment and adding a new one. > Index: LinuxBIOSv2/src/include/cpu/amd/lxdef.h > =================================================================== > --- LinuxBIOSv2.orig/src/include/cpu/amd/lxdef.h 2007-05-03 > 11:20:49.000000000 -0600 > +++ LinuxBIOSv2/src/include/cpu/amd/lxdef.h 2007-05-03 11:30:00.000000000 > -0600 > @@ -24,42 +24,21 @@ > > #ifndef CPU_AMD_LXDEF_H > #define CPU_AMD_LXDEF_H > -#define CPU_ID_1_X > 0x540 /* Stepping ID 1.x*/ > -#define CPU_ID_2_0 > 0x551 /* Stepping ID 2.0*/ > -#define CPU_ID_2_1 > 0x552 /* Stepping ID 2.1*/ > -#define CPU_ID_2_2 > 0x553 /* Stepping ID 2.2*/ [...] > + > +#define CPU_ID_1_X 0x00000560 > /* Stepping ID 1.x CPUbug fix to change it to 5A0*/ > +#define CPU_ID_2_0 0x000005A1 > +#define CPU_ID_3_0 0x000005A2 Apart from the massive whitespace changes also in this file, please comment on the code changes related to hardware revisions. If there are big changes between revs they should just be multiple CPU types.. Is it really nice to change the IDs? > -#define GL0_GLIU0 0 > -#define GL0_MC 1 > -#define GL0_GLIU1 2 > -#define GL0_CPU 3 > -#define GL0_VG 4 > -#define GL0_GP 5 > -//#define GL0_DF 6 //GX3 no such > thing as VP port > +#define GL0_GLIU0 0 > +#define GL0_MC 1 > +#define GL0_GLIU1 2 > +#define GL0_CPU 3 > +#define GL0_VG 4 > +#define GL0_GP 5 ... > Index: LinuxBIOSv2/src/include/cpu/amd/vr.h > =================================================================== > --- LinuxBIOSv2.orig/src/include/cpu/amd/vr.h 2007-05-03 11:20:49.000000000 > -0600 > +++ LinuxBIOSv2/src/include/cpu/amd/vr.h 2007-05-03 11:22:07.000000000 > -0600 Again full of whitespace changes. > - #define VSA_VERSION_NUM 0x00 > + #define VSA_VERSION_NUM 0x00 > #define HIGH_MEM_ACCESS 0x01 > - #define GET_VSM_INFO 0x02 // Used by INFO > - #define GET_BASICS 0x00 > - #define GET_EVENT 0x01 > - #define GET_STATISTICS 0x02 > - #define GET_HISTORY 0x03 > - #define GET_HARDWARE 0x04 > - #define GET_ERROR 0x05 > - #define SET_VSM_TYPE 0x06 > + #define GET_VSM_INFO 0x02 // Used by INFO > + #define GET_BASICS 0x00 > + #define GET_EVENT 0x01 > + #define GET_STATISTICS 0x02 > + #define GET_HISTORY 0x03 > + #define GET_HARDWARE 0x04 > + #define GET_ERROR 0x05 > + #define SET_VSM_TYPE 0x06 > #define SIGNATURE 0x03 > - #define VSA2_SIGNATURE 0x56534132 // 'VSA2' returned in > EAX > + #define VSA2_SIGNATURE 0x56534132 // 'VSA2' returned in > EAX So much noise my head actually hurts! Frankly, I think this patch makes AMD look bad and I don't want that in the repo. Should anyone ever diff over it they will curse long, hard and loud - at least I would. But, even if I feel strongly about this I don't feel very authoritative here. If others are fine with it I'll shut up, at least for this patchset. //Peter -- linuxbios mailing list [email protected] http://www.linuxbios.org/mailman/listinfo/linuxbios
