Re: [PATCH] libata: fix broken Kconfig setup

[Linus Torvalds <torvalds@osdl.org>]

On Mon, 17 Oct 2005, Jeff Garzik wrote:
> 
> CONFIG_SCSI_SATA is truly a boolean option, not a tristate.
> Since the Kconfig dependencies are insufficient to describe this (2.4
> had dep_mbool), we need to resort to 'if'.

Two problems:

 - this is ugly as hell

   First, you change SCSI_SATA to be boolean, then you change the 
   depends-on to be "if". Which makes no sense. Once SCSI_SATA is a 
   boolean, then the "depends on" works fine, since SCSI_SATA can no 
   longer be "m" anyway (ie your comment about "dep_mbool" doesn't make 
   sense).

   Second, if you want "dep_mbool", then you can have dep_mbool in Kconfig 
   too. Just do

	depends on (XYZ != n)

   which gets you what you want (ie if XYZ is "m", then the inequality 
   will be "y")

   So you might as well have just done something like

	 config SCSI_SATA
	-       tristate "Serial ATA (SATA) support"
	-       depends on SCSI
	+       bool "Serial ATA (SATA) support"
	+       depends on SCSI != n

   and then just added SCSI to the "depends on" lines to get the "m" 
   config. No "if" needed.

 - anything that depends on a module had better only be _inside_ that 
   module. Ie the "dep_mbool" kind of behaviour should _not_ affect 
   anything outside the module. The reason? Maybe you build the module 
   _later_, and maybe you don't ever load it.

   So it appears that your dependence on quirk_intel_ide_combined() is 
   fundamentally broken for the "m" case _anyway_.

Anyway, the second thing means that the whole configuration is somewhat 
broken, but on the whole, why not this _much_ more trivial patch?

It's still broken, exactly because it depends on the modules being defined 
when compiling the whole kernel, but hey, we probably have other cases 
like that.

My suggestion being that we should make it unconditional, but maybe that 
breaks the case where we don't actually load the SATA module?

		Linus

---
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 11ca443..cec631b 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1233,7 +1233,7 @@ static void __init quirk_alder_ioapic(st
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_EESSC,	quirk_alder_ioapic );
 #endif
 
-#ifdef CONFIG_SCSI_SATA
+#if defined(CONFIG_SCSI_SATA) || defined(CONFIG_SCSI_SATA_MODULE)
 static void __devinit quirk_intel_ide_combined(struct pci_dev *pdev)
 {
 	u8 prog, comb, tmp;
@@ -1310,7 +1310,7 @@ static void __devinit quirk_intel_ide_co
 		request_region(0x170, 8, "libata");	/* port 1 */
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,    PCI_ANY_ID,	  quirk_intel_ide_combined );
-#endif /* CONFIG_SCSI_SATA */
+#endif /* CONFIG_SCSI_SATA[_MODULE] */
 
 
 int pcie_mch_quirk;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/