Subject: [PATCH v4 03/27] ata: make SATA_PMP option selectable only if any SATA host driver is enabled

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" <[email protected]>
Reviewed-by: Martin K. Petersen <[email protected]> # for SCSI bits
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
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.

@@ -111,6 +116,7 @@ config SATA_MOBILE_LPM_POLICY

config SATA_AHCI_PLATFORM
tristate "Platform AHCI SATA support"
+ select SATA_HOST
help
This option enables support for Platform AHCI Serial ATA
controllers.
@@ -121,6 +127,7 @@ config AHCI_BRCM
tristate "Broadcom AHCI SATA support"
depends on ARCH_BRCMSTB || BMIPS_GENERIC || ARCH_BCM_NSP || \
ARCH_BCM_63XX
+ select SATA_HOST
help
This option enables support for the AHCI SATA3 controller found on
Broadcom SoC's.
@@ -130,6 +137,7 @@ config AHCI_BRCM
config AHCI_DA850
tristate "DaVinci DA850 AHCI SATA support"
depends on ARCH_DAVINCI_DA850
+ select SATA_HOST
help
This option enables support for the DaVinci DA850 SoC's
onboard AHCI SATA.
@@ -139,6 +147,7 @@ config AHCI_DA850
config AHCI_DM816
tristate "DaVinci DM816 AHCI SATA support"
depends on ARCH_OMAP2PLUS
+ select SATA_HOST
help
This option enables support for the DaVinci DM816 SoC's
onboard AHCI SATA controller.
@@ -148,6 +157,7 @@ config AHCI_DM816
config AHCI_ST
tristate "ST AHCI SATA support"
depends on ARCH_STI
+ select SATA_HOST
help
This option enables support for ST AHCI SATA controller.

@@ -157,6 +167,7 @@ config AHCI_IMX
tristate "Freescale i.MX AHCI SATA support"
depends on MFD_SYSCON && (ARCH_MXC || COMPILE_TEST)
depends on (HWMON && (THERMAL || !THERMAL_OF)) || !HWMON
+ select SATA_HOST
help
This option enables support for the Freescale i.MX SoC's
onboard AHCI SATA.
@@ -166,6 +177,7 @@ config AHCI_IMX
config AHCI_CEVA
tristate "CEVA AHCI SATA support"
depends on OF
+ select SATA_HOST
help
This option enables support for the CEVA AHCI SATA.
It can be found on the Xilinx Zynq UltraScale+ MPSoC.
@@ -176,6 +188,7 @@ config AHCI_MTK
tristate "MediaTek AHCI SATA support"
depends on ARCH_MEDIATEK
select MFD_SYSCON
+ select SATA_HOST
help
This option enables support for the MediaTek SoC's
onboard AHCI SATA controller.
@@ -185,6 +198,7 @@ config AHCI_MTK
config AHCI_MVEBU
tristate "Marvell EBU AHCI SATA support"
depends on ARCH_MVEBU
+ select SATA_HOST
help
This option enables support for the Marvebu EBU SoC's
onboard AHCI SATA.
@@ -203,6 +217,7 @@ config AHCI_OCTEON
config AHCI_SUNXI
tristate "Allwinner sunxi AHCI SATA support"
depends on ARCH_SUNXI
+ select SATA_HOST
help
This option enables support for the Allwinner sunxi SoC's
onboard AHCI SATA.
@@ -212,6 +227,7 @@ config AHCI_SUNXI
config AHCI_TEGRA
tristate "NVIDIA Tegra AHCI SATA support"
depends on ARCH_TEGRA
+ select SATA_HOST
help
This option enables support for the NVIDIA Tegra SoC's
onboard AHCI SATA.
@@ -221,12 +237,14 @@ config AHCI_TEGRA
config AHCI_XGENE
tristate "APM X-Gene 6.0Gbps AHCI SATA host controller support"
depends on PHY_XGENE
+ select SATA_HOST
help
This option enables support for APM X-Gene SoC SATA host controller.

config AHCI_QORIQ
tristate "Freescale QorIQ AHCI SATA support"
depends on OF
+ select SATA_HOST
help
This option enables support for the Freescale QorIQ AHCI SoC's
onboard AHCI SATA.
@@ -236,6 +254,7 @@ config AHCI_QORIQ
config SATA_FSL
tristate "Freescale 3.0Gbps SATA support"
depends on FSL_SOC
+ select SATA_HOST
help
This option enables support for Freescale 3.0Gbps SATA controller.
It can be found on MPC837x and MPC8315.
@@ -245,6 +264,7 @@ config SATA_FSL
config SATA_GEMINI
tristate "Gemini SATA bridge support"
depends on ARCH_GEMINI || COMPILE_TEST
+ select SATA_HOST
default ARCH_GEMINI
help
This enabled support for the FTIDE010 to SATA bridge
@@ -255,6 +275,7 @@ config SATA_GEMINI
config SATA_AHCI_SEATTLE
tristate "AMD Seattle 6.0Gbps AHCI SATA host controller support"
depends on ARCH_SEATTLE
+ select SATA_HOST
help
This option enables support for AMD Seattle SATA host controller.

@@ -263,12 +284,14 @@ config SATA_AHCI_SEATTLE
config SATA_INIC162X
tristate "Initio 162x SATA support (Very Experimental)"
depends on PCI
+ select SATA_HOST
help
This option enables support for Initio 162x Serial ATA.

config SATA_ACARD_AHCI
tristate "ACard AHCI variant (ATP 8620)"
depends on PCI
+ select SATA_HOST
help
This option enables support for Acard.

@@ -277,6 +300,7 @@ config SATA_ACARD_AHCI
config SATA_SIL24
tristate "Silicon Image 3124/3132 SATA support"
depends on PCI
+ select SATA_HOST
help
This option enables support for Silicon Image 3124/3132 Serial ATA.

@@ -326,6 +350,7 @@ config PATA_OCTEON_CF
config SATA_QSTOR
tristate "Pacific Digital SATA QStor support"
depends on PCI
+ select SATA_HOST
help
This option enables support for Pacific Digital Serial ATA QStor.

@@ -334,6 +359,7 @@ config SATA_QSTOR
config SATA_SX4
tristate "Promise SATA SX4 support (Experimental)"
depends on PCI
+ select SATA_HOST
help
This option enables support for Promise Serial ATA SX4.

@@ -357,6 +383,7 @@ comment "SATA SFF controllers with BMDMA"
config ATA_PIIX
tristate "Intel ESB, ICH, PIIX3, PIIX4 PATA/SATA support"
depends on PCI
+ select SATA_HOST
help
This option enables support for ICH5/6/7/8 Serial ATA
and support for PATA on the Intel ESB/ICH/PIIX3/PIIX4 series
@@ -368,6 +395,7 @@ config SATA_DWC
tristate "DesignWare Cores SATA support"
depends on DMADEVICES
select GENERIC_PHY
+ select SATA_HOST
help
This option enables support for the on-chip SATA controller of the
AppliedMicro processor 460EX.
@@ -398,6 +426,7 @@ config SATA_DWC_VDEBUG
config SATA_HIGHBANK
tristate "Calxeda Highbank SATA support"
depends on ARCH_HIGHBANK || COMPILE_TEST
+ select SATA_HOST
help
This option enables support for the Calxeda Highbank SoC's
onboard SATA.
@@ -409,6 +438,7 @@ config SATA_MV
depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
select GENERIC_PHY
+ select SATA_HOST
help
This option enables support for the Marvell Serial ATA family.
Currently supports 88SX[56]0[48][01] PCI(-X) chips,
@@ -419,6 +449,7 @@ config SATA_MV
config SATA_NV
tristate "NVIDIA SATA support"
depends on PCI
+ select SATA_HOST
help
This option enables support for NVIDIA Serial ATA.

@@ -427,6 +458,7 @@ config SATA_NV
config SATA_PROMISE
tristate "Promise SATA TX2/TX4 support"
depends on PCI
+ select SATA_HOST
help
This option enables support for Promise Serial ATA TX2/TX4.

@@ -435,6 +467,7 @@ config SATA_PROMISE
config SATA_RCAR
tristate "Renesas R-Car SATA support"
depends on ARCH_RENESAS || COMPILE_TEST
+ select SATA_HOST
help
This option enables support for Renesas R-Car Serial ATA.

@@ -443,6 +476,7 @@ config SATA_RCAR
config SATA_SIL
tristate "Silicon Image SATA support"
depends on PCI
+ select SATA_HOST
help
This option enables support for Silicon Image Serial ATA.

@@ -452,6 +486,7 @@ config SATA_SIS
tristate "SiS 964/965/966/180 SATA support"
depends on PCI
select PATA_SIS
+ select SATA_HOST
help
This option enables support for SiS Serial ATA on
SiS 964/965/966/180 and Parallel ATA on SiS 180.
@@ -462,6 +497,7 @@ config SATA_SIS
config SATA_SVW
tristate "ServerWorks Frodo / Apple K2 SATA support"
depends on PCI
+ select SATA_HOST
help
This option enables support for Broadcom/Serverworks/Apple K2
SATA support.
@@ -471,6 +507,7 @@ config SATA_SVW
config SATA_ULI
tristate "ULi Electronics SATA support"
depends on PCI
+ select SATA_HOST
help
This option enables support for ULi Electronics SATA.

@@ -479,6 +516,7 @@ config SATA_ULI
config SATA_VIA
tristate "VIA SATA support"
depends on PCI
+ select SATA_HOST
help
This option enables support for VIA Serial ATA.

@@ -487,6 +525,7 @@ config SATA_VIA
config SATA_VITESSE
tristate "VITESSE VSC-7174 / INTEL 31244 SATA support"
depends on PCI
+ select SATA_HOST
help
This option enables support for Vitesse VSC7174 and Intel 31244 Serial ATA.

@@ -1113,6 +1152,7 @@ config PATA_ACPI
config ATA_GENERIC
tristate "Generic ATA support"
depends on PCI && ATA_BMDMA
+ select SATA_HOST
help
This option enables support for generic BIOS configured
ATA controllers via the new ATA layer
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index b5be6f43ec3f..17feff174f57 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -980,6 +980,7 @@ config SCSI_SYM53C8XX_MMIO
config SCSI_IPR
tristate "IBM Power Linux RAID adapter support"
depends on PCI && SCSI && ATA
+ select SATA_HOST
select FW_LOADER
select IRQ_POLL
select SGL_ALLOC
diff --git a/drivers/scsi/libsas/Kconfig b/drivers/scsi/libsas/Kconfig
index 5c6a5eff2f8e..052ee3a26f6e 100644
--- a/drivers/scsi/libsas/Kconfig
+++ b/drivers/scsi/libsas/Kconfig
@@ -19,6 +19,7 @@ config SCSI_SAS_ATA
bool "ATA support for libsas (requires libata)"
depends on SCSI_SAS_LIBSAS
depends on ATA = y || ATA = SCSI_SAS_LIBSAS
+ select SATA_HOST
help
Builds in ATA support into libsas. Will necessitate
the loading of libata along with libsas.
--
2.24.1


2020-03-17 14:59:04

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH v4 03/27] ata: make SATA_PMP option selectable only if any SATA host driver is enabled

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" <[email protected]>
> Reviewed-by: Martin K. Petersen <[email protected]> # for
> SCSI bits
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> ---
> 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
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.

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?

James

Subject: Re: [PATCH v4 03/27] ata: make SATA_PMP option selectable only if any SATA host driver is enabled


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" <[email protected]>
>> Reviewed-by: Martin K. Petersen <[email protected]> # for
>> SCSI bits
>> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>> ---
>> 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

2020-03-26 09:46:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v4 03/27] ata: make SATA_PMP option selectable only if any SATA host driver is enabled

On Tue, Mar 17, 2020 at 03:43:09PM +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" <[email protected]>
> Reviewed-by: Martin K. Petersen <[email protected]> # for SCSI bits
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>

Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>