[PATCH v2] mtd: spi-nor: Fix the spi_nor_read() when config SPI_STACKED_PARALLEL is enabled
Marek Vasut
marex at denx.de
Mon Dec 16 00:50:49 CET 2024
On 12/11/24 1:06 PM, Venkatesh Yadav Abbarapu wrote:
> Update the spi_nor_read() function only for parallel configuration
> when config SPI_STACKED_PARALLEL is enabled and keep the default
> code as is. For parallel configurations fix the read issue for
> 4byte address width by passing the entire length to the read
> function, split the memory of 16MB size banks only when the
> address width is 3byte. Also update the size when the
> configuration is stacked.
>
> Fixes: 5d40b3d384 ("mtd: spi-nor: Add parallel and stacked memories support")
> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
> ---
> Changes in v2:
> - Update the nor read for parallel configuration and for other case
> keep the code as is.
> - Fixed the commit description.
> - Tested the changes with s25fl128s flash
> Zynq> sf probe 0 0 0
> SF: Detected s25fl128s with page size 256 Bytes, erase size 64 KiB, total 16 MiB
> Zynq> time sf erase 0x0 0x1000000;mw.b 0x8000 aabbccdd 0x1000000;time sf write 0x8000 0x0 0x1000000;mw.b 0x8008000 0x0 0x1000000;time sf read 0x8008000 0x0 0x1000000;cmp.b 0x8000 0x8008000 0x1000000
> SF: 16777216 bytes @ 0x0 Erased: OK
>
> time: 32.464 seconds
> device 0 whole chip
> SF: 16777216 bytes @ 0x0 Written: OK
>
> time: 15.515 seconds
> device 0 whole chip
> SF: 16777216 bytes @ 0x0 Read: OK
>
> time: 1.557 seconds
> Total of 16777216 byte(s) were the same
> ---
> drivers/mtd/spi/spi-nor-core.c | 50 ++++++++++++++++++++++------------
> 1 file changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index ec841fb13b..8d7087b5c9 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -1140,7 +1140,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
> nor->spi->flags &= ~SPI_XFER_U_PAGE;
> }
> #ifdef CONFIG_SPI_FLASH_BAR
> - ret = write_bar(nor, addr);
> + ret = write_bar(nor, offset);
This change is really inobvious, the code above likely needs to be
compiled out if the SPI_STACKED_PARALLEL stuff is disabled ?
"
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 8d7087b5c95..691962ef271 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -1130,6 +1130,7 @@ static int spi_nor_erase(struct mtd_info *mtd,
struct erase_info *instr)
goto erase_err;
}
offset = addr;
+ if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
if (nor->flags & SNOR_F_HAS_PARALLEL)
offset /= 2;
@@ -1139,6 +1140,7 @@ static int spi_nor_erase(struct mtd_info *mtd,
struct erase_info *instr)
else
nor->spi->flags &=
~SPI_XFER_U_PAGE;
}
+ }
#ifdef CONFIG_SPI_FLASH_BAR
ret = write_bar(nor, offset);
if (ret < 0)
"
> if (ret < 0)
> goto erase_err;
> #endif
> @@ -1152,7 +1152,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
> !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) {
> ret = spi_nor_erase_chip(nor);
> } else {
> - ret = spi_nor_erase_sector(nor, addr);
> + ret = spi_nor_erase_sector(nor, offset);
> }
> if (ret < 0)
> goto erase_err;
> @@ -1578,8 +1578,8 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> struct spi_nor *nor = mtd_to_spi_nor(mtd);
> int ret;
> loff_t offset = from;
> - u32 read_len = 0;
> u32 rem_bank_len = 0;
> + u32 stack_shift = 0;
> u8 bank;
> bool is_ofst_odd = false;
>
> @@ -1593,18 +1593,23 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> }
>
> while (len) {
> - bank = (u32)from / SZ_16M;
> - if (nor->flags & SNOR_F_HAS_PARALLEL)
> - bank /= 2;
> -
> - rem_bank_len = SZ_16M * (bank + 1);
> - if (nor->flags & SNOR_F_HAS_PARALLEL)
> - rem_bank_len *= 2;
> - rem_bank_len -= from;
> -
> + size_t read_len = len;
> offset = from;
>
> + if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
> + if (nor->addr_width == 3) {
> + bank = (u32)from / SZ_16M;
> + if (nor->flags & SNOR_F_HAS_PARALLEL)
> + bank /= 2;
> +
> + rem_bank_len = SZ_16M * (bank + 1);
> + if (nor->flags & SNOR_F_HAS_PARALLEL)
> + rem_bank_len *= 2;
> + rem_bank_len -= from;
> + }
> +
> if (nor->flags & SNOR_F_HAS_STACKED) {
> + stack_shift = 1;
> if (offset >= (mtd->size / 2)) {
> offset = offset - (mtd->size / 2);
> nor->spi->flags |= SPI_XFER_U_PAGE;
> @@ -1613,20 +1618,31 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> }
> }
>
> + if (nor->addr_width == 4)
> + rem_bank_len = (mtd->size >> stack_shift) - offset;
> +
> if (nor->flags & SNOR_F_HAS_PARALLEL)
> offset /= 2;
> + }
>
> #ifdef CONFIG_SPI_FLASH_BAR
> + u32 remain_len;
> +
> ret = write_bar(nor, offset);
> if (ret < 0)
> return log_ret(ret);
> -#endif
> -
> - if (len < rem_bank_len)
> + remain_len = (SZ_16M * (nor->bank_curr + 1)) - offset;
> + if (len < remain_len)
> read_len = len;
> else
> - read_len = rem_bank_len;
> -
> + read_len = remain_len;
> +#endif
> + if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
> + if (len < rem_bank_len)
> + read_len = len;
> + else
> + read_len = rem_bank_len;
> + }
If I look at this change with 'git show -w' , the change looks like this:
"
#ifdef CONFIG_SPI_FLASH_BAR
+ u32 remain_len;
+
ret = write_bar(nor, offset);
if (ret < 0)
return log_ret(ret);
+ remain_len = (SZ_16M * (nor->bank_curr + 1)) - offset;
+ if (len < remain_len)
+ read_len = len;
+ else
+ read_len = remain_len;
#endif
-
+ if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
if (len < rem_bank_len)
read_len = len;
else
read_len = rem_bank_len;
-
+ }
if (read_len == 0)
return -EIO;
"
Why is there this part of code twice now, ifdeffed out differently in
each case ?
"
if (len < rem_bank_len)
read_len = len;
else
read_len = rem_bank_len;
"
More information about the U-Boot
mailing list