[PATCH] mtd: maps: fix dead select of MTD_CFI_BE_BYTE_SWAP

[PATCH] mtd: maps: fix dead select of MTD_CFI_BE_BYTE_SWAP

[Julian Braha]

'select' does not work on config options in a 'choice', so currently it is
possible to enable MTD_PHYSMAP_IXP4XX without MTD_CFI_BE_BYTE_SWAP.

Let's replace the select with 'depends on'.

Note that, if we remove the select / dependency, the kernel will compile
with MTD_PHYSMAP_IXP4XX=y and MTD_CFI_BE_BYTE_SWAP=n so if it would be
better to remove the select, please advise as I do not have the hardware
to runtime test this.

This dead select was found by kconfirm, a static analysis tool for Kconfig.

Signed-off-by: Julian Braha <julianbraha@gmail.com>
---
 drivers/mtd/maps/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
index f447902d707e..f300953cf9fa 100644
--- a/drivers/mtd/maps/Kconfig
+++ b/drivers/mtd/maps/Kconfig
@@ -100,8 +100,8 @@ config MTD_PHYSMAP_IXP4XX
 	bool "Intel IXP4xx OF-based physical memory map handling"
 	depends on MTD_PHYSMAP_OF
 	depends on ARM
+	depends on MTD_CFI_BE_BYTE_SWAP if CPU_BIG_ENDIAN
 	select MTD_COMPLEX_MAPPINGS
-	select MTD_CFI_BE_BYTE_SWAP if CPU_BIG_ENDIAN
 	default ARCH_IXP4XX
 	help
 	  This provides some extra DT physmap parsing for the Intel IXP4xx
-- 
2.54.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: maps: fix dead select of MTD_CFI_BE_BYTE_SWAP

[Arnd Bergmann]

On Wed, Jul 22, 2026, at 02:10, Julian Braha wrote:
> 'select' does not work on config options in a 'choice', so currently it is
> possible to enable MTD_PHYSMAP_IXP4XX without MTD_CFI_BE_BYTE_SWAP.
>
> Let's replace the select with 'depends on'.
>
> Note that, if we remove the select / dependency, the kernel will compile
> with MTD_PHYSMAP_IXP4XX=y and MTD_CFI_BE_BYTE_SWAP=n so if it would be
> better to remove the select, please advise as I do not have the hardware
> to runtime test this.
>
> This dead select was found by kconfirm, a static analysis tool for Kconfig.

The choice is forced to be MTD_CFI_BE_BYTE_SWAP when building for
big-endian IXP4XX, which I think means this will currently always
work correctly:

config MTD_CFI_NOSWAP
        depends on !ARCH_IXP4XX || CPU_BIG_ENDIAN
        bool "NO"

config MTD_CFI_BE_BYTE_SWAP
        bool "BIG_ENDIAN_BYTE"

config MTD_CFI_LE_BYTE_SWAP
        depends on !ARCH_IXP4XX
        bool "LITTLE_ENDIAN_BYTE"

endchoice

However, this is about to change, as we are in the process of
merging the patch to allow little-endian ARCH_IXP4XX builds
again, and we probably want a different solution here.

Importantly, the logic above is now broken when building for
any multiplatform target that includes both IXP4xx and
some other target using CFI with a different default endianess.
This has not been possible in any release version but will
be in linux-7.3.

> diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
> index f447902d707e..f300953cf9fa 100644
> --- a/drivers/mtd/maps/Kconfig
> +++ b/drivers/mtd/maps/Kconfig
> @@ -100,8 +100,8 @@ config MTD_PHYSMAP_IXP4XX
>  	bool "Intel IXP4xx OF-based physical memory map handling"
>  	depends on MTD_PHYSMAP_OF
>  	depends on ARM
> +	depends on MTD_CFI_BE_BYTE_SWAP if CPU_BIG_ENDIAN
>  	select MTD_COMPLEX_MAPPINGS
> -	select MTD_CFI_BE_BYTE_SWAP if CPU_BIG_ENDIAN
>  	default ARCH_IXP4XX
>  	help
>  	  This provides some extra DT physmap parsing for the Intel IXP4xx

I would think we want to remove the select here without a
replacement and enforce this at runtime by overriding
map->swap like

--- a/drivers/mtd/maps/physmap-ixp4xx.c
+++ b/drivers/mtd/maps/physmap-ixp4xx.c
@@ -125,6 +125,7 @@ int of_flash_probe_ixp4xx(struct platform_device *pdev,
        map->write = ixp4xx_write16;
        map->copy_from = ixp4xx_copy_from;
        map->copy_to = NULL;
+       map->swap = CFI_BIG_ENDIAN; /* or whichever one we need */
 
        dev_info(dev, "initialized Intel IXP4xx-specific physmap control\n");
 
This will force the CFI layer to always perform the same type of
swapping for the ixp4xx driver, regardless of CONFIG_MTD_CFI_*SWAP,
and regardless of any 'big-endian' or 'little-endian' properties
in the cfi-flash DT node that don't work on ARMv5/BE32.

Since there is extra swizzling in both ixp4xx_copy_from()
and in the ixp4xx LE flash_read16()/flash_write16(), I can
no longer work out whether CFI_HOST_ENDIAN is the correct
number of swaps, or if we want CFI_BIG_ENDIAN instead.

If I got the number of swaps correctly, we may actually be
able to simplify this all to the diff below, 

      Arnd

diff --git a/drivers/mtd/chips/Kconfig b/drivers/mtd/chips/Kconfig
index 19726ebd973d..aef14990e5f7 100644
--- a/drivers/mtd/chips/Kconfig
+++ b/drivers/mtd/chips/Kconfig
@@ -55,14 +55,12 @@ choice
 	  LITTLE_ENDIAN_BYTE, if the bytes are reversed.
 
 config MTD_CFI_NOSWAP
-	depends on !ARCH_IXP4XX || CPU_BIG_ENDIAN
 	bool "NO"
 
 config MTD_CFI_BE_BYTE_SWAP
 	bool "BIG_ENDIAN_BYTE"
 
 config MTD_CFI_LE_BYTE_SWAP
-	depends on !ARCH_IXP4XX
 	bool "LITTLE_ENDIAN_BYTE"
 
 endchoice
diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
index 9cefa3f9e5cb..e898150e82e0 100644
--- a/drivers/mtd/maps/Kconfig
+++ b/drivers/mtd/maps/Kconfig
@@ -101,7 +101,6 @@ config MTD_PHYSMAP_IXP4XX
 	depends on MTD_PHYSMAP_OF
 	depends on ARM
 	select MTD_COMPLEX_MAPPINGS
-	select MTD_CFI_BE_BYTE_SWAP if CPU_BIG_ENDIAN
 	default ARCH_IXP4XX
 	help
 	  This provides some extra DT physmap parsing for the Intel IXP4xx
diff --git a/drivers/mtd/maps/physmap-ixp4xx.c b/drivers/mtd/maps/physmap-ixp4xx.c
index c561468f95f6..139528585f25 100644
--- a/drivers/mtd/maps/physmap-ixp4xx.c
+++ b/drivers/mtd/maps/physmap-ixp4xx.c
@@ -39,17 +39,14 @@
 
 static inline u16 flash_read16(void __iomem *addr)
 {
-	return be16_to_cpu(__raw_readw((void __iomem *)((unsigned long)addr ^ 0x2)));
+	return __raw_readw((void __iomem *)((unsigned long)addr ^ 0x2));
 }
 
 static inline void flash_write16(u16 d, void __iomem *addr)
 {
-	__raw_writew(cpu_to_be16(d), (void __iomem *)((unsigned long)addr ^ 0x2));
+	__raw_writew(d, (void __iomem *)((unsigned long)addr ^ 0x2));
 }
 
-#define	BYTE0(h)	((h) & 0xFF)
-#define	BYTE1(h)	(((h) >> 8) & 0xFF)
-
 #else
 
 static inline u16 flash_read16(const void __iomem *addr)
@@ -62,8 +59,6 @@ static inline void flash_write16(u16 d, void __iomem *addr)
 	__raw_writew(d, addr);
 }
 
-#define	BYTE0(h)	(((h) >> 8) & 0xFF)
-#define	BYTE1(h)	((h) & 0xFF)
 #endif
 
 static map_word ixp4xx_read16(struct map_info *map, unsigned long ofs)
@@ -79,6 +74,9 @@ static map_word ixp4xx_read16(struct map_info *map, unsigned long ofs)
  * when attached to a 16-bit wide device (such as the 28F128J3A),
  * so we can't just memcpy_fromio().
  */
+#define	BYTE0(h)	(((h) >> 8) & 0xFF)
+#define	BYTE1(h)	((h) & 0xFF)
+
 static void ixp4xx_copy_from(struct map_info *map, void *to,
 			     unsigned long from, ssize_t len)
 {
@@ -125,6 +123,7 @@ int of_flash_probe_ixp4xx(struct platform_device *pdev,
 	map->write = ixp4xx_write16;
 	map->copy_from = ixp4xx_copy_from;
 	map->copy_to = NULL;
+	map->swap = CFI_HOST_ENDIAN;
 
 	dev_info(dev, "initialized Intel IXP4xx-specific physmap control\n");
 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: maps: fix dead select of MTD_CFI_BE_BYTE_SWAP

[Julian Braha]

Hi Arnd,

On 7/22/26 08:42, Arnd Bergmann wrote:
>> 'select' does not work on config options in a 'choice', so currently it is
>> possible to enable MTD_PHYSMAP_IXP4XX without MTD_CFI_BE_BYTE_SWAP.
>>
>> Let's replace the select with 'depends on'.
>>
>> Note that, if we remove the select / dependency, the kernel will compile
>> with MTD_PHYSMAP_IXP4XX=y and MTD_CFI_BE_BYTE_SWAP=n so if it would be
>> better to remove the select, please advise as I do not have the hardware
>> to runtime test this.
>>
>> This dead select was found by kconfirm, a static analysis tool for Kconfig.
> The choice is forced to be MTD_CFI_BE_BYTE_SWAP when building for
> big-endian IXP4XX, which I think means this will currently always
> work correctly:
> 
> config MTD_CFI_NOSWAP
>         depends on !ARCH_IXP4XX || CPU_BIG_ENDIAN
>         bool "NO"
> 
> config MTD_CFI_BE_BYTE_SWAP
>         bool "BIG_ENDIAN_BYTE"
> 
> config MTD_CFI_LE_BYTE_SWAP
>         depends on !ARCH_IXP4XX
>         bool "LITTLE_ENDIAN_BYTE"
> 
> endchoice

Actually it is currently possible to have:
MTD_PHYSMAP_IXP4XX=y
MTD_CFI_BE_BYTE_SWAP=n
CPU_BIG_ENDIAN=y

by not satisfying the dependencies of the choice that
MTD_CFI_BE_BYTE_SWAP is part of,
for example, set:
MTD_CFI_ADV_OPTIONS=n

then, none of the choice options are enabled. This is the
configuration that I had compile-tested (but cannot runtime test).

In any case, should I send a v2 removing the select, and leave the
rest of the little-endian ARCH_IXP4XX changes to you?

- Julian Braha

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: maps: fix dead select of MTD_CFI_BE_BYTE_SWAP

[Arnd Bergmann]

On Wed, Jul 22, 2026, at 15:43, Julian Braha wrote:
> On 7/22/26 08:42, Arnd Bergmann wrote:
>> config MTD_CFI_NOSWAP
>>         depends on !ARCH_IXP4XX || CPU_BIG_ENDIAN
>>         bool "NO"
>> 
>> config MTD_CFI_BE_BYTE_SWAP
>>         bool "BIG_ENDIAN_BYTE"
>> 
>> config MTD_CFI_LE_BYTE_SWAP
>>         depends on !ARCH_IXP4XX
>>         bool "LITTLE_ENDIAN_BYTE"
>> 
>> endchoice
>
> Actually it is currently possible to have:
> MTD_PHYSMAP_IXP4XX=y
> MTD_CFI_BE_BYTE_SWAP=n
> CPU_BIG_ENDIAN=y
>
> by not satisfying the dependencies of the choice that
> MTD_CFI_BE_BYTE_SWAP is part of,
> for example, set:
> MTD_CFI_ADV_OPTIONS=n
>
> then, none of the choice options are enabled. This is the
> configuration that I had compile-tested (but cannot runtime test).

Right. In older kernels that would have run into an #error,
and I tried to express the same with Kconfig logic but clearly
failed here.

> In any case, should I send a v2 removing the select, and leave the
> rest of the little-endian ARCH_IXP4XX changes to you?

Sound good to me, if that helps you get through your list
backlog of known issues, otherwise we can just do my version
and drop yours. Hopefully Linus Walleij can give my patch
a spin on one of his machines.

     Arnd

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: maps: fix dead select of MTD_CFI_BE_BYTE_SWAP

[Julian Braha]

Hi Arnd,

On 7/22/26 14:59, Arnd Bergmann wrote:
> Sound good to me, if that helps you get through your list
> backlog of known issues,

Right, my bigger goal is to modify the Kconfig interpreter to disallow
select-choice instead of silently failing (as has been the case for 10+
years...), but we need to remove all existing usage first.

Thanks,
Julian Braha

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/