[greybus-dev] Re: [PATCH v2 10/13] mtd: spi-nor: Add stacked memories support in spi-nor

On 1/19/23 19:53, Amit Kumar Mahapatra wrote:

Each flash that is connected in stacked mode should have a separate parameter structure. So, the flash parameter member(*params) of the spi_nor structure is changed to an array (*params[2]). The array is used to store the parameters of each flash connected in stacked configuration.

The current implementation assumes that a maximum of two flashes are connected in stacked mode and both the flashes are of same make but can differ in sizes. So, except the sizes all other flash parameters of both the flashes are identical.

SPI-NOR is not aware of the chip_select values, for any incoming request SPI-NOR will decide the flash index with the help of individual flash size and the configuration type (single/stacked). SPI-NOR will pass on the flash index information to the SPI core & SPI driver by setting the appropriate bit in nor->spimem->spi->cs_index_mask. For example, if nth bit of nor->spimem->spi->cs_index_mask is set then the driver would assert/de-assert spi->chip_slect[n].

Signed-off-by: Amit Kumar Mahapatra amit.kumar-mahapatra@amd.com

drivers/mtd/spi-nor/core.c | 282 +++++++++++++++++++++++++++++------- drivers/mtd/spi-nor/core.h | 4 + drivers/mtd/spi-nor/otp.c | 4 +- include/linux/mtd/spi-nor.h | 12 +- 4 files changed, 246 insertions(+), 56 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 8a4a54bf2d0e..bb7326dc8b70 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -1441,13 +1441,18 @@ static int spi_nor_erase_multi_sectors(struct spi_nor *nor, u64 addr, u32 len) static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) { struct spi_nor *nor = mtd_to_spi_nor(mtd);

• u32 addr, len;

• struct spi_nor_flash_parameter *params;
• u32 addr, len, offset, cur_cs_num = 0; uint32_t rem; int ret;
• u64 sz;

dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr, (long long)instr->len);

• params = spi_nor_get_params(nor, 0);
• sz = params->size;
• if (spi_nor_has_uniform_erase(nor)) { div_u64_rem(instr->len, mtd->erasesize, &rem); if (rem)

@@ -1465,26 +1470,30 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) if (len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) { unsigned long timeout;

• ret = spi_nor_write_enable(nor);
  

• if (ret)
  

• 	goto erase_err;
  

• while (cur_cs_num < SNOR_FLASH_CNT_MAX && params) {
  

• 	nor->spimem->spi->cs_index_mask = 0x01 << cur_cs_num;
  

• 	ret = spi_nor_write_enable(nor);
  

• 	if (ret)
  

• 		goto erase_err;
  

• ret = spi_nor_erase_chip(nor);
  

• if (ret)
  

• 	goto erase_err;
  

• 	ret = spi_nor_erase_chip(nor);
  

• 	if (ret)
  

• 		goto erase_err;
  

• /*
  

•  * Scale the timeout linearly with the size of the flash, with
  

•  * a minimum calibrated to an old 2MB flash. We could try to
  

•  * pull these from CFI/SFDP, but these values should be good
  

•  * enough for now.
  

•  */
  

• timeout = max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES,
  

• 	      CHIP_ERASE_2MB_READY_WAIT_JIFFIES *
  

• 	      (unsigned long)(mtd->size / SZ_2M));
  

• ret = spi_nor_wait_till_ready_with_timeout(nor, timeout);
  

• if (ret)
  

• 	goto erase_err;
  

• 	/*
  

• 	 * Scale the timeout linearly with the size of the flash, with
  

• 	 * a minimum calibrated to an old 2MB flash. We could try to
  

• 	 * pull these from CFI/SFDP, but these values should be good
  

• 	 * enough for now.
  

• 	 */
  

• 	timeout = max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES,
  

• 		      CHIP_ERASE_2MB_READY_WAIT_JIFFIES *
  

• 		      (unsigned long)(params->size / SZ_2M));
  

• 	ret = spi_nor_wait_till_ready_with_timeout(nor, timeout);
  

• 	if (ret)
  

• 		goto erase_err;
  

• 	cur_cs_num++;
  

• }
  

/* REVISIT in some cases we could speed up erasing large regions * by using SPINOR_OP_SE instead of SPINOR_OP_BE_4K. We may have set up @@ -1493,12 +1502,26 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) /* "sector"-at-a-time erase */ } else if (spi_nor_has_uniform_erase(nor)) {

• /* Determine the flash from which the operation need to start */
  

• while ((cur_cs_num < SNOR_FLASH_CNT_MAX) && (addr > sz - 1) && params) {
  

• 	cur_cs_num++;
  

• 	params = spi_nor_get_params(nor, cur_cs_num);
  

• 	sz += params->size;
  

• }
  

• while (len) {

• 	nor->spimem->spi->cs_index_mask = 0x01 << cur_cs_num;
  ret = spi_nor_write_enable(nor);
  if (ret)
  	goto erase_err;
  

• 	ret = spi_nor_erase_sector(nor, addr);
  

• 	offset = addr;
  

• 	if (nor->flags & SNOR_F_HAS_STACKED) {
  

• 		params = spi_nor_get_params(nor, cur_cs_num);
  

• 		offset -= (sz - params->size);
  

• 	}
  

• 	ret = spi_nor_erase_sector(nor, offset);
  if (ret)
  	goto erase_err;
  

@@ -1508,13 +1531,45 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) addr += mtd->erasesize; len -= mtd->erasesize;

• 	/*
  

• 	 * Flash cross over condition in stacked mode.
  

• 	 */
  

• 	if ((nor->flags & SNOR_F_HAS_STACKED) && (addr > sz - 1)) {
  

• 		cur_cs_num++;
  

• 		params = spi_nor_get_params(nor, cur_cs_num);
  

• 		sz += params->size;
  

• 	}
  

  }

/* erase multiple sectors */ } else {

• ret = spi_nor_erase_multi_sectors(nor, addr, len);
  

• if (ret)
  

• 	goto erase_err;
  

• u64 erase_len = 0;
  

• /* Determine the flash from which the operation need to start */
  

• while ((cur_cs_num < SNOR_FLASH_CNT_MAX) && (addr > sz - 1) && params) {
  

• 	cur_cs_num++;
  

• 	params = spi_nor_get_params(nor, cur_cs_num);
  

• 	sz += params->size;
  

• }
  

• /* perform multi sector erase onec per Flash*/
  

• while (len) {
  

• 	erase_len = (len > (sz - addr)) ? (sz - addr) : len;
  

• 	offset = addr;
  

• 	nor->spimem->spi->cs_index_mask = 0x01 << cur_cs_num;
  

• 	if (nor->flags & SNOR_F_HAS_STACKED) {
  

• 		params = spi_nor_get_params(nor, cur_cs_num);
  

• 		offset -= (sz - params->size);
  

• 	}
  

• 	ret = spi_nor_erase_multi_sectors(nor, offset, erase_len);
  

• 	if (ret)
  

• 		goto erase_err;
  

• 	len -= erase_len;
  

• 	addr += erase_len;
  

• 	cur_cs_num++;
  

• 	params = spi_nor_get_params(nor, cur_cs_num);
  

• 	sz += params->size;
  

• }
  

  }

ret = spi_nor_write_disable(nor); @@ -1713,7 +1768,10 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf) { struct spi_nor *nor = mtd_to_spi_nor(mtd);

• ssize_t ret;

• struct spi_nor_flash_parameter *params;
• ssize_t ret, read_len;
• u32 cur_cs_num = 0;
• u64 sz;

dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len); @@ -1721,9 +1779,23 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, if (ret) return ret;

• params = spi_nor_get_params(nor, 0);
• sz = params->size;
• /* Determine the flash from which the operation need to start */
• while ((cur_cs_num < SNOR_FLASH_CNT_MAX) && (from > sz - 1) && params) {

• cur_cs_num++;
  

• params = spi_nor_get_params(nor, cur_cs_num);
  

• sz += params->size;
  

• } while (len) { loff_t addr = from;

• nor->spimem->spi->cs_index_mask = 0x01 << cur_cs_num;
  

• read_len = (len > (sz - addr)) ? (sz - addr) : len;
  

• params = spi_nor_get_params(nor, cur_cs_num);
  

• addr -= (sz - params->size);
  

• addr = spi_nor_convert_addr(nor, addr);

ret = spi_nor_read_data(nor, addr, len, buf); @@ -1735,11 +1807,22 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, if (ret < 0) goto read_err;

• WARN_ON(ret > len);
  

• WARN_ON(ret > read_len);
  

  *retlen += ret; buf += ret; from += ret; len -= ret;

• /*
  

•  * Flash cross over condition in stacked mode.
  

•  *
  

•  */
  

• if ((nor->flags & SNOR_F_HAS_STACKED) && (from > sz - 1)) {
  

• 	cur_cs_num++;
  

• 	params = spi_nor_get_params(nor, cur_cs_num);
  

• 	sz += params->size;
  

• }
  

• } ret = 0;

@@ -1759,13 +1842,22 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, struct spi_nor *nor = mtd_to_spi_nor(mtd); struct spi_nor_flash_parameter *params; size_t page_offset, page_remain, i;

• u32 page_size, cur_cs_num = 0; ssize_t ret;

• u32 page_size;

• u64 sz;

dev_dbg(nor->dev, "to 0x%08x, len %zd\n", (u32)to, len); params = spi_nor_get_params(nor, 0); page_size = params->page_size;

• sz = params->size;
• /* Determine the flash from which the operation need to start */
• while ((cur_cs_num < SNOR_FLASH_CNT_MAX) && (to > sz - 1) && params) {

• cur_cs_num++;
  

• params = spi_nor_get_params(nor, cur_cs_num);
  

• sz += params->size;
  

• }

ret = spi_nor_lock_and_prep(nor); if (ret) @@ -1790,6 +1882,10 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, /* the size of data remaining on the first page */ page_remain = min_t(size_t, page_size - page_offset, len - i);

• nor->spimem->spi->cs_index_mask = 0x01 << cur_cs_num;
  

• params = spi_nor_get_params(nor, cur_cs_num);
  

• addr -= (sz - params->size);
  

• addr = spi_nor_convert_addr(nor, addr);

ret = spi_nor_write_enable(nor); @@ -1806,6 +1902,15 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, goto write_err; *retlen += written; i += written;

• /*
  

•  * Flash cross over condition in stacked mode.
  

•  */
  

• if ((nor->flags & SNOR_F_HAS_STACKED) && ((to + i) > sz - 1)) {
  

• 	cur_cs_num++;
  

• 	params = spi_nor_get_params(nor, cur_cs_num);
  

• 	sz += params->size;
  

• }
  

  }

write_err: @@ -1918,8 +2023,6 @@ int spi_nor_hwcaps_pp2cmd(u32 hwcaps) static int spi_nor_spimem_check_op(struct spi_nor *nor, struct spi_mem_op *op) {

• struct spi_nor_flash_parameter *params = spi_nor_get_params(nor, 0);
• /*
  • First test with 4 address bytes. The opcode itself might
  • be a 3B addressing opcode but we don't care, because

@@ -1928,7 +2031,7 @@ static int spi_nor_spimem_check_op(struct spi_nor *nor, */ op->addr.nbytes = 4; if (!spi_mem_supports_op(nor->spimem, op)) {

• if (params->size > SZ_16M)
  

• if (nor->mtd.size > SZ_16M)
  return -EOPNOTSUPP;
  

/* If flash size <= 16MB, 3 address bytes are sufficient */ @@ -2516,6 +2619,10 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor) static void spi_nor_late_init_params(struct spi_nor *nor) { struct spi_nor_flash_parameter *params = spi_nor_get_params(nor, 0);

• struct device_node *np = spi_nor_get_flash_node(nor);
• u64 flash_size[SNOR_FLASH_CNT_MAX];
• u32 idx = 0, i = 0;
• int rc;

if (nor->manufacturer && nor->manufacturer->fixups && nor->manufacturer->fixups->late_init) @@ -2533,6 +2640,36 @@ static void spi_nor_late_init_params(struct spi_nor *nor) */ if (nor->flags & SNOR_F_HAS_LOCK && !params->locking_ops) spi_nor_init_default_locking_ops(nor);

• /*

• * The flashes that are connected in stacked mode should be of same make.
  

• * Except the flash size all other properties are identical for all the
  

• * flashes connected in stacked mode.
  

• * The flashes that are connected in parallel mode should be identical.
  

• */
  

• while (i < SNOR_FLASH_CNT_MAX) {

• rc = of_property_read_u64_index(np, "stacked-memories", idx, &flash_size[i]);
  

• if (rc == -EINVAL) {
  

• 	break;
  

• } else if (rc == -EOVERFLOW) {
  

• 	idx++;
  

• } else {
  

• 	idx++;
  

• 	i++;
  

• 	if (!(nor->flags & SNOR_F_HAS_STACKED))
  

• 		nor->flags |= SNOR_F_HAS_STACKED;
  

• }
  

• }
• if (nor->flags & SNOR_F_HAS_STACKED) {

• for (idx = 1; idx < SNOR_FLASH_CNT_MAX; idx++) {
  

• 	params = spi_nor_get_params(nor, idx);
  

• 	params = devm_kzalloc(nor->dev, sizeof(*params), GFP_KERNEL);
  

• 	if (params) {
  

• 		memcpy(params, spi_nor_get_params(nor, 0), sizeof(*params));
  

• 		params->size = flash_size[idx];
  

• 		spi_nor_set_params(nor, idx, params);
  

• 	}
  

• }
  

• } }

/** @@ -2741,22 +2878,36 @@ static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable) */ static int spi_nor_quad_enable(struct spi_nor *nor) {

• struct spi_nor_flash_parameter *params = spi_nor_get_params(nor, 0);

• struct spi_nor_flash_parameter *params;
• int err, idx;

• if (!params->quad_enable)

• return 0;
  

• for (idx = 0; idx < SNOR_FLASH_CNT_MAX; idx++) {

• params = spi_nor_get_params(nor, idx);
  

• if (params) {
  

• 	if (!params->quad_enable)
  

• 		return 0;
  

• if (!(spi_nor_get_protocol_width(nor->read_proto) == 4 ||

•      spi_nor_get_protocol_width(nor->write_proto) == 4))
  

• return 0;
  

• 	if (!(spi_nor_get_protocol_width(nor->read_proto) == 4 ||
  

• 	      spi_nor_get_protocol_width(nor->write_proto) == 4))
  

• 		return 0;
  

• 	/*
  

• 	 * Set the appropriate CS index before
  

• 	 * issuing the command.
  

• 	 */
  

• 	nor->spimem->spi->cs_index_mask = 0x01 << idx;
  

• return params->quad_enable(nor);

• 	err = params->quad_enable(nor);
  

• 	if (err)
  

• 		return err;
  

• }
  

• }
• return err; }

static int spi_nor_init(struct spi_nor *nor) {

• struct spi_nor_flash_parameter *params = spi_nor_get_params(nor, 0);
• int err;

• struct spi_nor_flash_parameter *params;
• int err, idx;

err = spi_nor_octal_dtr_enable(nor, true); if (err) { @@ -2797,9 +2948,19 @@ static int spi_nor_init(struct spi_nor *nor) */ WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET, "enabling reset hack; may not recover from unexpected reboots\n");

• err = params->set_4byte_addr_mode(nor, true);
  

• if (err && err != -ENOTSUPP)
  

• 	return err;
  

• for (idx = 0; idx < SNOR_FLASH_CNT_MAX; idx++) {
  

• 	params = spi_nor_get_params(nor, idx);
  

• 	if (params) {
  

• 		/*
  

• 		 * Select the appropriate CS index before
  

• 		 * issuing the command.
  

• 		 */
  

• 		nor->spimem->spi->cs_index_mask = 0x01 << idx;
  

• 		err = params->set_4byte_addr_mode(nor, true);
  

• 		if (err && err != -ENOTSUPP)
  

• 			return err;
  

• 	}
  

• }
  

  }

return 0; @@ -2915,19 +3076,31 @@ void spi_nor_restore(struct spi_nor *nor) { struct spi_nor_flash_parameter *params; int ret;

• int idx;

/* restore the addressing mode */ if (nor->addr_nbytes == 4 && !(nor->flags & SNOR_F_4B_OPCODES) && nor->flags & SNOR_F_BROKEN_RESET) {

• params = spi_nor_get_params(nor, 0);
  

• ret = params->set_4byte_addr_mode(nor, false);
  

• if (ret)
  

• 	/*
  

• 	 * Do not stop the execution in the hope that the flash
  

• 	 * will default to the 3-byte address mode after the
  

• 	 * software reset.
  

• 	 */
  

• 	dev_err(nor->dev, "Failed to exit 4-byte address mode, err = %d\n", ret);
  

• for (idx = 0; idx < SNOR_FLASH_CNT_MAX; idx++) {
  

• 	params = spi_nor_get_params(nor, idx);
  

• 	if (params) {
  

• 		/*
  

• 		 * Select the appropriate CS index before
  

• 		 * issuing the command.
  

• 		 */
  

• 		nor->spimem->spi->cs_index_mask = 0x01 << idx;
  

• 		ret = params->set_4byte_addr_mode(nor, false);
  

• 		if (ret)
  

• 			/*
  

• 			 * Do not stop the execution in the hope that the flash
  

• 			 * will default to the 3-byte address mode after the
  

• 			 * software reset.
  

• 			 */
  

• 			dev_err(nor->dev,
  

• 				"Failed to exit 4-byte address mode, err = %d\n",
  

• 				ret);
  

• 	}
  

• }
  

  }

if (nor->flags & SNOR_F_SOFT_RESET) @@ -2995,6 +3168,8 @@ static void spi_nor_set_mtd_info(struct spi_nor *nor) struct spi_nor_flash_parameter *params = spi_nor_get_params(nor, 0); struct mtd_info *mtd = &nor->mtd; struct device *dev = nor->dev;

• u64 total_sz = 0;
• int idx;

spi_nor_set_mtd_locking_ops(nor); spi_nor_set_mtd_otp_ops(nor); @@ -3010,7 +3185,12 @@ static void spi_nor_set_mtd_info(struct spi_nor *nor) mtd->_erase = spi_nor_erase; mtd->writesize = params->writesize; mtd->writebufsize = params->page_size;

• mtd->size = params->size;

• for (idx = 0; idx < SNOR_FLASH_CNT_MAX; idx++) {

• params = spi_nor_get_params(nor, idx);
  

• if (params)
  

• 	total_sz += params->size;
  

• }
• mtd->size = total_sz; mtd->_read = spi_nor_read; /* Might be already set by some SST flashes. */ if (!mtd->_write)

diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h index f03b55cf7e6f..e94107cc465e 100644 --- a/drivers/mtd/spi-nor/core.h +++ b/drivers/mtd/spi-nor/core.h @@ -11,6 +11,9 @@ #define SPI_NOR_MAX_ID_LEN 6 +/* In single configuration enable CS0 */ +#define SPI_NOR_ENABLE_CS0 BIT(0)

• /* Standard SPI NOR flash operations. */ #define SPI_NOR_READID_OP(naddr, ndummy, buf, len) \ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 0), \

@@ -130,6 +133,7 @@ enum spi_nor_option_flags { SNOR_F_IO_MODE_EN_VOLATILE = BIT(11), SNOR_F_SOFT_RESET = BIT(12), SNOR_F_SWP_IS_VOLATILE = BIT(13),

• SNOR_F_HAS_STACKED = BIT(14), };

struct spi_nor_read_command { diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c index a9c0844d55ef..b8c7e085a90d 100644 --- a/drivers/mtd/spi-nor/otp.c +++ b/drivers/mtd/spi-nor/otp.c @@ -11,8 +11,8 @@ #include "core.h" -#define spi_nor_otp_region_len(nor) ((nor)->params->otp.org->len) -#define spi_nor_otp_n_regions(nor) ((nor)->params->otp.org->n_regions) +#define spi_nor_otp_region_len(nor) ((nor)->params[0]->otp.org->len) +#define spi_nor_otp_n_regions(nor) ((nor)->params[0]->otp.org->n_regions)

I think this should be also converted to static inline functions and use spi_nor_get_params(nor, 0); instead of using arrays. Can be done on the top of this series if it is the only one problem with it.

Thanks, Michal