[PATCH] mtd: spi-nor: Fix the read issue when addr_width is 4byte

[Tudor Ambarus]

+ Marek

use get_maintainers script please

On 11/18/24 9:05 AM, Venkatesh Yadav Abbarapu wrote:
> 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>
> ---
>  drivers/mtd/spi/spi-nor-core.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index ec841fb13b..c37a9bce74 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -1580,6 +1580,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>  	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 +1594,20 @@ 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;
> -
> +		if (nor->addr_width == 3) {

No, this looks bad because you enter the body of this for all the
flashes with addr_width 3, not just for the parallel/stacked ones.

Instead you should do whatever you do here just for your
parallel/stacked cases.

> +			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;
> +		}
>  		offset = 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;
> @@ -1616,6 +1619,9 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>  		if (nor->flags & SNOR_F_HAS_PARALLEL)
>  			offset /= 2;
>  
> +		if (nor->addr_width == 4)
> +			rem_bank_len = (mtd->size >> stack_shift) - offset;

same here.

If we still want to keep this support until everything is implemented
above SPI NOR, other option is to duplicate core methods and introduce
dedicated methods for the parallel/stacked cases. You could keep the
parallel/stacked methods in dedicated file as well.

The advantage is that you won't interfere with current SPI NOR support
and that you won't break any flashes anymore. Then we can easily remove
everything once proper support is implemented above SPI NOR. The
disadvantage is that we duplicate code ... Marek?

> +
>  #ifdef CONFIG_SPI_FLASH_BAR
>  		ret = write_bar(nor, offset);
>  		if (ret < 0)