Re: [PATCH v2] mtd: spi-nor: Fix the spi_nor_read() when config SPI_STACKED_PARALLEL is enabled

[Jon Humphreys <j-humphreys@ti.com>]

Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com> writes:

> 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@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);
>  		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 (read_len == 0)
>  			return -EIO;
>  
> -- 
> 2.34.1

Tested-by: Jonathan Humphreys <j-humphreys@ti.com>