On 3/17/20 3:55 PM, James Bottomley wrote:
On Tue, 2020-03-17 at 15:43 +0100, Bartlomiej Zolnierkiewicz wrote:There is no reason to expose SATA_PMP config option when no SATA host drivers are enabled. To fix it add SATA_HOST config option, make all SATA host drivers select it and finally make SATA_PMP config options depend on it. This also serves as preparation for the future changes which optimize libata core code size on PATA only setups. CC: "James E.J. Bottomley" <jejb@xxxxxxxxxxxxx> Reviewed-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx> # for SCSI bits Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> --- drivers/ata/Kconfig | 40 +++++++++++++++++++++++++++++++++++++ drivers/scsi/Kconfig | 1 + drivers/scsi/libsas/Kconfig | 1 + 3 files changed, 42 insertions(+) diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index a6beb2c5a692..ad7760656f71 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -34,6 +34,9 @@ if ATA config ATA_NONSTANDARD bool +config SATA_HOST + bool + config ATA_VERBOSE_ERROR bool "Verbose ATA error reporting" default y @@ -73,6 +76,7 @@ config SATA_ZPODD config SATA_PMP bool "SATA Port Multiplier support" + depends on SATA_HOST default y help This option adds support for SATA Port Multipliers @@ -85,6 +89,7 @@ comment "Controllers with non-SFF native interface" config SATA_AHCI tristate "AHCI SATA support" depends on PCI + select SATA_HOST help This option enables support for AHCI Serial ATA.This is a bit fragile and not the way Kconfig should be done. The fragility comes because anyone adding a new host has also to remember to add the select, and there will be no real consequences for not doing
People adding the new host driver usually look at the existing Kconfig entries before introducing the new one (most probably just copy and then modify existing entry) so they should notice that the existing entries contain the select. Also we shouldn't have problem in catching potential select omissions during the code review. Not to mention that the addition of the new ATA host driver is quite a rare event nowadays.. ;)
so. The way to get rid of the fragility is to make SATA_HOST a menuconfig option enclosing all the hosts, which makes the patch much smaller as well. The hint implies you want to separate out all the PATA drivers, which also makes a menuconfig sound like the better option.
SATA_HOST is not for grouping SATA hosts, it is for core libata code to enable SATA support (please see patches #13-26 for details, maybe it should have been named ATA_SATA?) and menuconfig usage in this case is problematic: - we have host drivers supporting both SATA and PATA (i.e. ata_piix) - we have currently host drivers sorted in Kconfig by different criteria than SATA or PATA support - I would prefer to avoid making users to have explicitly select SATA_HOST option (right now it gets auto-selected when needed) - SCSI_SAS_ATA (which also needs to select SATA_HOST) lives in drivers/scsi/libsas/Kconfig so menuconfig will not cover it
I've also got to say that the problem doesn't seem to be one ... even if some raving lunatic disables all SATA hosts and then enables PMP it doesn't cause any problems does it?
It doesn't cause any real problems (just some dead code being present in the kernel image) but this patch is a prerequisite for patches #13-26 (patch description mentions that this change is needed for the future changes which optimize libata core code size on PATA only setups). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
