[PATCH v2] mtd: spi-nor: Describe locking support for MX25L51245G

[PATCH v2] mtd: spi-nor: Describe locking support for MX25L51245G

[Marek Vasut]

The MX25L51245G part has similar 4-BP locking mechanism as MX25L12805D,
however, the TB bit on this part is OTP. Introduce support for Top-only
locking in case the TB bit is not set, and disable locking in case the
TB bit is fused i.e. set to Bottom mode.

Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Cheng Ming Lin <chengminglin@mxic.com.tw>
Cc: Michael Walle <mwalle@kernel.org>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Pratyush Yadav <pratyush@kernel.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: Takahiro Kuwano <takahiro.kuwano@infineon.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
---
V2: Introduce .late_init callback which enables locking only in case
    the TB bit is not set
---
 drivers/mtd/spi-nor/macronix.c | 53 +++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index b3fdb4b2fa1ca..11e71e254a54a 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -15,6 +15,9 @@
 #define MXIC_NOR_REG_DOPI_EN	0x2		/* Enable Octal DTR */
 #define MXIC_NOR_REG_SPI_EN	0x0		/* Enable SPI */
 
+#define MX25L51245G_OP_RDCR	0x15
+#define MX25L51245G_CR_TB	BIT(3)
+
 /* Convert dummy cycles to bit pattern */
 #define MXIC_NOR_REG_DC(p) \
 	((20 - (p)) >> 1)
@@ -25,6 +28,49 @@
 		   SPI_MEM_OP_NO_DUMMY,				\
 		   SPI_MEM_OP_DATA_OUT(ndata, buf, 0))
 
+static int mx25l51245_read_cr(struct spi_nor *nor, u8 *cr)
+{
+	int ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(MX25L51245G_OP_RDCR, 0),
+						  SPI_MEM_OP_NO_ADDR, SPI_MEM_OP_NO_DUMMY,
+						  SPI_MEM_OP_DATA_IN(1, cr, 0));
+
+		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = spi_nor_controller_ops_read_reg(nor, MX25L51245G_OP_RDCR, cr, 1);
+	}
+
+	if (ret)
+		dev_dbg(nor->dev, "error %d reading CR\n", ret);
+
+	return ret;
+}
+
+static int mx25l51245_late_init_fixups(struct spi_nor *nor)
+{
+	int ret;
+
+	ret = mx25l51245_read_cr(nor, nor->bouncebuf);
+	if (ret)
+		return ret;
+
+	/*
+	 * MX25L51245G supports locking, but the CR register TB bit is OTP.
+	 * Enable locking only in case the TB bit is not programmed, or set
+	 * to 0.
+	 */
+	if (nor->bouncebuf[0] & MX25L51245G_CR_TB)
+		dev_dbg(nor->dev, "CR TB is set, bottom protection not supported\n");
+	else
+		nor->params->flags |= SNOR_F_HAS_LOCK | SNOR_F_HAS_4BIT_BP;
+
+	return 0;
+}
+
 static int
 mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
 			    const struct sfdp_parameter_header *bfpt_header,
@@ -108,6 +154,11 @@ static const struct spi_nor_fixups mx25l25635_fixups = {
 	.post_sfdp = macronix_qpp4b_post_sfdp_fixups,
 };
 
+static const struct spi_nor_fixups mx25l51245_fixups = {
+	.late_init = mx25l51245_late_init_fixups,
+	.post_sfdp = macronix_qpp4b_post_sfdp_fixups,
+};
+
 static const struct spi_nor_fixups macronix_qpp4b_fixups = {
 	.post_sfdp = macronix_qpp4b_post_sfdp_fixups,
 };
@@ -353,7 +404,7 @@ static const struct spi_nor_fixup macronix_fixups[] = {
 	{ .fixups = &macronix_nor_fixups },
 	{ .id = SNOR_ID(0xc2, 0x20, 0x18), .fixups = &mx25l12805d_4pp3b_fixups },
 	{ .id = SNOR_ID(0xc2, 0x20, 0x19), .fixups = &mx25l25635_fixups },
-	{ .id = SNOR_ID(0xc2, 0x20, 0x1a), .fixups = &macronix_qpp4b_fixups,
+	{ .id = SNOR_ID(0xc2, 0x20, 0x1a), .fixups = &mx25l51245_fixups,
 	  .fixup_flags = SPI_NOR_4B_OPCODES },
 	{ .id = SNOR_ID(0xc2, 0x20, 0x1b), .fixups = &macronix_qpp4b_fixups },
 	{ .id = SNOR_ID(0xc2, 0x20, 0x1c), .fixups = &macronix_qpp4b_fixups },
-- 
2.53.0

Re: [PATCH v2] mtd: spi-nor: Describe locking support for MX25L51245G

[Marek Vasut]

On 10/24/26 2:16 AM, Marek Vasut wrote:
> The MX25L51245G part has similar 4-BP locking mechanism as MX25L12805D,
> however, the TB bit on this part is OTP. Introduce support for Top-only
> locking in case the TB bit is not set, and disable locking in case the
> TB bit is fused i.e. set to Bottom mode.
Cheng Ming , can you (or someone from Macronix) please have a look at 
this patch and confirm that it is OK ?

Thank you for your help !

Re: [PATCH v2] mtd: spi-nor: Describe locking support for MX25L51245G

[Cheng Ming Lin]

Hi Marek,

On 10/24/26 2:18 AM, Marek Vasut wrote:
> Cheng Ming , can you (or someone from Macronix) please have a look at
> this patch and confirm that it is OK ?

I checked the patch against the MX25L51245G datasheet. RDCR (0x15),
CR bit 3 as TB, the TB bit being OTP, and the 4-BP locking scheme all
match the datasheet, so the description is correct from our side.

One concern, in line with the sashiko-bot comment: since locking is
an optional feature, I think failing to read CR should not fail the
probe. Controllers with a restricted opcode set (e.g. spi-intel) will
return -EOPNOTSUPP for 0x15, and this part probes fine on them today
with macronix_qpp4b_fixups. Rather than special-casing -EOPNOTSUPP,
how about just leaving locking disabled on any CR read error, e.g.:

	/*
	 * Locking is optional. If CR cannot be read (e.g. the controller
	 * does not support RDCR), leave locking disabled instead of
	 * failing the probe.
	 */
	if (mx25l51245_read_cr(nor, nor->bouncebuf))
		return 0;

Michael, a related question for you. Maarten posted a series last
year to make the RDCR opcode configurable in the core and use 0x15
for Macronix:

https://lore.kernel.org/linux-mtd/20250922155635.749975-1-maarten@zanders.be/

It stalled because setting 0x15 for all Macronix parts is wrong (e.g.
MX25L12805D has no RDCR), and because there was no user of RDCR at the
time. This patch is now such a user.

I have gone through the flashes currently in the Macronix ID table
and checked which ones do not have a CR / do not support RDCR. If it
helps, I can add this information per flash entry, so that 0x15 is
only used for the parts that actually support it. For IDs that are
shared by several parts, RDCR would only be enabled if all of them
support it. That should address the concern on Maarten's series, and
this patch could then simply use spi_nor_read_cr() instead of
implementing its own RDCR helper in macronix.c.

Would that be an acceptable direction?

Thanks,
Cheng Ming

Re: [PATCH v2] mtd: spi-nor: Describe locking support for MX25L51245G

[Marek Vasut]

On 10/28/26 9:15 AM, Cheng Ming Lin wrote:

Hello Cheng Ming,

> On 10/24/26 2:18 AM, Marek Vasut wrote:
>> Cheng Ming , can you (or someone from Macronix) please have a look at
>> this patch and confirm that it is OK ?
> 
> I checked the patch against the MX25L51245G datasheet. RDCR (0x15),
> CR bit 3 as TB, the TB bit being OTP, and the 4-BP locking scheme all
> match the datasheet, so the description is correct from our side.
> 
> One concern, in line with the sashiko-bot comment: since locking is
> an optional feature, I think failing to read CR should not fail the
> probe. Controllers with a restricted opcode set (e.g. spi-intel) will
> return -EOPNOTSUPP for 0x15, and this part probes fine on them today
> with macronix_qpp4b_fixups. Rather than special-casing -EOPNOTSUPP,
> how about just leaving locking disabled on any CR read error, e.g.:
> 
> 	/*
> 	 * Locking is optional. If CR cannot be read (e.g. the controller
> 	 * does not support RDCR), leave locking disabled instead of
> 	 * failing the probe.
> 	 */
> 	if (mx25l51245_read_cr(nor, nor->bouncebuf))
> 		return 0;

I can add an explicit check, if (ret == -EOPNOTSUPP) return 0; , that is 
probably better than blanked ignore of all errors ?

> Michael, a related question for you. Maarten posted a series last
> year to make the RDCR opcode configurable in the core and use 0x15
> for Macronix:
> 
> https://lore.kernel.org/linux-mtd/20250922155635.749975-1-maarten@zanders.be/
> 
> It stalled because setting 0x15 for all Macronix parts is wrong (e.g.
> MX25L12805D has no RDCR), and because there was no user of RDCR at the
> time. This patch is now such a user.
> 
> I have gone through the flashes currently in the Macronix ID table
> and checked which ones do not have a CR / do not support RDCR. If it
> helps, I can add this information per flash entry, so that 0x15 is
> only used for the parts that actually support it. For IDs that are
> shared by several parts, RDCR would only be enabled if all of them
> support it. That should address the concern on Maarten's series, and
> this patch could then simply use spi_nor_read_cr() instead of
> implementing its own RDCR helper in macronix.c.
> 
> Would that be an acceptable direction?

I'll wait for input from Michael .

Thank you for your help !

-- 
Best regards,
Marek Vasut