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;elseread_len = remain_len;
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 ?