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

["Arnd Bergmann" <arnd@arndb.de>]

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/