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

[sashiko-bot@kernel.org]

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/