Re: [PATCH v2 3/3] mtd: spi-nor: macronix: Add fixups for MX25L3255E

[Pratyush Yadav <pratyush@kernel.org>]

On Mon, Apr 07 2025, Cheng Ming Lin wrote:

> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>
> SFDP of MX25L3255E is JESD216, which does not include the Quad
> Enable bit Requirement in BFPT. As a result, during BFPT parsing,
> the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
> Therefore, it is necessary to correct this setting by late_init.
>
> In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
> address mode. However, since the 3-byte address 1-4-4 page program
> is not defined in SFDP, it needs to be configured in late_init.
>
> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/macronix.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 07e0bd0b70a0..4bbd3b651cec 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -58,6 +58,31 @@ macronix_qpp4b_post_sfdp_fixups(struct spi_nor *nor)
>  	return 0;
>  }
>  
> +static int
> +mx25l3255e_late_init_fixups(struct spi_nor *nor)
> +{
> +	/*
> +	 * SFDP of MX25L3255E is JESD216, which does not include the Quad
> +	 * Enable bit Requirement in BFPT. As a result, during BFPT parsing,
> +	 * the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
> +	 * Therefore, it is necessary to correct this setting by late_init.
> +	 */
> +	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> +
> +	/*
> +	 * In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
> +	 * address mode. However, since the 3-byte address 1-4-4 page program
> +	 * is not defined in SFDP, it needs to be configured in late_init.
> +	 */
> +	struct spi_nor_flash_parameter *params = nor->params;

Please don't have variable declarations in the middle of the function.
It looks odd. Though neither checkpatch nor cc seem to complain about
this. I thought they did...

Anyway, no need for a new revision, this can be fixed when applying by
me or Tudor.

Acked-by: Pratyush Yadav <pratyush@kernel.org>

> +
> +	params->hwcaps.mask |= SNOR_HWCAPS_PP_1_4_4;
> +	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_4_4],
> +				SPINOR_OP_PP_1_4_4, SNOR_PROTO_1_4_4);
> +
> +	return 0;
> +}
> +
[...]

-- 
Regards,
Pratyush Yadav