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@xxxxxxxxxx/

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