Re: [PATCH v4] mtd: spi-nor: Fix quad-enable for flashes with QER bit in SR1

Re: [PATCH v4] mtd: spi-nor: Fix quad-enable for flashes with QER bit in SR1

[Jon Hunter]

On 11/09/2026 18:22, Miquel Raynal wrote:
> Some flashes (eg. from Macronix) do set BFPT_DWORD15_QER_SR1_BIT6, which
> means they do not have an SR2 to read from/write to. The new generic QE
> helper was supposed to accommodate this situation but in the last version
> that got merged, parts of that specific handling has been moved to a
> more contained location, swp.c (which needed most of the extra code),
> yet the Macronix case has been forgotten about in that generic QE
> handling helper. Booting with such flashes will always fail probing.
> 
> Fix the situation by making sure SR2 reads just return 0 if
> unsupported. This is safe since there is no chip with a write SR2 path
> but no read SR2 path (which is now enforced in the SFDP parsing step).
> 
> This way, callers still do not have to care about the internal device
> capabilities. Calling sr1_and_sr2 read/write helpers is safe in both
> directions (not risk to get a spurious error). The behavior for SR1-only
> chips is respected, the complexity in the core kept to its minimum.
> 
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Closes: https://lore.kernel.org/linux-mtd/178876719232.3543902.14451625037676421254.b4-ty@b4/T/#m5bc4ba6776436f2870ced0eb5789d229037ad840
> Fixes: 63489002d397 ("mtd: spi-nor: Refactor Read Status/Write Status support")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> Changes in v4:
> - Make sure we perfom the "_and_check" over SR2 only if SR2 can be read
>    back, otherwise it does not make much sense to do it.
> - Link to v3: https://lore.kernel.org/r/20260911-perso-fix-spi-nor-qe-mxic-v3-1-2ecaefb2ef2c@bootlin.com
> 
> Changes in v3:
> - Make sure the read helper returns a "valid" SR2 in the sense that it
>    cannot be random data by returning 0 for chips that do not feature a
>    read_sr2 opcode. The major thread was to get a wrong comparison
>    (against the tmp variable) in the _and_check() helper.
> - Link to v2: https://lore.kernel.org/r/20260911-perso-fix-spi-nor-qe-mxic-v2-1-70c324e9f30e@bootlin.com
> 
> Changes in v2:
> - Change the approach, see v1 thread below.
> - Link to v1: https://lore.kernel.org/r/20260911-perso-fix-spi-nor-qe-mxic-v1-1-fd6d91416a2a@bootlin.com
> ---
>   drivers/mtd/spi-nor/core.c | 15 ++++++++++++---
>   drivers/mtd/spi-nor/sfdp.c |  3 ++-
>   2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index e2b6efafdd8d..4377b73e57fb 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -849,7 +849,8 @@ int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
>   
>   /**
>    * spi_nor_read_sr1_and_sr2() - Read SR1 then SR2
> - * General purpose helper.
> + * General purpose helper, always safe to call. Will expectedly ignore
> + * SR2 on certain chips.
>    *
>    * @nor: the spi_nor structure
>    * @sr: pointer to a valid 2-byte array
> @@ -864,7 +865,12 @@ int spi_nor_read_sr1_and_sr2(struct spi_nor *nor, u8 *sr)
>   	if (ret)
>   		return ret;
>   
> -	return spi_nor_read_sr2(nor, &sr[1]);
> +	if (nor->params->opcodes.read_sr2)
> +		ret = spi_nor_read_sr2(nor, &sr[1]);
> +	else
> +		sr[1] = 0;
> +
> +	return ret;
>   }
>   
>   /**
> @@ -960,7 +966,10 @@ int spi_nor_write_sr1_and_sr2_and_check(struct spi_nor *nor, const u8 *sr)
>   	if (ret)
>   		return ret;
>   
> -	if (sr[0] != tmp[0] || sr[1] != tmp[1])
> +	if (sr[0] != tmp[0])
> +		return -EIO;
> +
> +	if (nor->params->opcodes.read_sr2 && sr[1] != tmp[1])
>   		return -EIO;
>   
>   	return 0;
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index c21a6953db96..641f17ad51a0 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -656,7 +656,8 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>   
>   	/* opcodes sanity check */
>   	WARN_ON(!params->opcodes.read_sr1 ||
> -		(!params->opcodes.write_sr1 && !params->opcodes.write_sr1_and_sr2));
> +		(!params->opcodes.write_sr1 && !params->opcodes.write_sr1_and_sr2) ||
> +		(!params->opcodes.read_sr2 && params->opcodes.write_sr2));
>   
>   	dword = bfpt.dwords[SFDP_DWORD(16)] & BFPT_DWORD16_4B_ADDR_MODE_MASK;
>   	if (SFDP_MASK_CHECK(dword, BFPT_DWORD16_4B_ADDR_MODE_BRWR))
> 
> ---
> base-commit: 700bf34058ca7cd792236b1ba5caad3770d66208
> change-id: 20260911-perso-fix-spi-nor-qe-mxic-d0513e9f4421

Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks
Jon

-- 
nvpublic