| Submitter | Myles Watson |
|---|---|
| Date | 2009-10-12 19:58:34 |
| Message ID | <2831fecf0910121258m2ed0be0of1301b15e299cb38@mail.gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/392/ |
| State | Accepted |
| Headers | show |
Comments
Am Montag, den 12.10.2009, 13:58 -0600 schrieb Myles Watson:
> Signed-off-by: Myles Watson <mylesgw@gmail.com>
+ -e "/^CONFIG_HAVE_MP_TABLE / d" \
+ -e "/^CONFIG_HAVE_PIRQ_TABLE / d" \
+ -e "/^CONFIG_HAVE_ACPI_TABLES / d" \
Why do you want to exclude these from comparison?
+ -e "/^CONFIG_IDE / d" \
that one is a curious case: it seems to be about the ide stream loader
thingy (which doesn't really exist anymore), but it seems to be
repurposed for some IDE drivers
In src/southbridge/intel/i82801ca/Makefile.inc:
driver-$(CONFIG_IDE) += i82801ca_ide.o
Maybe that should be changed, and CONFIG_IDE be ignored
Regards,
Patrick
> Am Montag, den 12.10.2009, 13:58 -0600 schrieb Myles Watson: > > Signed-off-by: Myles Watson <mylesgw@gmail.com> > > + -e "/^CONFIG_HAVE_MP_TABLE / d" \ > + -e "/^CONFIG_HAVE_PIRQ_TABLE / d" \ > + -e "/^CONFIG_HAVE_ACPI_TABLES / d" \ > Why do you want to exclude these from comparison? They are options, and they aren't selected automatically by kbuildall. Without the distinction between HAVE_ and GEN_ it would be an error to force boards to build all the tables they have. > + -e "/^CONFIG_IDE / d" \ > that one is a curious case: it seems to be about the ide stream loader > thingy (which doesn't really exist anymore), but it seems to be > repurposed for some IDE drivers > In src/southbridge/intel/i82801ca/Makefile.inc: > driver-$(CONFIG_IDE) += i82801ca_ide.o OK. I didn't see that. Thanks, Myles
On Tue, Oct 13, 2009 at 5:39 AM, Myles Watson <mylesgw@gmail.com> wrote: >> Am Montag, den 12.10.2009, 13:58 -0600 schrieb Myles Watson: >> > Signed-off-by: Myles Watson <mylesgw@gmail.com> >> >> + -e "/^CONFIG_HAVE_MP_TABLE / d" \ >> + -e "/^CONFIG_HAVE_PIRQ_TABLE / d" \ >> + -e "/^CONFIG_HAVE_ACPI_TABLES / d" \ >> Why do you want to exclude these from comparison? > They are options, and they aren't selected automatically by kbuildall. > Without the distinction between HAVE_ and GEN_ it would be an error to force > boards to build all the tables they have. > >> + -e "/^CONFIG_IDE / d" \ >> that one is a curious case: it seems to be about the ide stream loader >> thingy (which doesn't really exist anymore), but it seems to be >> repurposed for some IDE drivers >> In src/southbridge/intel/i82801ca/Makefile.inc: >> driver-$(CONFIG_IDE) += i82801ca_ide.o > OK. I didn't see that. we had problems in the past where a vendor would have a southbridge with IDE but add a different chip for IDE. So you had to know whether to enable the SB ide or the other IDE. I've seen this several times. CONFIG_IDE kind of fails for this purpose because there might be multiple parts on the board with IDE and you will end up compiling them all in. It's going to enable anything that has an IDE and has this conditional compilation, even if there is a conflict, I think. But we might need that i82801ca_ide code just so we can disable IDE! That happens too ... I think this should be a -y and I think CONFIG_IDE should die. If we need conditional inclusion of this code, why not CONFIG_SOUTHBRIDGE_INTEL_I82801ER_IDE? But CONFIG_IDE was really for the now-removed IDE streams, and it has been incorrectly used for a different purpose. ron
Patch
Index: svn/util/compareboard/compareboard =================================================================== --- svn.orig/util/compareboard/compareboard +++ svn/util/compareboard/compareboard @@ -30,21 +30,29 @@ sed \ -e "/^CONFIG_PRECOMPRESSED_PAYLOAD / d" \ -e "/^CONFIG_MULTIBOOT / d" \ -e "/^CONFIG_ARCH_POWERPC / d" \ + -e "/^CONFIG_EXCEPTION_VECTORS / d" \ -e "/^CONFIG_RESET / d" \ -e "/^CONFIG_ROM_PAYLOAD / d" \ -e "/^CONFIG_ROM_SECTION_/ d" \ -e "/^CONFIG_UNCOMPRESSED / d" \ -e "/^CONFIG_COMPRESS / d" \ -e "/^CONFIG_COMPRESSED_PAYLOAD_LZMA / d" \ + -e "/^CONFIG_COMPRESSED_PAYLOAD_NRV2B / d" \ + -e "/^CONFIG_HAVE_MP_TABLE / d" \ + -e "/^CONFIG_HAVE_PIRQ_TABLE / d" \ + -e "/^CONFIG_HAVE_ACPI_TABLES / d" \ -e "/^CONFIG_ASSEMBLER_DEBUG / d" \ -e "/^CONFIG_HAVE_FAILOVER_BOOT / d" \ -e "/^CONFIG_FAILOVER_SIZE / d" \ -e "/^CONFIG_FALLBACK_SIZE / d" \ -e "/^CONFIG_ROMBASE / d" \ -e "/^CONFIG_ROM_IMAGE_SIZE / d" \ + -e "/^CONFIG_ROM_SIZE/ d" \ -e "/^CONFIG_STACK_SIZE / d" \ -e "/^CONFIG_IDE_BOOT_DRIVE / d" \ -e "/^CONFIG_IDE_OFFSET / d" \ + -e "/^CONFIG_IDE / d" \ + -e "/^CONFIG_PCI_OPTION_ROM_RUN.* / d" \ -e "/^CONFIG_GDB_STUB / d" \ $A/old > $A/old.filtered sed \ @@ -67,8 +75,14 @@ sed \ -e "/^CONFIG_CPU_[A-Z]*_SOCKET_/ d" \ -e "/^CONFIG_ROMBASE / d" \ -e "/^CONFIG_ROM_IMAGE_SIZE / d" \ + -e "/^CONFIG_ROM_SIZE/ d" \ + -e "/^CONFIG_HAVE_MP_TABLE / d" \ + -e "/^CONFIG_HAVE_PIRQ_TABLE / d" \ + -e "/^CONFIG_HAVE_ACPI_TABLES / d" \ -e "/^CONFIG_STACK_SIZE / d" \ -e "/^CONFIG_GDB_STUB / d" \ + -e "/^CONFIG_VGA_BIOS / d" \ + -e "/^CONFIG_PCI_OPTION_ROM_RUN.* / d" \ $A/new > $A/new.filtered normalize $A/old.filtered > $A/old.normalized
Signed-off-by: Myles Watson <mylesgw@gmail.com> Thanks, Myles