On 12/20/24 3:30 PM, Venkatesh Yadav Abbarapu wrote:
[...]
@@ -1578,8 +1580,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 +1595,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;
size_t read_len should be declared at the beginning of this function.
offset = from;
- if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
The indent here needs to be pushed on tab RIGHT -> .
Do you need this functionality in SPL ? If so, you might need a matching Kconfig SPL_SPI_STACKED_PARALLEL symbol .
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 +1620,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;
Is this calculation correct?
It used to be calculated as:
bank = (u32)from / SZ_16M; rem_bank_len = SZ_16M * (bank + 1); rem_bank_len -= from;
Now the bank is calculated as:
bank = nor->bank_curr + 1;
?
Why not keep calculating bank from offset ?
Maybe this will work ?
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 691962ef271..bdfbd44f30c 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1578,10 +1578,11 @@ 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); - int ret; loff_t offset = from; u32 rem_bank_len = 0; u32 stack_shift = 0; + size_t read_len; + int ret; u8 bank; bool is_ofst_odd = false;
@@ -1595,7 +1596,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, }
while (len) { - size_t read_len = len; + read_len = len; offset = from;
if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) { @@ -1627,24 +1628,24 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, offset /= 2; }
-#ifdef CONFIG_SPI_FLASH_BAR - u32 remain_len; + if (CONFIG_IS_ENABLED(SPI_FLASH_BAR)) { + bank = (u32)from / SZ_16M; + rem_bank_len = SZ_16M * (bank + 1); + rem_bank_len -= from;
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 (CONFIG_IS_ENABLED(SPI_FLASH_BAR) || + 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;
I am still worried about using stacked parallel and BAR together, is that even possible or are they mutually exclusive ?