[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


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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 !

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: spi-nor: macronix: unhandled -EOPNOTSUPP in mx25l51245_late_init_fixups()
--

commit 98a395302d0bbf059e1d12c88df4c630f7de23f8
Author: Marek Vasut <marex@nabladev.com>

mtd: spi-nor: Describe locking support for MX25L51245G

This patch introduces a .late_init fixup for MX25L51245G (JEDEC ID 0xc2201a)
that reads the configuration register (opcode 0x15) to check the OTP TB bit
and conditionally enables 4-BP top-only locking support.

> 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;

[Severity: High]
Could returning -EOPNOTSUPP from mx25l51245_late_init_fixups() cause a probe
regression on controllers with restricted opcode sets?

During probe, the call chain is:

  spi_nor_probe()
    spi_nor_scan()
      spi_nor_init_params()
        spi_nor_late_init_params()
          mx25l51245_late_init_fixups()
            mx25l51245_read_cr()
              spi_mem_exec_op()

In drivers/spi/spi-mem.c:spi_mem_exec_op():

	if (!spi_mem_internal_supports_op(mem, op))
		return -EOPNOTSUPP;

On restricted SPI controllers such as drivers/spi/spi-intel.c,
intel_spi_supports_mem_op() checks the operation against
INTEL_SPI_GENERIC_OPS via intel_spi_match_mem_op():

	iop = intel_spi_match_mem_op(ispi, op);
	if (!iop) {
		dev_dbg(ispi->dev, "%#x not supported\n", op->cmd.opcode);
		return false;
	}

Previously, flashes matching JEDEC ID 0xc2201a used macronix_qpp4b_fixups and
probed using standard opcodes present in INTEL_SPI_GENERIC_OPS. Because
MX25L51245G_OP_RDCR (0x15) is not in INTEL_SPI_GENERIC_OPS,
mx25l51245_read_cr() returns -EOPNOTSUPP, which causes
spi_nor_late_init_params() and spi_nor_scan() to fail probe entirely.

Should mx25l51245_late_init_fixups() ignore -EOPNOTSUPP (leaving
SNOR_F_HAS_LOCK unset and returning 0) when the controller does not support
reading CR?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024001658.222147-1-marex@nabladev.com?part=1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/