Hi,
On 01/10/20 01:56AM, Bert Vermeulen wrote:
> Flash chips that announce BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 capability
> get an addr_width of 3. This breaks when the flash chip is actually
> larger than 16MB, since that requires a 4-byte address. The MX25L25635F
> does exactly this, breaking anything over 16MB.
>
> spi-nor only enables 4-byte opcodes or 4-byte address mode if addr_width
> is 4, so no 4-byte mode is ever enabled. The > 16MB check in
> spi_nor_set_addr_width() only works if addr_width wasn't already set
> by the SFDP, which it was.
>
> It could be fixed in a post_bfpt fixup for the MX25L25635F, but setting
> addr_width to 4 when BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 is found fixes the
> problem for all such cases.
JESD216D.01 says: "01b: 3- or 4-Byte addressing (e.g., defaults to
3-Byte mode; enters 4-Byte mode on command)"
So using an address width of 4 here is not necessarily the right thing
to do. This change would break SMPT parsing for all flashes that use
3-byte addressing by default because SMPT parsing can involve register
reads/writes. One such device is the Cypress S28HS flash. In fact, this
was what prompted me to write the patch [0].
Before that patch, how did MX25L25635F decide to use 4-byte addressing?
Coming out of BFPT parsing addr_width would still be 0. My guess is that
it would go into spi_nor_set_addr_width() with addr_width still 0 and
then the check for (nor->mtd.size > 0x1000000) would set it to 4. Do I
guess correctly?
In that case maybe we can do a better job of deciding what gets priority
in the if-else chain. For example, giving addr_width from nor->info
precedence over the one configured by SFDP can solve this problem. Then
all you have to do is set the addr_width in the info struct, which is
certainly easier than adding a fixup hook. There may be a more elegant
solution to this but I haven't given it much thought.
So from my side, NACK!
>
> Signed-off-by: Bert Vermeulen <b...@biot.com>
> ---
> drivers/mtd/spi-nor/sfdp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index e2a43d39eb5f..6fedc425bcf7 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -456,10 +456,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> /* Number of address bytes. */
> switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> - case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> nor->addr_width = 3;
> break;
>
> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> nor->addr_width = 4;
> break;
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f9acd7fa80be6ee14aecdc54429f2a48e56224e8
--
Regards,
Pratyush Yadav
Texas Instruments India
