On Thu, Dec 18, 2008 at 11:08 AM, Marc Jones <[email protected]> wrote:
> On Thu, Dec 18, 2008 at 10:22 AM, Myles Watson <[email protected]> wrote: > > > > > > On Thu, Dec 18, 2008 at 7:52 AM, Jordan Crouse <[email protected] > > > > wrote: > >> > >> Myles Watson wrote: > >>> > >>> On Thu, Dec 18, 2008 at 3:18 AM, Carl-Daniel Hailfinger < > >>> [email protected]> wrote: > >>> > >>>> Bao, Zheng found a bug which killed SATA booting on my board. > >>>> > >>>> This happened because we do not error out on implicit function > >>>> declarations. The linker has no way of checking whether the implicitly > >>>> assumed function signature is identical to the real signature, so > >>>> mismatches can occur and these mismatches are practically impossible > to > >>>> debug because the code looks completely correct. > >>>> > >>>> Adding -Werror-implicit-function-declaration to our CFLAGS would solve > >>>> this problem nicely, but a lot of files in the tree need to be fixed. > >>>> > >>> > >>> I think this is a great idea. Isn't the correct order to fix all the > >>> warnings, then make it an error? > >> > >> Yeah - the unfortunate thing about changes like this is that you end up > >> being responsible for fixing the errors.. :) > > > > Here's my first patch. It clears up all of them except get_nodes for > > serengeti. > > > > coreboot/svn/src/cpu/amd/dualcore/dualcore.c:63: warning: implicit > > declaration of function 'get_nodes' > > > > The rest were easy. This one I'm not sure what was supposed to be here. > > > >> > >> Carl-Daniel - if you post a list of offending files, we'll all help > clear > >> them up. Dumping the log through grep "implicit declaration of > function" > >> should suffice. > > > > If you want to take the get_nodes reference, that would be great. If > this > > is the way its supposed to be cleaned up, I'll keep going a little more. > I > > think we should divide it up based on processor type so we don't > duplicate > > work. > > > > Signed-off-by: Myles Watson <[email protected]> > Acked-by: Marc Jones <[email protected]> Rev 3818. Thanks. > This is a much needed cleanup. Thanks Carl-Daniel and Myles for starting on > it. > I think that the get_node is a little tricky because it is used in > CAR and in main coreboot code? > It's also inlined some places but not others. I'm attaching a patch which I thought was trivial, but breaks things all over the place. I'm not sure how this will need to be done. It looks like a problem with romcc vs. CAR. Thanks, Myles
Index: svn/src/mainboard/supermicro/x6dai_g/reset.c =================================================================== --- svn.orig/src/mainboard/supermicro/x6dai_g/reset.c +++ svn/src/mainboard/supermicro/x6dai_g/reset.c @@ -1,6 +1,7 @@ #include <arch/io.h> #include <device/pci_def.h> #include <device/pci_ids.h> +#include <device/pci.h> #ifndef __ROMCC__ #include <device/device.h> #define PCI_ID(VENDOR_ID, DEVICE_ID) \ Index: svn/src/mainboard/supermicro/x6dhe_g/reset.c =================================================================== --- svn.orig/src/mainboard/supermicro/x6dhe_g/reset.c +++ svn/src/mainboard/supermicro/x6dhe_g/reset.c @@ -1,6 +1,7 @@ #include <arch/io.h> #include <device/pci_def.h> #include <device/pci_ids.h> +#include <device/pci.h> #ifndef __ROMCC__ #include <device/device.h> #define PCI_ID(VENDOR_ID, DEVICE_ID) \ Index: svn/src/mainboard/supermicro/x6dhe_g2/reset.c =================================================================== --- svn.orig/src/mainboard/supermicro/x6dhe_g2/reset.c +++ svn/src/mainboard/supermicro/x6dhe_g2/reset.c @@ -1,6 +1,7 @@ #include <arch/io.h> #include <device/pci_def.h> #include <device/pci_ids.h> +#include <device/pci.h> #ifndef __ROMCC__ #include <device/device.h> #define PCI_ID(VENDOR_ID, DEVICE_ID) \ Index: svn/src/mainboard/supermicro/x6dhr_ig/reset.c =================================================================== --- svn.orig/src/mainboard/supermicro/x6dhr_ig/reset.c +++ svn/src/mainboard/supermicro/x6dhr_ig/reset.c @@ -1,6 +1,7 @@ #include <arch/io.h> #include <device/pci_def.h> #include <device/pci_ids.h> +#include <device/pci.h> #ifndef __ROMCC__ #include <device/device.h> #define PCI_ID(VENDOR_ID, DEVICE_ID) \ Index: svn/src/mainboard/supermicro/x6dhr_ig2/reset.c =================================================================== --- svn.orig/src/mainboard/supermicro/x6dhr_ig2/reset.c +++ svn/src/mainboard/supermicro/x6dhr_ig2/reset.c @@ -1,6 +1,7 @@ #include <arch/io.h> #include <device/pci_def.h> #include <device/pci_ids.h> +#include <device/pci.h> #ifndef __ROMCC__ #include <device/device.h> #define PCI_ID(VENDOR_ID, DEVICE_ID) \ Index: svn/src/mainboard/dell/s1850/reset.c =================================================================== --- svn.orig/src/mainboard/dell/s1850/reset.c +++ svn/src/mainboard/dell/s1850/reset.c @@ -1,6 +1,7 @@ #include <arch/io.h> #include <device/pci_def.h> #include <device/pci_ids.h> +#include <device/pci.h> #ifndef __ROMCC__ #include <device/device.h> #define PCI_ID(VENDOR_ID, DEVICE_ID) \ Index: svn/src/mainboard/intel/jarrell/reset.c =================================================================== --- svn.orig/src/mainboard/intel/jarrell/reset.c +++ svn/src/mainboard/intel/jarrell/reset.c @@ -1,6 +1,7 @@ #include <arch/io.h> #include <device/pci_def.h> #include <device/pci_ids.h> +#include <device/pci.h> #ifndef __ROMCC__ #include <device/device.h> #define PCI_ID(VENDOR_ID, DEVICE_ID) \ Index: svn/src/northbridge/amd/amdk8/reset_test.c =================================================================== --- svn.orig/src/northbridge/amd/amdk8/reset_test.c +++ svn/src/northbridge/amd/amdk8/reset_test.c @@ -1,5 +1,6 @@ #include <stdint.h> #include <cpu/x86/lapic.h> +#include <device/pci.h> #define NODE_ID 0x60 #define HT_INIT_CONTROL 0x6c
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
