Skip to contentSkip to conversation

mtd: spi-nor: allow force unlocking via DT property

PATCH In review Chen Minqiang 2 participants 4 messages Aug 6 – Sep 11 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

Hi,

Thank you for your valuable insight!

You are completely right that for unlisted/generic chips that feature a 4-bit BP layout (BP3 at bit 5 or bit 6) or a CMP (Complement Protect) bit in SR2, spi_nor_unlock() won't clear those extra bits without the corresponding flags (SNOR_F_HAS_4BIT_BP / SNOR_F_HAS_SR2_CMP_BIT6) set by the ID database or SFDP.

However, in practice:
1. The vast majority of 3.3V/1.8V generic SPI NOR flashes (e.g. 4MB-16MB chips commonly found in vendor devices like Tenda AX12L Pro) use the standard 3-bit BP (BP0-BP2, SR1 bits 2..4).
2. The current main issue is that even for these standard 3-bit BP chips, the kernel currently skips spi_nor_try_unlock_all() completely at boot time if CONFIG_MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE is set (for non-volatile chips) or if SNOR_F_HAS_LOCK is not set in chip flags. As a result, status registers locked by factory bootloaders are never cleared.

linux,force-sr-unlock serves as a pragmatic DT override to force the unlock attempt at probe time.

To address your point regarding 4-bit BP and CMP bits for unlisted chips, we have two potential options:

Option A (Current Best-Effort):
Keep the patch as-is, treating linux,force-sr-unlock as a best-effort DT trigger to invoke standard spi_nor_unlock(). It successfully unlocks the vast majority of standard 3-bit BP generic chips. For rare unlisted chips with 4-bit BP or CMP bits, explicit entries can still be added to the ID database when discovered.

Option B (Aggressive Force-Clear):
When linux,force-sr-unlock is present in DT, enhance spi_nor_try_unlock_all() to perform a broader clear operation on SR1 (masking bits 2..6 to clear BP0-BP3/TB) and SR2 (clearing CMP bit if SR2 is readable).

Which approach would you prefer? I'd be happy to revise the patch based on your guidance.

Best regards,
Chen Minqiang

maintainer
#2 lore raw
Quoted 8 lines · Chen Minqiang

Hi,

Thank you for your valuable insight!

You are completely right that for unlisted/generic chips that feature a 4-bit BP layout (BP3 at bit 5 or bit 6) or a CMP (Complement Protect) bit in SR2, spi_nor_unlock() won't clear those extra bits without the corresponding flags (SNOR_F_HAS_4BIT_BP / SNOR_F_HAS_SR2_CMP_BIT6) set by the ID database or SFDP.

Is this AI assisted?

Quoted 23 lines

However, in practice:
1. The vast majority of 3.3V/1.8V generic SPI NOR flashes (e.g. 4MB-16MB chips commonly found in vendor devices like Tenda AX12L Pro) use the standard 3-bit BP (BP0-BP2, SR1 bits 2..4).
2. The current main issue is that even for these standard 3-bit BP chips, the kernel currently skips spi_nor_try_unlock_all() completely at boot time if CONFIG_MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE is set (for non-volatile chips) or if SNOR_F_HAS_LOCK is not set in chip flags. As a result, status registers locked by factory bootloaders are never cleared.

linux,force-sr-unlock serves as a pragmatic DT override to force the unlock attempt at probe time.

To address your point regarding 4-bit BP and CMP bits for unlisted chips, we have two potential options:

Option A (Current Best-Effort):
Keep the patch as-is, treating linux,force-sr-unlock as a best-effort DT trigger to invoke standard spi_nor_unlock(). It successfully unlocks the vast majority of standard 3-bit BP generic chips. For rare unlisted chips with 4-bit BP or CMP bits, explicit entries can still be added to the ID database when discovered.

Quoted 7 lines

Option B (Aggressive Force-Clear):
When linux,force-sr-unlock is present in DT, enhance spi_nor_try_unlock_all() to perform a broader clear operation on SR1 (masking bits 2..6 to clear BP0-BP3/TB) and SR2 (clearing CMP bit if SR2 is readable).

Which approach would you prefer? I'd be happy to revise the patch based on your guidance.

This patch won't fly as is. You're missing the dt-bindings patch (which I doubt will be accepted) and I really don't like this approach. First, why would you unlock a flash automatically? I've worked hard, to get rid of that anti-feature. Second, what if you still loose the flash "lottery"?

-michael

#3 lore raw

PATCH v2 1/1 mtd: spi-nor: allow force unlocking via DT property

In reply to a message not in the archive

Some SPI NOR flash chips (such as generic or unlisted chips used in vendor devices like Tenda AX12L Pro) have Block Protection (BP) bits set in the Status Register by bootloaders or factory settings, locking flash blocks.

Because vendors frequently switch between various generic SPI NOR flash chips ("Flash Lottery"), it is impractical to upstream explicit chip ID flags (SNOR_F_HAS_LOCK) for every possible generic chip variant.

This patch introduces support for the "linux,force-sr-unlock" Device Tree property:
1. In spi_nor_init_flags(), set the SNOR_F_HAS_LOCK flag on the flash instance if "linux,force-sr-unlock" is present in the flash DT node. This allows spi_nor_late_init_params() to automatically populate default locking_ops without altering swp.c.
2. In spi_nor_init(), trigger spi_nor_try_unlock_all() if "linux,force-sr-unlock" is set, invoking Linux kernel's native spi_nor_unlock() mechanism.

---
v1 -> v2:
- Set SNOR_F_HAS_LOCK in spi_nor_init_flags() when "linux,force-sr-unlock" is present in DT, allowing spi_nor_late_init_params() to set default locking_ops automatically without modifying swp.c.

---
 drivers/mtd/spi-nor/core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/mtd/spi-nor/core.c +5 -1
index ccf4396cdcd0..9aa2aaa10c38 100644
@@ -2956,6 +2956,9 @@ static void spi_nor_init_flags(struct spi_nor *nor)
 	if (of_property_read_bool(np, "no-wp"))
 		nor->flags |= SNOR_F_NO_WP;
 
+	if (of_property_read_bool(np, "linux,force-sr-unlock"))
+		nor->flags |= SNOR_F_HAS_LOCK;
+
 	if (flags & SPI_NOR_SWP_IS_VOLATILE)
 		nor->flags |= SNOR_F_SWP_IS_VOLATILE;
 
@@ -3332,7 +3335,8 @@ static int spi_nor_init(struct spi_nor *nor)
 	spi_nor_cache_sr_lock_bits(nor, NULL);
 	if (IS_ENABLED(CONFIG_MTD_SPI_NOR_SWP_DISABLE) ||
 	    (IS_ENABLED(CONFIG_MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE) &&
-	     nor->flags & SNOR_F_SWP_IS_VOLATILE)) {
+	     nor->flags & SNOR_F_SWP_IS_VOLATILE) ||
+	    of_property_read_bool(spi_nor_get_flash_node(nor), "linux,force-sr-unlock")) {
 		spi_nor_try_unlock_all(nor);
 	}
 
Signature · 1 line
2.17.1
maintainer
#4 lore raw
Quoted 17 lines · Chen Minqiang

Some SPI NOR flash chips (such as generic or unlisted chips used in vendor devices like Tenda AX12L Pro) have Block Protection (BP) bits set in the Status Register by bootloaders or factory settings, locking flash blocks.

Because vendors frequently switch between various generic SPI NOR flash chips ("Flash Lottery"), it is impractical to upstream explicit chip ID flags (SNOR_F_HAS_LOCK) for every possible generic chip variant.

This patch introduces support for the "linux,force-sr-unlock" Device Tree property:
1. In spi_nor_init_flags(), set the SNOR_F_HAS_LOCK flag on the flash instance if "linux,force-sr-unlock" is present in the flash DT node. This allows spi_nor_late_init_params() to automatically populate default locking_ops without altering swp.c.
2. In spi_nor_init(), trigger spi_nor_try_unlock_all() if "linux,force-sr-unlock" is set, invoking Linux kernel's native spi_nor_unlock() mechanism.

Quoted 6 lines

---
v1 -> v2:
- Set SNOR_F_HAS_LOCK in spi_nor_init_flags() when "linux,force-sr-unlock" is present in DT, allowing spi_nor_late_init_params() to set default locking_ops automatically without modifying swp.c.

I've given you feedback on the v1, there was no reply, so I'm going to ignore this.

-michael