On 12/16/24 2:58 PM, Abbarapu, Venkatesh wrote:
+++ 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 ?
In the spi_nor_erase()
offset = addr;
if(PARALLEL)
offset/=2;
so for parallel or single configuration we need to pass "offset" to
write_bar()
write_bar(nor, offset");
The code above likely needs to be compiled out if the SPI_STACKED_PARALLEL
stuff is disabled ?
[...]
Already the code here is being checked with the flags SNOR_F_HAS_PARALLEL and
SNOR_F_HAS_STACKED. Do you want to add the check SPI_STACKED_PARALLEL apart
from these flags?
Yes, to compile the code out completely.
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; "
For parallel/stacked configuration and address width the "rem_bank_len" will
vary
and as we don't want to disturb the default read functionality added the ifdef
separately.
What would happen if both SPI_FLASH_BAR and SPI_STACKED_PARALLEL are
enabled on a system that only has one SPI NOR attached
(non-stacked/parallel) ? I noticed the second "copy" of the code behaves
slightly
differently in the else branch, so does that mean this would break such setup ?
If both SPI_FLASH_BAR and SPI_STACKED_PARALLEL are enabled, the "rem_bank_len"
manipulation is done under the CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL) code and this
won't break any default functionality.
Wouldn't read_len calculation be done twice ?
