2024-04-19 14:30:02

by Michael Walle

[permalink] [raw]
Subject: [PATCH v2 0/6] mtd: spi-nor: spring cleaning

It's time for some spring cleaning. Remove the oddball xilinx
flashes with non-power-of-2 page sizes.
Remove the .setup() callback, only the default callback is ever
used and it is unlikely there is need for a custom setup.

Finally, the last patch is a proposal how to deprecate flashes,
which are just detected by their id. We cannot really find out if
there are boards out there which are using a particular flash. Thus,
as a first step, we can print a warning during kernel startup. As a
second step we might introduce a kernel config option to actually
disable the flashes which has the deprecated flag.

v2:
- remove convert_addr, I've left page_size in the info, that might
still come in handy for future flashes as they are getting
bigger, although let's hope they have SFDP support..
- simplify the fast read flag handling
- reword the commit message of the .setup() callback removal patch
- introduce .deprecation_version and simplify the checking

Michael Walle (6):
mtd: spi-nor: Remove support for Xilinx S3AN flashes
mtd: spi-nor: get rid of non-power-of-2 page size handling
mtd: spi-nor: remove .setup() callback
mtd: spi-nor: get rid of SPI_NOR_NO_FR
mtd: spi-nor: simplify spi_nor_get_flash_info()
mtd: spi-nor: introduce support for displaying deprecation message

drivers/mtd/spi-nor/Makefile | 1 -
drivers/mtd/spi-nor/core.c | 208 ++++++++++++++-------------------
drivers/mtd/spi-nor/core.h | 15 +--
drivers/mtd/spi-nor/everspin.c | 19 ++-
drivers/mtd/spi-nor/xilinx.c | 169 ---------------------------
5 files changed, 105 insertions(+), 307 deletions(-)
delete mode 100644 drivers/mtd/spi-nor/xilinx.c

--
2.39.2



2024-04-19 14:30:27

by Michael Walle

[permalink] [raw]
Subject: [PATCH v2 1/6] mtd: spi-nor: Remove support for Xilinx S3AN flashes

These flashes are kind of an oddball for the very old Xilinx Spartan 3
FPGAs to store their bitstream. More importantly, they reuse the Atmel
JEDEC manufacturer ID and in fact the at45db081d already blocks the use
of the 3S700AN flash chip. It's time to sunset support for these
flashes.

Signed-off-by: Michael Walle <[email protected]>
Acked-by: Tudor Ambarus <[email protected]>
Reviewed-by: Pratyush Yadav <[email protected]>
Cc: Ricardo Ribalda <[email protected]>
---
drivers/mtd/spi-nor/Makefile | 1 -
drivers/mtd/spi-nor/core.c | 1 -
drivers/mtd/spi-nor/core.h | 1 -
drivers/mtd/spi-nor/xilinx.c | 169 -----------------------------------
4 files changed, 172 deletions(-)
delete mode 100644 drivers/mtd/spi-nor/xilinx.c

diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 5e68468b72fc..5dd9c35f6b6f 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -13,7 +13,6 @@ spi-nor-objs += micron-st.o
spi-nor-objs += spansion.o
spi-nor-objs += sst.o
spi-nor-objs += winbond.o
-spi-nor-objs += xilinx.o
spi-nor-objs += xmc.o
spi-nor-$(CONFIG_DEBUG_FS) += debugfs.o
obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 3e1f1913536b..cbe5f92eb0af 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1986,7 +1986,6 @@ static const struct spi_nor_manufacturer *manufacturers[] = {
&spi_nor_spansion,
&spi_nor_sst,
&spi_nor_winbond,
- &spi_nor_xilinx,
&spi_nor_xmc,
};

diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 442786685515..072c69b0d06c 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -603,7 +603,6 @@ extern const struct spi_nor_manufacturer spi_nor_st;
extern const struct spi_nor_manufacturer spi_nor_spansion;
extern const struct spi_nor_manufacturer spi_nor_sst;
extern const struct spi_nor_manufacturer spi_nor_winbond;
-extern const struct spi_nor_manufacturer spi_nor_xilinx;
extern const struct spi_nor_manufacturer spi_nor_xmc;

extern const struct attribute_group *spi_nor_sysfs_groups[];
diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
deleted file mode 100644
index f99118c691b0..000000000000
--- a/drivers/mtd/spi-nor/xilinx.c
+++ /dev/null
@@ -1,169 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2005, Intec Automation Inc.
- * Copyright (C) 2014, Freescale Semiconductor, Inc.
- */
-
-#include <linux/mtd/spi-nor.h>
-
-#include "core.h"
-
-#define XILINX_OP_SE 0x50 /* Sector erase */
-#define XILINX_OP_PP 0x82 /* Page program */
-#define XILINX_OP_RDSR 0xd7 /* Read status register */
-
-#define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
-#define XSR_RDY BIT(7) /* Ready */
-
-#define XILINX_RDSR_OP(buf) \
- SPI_MEM_OP(SPI_MEM_OP_CMD(XILINX_OP_RDSR, 0), \
- SPI_MEM_OP_NO_ADDR, \
- SPI_MEM_OP_NO_DUMMY, \
- SPI_MEM_OP_DATA_IN(1, buf, 0))
-
-#define S3AN_FLASH(_id, _name, _n_sectors, _page_size) \
- .id = _id, \
- .name = _name, \
- .size = 8 * (_page_size) * (_n_sectors), \
- .sector_size = (8 * (_page_size)), \
- .page_size = (_page_size), \
- .flags = SPI_NOR_NO_FR
-
-/* Xilinx S3AN share MFR with Atmel SPI NOR */
-static const struct flash_info xilinx_nor_parts[] = {
- /* Xilinx S3AN Internal Flash */
- { S3AN_FLASH(SNOR_ID(0x1f, 0x22, 0x00), "3S50AN", 64, 264) },
- { S3AN_FLASH(SNOR_ID(0x1f, 0x24, 0x00), "3S200AN", 256, 264) },
- { S3AN_FLASH(SNOR_ID(0x1f, 0x24, 0x00), "3S400AN", 256, 264) },
- { S3AN_FLASH(SNOR_ID(0x1f, 0x25, 0x00), "3S700AN", 512, 264) },
- { S3AN_FLASH(SNOR_ID(0x1f, 0x26, 0x00), "3S1400AN", 512, 528) },
-};
-
-/*
- * This code converts an address to the Default Address Mode, that has non
- * power of two page sizes. We must support this mode because it is the default
- * mode supported by Xilinx tools, it can access the whole flash area and
- * changing over to the Power-of-two mode is irreversible and corrupts the
- * original data.
- * Addr can safely be unsigned int, the biggest S3AN device is smaller than
- * 4 MiB.
- */
-static u32 s3an_nor_convert_addr(struct spi_nor *nor, u32 addr)
-{
- u32 page_size = nor->params->page_size;
- u32 offset, page;
-
- offset = addr % page_size;
- page = addr / page_size;
- page <<= (page_size > 512) ? 10 : 9;
-
- return page | offset;
-}
-
-/**
- * xilinx_nor_read_sr() - Read the Status Register on S3AN flashes.
- * @nor: pointer to 'struct spi_nor'.
- * @sr: pointer to a DMA-able buffer where the value of the
- * Status Register will be written.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int xilinx_nor_read_sr(struct spi_nor *nor, u8 *sr)
-{
- int ret;
-
- if (nor->spimem) {
- struct spi_mem_op op = XILINX_RDSR_OP(sr);
-
- spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = spi_nor_controller_ops_read_reg(nor, XILINX_OP_RDSR, sr,
- 1);
- }
-
- if (ret)
- dev_dbg(nor->dev, "error %d reading SR\n", ret);
-
- return ret;
-}
-
-/**
- * xilinx_nor_sr_ready() - Query the Status Register of the S3AN flash to see
- * if the flash is ready for new commands.
- * @nor: pointer to 'struct spi_nor'.
- *
- * Return: 1 if ready, 0 if not ready, -errno on errors.
- */
-static int xilinx_nor_sr_ready(struct spi_nor *nor)
-{
- int ret;
-
- ret = xilinx_nor_read_sr(nor, nor->bouncebuf);
- if (ret)
- return ret;
-
- return !!(nor->bouncebuf[0] & XSR_RDY);
-}
-
-static int xilinx_nor_setup(struct spi_nor *nor,
- const struct spi_nor_hwcaps *hwcaps)
-{
- u32 page_size;
- int ret;
-
- ret = xilinx_nor_read_sr(nor, nor->bouncebuf);
- if (ret)
- return ret;
-
- nor->erase_opcode = XILINX_OP_SE;
- nor->program_opcode = XILINX_OP_PP;
- nor->read_opcode = SPINOR_OP_READ;
- nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
-
- /*
- * This flashes have a page size of 264 or 528 bytes (known as
- * Default addressing mode). It can be changed to a more standard
- * Power of two mode where the page size is 256/512. This comes
- * with a price: there is 3% less of space, the data is corrupted
- * and the page size cannot be changed back to default addressing
- * mode.
- *
- * The current addressing mode can be read from the XRDSR register
- * and should not be changed, because is a destructive operation.
- */
- if (nor->bouncebuf[0] & XSR_PAGESIZE) {
- /* Flash in Power of 2 mode */
- page_size = (nor->params->page_size == 264) ? 256 : 512;
- nor->params->page_size = page_size;
- nor->mtd.writebufsize = page_size;
- nor->params->size = nor->info->size;
- nor->mtd.erasesize = 8 * page_size;
- } else {
- /* Flash in Default addressing mode */
- nor->params->convert_addr = s3an_nor_convert_addr;
- nor->mtd.erasesize = nor->info->sector_size;
- }
-
- return 0;
-}
-
-static int xilinx_nor_late_init(struct spi_nor *nor)
-{
- nor->params->setup = xilinx_nor_setup;
- nor->params->ready = xilinx_nor_sr_ready;
-
- return 0;
-}
-
-static const struct spi_nor_fixups xilinx_nor_fixups = {
- .late_init = xilinx_nor_late_init,
-};
-
-const struct spi_nor_manufacturer spi_nor_xilinx = {
- .name = "xilinx",
- .parts = xilinx_nor_parts,
- .nparts = ARRAY_SIZE(xilinx_nor_parts),
- .fixups = &xilinx_nor_fixups,
-};
--
2.39.2


2024-04-19 15:16:59

by Ricardo Ribalda Delgado

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] mtd: spi-nor: Remove support for Xilinx S3AN flashes

Hi Michael

Thanks for the cleanup. I have pinged my previous employer and they do
not have any product in production using S3AN.
So no blockers from my side.

[ccing Marek In case he knows about other users]

Regards

On Fri, Apr 19, 2024 at 4:14 PM Michael Walle <[email protected]> wrote:
>
> These flashes are kind of an oddball for the very old Xilinx Spartan 3
> FPGAs to store their bitstream. More importantly, they reuse the Atmel
> JEDEC manufacturer ID and in fact the at45db081d already blocks the use
> of the 3S700AN flash chip. It's time to sunset support for these
> flashes.
>
> Signed-off-by: Michael Walle <[email protected]>
> Acked-by: Tudor Ambarus <[email protected]>
> Reviewed-by: Pratyush Yadav <[email protected]>
> Cc: Ricardo Ribalda <[email protected]>
Acked-by: Ricardo Ribalda <[email protected]>
> ---
> drivers/mtd/spi-nor/Makefile | 1 -
> drivers/mtd/spi-nor/core.c | 1 -
> drivers/mtd/spi-nor/core.h | 1 -
> drivers/mtd/spi-nor/xilinx.c | 169 -----------------------------------
> 4 files changed, 172 deletions(-)
> delete mode 100644 drivers/mtd/spi-nor/xilinx.c
>
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 5e68468b72fc..5dd9c35f6b6f 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -13,7 +13,6 @@ spi-nor-objs += micron-st.o
> spi-nor-objs += spansion.o
> spi-nor-objs += sst.o
> spi-nor-objs += winbond.o
> -spi-nor-objs += xilinx.o
> spi-nor-objs += xmc.o
> spi-nor-$(CONFIG_DEBUG_FS) += debugfs.o
> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 3e1f1913536b..cbe5f92eb0af 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1986,7 +1986,6 @@ static const struct spi_nor_manufacturer *manufacturers[] = {
> &spi_nor_spansion,
> &spi_nor_sst,
> &spi_nor_winbond,
> - &spi_nor_xilinx,
> &spi_nor_xmc,
> };
>
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 442786685515..072c69b0d06c 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -603,7 +603,6 @@ extern const struct spi_nor_manufacturer spi_nor_st;
> extern const struct spi_nor_manufacturer spi_nor_spansion;
> extern const struct spi_nor_manufacturer spi_nor_sst;
> extern const struct spi_nor_manufacturer spi_nor_winbond;
> -extern const struct spi_nor_manufacturer spi_nor_xilinx;
> extern const struct spi_nor_manufacturer spi_nor_xmc;
>
> extern const struct attribute_group *spi_nor_sysfs_groups[];
> diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
> deleted file mode 100644
> index f99118c691b0..000000000000
> --- a/drivers/mtd/spi-nor/xilinx.c
> +++ /dev/null
> @@ -1,169 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Copyright (C) 2005, Intec Automation Inc.
> - * Copyright (C) 2014, Freescale Semiconductor, Inc.
> - */
> -
> -#include <linux/mtd/spi-nor.h>
> -
> -#include "core.h"
> -
> -#define XILINX_OP_SE 0x50 /* Sector erase */
> -#define XILINX_OP_PP 0x82 /* Page program */
> -#define XILINX_OP_RDSR 0xd7 /* Read status register */
> -
> -#define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
> -#define XSR_RDY BIT(7) /* Ready */
> -
> -#define XILINX_RDSR_OP(buf) \
> - SPI_MEM_OP(SPI_MEM_OP_CMD(XILINX_OP_RDSR, 0), \
> - SPI_MEM_OP_NO_ADDR, \
> - SPI_MEM_OP_NO_DUMMY, \
> - SPI_MEM_OP_DATA_IN(1, buf, 0))
> -
> -#define S3AN_FLASH(_id, _name, _n_sectors, _page_size) \
> - .id = _id, \
> - .name = _name, \
> - .size = 8 * (_page_size) * (_n_sectors), \
> - .sector_size = (8 * (_page_size)), \
> - .page_size = (_page_size), \
> - .flags = SPI_NOR_NO_FR
> -
> -/* Xilinx S3AN share MFR with Atmel SPI NOR */
> -static const struct flash_info xilinx_nor_parts[] = {
> - /* Xilinx S3AN Internal Flash */
> - { S3AN_FLASH(SNOR_ID(0x1f, 0x22, 0x00), "3S50AN", 64, 264) },
> - { S3AN_FLASH(SNOR_ID(0x1f, 0x24, 0x00), "3S200AN", 256, 264) },
> - { S3AN_FLASH(SNOR_ID(0x1f, 0x24, 0x00), "3S400AN", 256, 264) },
> - { S3AN_FLASH(SNOR_ID(0x1f, 0x25, 0x00), "3S700AN", 512, 264) },
> - { S3AN_FLASH(SNOR_ID(0x1f, 0x26, 0x00), "3S1400AN", 512, 528) },
> -};
> -
> -/*
> - * This code converts an address to the Default Address Mode, that has non
> - * power of two page sizes. We must support this mode because it is the default
> - * mode supported by Xilinx tools, it can access the whole flash area and
> - * changing over to the Power-of-two mode is irreversible and corrupts the
> - * original data.
> - * Addr can safely be unsigned int, the biggest S3AN device is smaller than
> - * 4 MiB.
> - */
> -static u32 s3an_nor_convert_addr(struct spi_nor *nor, u32 addr)
> -{
> - u32 page_size = nor->params->page_size;
> - u32 offset, page;
> -
> - offset = addr % page_size;
> - page = addr / page_size;
> - page <<= (page_size > 512) ? 10 : 9;
> -
> - return page | offset;
> -}
> -
> -/**
> - * xilinx_nor_read_sr() - Read the Status Register on S3AN flashes.
> - * @nor: pointer to 'struct spi_nor'.
> - * @sr: pointer to a DMA-able buffer where the value of the
> - * Status Register will be written.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int xilinx_nor_read_sr(struct spi_nor *nor, u8 *sr)
> -{
> - int ret;
> -
> - if (nor->spimem) {
> - struct spi_mem_op op = XILINX_RDSR_OP(sr);
> -
> - spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> - ret = spi_mem_exec_op(nor->spimem, &op);
> - } else {
> - ret = spi_nor_controller_ops_read_reg(nor, XILINX_OP_RDSR, sr,
> - 1);
> - }
> -
> - if (ret)
> - dev_dbg(nor->dev, "error %d reading SR\n", ret);
> -
> - return ret;
> -}
> -
> -/**
> - * xilinx_nor_sr_ready() - Query the Status Register of the S3AN flash to see
> - * if the flash is ready for new commands.
> - * @nor: pointer to 'struct spi_nor'.
> - *
> - * Return: 1 if ready, 0 if not ready, -errno on errors.
> - */
> -static int xilinx_nor_sr_ready(struct spi_nor *nor)
> -{
> - int ret;
> -
> - ret = xilinx_nor_read_sr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - return !!(nor->bouncebuf[0] & XSR_RDY);
> -}
> -
> -static int xilinx_nor_setup(struct spi_nor *nor,
> - const struct spi_nor_hwcaps *hwcaps)
> -{
> - u32 page_size;
> - int ret;
> -
> - ret = xilinx_nor_read_sr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - nor->erase_opcode = XILINX_OP_SE;
> - nor->program_opcode = XILINX_OP_PP;
> - nor->read_opcode = SPINOR_OP_READ;
> - nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
> -
> - /*
> - * This flashes have a page size of 264 or 528 bytes (known as
> - * Default addressing mode). It can be changed to a more standard
> - * Power of two mode where the page size is 256/512. This comes
> - * with a price: there is 3% less of space, the data is corrupted
> - * and the page size cannot be changed back to default addressing
> - * mode.
> - *
> - * The current addressing mode can be read from the XRDSR register
> - * and should not be changed, because is a destructive operation.
> - */
> - if (nor->bouncebuf[0] & XSR_PAGESIZE) {
> - /* Flash in Power of 2 mode */
> - page_size = (nor->params->page_size == 264) ? 256 : 512;
> - nor->params->page_size = page_size;
> - nor->mtd.writebufsize = page_size;
> - nor->params->size = nor->info->size;
> - nor->mtd.erasesize = 8 * page_size;
> - } else {
> - /* Flash in Default addressing mode */
> - nor->params->convert_addr = s3an_nor_convert_addr;
> - nor->mtd.erasesize = nor->info->sector_size;
> - }
> -
> - return 0;
> -}
> -
> -static int xilinx_nor_late_init(struct spi_nor *nor)
> -{
> - nor->params->setup = xilinx_nor_setup;
> - nor->params->ready = xilinx_nor_sr_ready;
> -
> - return 0;
> -}
> -
> -static const struct spi_nor_fixups xilinx_nor_fixups = {
> - .late_init = xilinx_nor_late_init,
> -};
> -
> -const struct spi_nor_manufacturer spi_nor_xilinx = {
> - .name = "xilinx",
> - .parts = xilinx_nor_parts,
> - .nparts = ARRAY_SIZE(xilinx_nor_parts),
> - .fixups = &xilinx_nor_fixups,
> -};
> --
> 2.39.2
>

2024-05-27 15:20:37

by Pratyush Yadav

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] mtd: spi-nor: spring cleaning

Hi Michael,

On Fri, Apr 19 2024, Michael Walle wrote:

[...]

Applied the below 4 patches with minor fixups mentioned below. Since
v6.10-rc1 just came out yesterday, this should let the S3AN flash
removal get plenty of time to cook in linux-next, and give people chance
to notice any breakages.

> mtd: spi-nor: Remove support for Xilinx S3AN flashes
> mtd: spi-nor: get rid of non-power-of-2 page size handling

Touched up commit message a bit.

> mtd: spi-nor: remove .setup() callback
> mtd: spi-nor: get rid of SPI_NOR_NO_FR

s/evervision/everspin/g both in code and commit message.

These 2 patches need rework:

> mtd: spi-nor: simplify spi_nor_get_flash_info()

This one has some comments from Tudor. I don't think those are big
blockers, but at the same time I don't think this patch needs too much
time to cook in linux-next anyway. So I will wait for a respin.

> mtd: spi-nor: introduce support for displaying deprecation message

Same for this one. I will wait for a respin with some actual
deprecations.

Thanks for working on this!

[...]

--
Regards,
Pratyush Yadav