Re: [PATCH] libata: fix broken Kconfig setup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Linus Torvalds wrote:

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_.

CONFIG_SCSI_SATA does two things:
* Enables/disables the display of the SATA driver menu.
* Enables/disables the compiled-in PCI quirk.

Both of these are boolean, and have absolutely nothing to do with modules.

The original problem that caused me to merge the problematic patch (my fault for bad merge) was that Kconfig wouldn't allow SATA drivers to be built as modules if you did
	bool SCSI_SATA depends on SCSI
	tristate SCSI_SATA_driver depends on SCSI_SATA

That's why I used an 'if' in my patch. I suppose we could split it into two Kconfig options, one for the menu (CONFIG_SCSI_SATA) using 'if', and one specifically for the PCI quirk. Such a split would make it obvious that CONFIG_SCSI_SATA controls display of a menu, and nothing more.

Comments?

The entire situation is a hack, because we're fighting the always-built-in IDE driver for control of the hardware. IDE driver picks up the PATA+SATA device even though it isn't listed in drivers/ide/pci/piix.c PCI table, because it blindly probes at the IDE ports after exhausting other options.


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

Because it's fundamental a boolean, and has -zero- to do with modules. Encouraging people to think otherwise will just lead to more confusion.

	Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux