| Submitter | Myles Watson |
|---|---|
| Date | 2010-03-17 21:33:09 |
| Message ID | <2831fecf1003171433j16908419u2eb5828be97c0946@mail.gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/1082/ |
| State | Superseded |
| Headers | show |
Comments
On 3/17/10 10:33 PM, Myles Watson wrote: > +#ifndef __ROMCC__ > +#include <lib.h> /* Prototypes */ > +#endif I think the ifndef __ROMCC__ should go into lib.h just as __PRE_RAM__ did.. Acked-by: Stefan Reinauer <stepan@coresystems.de>
On 3/17/10 10:33 PM, Myles Watson wrote: > -extern unsigned char AmlCode[]; > +extern void* AmlCode; > Is this really a void *? Did you test that this works?
On 3/17/10 11:11 PM, Stefan Reinauer wrote: > On 3/17/10 10:33 PM, Myles Watson wrote: > >> +#ifndef __ROMCC__ >> +#include <lib.h> /* Prototypes */ >> +#endif >> > I think the ifndef __ROMCC__ should go into lib.h just as __PRE_RAM__ did.. > > > Acked-by: Stefan Reinauer <stepan@coresystems.de> > Sorry, got to N-Ack again.... unsigned char AmlCode[] = { 0x44,0x53,0x44,0x54,0xE6,0x27,0x00,0x00, /* 00000000 "DSDT.'.." */ 0x02,0x7C,0x41,0x4D,0x44,0x20,0x20,0x20, /* 00000008 ".|AMD " */ 0x4D,0x41,0x48,0x4F,0x47,0x41,0x4E,0x59, /* 00000010 "MAHOGANY" */ .... } and in another file it looks like this: -extern unsigned char AmlCode[]; +extern void* AmlCode; So AmlCode is used as a void * pointing to the address 0x54445344 which is definitely not what you want... Even though it might appear that char *foo and char foo[] are the same in C, they're in fact not. Stefan
On Wed, Mar 17, 2010 at 4:16 PM, Stefan Reinauer <stepan@coresystems.de>wrote: > On 3/17/10 11:11 PM, Stefan Reinauer wrote: > > On 3/17/10 10:33 PM, Myles Watson wrote: > > > >> +#ifndef __ROMCC__ > >> +#include <lib.h> /* Prototypes */ > >> +#endif > >> > > I think the ifndef __ROMCC__ should go into lib.h just as __PRE_RAM__ > did.. > > > > > > Acked-by: Stefan Reinauer <stepan@coresystems.de> > > > Sorry, got to N-Ack again.... > > unsigned char AmlCode[] = > { > 0x44,0x53,0x44,0x54,0xE6,0x27,0x00,0x00, /* 00000000 "DSDT.'.." */ > 0x02,0x7C,0x41,0x4D,0x44,0x20,0x20,0x20, /* 00000008 ".|AMD " */ > 0x4D,0x41,0x48,0x4F,0x47,0x41,0x4E,0x59, /* 00000010 "MAHOGANY" */ > .... > } > > > and in another file it looks like this: > > -extern unsigned char AmlCode[]; > +extern void* AmlCode; > > > So AmlCode is used as a void * pointing to the address 0x54445344 > > which is definitely not what you want... > You're right. And that's definitely not what I expected to happen. It seems like there ought to be a correct way to do this. I'll keep looking. Thanks, Myles > > > Even though it might appear that char *foo and char foo[] are the same > in C, they're in fact not. > > > Stefan > > > -- > coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br. > Tel.: +49 761 7668825 • Fax: +49 761 7664613 > Email: info@coresystems.de • http://www.coresystems.de/ > Registergericht: Amtsgericht Freiburg • HRB 7656 > Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866 > > > -- > coreboot mailing list: coreboot@coreboot.org > http://www.coreboot.org/mailman/listinfo/coreboot >
On 3/18/10 12:05 AM, Myles Watson wrote: > > unsigned char AmlCode[] = > { > 0x44,0x53,0x44,0x54,0xE6,0x27,0x00,0x00, /* 00000000 > "DSDT.'.." */ > > and in another file it looks like this: > > -extern unsigned char AmlCode[]; > +extern void* AmlCode; > > > So AmlCode is used as a void * pointing to the address 0x54445344 > > which is definitely not what you want... > > > You're right. And that's definitely not what I expected to happen. > It seems like there ought to be a correct way to do this. I'll keep > looking. What is it, you want to do?
> >> -extern unsigned char AmlCode[]; >> +extern void* AmlCode; >> >> >> So AmlCode is used as a void * pointing to the address 0x54445344 >> >> which is definitely not what you want... >> > > You're right. And that's definitely not what I expected to happen. It > seems like there ought to be a correct way to do this. I'll keep looking. > > What is it, you want to do? > Get rid of the type punned warning that gcc gives for that code where AmlCode gets cast to acpi_header_t. Thanks, Myles
Hi Segher, you're probably able to solve this one easily. On 18.03.2010 00:45, Myles Watson wrote: >>> -extern unsigned char AmlCode[]; >>> +extern void* AmlCode; >>> >>> >>> So AmlCode is used as a void * pointing to the address 0x54445344 >>> >>> which is definitely not what you want... >>> >>> >> You're right. And that's definitely not what I expected to happen. It >> seems like there ought to be a correct way to do this. I'll keep looking. >> >> What is it, you want to do? >> >> > Get rid of the type punned warning that gcc gives for that code where > AmlCode gets cast to acpi_header_t. > Could __attribute__((may_alias)) help? Then again, a Google search turns up a few miscompiles with that attribute, so another option may be to run memcpy(AmlCode,AmlCode, ...). Regards, Carl-Daniel
On Wed, Mar 17, 2010 at 6:17 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote: > Hi Segher, > > you're probably able to solve this one easily. > > On 18.03.2010 00:45, Myles Watson wrote: > >>> -extern unsigned char AmlCode[]; > >>> +extern void* AmlCode; > >>> > >>> > >>> So AmlCode is used as a void * pointing to the address 0x54445344 > > >> What is it, you want to do? > >> > >> > > Get rid of the type punned warning that gcc gives for that code where > > AmlCode gets cast to acpi_header_t. > > > > Could __attribute__((may_alias)) help? > I'd rather not use an attribute if we can just use a cast. It seems to work to have an intermediate void*: void_ptr = &AmlCode_ssdt; current += ((acpi_header_t *)void_ptr)->length; It compiles without warning and is functionally correct, but it may be too ugly. I'm still surprised that it needs to be &AmlCode_ssdt. I really expected Amlcode_ssdt to be a cast-able pointer. Thanks, Myles
Myles Watson wrote: > > > On Wed, Mar 17, 2010 at 6:17 PM, Carl-Daniel Hailfinger > <c-d.hailfinger.devel.2006@gmx.net > <mailto:c-d.hailfinger.devel.2006@gmx.net>> wrote: > > Hi Segher, > > you're probably able to solve this one easily. > > On 18.03.2010 00:45, Myles Watson wrote: > >>> -extern unsigned char AmlCode[]; > >>> +extern void* AmlCode; > >>> > >>> > >>> So AmlCode is used as a void * pointing to the address 0x54445344 > > >> What is it, you want to do? > >> > >> > > Get rid of the type punned warning that gcc gives for that code where > > AmlCode gets cast to acpi_header_t. > > > > Could __attribute__((may_alias)) help? > > I'd rather not use an attribute if we can just use a cast. > > It seems to work to have an intermediate void*: > > void_ptr = &AmlCode_ssdt; > current += ((acpi_header_t *)void_ptr)->length; > > It compiles without warning and is functionally correct, but it may be > too ugly. I'm still surprised that it needs to be &AmlCode_ssdt. I > really expected Amlcode_ssdt to be a cast-able pointer. If you have this in one C file: static const char AmlCode[] = {0,1,2,3,4,5,6,7}; You can then have this in another C file, which is what you want: struct AmlCode_s { /* stuff ? */ int a, b; }; extern const struct AmlCode_s AmlCode; This will mean that you've got wrong debug info, of course, but it is what you seem to want. The first file defines a symbol, and the second files says that a symbol is defined somewhere else. The types don't match, but the C compiler doesn't know / care about that. (Image what would happen if you were to replace the first file with assembler.) I'm not sure I like this solution. MM
Mark Marshall wrote: > If you have this in one C file: > > static const char AmlCode[] = {0,1,2,3,4,5,6,7}; const char AmlCode[] = {0,1,2,3,4,5,6,7}; Doh!
On 3/18/10 11:35 AM, Mark Marshall wrote: > > > This will mean that you've got wrong debug info, of course, but it is > what you seem to want. The first file defines a symbol, and the second > files says that a symbol is defined somewhere else. The types don't > match, but the C compiler doesn't know / care about that. Since the type punning issue is about wrong alignment, I think the C compiler does not do the right thing in that case, we just trick it around the warning. Stefan
> > This will mean that you've got wrong debug info, of course, but it is > > what you seem to want. The first file defines a symbol, and the second > > files says that a symbol is defined somewhere else. The types don't > > match, but the C compiler doesn't know / care about that. > Since the type punning issue is about wrong alignment, I think the C > compiler does not do the right thing in that case, we just trick it > around the warning. So it seems like we should make sure the alignment is correct and then trick it around the warning, right? What's the best way to make sure that a u8 array is aligned on a four-byte boundary? Thanks, Myles
On 18.03.2010 15:05, Stefan Reinauer wrote: > On 3/18/10 11:35 AM, Mark Marshall wrote: > >> This will mean that you've got wrong debug info, of course, but it is >> what you seem to want. The first file defines a symbol, and the second >> files says that a symbol is defined somewhere else. The types don't >> match, but the C compiler doesn't know / care about that. >> > Since the type punning issue is about wrong alignment, I think the C > compiler does not do the right thing in that case, we just trick it > around the warning. > AFAIK (and I could be wrong) type punning is not only about alignment, but the C compiler is free to optimize the generated code in a way that may break your expectations if you're violating strict aliasing rules. Regards, Carl-Daniel
On 3/18/10 3:47 PM, Carl-Daniel Hailfinger wrote: > On 18.03.2010 15:05, Stefan Reinauer wrote: > >> On 3/18/10 11:35 AM, Mark Marshall wrote: >> >> >>> This will mean that you've got wrong debug info, of course, but it is >>> what you seem to want. The first file defines a symbol, and the second >>> files says that a symbol is defined somewhere else. The types don't >>> match, but the C compiler doesn't know / care about that. >>> >>> >> Since the type punning issue is about wrong alignment, I think the C >> compiler does not do the right thing in that case, we just trick it >> around the warning. >> >> > AFAIK (and I could be wrong) type punning is not only about alignment, > but the C compiler is free to optimize the generated code in a way that > may break your expectations if you're violating strict aliasing rules. > > That's what I read over and over, too but I can't make much sense of it... what does it mean? What are we not supposed to expect? What is it about, more than alignment?
On Sun, Mar 21, 2010 at 01:04:44AM +0100, Stefan Reinauer wrote: > On 3/18/10 3:47 PM, Carl-Daniel Hailfinger wrote: > > AFAIK (and I could be wrong) type punning is not only about alignment, > > but the C compiler is free to optimize the generated code in a way that > > may break your expectations if you're violating strict aliasing rules. > > > That's what I read over and over, too but I can't make much sense of > it... what does it mean? What are we not supposed to expect? What is it > about, more than alignment? It's about pointer aliases. Let's say one had code like: void myfunc(u16 *s, u32 *l) { printf("%d", *l); *s += 1; *l += 1; printf("%d", *l); } Then gcc is free to assume that *s and *l don't point to the same memory location. Thus, myfunc((u16*)&myint, &myint) could give unexpected results. The gcc warnings point out risky looking casts, but it is not exhaustive. Fixing all the warnings will not prevent all aliasing issues. Also, I didn't think alignment made any real impact on embedded x86 - as I didn't think gcc would emit any code that required alignment. -Kevin
On 3/21/10 4:57 PM, Kevin O'Connor wrote: > It's about pointer aliases. Let's say one had code like: > > void myfunc(u16 *s, u32 *l) { > printf("%d", *l); > *s += 1; > *l += 1; > printf("%d", *l); > } > > Then gcc is free to assume that *s and *l don't point to the same > memory location. Very obviously they don't. But why would that prevent us from casting a char [] to struct acpi_hdr? Stefan
On Sun, Mar 21, 2010 at 05:27:31PM +0100, Stefan Reinauer wrote: > On 3/21/10 4:57 PM, Kevin O'Connor wrote: > > It's about pointer aliases. Let's say one had code like: > > > > void myfunc(u16 *s, u32 *l) { > > printf("%d", *l); > > *s += 1; > > *l += 1; > > printf("%d", *l); > > } > > > > Then gcc is free to assume that *s and *l don't point to the same > > memory location. > Very obviously they don't. But why would that prevent us from casting a > char [] to struct acpi_hdr? The cast was from an "unsigned char []" - which is actually important as "char *" is treated specially while "unsigned char *" is not. Otherwise, the compiler didn't stop the cast - it's just generating a warning that it's dangerous. (That is, it's letting the user know that any memory accesses via the "unsigned char *" reference wont necessarily be seen from the "struct acpi_hdr *" reference and vice-versa.) Did I miss something? -Kevin
On 3/21/10 5:38 PM, Kevin O'Connor wrote: > Otherwise, the compiler didn't stop the cast - it's just generating a > warning that it's dangerous. (That is, it's letting the user know > that any memory accesses via the "unsigned char *" reference wont > necessarily be seen from the "struct acpi_hdr *" reference and > vice-versa.) > So making both (or even one of them) const should help?
On Sun, Mar 21, 2010 at 06:23:43PM +0100, Stefan Reinauer wrote: > On 3/21/10 5:38 PM, Kevin O'Connor wrote: > > Otherwise, the compiler didn't stop the cast - it's just generating a > > warning that it's dangerous. (That is, it's letting the user know > > that any memory accesses via the "unsigned char *" reference wont > > necessarily be seen from the "struct acpi_hdr *" reference and > > vice-versa.) > > > > So making both (or even one of them) const should help? I don't know. It's probably worth testing. A const pointer doesn't mean the memory can't change - it just means the memory can't be changed via that pointer - so, gcc might still warn. To be honest, I just turned off the warning in SeaBIOS (-Wno-strict-aliasing). Last time I looked, different versions of the compiler had different heuristics for warning, and I couldn't see anything that I was doing that was wrong. So, I'm probably not the best person to ask on how to prevent the warnings. :-) -Kevin
Patch
Index: svn/src/arch/i386/lib/console.c =================================================================== --- svn.orig/src/arch/i386/lib/console.c +++ svn/src/arch/i386/lib/console.c @@ -4,6 +4,7 @@ #if CONFIG_USE_PRINTK_IN_CAR == 0 #include "console_print.c" #else /* CONFIG_USE_PRINTK_IN_CAR == 1 */ +#include <console/console.h> #include "console_printk.c" #endif /* CONFIG_USE_PRINTK_IN_CAR */ Index: svn/src/mainboard/tyan/s2895/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/tyan/s2895/acpi_tables.c +++ svn/src/mainboard/tyan/s2895/acpi_tables.c @@ -20,7 +20,7 @@ #include <../../../northbridge/amd/amdk8/amdk8_acpi.h> #include <cpu/amd/model_fxx_powernow.h> -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/mainboard/tyan/s2895/romstage.c =================================================================== --- svn.orig/src/mainboard/tyan/s2895/romstage.c +++ svn/src/mainboard/tyan/s2895/romstage.c @@ -149,7 +149,7 @@ static void sio_setup(void) } -void failover_process(unsigned long bist, unsigned long cpu_init_detectedx) +static void failover_process(unsigned long bist, unsigned long cpu_init_detectedx) { unsigned last_boot_normal_x = last_boot_normal(); @@ -204,6 +204,7 @@ void failover_process(unsigned long bist #endif void real_main(unsigned long bist, unsigned long cpu_init_detectedx); +void cache_as_ram_main(unsigned long bist, unsigned long cpu_init_detectedx); void cache_as_ram_main(unsigned long bist, unsigned long cpu_init_detectedx) { Index: svn/src/include/lib.h =================================================================== --- svn.orig/src/include/lib.h +++ svn/src/include/lib.h @@ -19,8 +19,13 @@ /* This file is for "nuisance prototypes" that have no other home. */ +#ifndef __LIB_H__ +#define __LIB_H__ + +#ifndef __PRE_RAM__ /* Conflicts with romcc_io.h */ /* Defined in src/lib/clog2.c */ unsigned long log2(unsigned long x); +#endif /* Defined in src/lib/lzma.c */ unsigned long ulzma(unsigned char *src, unsigned char *dst); @@ -28,3 +33,10 @@ unsigned long ulzma(unsigned char *src, /* Defined in src/arch/i386/boot/gdt.c */ void move_gdt(void); +/* Defined in src/lib/ramtest.c */ +void ram_check(unsigned long start, unsigned long stop); + +/* Defined in src/pc80/serial.c */ +void uart_init(void); + +#endif /* __LIB_H__ */ Index: svn/src/lib/ramtest.c =================================================================== --- svn.orig/src/lib/ramtest.c +++ svn/src/lib/ramtest.c @@ -1,3 +1,7 @@ +#ifndef __ROMCC__ +#include <lib.h> /* Prototypes */ +#endif + static void write_phys(unsigned long addr, unsigned long value) { // Assembler in lib/ is very ugly. But we properly guarded Index: svn/src/pc80/serial.c =================================================================== --- svn.orig/src/pc80/serial.c +++ svn/src/pc80/serial.c @@ -1,3 +1,7 @@ +#ifndef __ROMCC__ +#include <lib.h> /* Prototypes */ +#endif + /* Base Address */ #ifndef CONFIG_TTYS0_BASE #define CONFIG_TTYS0_BASE 0x3f8 Index: svn/src/lib/generic_sdram.c =================================================================== --- svn.orig/src/lib/generic_sdram.c +++ svn/src/lib/generic_sdram.c @@ -1,3 +1,6 @@ +#ifndef __ROMCC__ +#include <lib.h> /* Prototypes */ +#endif #ifndef RAMINIT_SYSINFO #define RAMINIT_SYSINFO 0 @@ -12,14 +15,6 @@ static inline void print_debug_sdram_8(c #endif } -void sdram_no_memory(void) -{ - print_err("No memory!!\r\n"); - while(1) { - hlt(); - } -} - /* Setup SDRAM */ #if RAMINIT_SYSINFO == 1 void sdram_initialize(int controllers, const struct mem_controller *ctrl, void *sysinfo) Index: svn/src/mainboard/amd/serengeti_cheetah/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/amd/serengeti_cheetah/acpi_tables.c +++ svn/src/mainboard/amd/serengeti_cheetah/acpi_tables.c @@ -38,11 +38,11 @@ static void dump_mem(unsigned start, uns } #endif -extern unsigned char AmlCode[]; +extern void* AmlCode; #if CONFIG_ACPI_SSDTX_NUM >= 1 -extern unsigned char AmlCode_ssdt2[]; -extern unsigned char AmlCode_ssdt3[]; -extern unsigned char AmlCode_ssdt4[]; +extern void* AmlCode_ssdt2; +extern void* AmlCode_ssdt3; +extern void* AmlCode_ssdt4; #endif #define IO_APIC_ADDR 0xfec00000UL Index: svn/src/mainboard/asus/a8v-e_se/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/asus/a8v-e_se/acpi_tables.c +++ svn/src/mainboard/asus/a8v-e_se/acpi_tables.c @@ -31,7 +31,7 @@ #include <../../../southbridge/via/vt8237r/vt8237r.h> #include <../../../southbridge/via/k8t890/k8t890.h> -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/mainboard/asus/m2v-mx_se/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/asus/m2v-mx_se/acpi_tables.c +++ svn/src/mainboard/asus/m2v-mx_se/acpi_tables.c @@ -33,7 +33,7 @@ #include <../../../northbridge/amd/amdk8/amdk8_acpi.h> #include <cpu/amd/model_fxx_powernow.h> -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/mainboard/gigabyte/m57sli/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/gigabyte/m57sli/acpi_tables.c +++ svn/src/mainboard/gigabyte/m57sli/acpi_tables.c @@ -34,7 +34,7 @@ #include <device/pci.h> #include <cpu/amd/amdk8_sysconf.h> -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/mainboard/intel/d945gclf/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/intel/d945gclf/acpi_tables.c +++ svn/src/mainboard/intel/d945gclf/acpi_tables.c @@ -31,7 +31,7 @@ #define OLD_ACPI 0 -extern unsigned char AmlCode[]; +extern void* AmlCode; #if CONFIG_HAVE_ACPI_SLIC unsigned long acpi_create_slic(unsigned long current); #endif Index: svn/src/mainboard/intel/eagleheights/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/intel/eagleheights/acpi_tables.c +++ svn/src/mainboard/intel/eagleheights/acpi_tables.c @@ -28,7 +28,7 @@ #include <device/pci_ids.h> #include "ioapic.h" -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/mainboard/iwill/dk8_htx/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/iwill/dk8_htx/acpi_tables.c +++ svn/src/mainboard/iwill/dk8_htx/acpi_tables.c @@ -38,13 +38,13 @@ static void dump_mem(unsigned start, uns } #endif -extern unsigned char AmlCode[]; +extern void* AmlCode; #if CONFIG_ACPI_SSDTX_NUM >= 1 -extern unsigned char AmlCode_ssdt2[]; -extern unsigned char AmlCode_ssdt3[]; -extern unsigned char AmlCode_ssdt4[]; -extern unsigned char AmlCode_ssdt5[]; +extern void* AmlCode_ssdt2; +extern void* AmlCode_ssdt3; +extern void* AmlCode_ssdt4; +extern void* AmlCode_ssdt5; #endif #define IO_APIC_ADDR 0xfec00000UL Index: svn/src/mainboard/kontron/986lcd-m/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/kontron/986lcd-m/acpi_tables.c +++ svn/src/mainboard/kontron/986lcd-m/acpi_tables.c @@ -29,7 +29,7 @@ #include <cpu/x86/msr.h> #include "dmi.h" -extern unsigned char AmlCode[]; +extern void* AmlCode; #if CONFIG_HAVE_ACPI_SLIC unsigned long acpi_create_slic(unsigned long current); #endif Index: svn/src/mainboard/msi/ms9652_fam10/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/msi/ms9652_fam10/acpi_tables.c +++ svn/src/mainboard/msi/ms9652_fam10/acpi_tables.c @@ -35,7 +35,7 @@ #include <cpu/amd/amdfam10_sysconf.h> #include "mb_sysconf.h" -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/mainboard/roda/rk886ex/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/roda/rk886ex/acpi_tables.c +++ svn/src/mainboard/roda/rk886ex/acpi_tables.c @@ -30,7 +30,7 @@ #include <device/pci_ids.h> #include "dmi.h" -extern unsigned char AmlCode[]; +extern void* AmlCode; #if CONFIG_HAVE_ACPI_SLIC unsigned long acpi_create_slic(unsigned long current); #endif Index: svn/src/mainboard/tyan/s2891/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/tyan/s2891/acpi_tables.c +++ svn/src/mainboard/tyan/s2891/acpi_tables.c @@ -20,7 +20,7 @@ #include <../../../northbridge/amd/amdk8/amdk8_acpi.h> #include <cpu/amd/model_fxx_powernow.h> -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/mainboard/tyan/s2892/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/tyan/s2892/acpi_tables.c +++ svn/src/mainboard/tyan/s2892/acpi_tables.c @@ -20,7 +20,7 @@ #include <../../../northbridge/amd/amdk8/amdk8_acpi.h> #include <cpu/amd/model_fxx_powernow.h> -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/mainboard/via/epia-m/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/via/epia-m/acpi_tables.c +++ svn/src/mainboard/via/epia-m/acpi_tables.c @@ -11,7 +11,7 @@ #include <string.h> #include <arch/acpi.h> -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/mainboard/via/epia-m700/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/via/epia-m700/acpi_tables.c +++ svn/src/mainboard/via/epia-m700/acpi_tables.c @@ -35,8 +35,8 @@ #include <device/pci_ids.h> #include <../../../northbridge/via/vx800/vx800.h> -extern unsigned char AmlCode_dsdt[]; -extern unsigned char AmlCode_ssdt[]; +extern void* AmlCode_dsdt; +extern void* AmlCode_ssdt; extern u32 wake_vec; Index: svn/src/mainboard/via/epia-n/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/via/epia-n/acpi_tables.c +++ svn/src/mainboard/via/epia-n/acpi_tables.c @@ -35,7 +35,7 @@ #include <device/pci_ids.h> #include "../../../southbridge/via/vt8237r/vt8237r.h" -extern unsigned char AmlCode[]; +extern void* AmlCode; /* * These four macros are copied from <arch/smp/mpspec.h>, I have to do this Index: svn/src/mainboard/via/vt8454c/acpi_tables.c =================================================================== --- svn.orig/src/mainboard/via/vt8454c/acpi_tables.c +++ svn/src/mainboard/via/vt8454c/acpi_tables.c @@ -27,7 +27,7 @@ #include <device/pci_ids.h> #include "dmi.h" -extern unsigned char AmlCode[]; +extern void* AmlCode; unsigned long acpi_fill_mcfg(unsigned long current) { Index: svn/src/northbridge/amd/amdfam10/amdfam10_acpi.c =================================================================== --- svn.orig/src/northbridge/amd/amdfam10/amdfam10_acpi.c +++ svn/src/northbridge/amd/amdfam10/amdfam10_acpi.c @@ -305,11 +305,11 @@ void update_sspr(void *sspr, u32 nodeid, } } -extern unsigned char AmlCode_sspr5[]; -extern unsigned char AmlCode_sspr4[]; -extern unsigned char AmlCode_sspr3[]; -extern unsigned char AmlCode_sspr2[]; -extern unsigned char AmlCode_sspr1[]; +extern void* AmlCode_sspr5; +extern void* AmlCode_sspr4; +extern void* AmlCode_sspr3; +extern void* AmlCode_sspr2; +extern void* AmlCode_sspr1; /* fixme: find one good way for different p_state_num */ unsigned long acpi_add_ssdt_pstates(acpi_rsdp_t *rsdp, unsigned long current)
1. gcc doesn't like a character array being cast to a struct. -extern unsigned char AmlCode[]; +extern void* AmlCode; 2. Include <console/console.h> if you're using PRINTK_IN_CAR (not ROMCC) 3. Add a few more "nuisance" prototypes. Signed-off-by: Myles Watson <mylesgw@gmail.com> I think it would be nice to add the number of warnings for each board on one of the build pages. It looks like there are many that could compile without any warnings, and some that look like they have a few too many. Here's the list after this patch: coreboot-builds/a-trend_atc-6220/make.log:2 coreboot-builds/a-trend_atc-6240/make.log:2 coreboot-builds/abit_be6-ii_v2_0/make.log:2 coreboot-builds/advantech_pcm-5820/make.log:76 coreboot-builds/amd_db800/make.log:111 coreboot-builds/amd_dbm690t/make.log:132 coreboot-builds/amd_mahogany/make.log:182 coreboot-builds/amd_mahogany_fam10/make.log:378 coreboot-builds/amd_norwich/make.log:110 coreboot-builds/amd_pistachio/make.log:127 coreboot-builds/amd_rumba/make.log:65 coreboot-builds/amd_serengeti_cheetah/make.log:74 coreboot-builds/amd_serengeti_cheetah_fam10/make.log:270 coreboot-builds/arima_hdama/make.log:49 coreboot-builds/artecgroup_dbe61/make.log:112 coreboot-builds/asi_mb_5blgp/make.log:2 coreboot-builds/asi_mb_5blmp/make.log:2 coreboot-builds/asus_a8n_e/make.log:54 coreboot-builds/asus_a8v-e_se/make.log:74 coreboot-builds/asus_m2v-mx_se/make.log:103 coreboot-builds/asus_mew-am/make.log:9 coreboot-builds/asus_mew-vm/make.log:10 coreboot-builds/asus_p2b-d/make.log:3 coreboot-builds/asus_p2b-ds/make.log:3 coreboot-builds/asus_p2b-f/make.log:2 coreboot-builds/asus_p2b-ls/make.log:2 coreboot-builds/asus_p2b/make.log:2 coreboot-builds/asus_p3b-f/make.log:2 coreboot-builds/axus_tc320/make.log:2 coreboot-builds/azza_pt-6ibd/make.log:2 coreboot-builds/bcom_winnet100/make.log:2 coreboot-builds/bcom_winnetp680/make.log:100 coreboot-builds/biostar_m6tba/make.log:2 coreboot-builds/broadcom_blast/make.log:78 coreboot-builds/compaq_deskpro_en_sff_p600/make.log:2 coreboot-builds/dell_s1850/make.log:5 coreboot-builds/digitallogic_adl855pc/make.log:10 coreboot-builds/digitallogic_msm586seg/make.log:12 coreboot-builds/digitallogic_msm800sev/make.log:111 coreboot-builds/eaglelion_5bcm/make.log:2 coreboot-builds/emulation_qemu-x86/make.log:0 coreboot-builds/gigabyte_ga-6bxc/make.log:2 coreboot-builds/gigabyte_ga_2761gxdk/make.log:128 coreboot-builds/gigabyte_m57sli/make.log:133 coreboot-builds/hp_dl145_g3/make.log:110 coreboot-builds/hp_e_vectra_p2706t/make.log:9 coreboot-builds/ibm_e325/make.log:48 coreboot-builds/ibm_e326/make.log:50 coreboot-builds/iei_juki-511p/make.log:2 coreboot-builds/iei_nova4899r/make.log:2 coreboot-builds/iei_pcisa-lx-800-r10/make.log:111 coreboot-builds/intel_d945gclf/make.log:24 coreboot-builds/intel_eagleheights/make.log:49 coreboot-builds/intel_jarrell/make.log:13 coreboot-builds/intel_mtarvon/make.log:6 coreboot-builds/intel_truxton/make.log:6 coreboot-builds/intel_xe7501devkit/make.log:15 coreboot-builds/iwill_dk8_htx/make.log:51 coreboot-builds/iwill_dk8s2/make.log:58 coreboot-builds/iwill_dk8x/make.log:53 coreboot-builds/jetway_j7f24/make.log:99 coreboot-builds/kontron_986lcd-m/make.log:15 coreboot-builds/kontron_kt690/make.log:131 coreboot-builds/lippert_frontrunner/make.log:42 coreboot-builds/lippert_roadrunner-lx/make.log:113 coreboot-builds/lippert_spacerunner-lx/make.log:114 coreboot-builds/mitac_6513wu/make.log:9 coreboot-builds/msi_ms6119/make.log:2 coreboot-builds/msi_ms6147/make.log:2 coreboot-builds/msi_ms6156/make.log:2 coreboot-builds/msi_ms6178/make.log:9 coreboot-builds/msi_ms7135/make.log:58 coreboot-builds/msi_ms7260/make.log:131 coreboot-builds/msi_ms9185/make.log:108 coreboot-builds/msi_ms9282/make.log:132 coreboot-builds/msi_ms9652_fam10/make.log:298 coreboot-builds/nec_powermate2000/make.log:9 coreboot-builds/newisys_khepri/make.log:48 coreboot-builds/nvidia_l1_2pvv/make.log:135 coreboot-builds/olpc_btest/make.log:72 coreboot-builds/olpc_rev_a/make.log:71 coreboot-builds/pcengines_alix1c/make.log:112 coreboot-builds/rca_rm4100/make.log:1 coreboot-builds/roda_rk886ex/make.log:25 coreboot-builds/soyo_sy-6ba-plus-iii/make.log:2 coreboot-builds/sunw_ultra40/make.log:59 coreboot-builds/supermicro_h8dme/make.log:126 coreboot-builds/supermicro_h8dmr/make.log:124 coreboot-builds/supermicro_h8dmr_fam10/make.log:293 coreboot-builds/supermicro_h8qme_fam10/make.log:293 coreboot-builds/supermicro_x6dai_g/make.log:13 coreboot-builds/supermicro_x6dhe_g/make.log:17 coreboot-builds/supermicro_x6dhe_g2/make.log:9 coreboot-builds/supermicro_x6dhr_ig/make.log:6 coreboot-builds/supermicro_x6dhr_ig2/make.log:6 coreboot-builds/technexion_tim5690/make.log:130 coreboot-builds/technexion_tim8690/make.log:130 coreboot-builds/technologic_ts5300/make.log:10 coreboot-builds/televideo_tc7020/make.log:2 coreboot-builds/thomson_ip1000/make.log:0 coreboot-builds/tyan_s1846/make.log:2 coreboot-builds/tyan_s2735/make.log:78 coreboot-builds/tyan_s2850/make.log:52 coreboot-builds/tyan_s2875/make.log:50 coreboot-builds/tyan_s2880/make.log:54 coreboot-builds/tyan_s2881/make.log:53 coreboot-builds/tyan_s2882/make.log:51 coreboot-builds/tyan_s2885/make.log:49 coreboot-builds/tyan_s2891/make.log:58 coreboot-builds/tyan_s2892/make.log:4 coreboot-builds/tyan_s2895/make.log:49 coreboot-builds/tyan_s2912/make.log:136 coreboot-builds/tyan_s2912_fam10/make.log:298 coreboot-builds/tyan_s4880/make.log:53 coreboot-builds/tyan_s4882/make.log:44 coreboot-builds/via_epia-cn/make.log:48 coreboot-builds/via_epia-m/make.log:135 coreboot-builds/via_epia-m700/make.log:204 coreboot-builds/via_epia-n/make.log:132 coreboot-builds/via_epia/make.log:62 coreboot-builds/via_pc2500e/make.log:45 coreboot-builds/via_vt8454c/make.log:60 coreboot-builds/winent_pl6064/make.log:110 Thanks, Myles