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

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

[Michael Walle]

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


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

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

[Michael Walle]

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 <mwalle@kernel.org>
Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
Cc: Ricardo Ribalda <ribalda@kernel.org>
---
 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


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

[PATCH v2 2/6] mtd: spi-nor: get rid of non-power-of-2 page size handling

[Michael Walle]

The Xilinx flashes were the only user of the page sized that were no
power of 2. Support for them were dropped, thus we can also get rid of
the special page size handling for it.

Signed-off-by: Michael Walle <mwalle@kernel.org>
Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/mtd/spi-nor/core.c | 41 ++++++++++----------------------------
 drivers/mtd/spi-nor/core.h |  4 ----
 2 files changed, 11 insertions(+), 34 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index cbe5f92eb0af..31dfdbf96765 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1463,14 +1463,6 @@ static void spi_nor_unlock_and_unprep_rd(struct spi_nor *nor, loff_t start, size
 	spi_nor_unprep(nor);
 }
 
-static u32 spi_nor_convert_addr(struct spi_nor *nor, loff_t addr)
-{
-	if (!nor->params->convert_addr)
-		return addr;
-
-	return nor->params->convert_addr(nor, addr);
-}
-
 /*
  * Initiate the erasure of a single sector
  */
@@ -1478,8 +1470,6 @@ int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
 {
 	int i;
 
-	addr = spi_nor_convert_addr(nor, addr);
-
 	if (nor->spimem) {
 		struct spi_mem_op op =
 			SPI_NOR_SECTOR_ERASE_OP(nor->erase_opcode,
@@ -2064,8 +2054,6 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
 	while (len) {
 		loff_t addr = from;
 
-		addr = spi_nor_convert_addr(nor, addr);
-
 		ret = spi_nor_read_data(nor, addr, len, buf);
 		if (ret == 0) {
 			/* We shouldn't see 0-length reads */
@@ -2098,7 +2086,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 	size_t *retlen, const u_char *buf)
 {
 	struct spi_nor *nor = mtd_to_spi_nor(mtd);
-	size_t page_offset, page_remain, i;
+	size_t i;
 	ssize_t ret;
 	u32 page_size = nor->params->page_size;
 
@@ -2111,23 +2099,9 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 	for (i = 0; i < len; ) {
 		ssize_t written;
 		loff_t addr = to + i;
-
-		/*
-		 * If page_size is a power of two, the offset can be quickly
-		 * calculated with an AND operation. On the other cases we
-		 * need to do a modulus operation (more expensive).
-		 */
-		if (is_power_of_2(page_size)) {
-			page_offset = addr & (page_size - 1);
-		} else {
-			u64 aux = addr;
-
-			page_offset = do_div(aux, page_size);
-		}
+		size_t page_offset = addr & (page_size - 1);
 		/* the size of data remaining on the first page */
-		page_remain = min_t(size_t, page_size - page_offset, len - i);
-
-		addr = spi_nor_convert_addr(nor, addr);
+		size_t page_remain = min_t(size_t, page_size - page_offset, len - i);
 
 		ret = spi_nor_lock_device(nor);
 		if (ret)
@@ -3054,7 +3028,14 @@ static int spi_nor_init_params(struct spi_nor *nor)
 		spi_nor_init_params_deprecated(nor);
 	}
 
-	return spi_nor_late_init_params(nor);
+	ret = spi_nor_late_init_params(nor);
+	if (ret)
+		return ret;
+
+	if (WARN_ON(!is_power_of_2(nor->params->page_size)))
+		return -EINVAL;
+
+	return 0;
 }
 
 /** spi_nor_set_octal_dtr() - enable or disable Octal DTR I/O.
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 072c69b0d06c..3043d583eac2 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -366,9 +366,6 @@ struct spi_nor_otp {
  * @set_octal_dtr:	enables or disables SPI NOR octal DTR mode.
  * @quad_enable:	enables SPI NOR quad mode.
  * @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
- * @convert_addr:	converts an absolute address into something the flash
- *                      will understand. Particularly useful when pagesize is
- *                      not a power-of-2.
  * @setup:		(optional) configures the SPI NOR memory. Useful for
  *			SPI NOR flashes that have peculiarities to the SPI NOR
  *			standard e.g. different opcodes, specific address
@@ -403,7 +400,6 @@ struct spi_nor_flash_parameter {
 	int (*set_octal_dtr)(struct spi_nor *nor, bool enable);
 	int (*quad_enable)(struct spi_nor *nor);
 	int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
-	u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
 	int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
 	int (*ready)(struct spi_nor *nor);
 
-- 
2.39.2


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

[PATCH v2 3/6] mtd: spi-nor: remove .setup() callback

[Michael Walle]

With the removal of the Xilinx flashes, there is no more flash driver
using that hook. The original intention was to let the driver configure
special requirements like page size an opcodes.  This is already
possible by other means and it is unlikely a flash will overwrite the
(more or less complex) setup function.

Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 drivers/mtd/spi-nor/core.c | 105 ++++++++++++++++---------------------
 drivers/mtd/spi-nor/core.h |   5 --
 2 files changed, 45 insertions(+), 65 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 31dfdbf96765..f4c0b5185818 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2554,8 +2554,51 @@ static int spi_nor_select_erase(struct spi_nor *nor)
 	return 0;
 }
 
-static int spi_nor_default_setup(struct spi_nor *nor,
-				 const struct spi_nor_hwcaps *hwcaps)
+static int spi_nor_set_addr_nbytes(struct spi_nor *nor)
+{
+	if (nor->params->addr_nbytes) {
+		nor->addr_nbytes = nor->params->addr_nbytes;
+	} else if (nor->read_proto == SNOR_PROTO_8_8_8_DTR) {
+		/*
+		 * In 8D-8D-8D mode, one byte takes half a cycle to transfer. So
+		 * in this protocol an odd addr_nbytes cannot be used because
+		 * then the address phase would only span a cycle and a half.
+		 * Half a cycle would be left over. We would then have to start
+		 * the dummy phase in the middle of a cycle and so too the data
+		 * phase, and we will end the transaction with half a cycle left
+		 * over.
+		 *
+		 * Force all 8D-8D-8D flashes to use an addr_nbytes of 4 to
+		 * avoid this situation.
+		 */
+		nor->addr_nbytes = 4;
+	} else if (nor->info->addr_nbytes) {
+		nor->addr_nbytes = nor->info->addr_nbytes;
+	} else {
+		nor->addr_nbytes = 3;
+	}
+
+	if (nor->addr_nbytes == 3 && nor->params->size > 0x1000000) {
+		/* enable 4-byte addressing if the device exceeds 16MiB */
+		nor->addr_nbytes = 4;
+	}
+
+	if (nor->addr_nbytes > SPI_NOR_MAX_ADDR_NBYTES) {
+		dev_dbg(nor->dev, "The number of address bytes is too large: %u\n",
+			nor->addr_nbytes);
+		return -EINVAL;
+	}
+
+	/* Set 4byte opcodes when possible. */
+	if (nor->addr_nbytes == 4 && nor->flags & SNOR_F_4B_OPCODES &&
+	    !(nor->flags & SNOR_F_HAS_4BAIT))
+		spi_nor_set_4byte_opcodes(nor);
+
+	return 0;
+}
+
+static int spi_nor_setup(struct spi_nor *nor,
+			 const struct spi_nor_hwcaps *hwcaps)
 {
 	struct spi_nor_flash_parameter *params = nor->params;
 	u32 ignored_mask, shared_mask;
@@ -2612,64 +2655,6 @@ static int spi_nor_default_setup(struct spi_nor *nor,
 		return err;
 	}
 
-	return 0;
-}
-
-static int spi_nor_set_addr_nbytes(struct spi_nor *nor)
-{
-	if (nor->params->addr_nbytes) {
-		nor->addr_nbytes = nor->params->addr_nbytes;
-	} else if (nor->read_proto == SNOR_PROTO_8_8_8_DTR) {
-		/*
-		 * In 8D-8D-8D mode, one byte takes half a cycle to transfer. So
-		 * in this protocol an odd addr_nbytes cannot be used because
-		 * then the address phase would only span a cycle and a half.
-		 * Half a cycle would be left over. We would then have to start
-		 * the dummy phase in the middle of a cycle and so too the data
-		 * phase, and we will end the transaction with half a cycle left
-		 * over.
-		 *
-		 * Force all 8D-8D-8D flashes to use an addr_nbytes of 4 to
-		 * avoid this situation.
-		 */
-		nor->addr_nbytes = 4;
-	} else if (nor->info->addr_nbytes) {
-		nor->addr_nbytes = nor->info->addr_nbytes;
-	} else {
-		nor->addr_nbytes = 3;
-	}
-
-	if (nor->addr_nbytes == 3 && nor->params->size > 0x1000000) {
-		/* enable 4-byte addressing if the device exceeds 16MiB */
-		nor->addr_nbytes = 4;
-	}
-
-	if (nor->addr_nbytes > SPI_NOR_MAX_ADDR_NBYTES) {
-		dev_dbg(nor->dev, "The number of address bytes is too large: %u\n",
-			nor->addr_nbytes);
-		return -EINVAL;
-	}
-
-	/* Set 4byte opcodes when possible. */
-	if (nor->addr_nbytes == 4 && nor->flags & SNOR_F_4B_OPCODES &&
-	    !(nor->flags & SNOR_F_HAS_4BAIT))
-		spi_nor_set_4byte_opcodes(nor);
-
-	return 0;
-}
-
-static int spi_nor_setup(struct spi_nor *nor,
-			 const struct spi_nor_hwcaps *hwcaps)
-{
-	int ret;
-
-	if (nor->params->setup)
-		ret = nor->params->setup(nor, hwcaps);
-	else
-		ret = spi_nor_default_setup(nor, hwcaps);
-	if (ret)
-		return ret;
-
 	return spi_nor_set_addr_nbytes(nor);
 }
 
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 3043d583eac2..497957b64906 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -366,10 +366,6 @@ struct spi_nor_otp {
  * @set_octal_dtr:	enables or disables SPI NOR octal DTR mode.
  * @quad_enable:	enables SPI NOR quad mode.
  * @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
- * @setup:		(optional) configures the SPI NOR memory. Useful for
- *			SPI NOR flashes that have peculiarities to the SPI NOR
- *			standard e.g. different opcodes, specific address
- *			calculation, page size, etc.
  * @ready:		(optional) flashes might use a different mechanism
  *			than reading the status register to indicate they
  *			are ready for a new command
@@ -400,7 +396,6 @@ struct spi_nor_flash_parameter {
 	int (*set_octal_dtr)(struct spi_nor *nor, bool enable);
 	int (*quad_enable)(struct spi_nor *nor);
 	int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
-	int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
 	int (*ready)(struct spi_nor *nor);
 
 	const struct spi_nor_locking_ops *locking_ops;
-- 
2.39.2


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

[PATCH v2 4/6] mtd: spi-nor: get rid of SPI_NOR_NO_FR

[Michael Walle]

The evervision FRAM devices are the only user of the NO_FR flag. Drop
the global flag and instead use a manufacturer fixup for the evervision
flashes to drop the fast read support.

Signed-off-by: Michael Walle <mwalle@kernel.org>
---
Please note, that the fast read opcode will still be set in
spi_nor_init_default_params(), but the selection of the read opcodes
just depends on the mask.

That is also something I want to fix soon: the opcodes can always
be set and the drivers/SFDP will only set the mask. Opcodes then can be
switched between 3b and 4b ones if necessary.
---
 drivers/mtd/spi-nor/core.c     |  9 ++-------
 drivers/mtd/spi-nor/core.h     |  2 --
 drivers/mtd/spi-nor/everspin.c | 19 +++++++++++++++----
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index f4c0b5185818..4e2ae6642d4c 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2923,15 +2923,10 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
 	params->page_size = info->page_size ?: SPI_NOR_DEFAULT_PAGE_SIZE;
 	params->n_banks = info->n_banks ?: SPI_NOR_DEFAULT_N_BANKS;
 
-	if (!(info->flags & SPI_NOR_NO_FR)) {
-		/* Default to Fast Read for DT and non-DT platform devices. */
+	/* Default to Fast Read for non-DT and enable it if requested by DT. */
+	if (!np || of_property_read_bool(np, "m25p,fast-read"))
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
 
-		/* Mask out Fast Read if not requested at DT instantiation. */
-		if (np && !of_property_read_bool(np, "m25p,fast-read"))
-			params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
-	}
-
 	/* (Fast) Read settings. */
 	params->hwcaps.mask |= SNOR_HWCAPS_READ;
 	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 497957b64906..1516b6d0dc37 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -470,7 +470,6 @@ struct spi_nor_id {
  *                            Usually these will power-up in a write-protected
  *                            state.
  *   SPI_NOR_NO_ERASE:        no erase command needed.
- *   SPI_NOR_NO_FR:           can't do fastread.
  *   SPI_NOR_QUAD_PP:         flash supports Quad Input Page Program.
  *   SPI_NOR_RWW:             flash supports reads while write.
  *
@@ -519,7 +518,6 @@ struct flash_info {
 #define SPI_NOR_BP3_SR_BIT6		BIT(4)
 #define SPI_NOR_SWP_IS_VOLATILE		BIT(5)
 #define SPI_NOR_NO_ERASE		BIT(6)
-#define SPI_NOR_NO_FR			BIT(7)
 #define SPI_NOR_QUAD_PP			BIT(8)
 #define SPI_NOR_RWW			BIT(9)
 
diff --git a/drivers/mtd/spi-nor/everspin.c b/drivers/mtd/spi-nor/everspin.c
index 5f321e24ae7d..0720a61947e7 100644
--- a/drivers/mtd/spi-nor/everspin.c
+++ b/drivers/mtd/spi-nor/everspin.c
@@ -14,28 +14,39 @@ static const struct flash_info everspin_nor_parts[] = {
 		.size = SZ_16K,
 		.sector_size = SZ_16K,
 		.addr_nbytes = 2,
-		.flags = SPI_NOR_NO_ERASE | SPI_NOR_NO_FR,
+		.flags = SPI_NOR_NO_ERASE,
 	}, {
 		.name = "mr25h256",
 		.size = SZ_32K,
 		.sector_size = SZ_32K,
 		.addr_nbytes = 2,
-		.flags = SPI_NOR_NO_ERASE | SPI_NOR_NO_FR,
+		.flags = SPI_NOR_NO_ERASE,
 	}, {
 		.name = "mr25h10",
 		.size = SZ_128K,
 		.sector_size = SZ_128K,
-		.flags = SPI_NOR_NO_ERASE | SPI_NOR_NO_FR,
+		.flags = SPI_NOR_NO_ERASE,
 	}, {
 		.name = "mr25h40",
 		.size = SZ_512K,
 		.sector_size = SZ_512K,
-		.flags = SPI_NOR_NO_ERASE | SPI_NOR_NO_FR,
+		.flags = SPI_NOR_NO_ERASE,
 	}
 };
 
+static void evervision_nor_default_init(struct spi_nor *nor)
+{
+	/* Everspin FRAMs don't support the fast read opcode. */
+	nor->params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
+}
+
+static const struct spi_nor_fixups evervision_nor_fixups = {
+	.default_init = evervision_nor_default_init,
+};
+
 const struct spi_nor_manufacturer spi_nor_everspin = {
 	.name = "everspin",
 	.parts = everspin_nor_parts,
 	.nparts = ARRAY_SIZE(everspin_nor_parts),
+	.fixups = &evervision_nor_fixups,
 };
-- 
2.39.2


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

[PATCH v2 5/6] mtd: spi-nor: simplify spi_nor_get_flash_info()

[Michael Walle]

Rework spi_nor_get_flash_info() to make it look more straight forward
and esp. don't return early. The latter is a preparation to check for
deprecated flashes.

Signed-off-by: Michael Walle <mwalle@kernel.org>
Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
---
 drivers/mtd/spi-nor/core.c | 45 ++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 4e2ae6642d4c..8e4ae1317870 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3294,39 +3294,36 @@ static const struct flash_info *spi_nor_match_name(struct spi_nor *nor,
 static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
 						       const char *name)
 {
-	const struct flash_info *info = NULL;
+	const struct flash_info *jinfo = NULL, *info = NULL;
 
 	if (name)
 		info = spi_nor_match_name(nor, name);
-	/* Try to auto-detect if chip name wasn't specified or not found */
-	if (!info)
-		return spi_nor_detect(nor);
-
 	/*
-	 * If caller has specified name of flash model that can normally be
-	 * detected using JEDEC, let's verify it.
+	 * Auto-detect if chip name wasn't specified or not found, or the chip
+	 * has an ID. If the chip supposedly has an ID, we also do an
+	 * auto-detection to compare it later.
 	 */
-	if (name && info->id) {
-		const struct flash_info *jinfo;
-
+	if (!info || info->id) {
 		jinfo = spi_nor_detect(nor);
-		if (IS_ERR(jinfo)) {
+		if (IS_ERR(jinfo))
 			return jinfo;
-		} else if (jinfo != info) {
-			/*
-			 * JEDEC knows better, so overwrite platform ID. We
-			 * can't trust partitions any longer, but we'll let
-			 * mtd apply them anyway, since some partitions may be
-			 * marked read-only, and we don't want to loose that
-			 * information, even if it's not 100% accurate.
-			 */
-			dev_warn(nor->dev, "found %s, expected %s\n",
-				 jinfo->name, info->name);
-			info = jinfo;
-		}
 	}
 
-	return info;
+	/*
+	 * If caller has specified name of flash model that can normally be
+	 * detected using JEDEC, let's verify it.
+	 */
+	if (info && jinfo && jinfo != info)
+		dev_warn(nor->dev, "found %s, expected %s\n",
+			 jinfo->name, info->name);
+
+	/*
+	 * JEDEC knows better, so overwrite platform ID. We can't trust
+	 * partitions any longer, but we'll let mtd apply them anyway, since
+	 * some partitions may be marked read-only, and we don't want to loose
+	 * that information, even if it's not 100% accurate.
+	 */
+	return jinfo ?: info;
 }
 
 static u32
-- 
2.39.2


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

[PATCH v2 6/6] mtd: spi-nor: introduce support for displaying deprecation message

[Michael Walle]

SPI-NOR will automatically detect the attached flash device most of the
time. We cannot easily find out if boards are using a given flash.
Therefore, add a .deprecation_version to the flash_info struct which
indicates the kernel version after which the driver support will be
removed.

Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 drivers/mtd/spi-nor/core.c | 11 ++++++++++-
 drivers/mtd/spi-nor/core.h |  3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 8e4ae1317870..bed5209b94de 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3323,7 +3323,16 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
 	 * some partitions may be marked read-only, and we don't want to loose
 	 * that information, even if it's not 100% accurate.
 	 */
-	return jinfo ?: info;
+	if (jinfo)
+		info = jinfo;
+
+	if (info && info->deprecation_version)
+		pr_warn("Your board is using a SPI NOR flash (%s) with deprecated driver\n"
+			"support. It will be removed after kernel version %s.\n"
+			"If you feel this shouldn't be the case, please contact us at\n"
+			"linux-mtd@lists.infradead.org\n",
+			info->name, info->deprecation_version);
+	return info;
 }
 
 static u32
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 1516b6d0dc37..984155d10fd8 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -447,6 +447,8 @@ struct spi_nor_id {
  * @id:   pointer to struct spi_nor_id or NULL, which means "no ID" (mostly
  *        older chips).
  * @name: (obsolete) the name of the flash. Do not set it for new additions.
+ * @deprecation_version:      the kernel version after which the support for
+ *	                      this flash will be removed.
  * @size:           the size of the flash in bytes.
  * @sector_size:    (optional) the size listed here is what works with
  *                  SPINOR_OP_SE, which isn't necessarily called a "sector" by
@@ -504,6 +506,7 @@ struct spi_nor_id {
 struct flash_info {
 	char *name;
 	const struct spi_nor_id *id;
+	const char *deprecation_version;
 	size_t size;
 	unsigned sector_size;
 	u16 page_size;
-- 
2.39.2


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

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

[Ricardo Ribalda Delgado]

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 <mwalle@kernel.org> 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 <mwalle@kernel.org>
> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
> Cc: Ricardo Ribalda <ribalda@kernel.org>
Acked-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  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
>

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

Re: [PATCH v2 2/6] mtd: spi-nor: get rid of non-power-of-2 page size handling

[Pratyush Yadav]

On Fri, Apr 19 2024, Michael Walle wrote:

> The Xilinx flashes were the only user of the page sized that were no
> power of 2. Support for them were dropped, thus we can also get rid of
> the special page size handling for it.
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

-- 
Regards,
Pratyush Yadav

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

Re: [PATCH v2 4/6] mtd: spi-nor: get rid of SPI_NOR_NO_FR

[Pratyush Yadav]

On Fri, Apr 19 2024, Michael Walle wrote:

> The evervision FRAM devices are the only user of the NO_FR flag. Drop
> the global flag and instead use a manufacturer fixup for the evervision
> flashes to drop the fast read support.
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

-- 
Regards,
Pratyush Yadav

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

Re: [PATCH v2 6/6] mtd: spi-nor: introduce support for displaying deprecation message

[Pratyush Yadav]

On Fri, Apr 19 2024, Michael Walle wrote:

> SPI-NOR will automatically detect the attached flash device most of the
> time. We cannot easily find out if boards are using a given flash.
> Therefore, add a .deprecation_version to the flash_info struct which
> indicates the kernel version after which the driver support will be
> removed.
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

Would also like to get an ACK from Tudor for this patch.

-- 
Regards,
Pratyush Yadav

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

Re: [PATCH v2 3/6] mtd: spi-nor: remove .setup() callback

[Tudor Ambarus]

Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>

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

Re: [PATCH v2 4/6] mtd: spi-nor: get rid of SPI_NOR_NO_FR

[Tudor Ambarus]

On 4/19/24 15:12, Michael Walle wrote:
> The evervision FRAM devices are the only user of the NO_FR flag. Dro

everspin is evervision?

> the global flag and instead use a manufacturer fixup for the evervision
> flashes to drop the fast read support.
> 
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> Please note, that the fast read opcode will still be set in
> spi_nor_init_default_params(), but the selection of the read opcodes
> just depends on the mask.
> 
> That is also something I want to fix soon: the opcodes can always
> be set and the drivers/SFDP will only set the mask. Opcodes then can be
> switched between 3b and 4b ones if necessary.
> ---
>  drivers/mtd/spi-nor/core.c     |  9 ++-------
>  drivers/mtd/spi-nor/core.h     |  2 --
>  drivers/mtd/spi-nor/everspin.c | 19 +++++++++++++++----
>  3 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index f4c0b5185818..4e2ae6642d4c 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2923,15 +2923,10 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
>  	params->page_size = info->page_size ?: SPI_NOR_DEFAULT_PAGE_SIZE;
>  	params->n_banks = info->n_banks ?: SPI_NOR_DEFAULT_N_BANKS;
>  
> -	if (!(info->flags & SPI_NOR_NO_FR)) {
> -		/* Default to Fast Read for DT and non-DT platform devices. */
> +	/* Default to Fast Read for non-DT and enable it if requested by DT. */
> +	if (!np || of_property_read_bool(np, "m25p,fast-read"))
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
>  
> -		/* Mask out Fast Read if not requested at DT instantiation. */
> -		if (np && !of_property_read_bool(np, "m25p,fast-read"))
> -			params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> -	}
> -

You could do this in a dedicated patch, but I'm fine either way.

Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>

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

Re: [PATCH v2 6/6] mtd: spi-nor: introduce support for displaying deprecation message

[Tudor Ambarus]

On 4/19/24 15:12, Michael Walle wrote:
> SPI-NOR will automatically detect the attached flash device most of the

SPI NOR. Looks good. Please follow up with a patch in the same patch set
to use deprecation_version, otherwise we introduce code that's not used
and we risk to get patches removing it.

Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>

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

Re: [PATCH v2 5/6] mtd: spi-nor: simplify spi_nor_get_flash_info()

[Tudor Ambarus]

On 4/19/24 15:12, Michael Walle wrote:
> Rework spi_nor_get_flash_info() to make it look more straight forward
> and esp. don't return early. The latter is a preparation to check for
> deprecated flashes.
> 
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
> ---
>  drivers/mtd/spi-nor/core.c | 45 ++++++++++++++++++--------------------
>  1 file changed, 21 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 4e2ae6642d4c..8e4ae1317870 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3294,39 +3294,36 @@ static const struct flash_info *spi_nor_match_name(struct spi_nor *nor,
>  static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
>  						       const char *name)
>  {
> -	const struct flash_info *info = NULL;
> +	const struct flash_info *jinfo = NULL, *info = NULL;
>  
>  	if (name)
>  		info = spi_nor_match_name(nor, name);
> -	/* Try to auto-detect if chip name wasn't specified or not found */
> -	if (!info)
> -		return spi_nor_detect(nor);
> -
>  	/*
> -	 * If caller has specified name of flash model that can normally be
> -	 * detected using JEDEC, let's verify it.
> +	 * Auto-detect if chip name wasn't specified or not found, or the chip
> +	 * has an ID. If the chip supposedly has an ID, we also do an
> +	 * auto-detection to compare it later.
>  	 */
> -	if (name && info->id) {
> -		const struct flash_info *jinfo;
> -
> +	if (!info || info->id) {
>  		jinfo = spi_nor_detect(nor);
> -		if (IS_ERR(jinfo)) {
> +		if (IS_ERR(jinfo))
>  			return jinfo;
> -		} else if (jinfo != info) {

you can remove else if with if (jinfo != info)

> -			/*
> -			 * JEDEC knows better, so overwrite platform ID. We
> -			 * can't trust partitions any longer, but we'll let
> -			 * mtd apply them anyway, since some partitions may be
> -			 * marked read-only, and we don't want to loose that
> -			 * information, even if it's not 100% accurate.
> -			 */
> -			dev_warn(nor->dev, "found %s, expected %s\n",
> -				 jinfo->name, info->name);

keep the warning where it was
> -			info = jinfo;

move this so that it belongs to if (!info || info->id)
> -		}
>  	}
>

and then return info. Does it work?


	if (name)
		info = spi_nor_match_name(nor, name);

	if (!info || info->id) {
		jinfo = spi_nor_detect(nor);
		if (IS_ERR(jinfo))
			return jinfo;
		
		if (jinfo != info)
			dev_warn(();
		info = jinfo;
	}

	return info;

> -	return info;
> +	/*
> +	 * If caller has specified name of flash model that can normally be
> +	 * detected using JEDEC, let's verify it.
> +	 */
> +	if (info && jinfo && jinfo != info)> +		dev_warn(nor->dev, "found %s, expected %s\n",
> +			 jinfo->name, info->name);
> +
> +	/*
> +	 * JEDEC knows better, so overwrite platform ID. We can't trust
> +	 * partitions any longer, but we'll let mtd apply them anyway, since
> +	 * some partitions may be marked read-only, and we don't want to loose
> +	 * that information, even if it's not 100% accurate.
> +	 */
> +	return jinfo ?: info;

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

Re: [PATCH v2 6/6] mtd: spi-nor: introduce support for displaying deprecation message

[Tudor Ambarus]

On 4/19/24 15:12, Michael Walle wrote:
> +	if (info && info->deprecation_version)

and here you won't need to check for info again

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

Re: [PATCH v2 5/6] mtd: spi-nor: simplify spi_nor_get_flash_info()

[Michael Walle]

[-- Attachment #1.1: Type: text/plain, Size: 380 bytes --]

Hi,

> 	if (name)
> 		info = spi_nor_match_name(nor, name);
>
> 	if (!info || info->id) {
> 		jinfo = spi_nor_detect(nor);
> 		if (IS_ERR(jinfo))
> 			return jinfo;
> 		
> 		if (jinfo != info)

info could be NULL here. So "info &&", apart from that looks good.

> 			dev_warn(();
> 		info = jinfo;
> 	}

Pratyush, should I'll drop your Rb tag then.

-michael

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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

Re: [PATCH v2 6/6] mtd: spi-nor: introduce support for displaying deprecation message

[Michael Walle]

[-- Attachment #1.1: Type: text/plain, Size: 703 bytes --]

On Mon Apr 22, 2024 at 8:06 AM CEST, Tudor Ambarus wrote:
> On 4/19/24 15:12, Michael Walle wrote:
> > SPI-NOR will automatically detect the attached flash device most of the
>
> SPI NOR. Looks good. Please follow up with a patch in the same patch set
> to use deprecation_version, otherwise we introduce code that's not used
> and we risk to get patches removing it.

I missed the RFC tag this time. My plan was actually to have this
patch already reviewed and then repost it with a deprecation of
some flashes.

But apart from that, it's the nature of this flag that it might be
unused from time to time.

> Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>

Thanks!

-michael

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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

Re: [PATCH v2 5/6] mtd: spi-nor: simplify spi_nor_get_flash_info()

[Tudor Ambarus]

On 4/22/24 10:53, Michael Walle wrote:
> Hi,
> 
>> 	if (name)
>> 		info = spi_nor_match_name(nor, name);
>>
>> 	if (!info || info->id) {

here

>> 		jinfo = spi_nor_detect(nor);
>> 		if (IS_ERR(jinfo))
>> 			return jinfo;
>> 		
>> 		if (jinfo != info)
> 
> info could be NULL here. So "info &&", apart from that looks good.

it can't be NULL, the parent if indicated above assures info isn't NULL

> 
>> 			dev_warn(();
>> 		info = jinfo;
>> 	}
> 
> Pratyush, should I'll drop your Rb tag then.
> 
> -michael

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

Re: [PATCH v2 5/6] mtd: spi-nor: simplify spi_nor_get_flash_info()

[Tudor Ambarus]

On 4/22/24 11:12, Tudor Ambarus wrote:
> 
> 
> On 4/22/24 10:53, Michael Walle wrote:
>> Hi,
>>
>>> 	if (name)
>>> 		info = spi_nor_match_name(nor, name);
>>>
>>> 	if (!info || info->id) {
> 
> here
> 
>>> 		jinfo = spi_nor_detect(nor);
>>> 		if (IS_ERR(jinfo))
>>> 			return jinfo;
>>> 		
>>> 		if (jinfo != info)
>>
>> info could be NULL here. So "info &&", apart from that looks good.
> 
> it can't be NULL, the parent if indicated above assures info isn't NULL

ah, I read it wrong it's if (!info), you're right!

> 
>>
>>> 			dev_warn(();
>>> 		info = jinfo;
>>> 	}
>>
>> Pratyush, should I'll drop your Rb tag then.
>>
>> -michael

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