While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
be selectable for platforms using fsl_ifc with NOR flash.
Fixes: ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers")
To: Krzysztof Kozlowski <[email protected]>
To: Tudor Ambarus <[email protected]>
To: Pratyush Yadav <[email protected]>
To: Michael Walle <[email protected]>
To: Miquel Raynal <[email protected]>
To: Richard Weinberger <[email protected]>
To: Vignesh Raghavendra <[email protected]>
To: Michael Ellerman <[email protected]>
To: Nicholas Piggin <[email protected]>
To: Christophe Leroy <[email protected]>
To: Aneesh Kumar K.V <[email protected]>
To: Naveen N. Rao <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Esben Haabendal <[email protected]>
Changes in v2:
- CONFIG_MTD_NAND_FSL_IFC depends on CONFIG_FSL_IFC instead of select.
- Refresh powerpc config snippet accordingly.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Esben Haabendal (2):
memory: fsl_ifc: Make FSL_IFC config visible and selectable
powerpc/configs: Update defconfig with now user-visible CONFIG_FSL_IFC
arch/powerpc/configs/85xx-hw.config | 2 ++
drivers/memory/Kconfig | 2 +-
drivers/mtd/nand/raw/Kconfig | 3 +--
3 files changed, 4 insertions(+), 3 deletions(-)
---
base-commit: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6
change-id: 20240523-fsl-ifc-config-c877902b297e
Best regards,
--
Esben Haabendal <[email protected]>
With CONFIG_FSL_IFC now being user-visible, and thus changed from a select
to depends in CONFIG_MTD_NAND_FSL_IFC, the dependencies needs to be
selected in config snippets.
Signed-off-by: Esben Haabendal <[email protected]>
---
arch/powerpc/configs/85xx-hw.config | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/configs/85xx-hw.config b/arch/powerpc/configs/85xx-hw.config
index 524db76f47b7..8aff83217397 100644
--- a/arch/powerpc/configs/85xx-hw.config
+++ b/arch/powerpc/configs/85xx-hw.config
@@ -24,6 +24,7 @@ CONFIG_FS_ENET=y
CONFIG_FSL_CORENET_CF=y
CONFIG_FSL_DMA=y
CONFIG_FSL_HV_MANAGER=y
+CONFIG_FSL_IFC=y
CONFIG_FSL_PQ_MDIO=y
CONFIG_FSL_RIO=y
CONFIG_FSL_XGMAC_MDIO=y
@@ -58,6 +59,7 @@ CONFIG_INPUT_FF_MEMLESS=m
CONFIG_MARVELL_PHY=y
CONFIG_MDIO_BUS_MUX_GPIO=y
CONFIG_MDIO_BUS_MUX_MMIOREG=y
+CONFIG_MEMORY=y
CONFIG_MMC_SDHCI_OF_ESDHC=y
CONFIG_MMC_SDHCI_PLTFM=y
CONFIG_MMC_SDHCI=y
--
2.45.1
While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
be selectable for platforms using fsl_ifc with NOR flash.
Fixes: ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers")
Signed-off-by: Esben Haabendal <[email protected]>
---
drivers/memory/Kconfig | 2 +-
drivers/mtd/nand/raw/Kconfig | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 8efdd1f97139..c82d8d8a16ea 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -167,7 +167,7 @@ config FSL_CORENET_CF
represents a coherency violation.
config FSL_IFC
- bool "Freescale IFC driver" if COMPILE_TEST
+ bool "Freescale IFC driver"
depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
depends on HAS_IOMEM
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index cbf8ae85e1ae..614257308516 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -234,8 +234,7 @@ config MTD_NAND_FSL_IFC
tristate "Freescale IFC NAND controller"
depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
depends on HAS_IOMEM
- select FSL_IFC
- select MEMORY
+ depends on FSL_IFC
help
Various Freescale chips e.g P1010, include a NAND Flash machine
with built-in hardware ECC capabilities.
--
2.45.1
On 28/05/2024 14:28, Esben Haabendal wrote:
> With CONFIG_FSL_IFC now being user-visible, and thus changed from a select
> to depends in CONFIG_MTD_NAND_FSL_IFC, the dependencies needs to be
> selected in config snippets.
>
> Signed-off-by: Esben Haabendal <[email protected]>
> ---
> arch/powerpc/configs/85xx-hw.config | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/configs/85xx-hw.config b/arch/powerpc/configs/85xx-hw.config
> index 524db76f47b7..8aff83217397 100644
> --- a/arch/powerpc/configs/85xx-hw.config
> +++ b/arch/powerpc/configs/85xx-hw.config
> @@ -24,6 +24,7 @@ CONFIG_FS_ENET=y
> CONFIG_FSL_CORENET_CF=y
> CONFIG_FSL_DMA=y
> CONFIG_FSL_HV_MANAGER=y
> +CONFIG_FSL_IFC=y
Does not look like placed according to config order. This is not
alphabetically sorted, but as Kconfig creates it (make savedefconfig).
> CONFIG_FSL_PQ_MDIO=y
> CONFIG_FSL_RIO=y
You also missed to update second defconfig - arm64.
Best regards,
Krzysztof
Le 28/05/2024 à 14:28, Esben Haabendal a écrit :
> [Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>
> While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
> driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
> be selectable for platforms using fsl_ifc with NOR flash.
I don't understand.
Shall I understand :
While use of fsl_ifc driver with NAND flash is fine as the fsl_ifc_nand
driver selects FSL_IFC automatically, ....
or
..., as the fsl_ifc_nand driver selects FSL_IFC automatically we need
the CONFIG_FSL_IFC option to be selectable for platforms using fsl_ifc
with NOR flash
I'm fine with the fact that you want to be able to select it when you
use NOR flashes, allthough I can't see why, but why do you need to
change the "select" to a "depend" ? You should be able to leave it as a
"select" in which case patch 2 wouldn't be necessary.
Christophe
>
> Fixes: ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers")
> Signed-off-by: Esben Haabendal <[email protected]>
> ---
> drivers/memory/Kconfig | 2 +-
> drivers/mtd/nand/raw/Kconfig | 3 +--
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 8efdd1f97139..c82d8d8a16ea 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -167,7 +167,7 @@ config FSL_CORENET_CF
> represents a coherency violation.
>
> config FSL_IFC
> - bool "Freescale IFC driver" if COMPILE_TEST
> + bool "Freescale IFC driver"
> depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
> depends on HAS_IOMEM
>
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index cbf8ae85e1ae..614257308516 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -234,8 +234,7 @@ config MTD_NAND_FSL_IFC
> tristate "Freescale IFC NAND controller"
> depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
> depends on HAS_IOMEM
> - select FSL_IFC
> - select MEMORY
> + depends on FSL_IFC
> help
> Various Freescale chips e.g P1010, include a NAND Flash machine
> with built-in hardware ECC capabilities.
>
> --
> 2.45.1
>
On 28/05/2024 15:15, Christophe Leroy wrote:
>
>
> Le 28/05/2024 à 14:28, Esben Haabendal a écrit :
>> [Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>>
>> While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
>> driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
>> be selectable for platforms using fsl_ifc with NOR flash.
>
> I don't understand.
>
> Shall I understand :
>
> While use of fsl_ifc driver with NAND flash is fine as the fsl_ifc_nand
> driver selects FSL_IFC automatically, ....
>
> or
>
> ..., as the fsl_ifc_nand driver selects FSL_IFC automatically we need
> the CONFIG_FSL_IFC option to be selectable for platforms using fsl_ifc
> with NOR flash
>
>
>
> I'm fine with the fact that you want to be able to select it when you
> use NOR flashes,
But users are not fine... their memory won't work if they cannot choose
it (if you meant select=choose).
> allthough I can't see why, but why do you need to
> change the "select" to a "depend" ? You should be able to leave it as a
As explained in v1: user-visible symbols should not be selected. Nothing
odd here.
> "select" in which case patch 2 wouldn't be necessary.
Best regards,
Krzysztof
Krzysztof Kozlowski <[email protected]> writes:
> On 28/05/2024 14:28, Esben Haabendal wrote:
>> With CONFIG_FSL_IFC now being user-visible, and thus changed from a select
>> to depends in CONFIG_MTD_NAND_FSL_IFC, the dependencies needs to be
>> selected in config snippets.
>>
>> Signed-off-by: Esben Haabendal <[email protected]>
>> ---
>> arch/powerpc/configs/85xx-hw.config | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/configs/85xx-hw.config b/arch/powerpc/configs/85xx-hw.config
>> index 524db76f47b7..8aff83217397 100644
>> --- a/arch/powerpc/configs/85xx-hw.config
>> +++ b/arch/powerpc/configs/85xx-hw.config
>> @@ -24,6 +24,7 @@ CONFIG_FS_ENET=y
>> CONFIG_FSL_CORENET_CF=y
>> CONFIG_FSL_DMA=y
>> CONFIG_FSL_HV_MANAGER=y
>> +CONFIG_FSL_IFC=y
>
> Does not look like placed according to config order.
Correct.
> This is not alphabetically sorted, but as Kconfig creates it (make
> savedefconfig).
Are you sure about this?
It looks very much alphabetically sorted, with only two "errors"
$ diff -u 85xx-hw.config 85xx-hw.config.sorted
--- 85xx-hw.config 2024-05-28 15:05:44.665354428 +0200
+++ 85xx-hw.config.sorted 2024-05-28 15:05:56.102019081 +0200
@@ -15,8 +15,8 @@
CONFIG_DMADEVICES=y
CONFIG_E1000E=y
CONFIG_E1000=y
-CONFIG_EDAC=y
CONFIG_EDAC_MPC85XX=y
+CONFIG_EDAC=y
CONFIG_EEPROM_AT24=y
CONFIG_EEPROM_LEGACY=y
CONFIG_FB_FSL_DIU=y
@@ -71,10 +71,10 @@
CONFIG_MTD_CMDLINE_PARTS=y
CONFIG_MTD_NAND_FSL_ELBC=y
CONFIG_MTD_NAND_FSL_IFC=y
-CONFIG_MTD_RAW_NAND=y
CONFIG_MTD_PHYSMAP_OF=y
CONFIG_MTD_PHYSMAP=y
CONFIG_MTD_PLATRAM=y
+CONFIG_MTD_RAW_NAND=y
CONFIG_MTD_SPI_NOR=y
CONFIG_NETDEVICES=y
CONFIG_NVRAM=y
I don't think that this file has ever been Kconfig sorted since it was
created back in ancient times.
And as it is merged with other config snippets using merge_into_defconfig
function. I have no idea how to use savedefconfig to maintain such a snippet.
It would require doing the reverse of the merge_into_defconfig.
>> CONFIG_FSL_PQ_MDIO=y
>> CONFIG_FSL_RIO=y
>
> You also missed to update second defconfig - arm64.
Argh. I thought I checked, and it did not need any changes. But it needs
to have CONFIG_FSL_IFC=y added.
I will add that for v3.
/Esben
Krzysztof Kozlowski <[email protected]> writes:
> On 28/05/2024 15:15, Christophe Leroy wrote:
>> Le 28/05/2024 à 14:28, Esben Haabendal a écrit :
>>> [Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
>>> driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
>>> be selectable for platforms using fsl_ifc with NOR flash.
>>
>> I don't understand.
>>
>> Shall I understand :
>>
>> While use of fsl_ifc driver with NAND flash is fine as the fsl_ifc_nand
>> driver selects FSL_IFC automatically, ....
>>
>> or
>>
>> ..., as the fsl_ifc_nand driver selects FSL_IFC automatically we need
>> the CONFIG_FSL_IFC option to be selectable for platforms using fsl_ifc
>> with NOR flash
>>
>>
>>
>> I'm fine with the fact that you want to be able to select it when you
>> use NOR flashes,
>
> But users are not fine... their memory won't work if they cannot choose
> it (if you meant select=choose).
Exactly. The IFC controller supports both NAND and NOR flashes. Since
commit ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers"),
it has not been possible to use IFC controller for NOR flashes without
selecting the IFC NAND driver (CONFIG_MTD_NAND_FSL_IFC), which is
obviously not how it is supposed to be.
/Esben
On 28/05/2024 15:33, Esben Haabendal wrote:
> Krzysztof Kozlowski <[email protected]> writes:
>
>> On 28/05/2024 14:28, Esben Haabendal wrote:
>>> With CONFIG_FSL_IFC now being user-visible, and thus changed from a select
>>> to depends in CONFIG_MTD_NAND_FSL_IFC, the dependencies needs to be
>>> selected in config snippets.
>>>
>>> Signed-off-by: Esben Haabendal <[email protected]>
>>> ---
>>> arch/powerpc/configs/85xx-hw.config | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/powerpc/configs/85xx-hw.config b/arch/powerpc/configs/85xx-hw.config
>>> index 524db76f47b7..8aff83217397 100644
>>> --- a/arch/powerpc/configs/85xx-hw.config
>>> +++ b/arch/powerpc/configs/85xx-hw.config
>>> @@ -24,6 +24,7 @@ CONFIG_FS_ENET=y
>>> CONFIG_FSL_CORENET_CF=y
>>> CONFIG_FSL_DMA=y
>>> CONFIG_FSL_HV_MANAGER=y
>>> +CONFIG_FSL_IFC=y
>>
>> Does not look like placed according to config order.
>
> Correct.
>
>> This is not alphabetically sorted, but as Kconfig creates it (make
>> savedefconfig).
>
> Are you sure about this?
>
> It looks very much alphabetically sorted, with only two "errors"
Uh, ok, in general this is savedefconfig-sorted, but of course some
platforms can choose other way. At least all arm and arm64 are
savedefconfig-sorted.
Follow whatever is there already, so in this case alphabetical.
..
>
>>> CONFIG_FSL_PQ_MDIO=y
>>> CONFIG_FSL_RIO=y
>>
>> You also missed to update second defconfig - arm64.
>
> Argh. I thought I checked, and it did not need any changes. But it needs
> to have CONFIG_FSL_IFC=y added.
>
> I will add that for v3.
>
> /Esben
Best regards,
Krzysztof
Esben Haabendal <[email protected]> writes:
> Krzysztof Kozlowski <[email protected]> writes:
>
>> On 28/05/2024 14:28, Esben Haabendal wrote:
>>> With CONFIG_FSL_IFC now being user-visible, and thus changed from a select
>>> to depends in CONFIG_MTD_NAND_FSL_IFC, the dependencies needs to be
>>> selected in config snippets.
>>>
>>> Signed-off-by: Esben Haabendal <[email protected]>
>>> ---
>>> arch/powerpc/configs/85xx-hw.config | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/powerpc/configs/85xx-hw.config b/arch/powerpc/configs/85xx-hw.config
>>> index 524db76f47b7..8aff83217397 100644
>>> --- a/arch/powerpc/configs/85xx-hw.config
>>> +++ b/arch/powerpc/configs/85xx-hw.config
>>> @@ -24,6 +24,7 @@ CONFIG_FS_ENET=y
>>> CONFIG_FSL_CORENET_CF=y
>>> CONFIG_FSL_DMA=y
>>> CONFIG_FSL_HV_MANAGER=y
>>> +CONFIG_FSL_IFC=y
>>
>> Does not look like placed according to config order.
>
> Correct.
>
>> This is not alphabetically sorted, but as Kconfig creates it (make
>> savedefconfig).
>
> Are you sure about this?
>
> It looks very much alphabetically sorted, with only two "errors"
>
> $ diff -u 85xx-hw.config 85xx-hw.config.sorted
> --- 85xx-hw.config 2024-05-28 15:05:44.665354428 +0200
> +++ 85xx-hw.config.sorted 2024-05-28 15:05:56.102019081 +0200
> @@ -15,8 +15,8 @@
> CONFIG_DMADEVICES=y
> CONFIG_E1000E=y
> CONFIG_E1000=y
> -CONFIG_EDAC=y
> CONFIG_EDAC_MPC85XX=y
> +CONFIG_EDAC=y
> CONFIG_EEPROM_AT24=y
> CONFIG_EEPROM_LEGACY=y
> CONFIG_FB_FSL_DIU=y
> @@ -71,10 +71,10 @@
> CONFIG_MTD_CMDLINE_PARTS=y
> CONFIG_MTD_NAND_FSL_ELBC=y
> CONFIG_MTD_NAND_FSL_IFC=y
> -CONFIG_MTD_RAW_NAND=y
> CONFIG_MTD_PHYSMAP_OF=y
> CONFIG_MTD_PHYSMAP=y
> CONFIG_MTD_PLATRAM=y
> +CONFIG_MTD_RAW_NAND=y
> CONFIG_MTD_SPI_NOR=y
> CONFIG_NETDEVICES=y
> CONFIG_NVRAM=y
>
> I don't think that this file has ever been Kconfig sorted since it was
> created back in ancient times.
>
> And as it is merged with other config snippets using merge_into_defconfig
> function. I have no idea how to use savedefconfig to maintain such a snippet.
> It would require doing the reverse of the merge_into_defconfig.
Right. This is a config fragment, not a full config, so it's not managed
with savedefconfig.
Alphabetical order is preferable when adding new symbols.
cheers