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

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

[Miquel Raynal]

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

Best regards,
-- 
Miquel Raynal <miquel.raynal@bootlin.com>

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

[Michael Walle]

On Fri, 11 Sep 2026 19:22:45 +0200, 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.
> 
> [...]

Applied, thanks!

[1/1] mtd: spi-nor: Fix quad-enable for flashes with QER bit in SR1
      commit: 90f242c92d3ab241f0d22e2368b57750b8755667

Best regards,
-- 
Michael Walle <mwalle@kernel.org>

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