Re: [PATCH 3/3] mtd: spi-nor: macronix: Move macronix_nor_default_init logic to macronix_nor_late_init

[Tudor Ambarus <tudor.ambarus@linaro.org>]

On 4/2/25 9:51 AM, Cheng Ming Lin wrote:
> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
> 
> Remove macronix_nor_default_init and move its functionality to
> macronix_nor_late_init to ensure proper quad_enable initialization.
> 
> For MX25L3255E, SFDP follows JESD216, which does not include the Quad
> Enable bit Requirements field in its version. When the size field is
> removed, manufacturer->fixups->default_init hook is not executed, causing
> params->quad_enable not being overwritten with the intended function.
> Consequently, it remains as the default spi_nor_sr2_bit1_quad_enable.
> 
> By moving quad_enable setup from default_init to late_init, quad_enable
> is correctly assigned after spi_nor_init_params, regardless of the size
> field removal.
> 
> Additionally, according to spi-nor/core.h, quad_enable is more
> appropriately placed in late_init, as older SFDP versions did not define
> the Quad Enable bit Requirements. This change removes default_init and
> moves quad_enable handling to late_init accordingly.
> 
> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/macronix.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 07e0bd0b70a0..216c02b92bfe 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -282,22 +282,17 @@ static int macronix_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
>  	return enable ? macronix_nor_octal_dtr_en(nor) : macronix_nor_octal_dtr_dis(nor);
>  }
>  
> -static void macronix_nor_default_init(struct spi_nor *nor)
> -{
> -	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> -}
> -
>  static int macronix_nor_late_init(struct spi_nor *nor)
>  {
>  	if (!nor->params->set_4byte_addr_mode)
>  		nor->params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_en4b_ex4b;
> +	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;

Not at manufacturer level, please. You change the behavior and overwrite
whatever is retrieved from SFDP for all the flashes. Instead you should
introduce late init just for MX25L3255E, because set_4byte_addr_mode is
not covered in rev A of JESD216 I assume. Then if other flashes get this
param wrong from BFPT, amend them with post_bfpt hooks.


>  	nor->params->set_octal_dtr = macronix_nor_set_octal_dtr;
>  
>  	return 0;
>  }
>  
>  static const struct spi_nor_fixups macronix_nor_fixups = {
> -	.default_init = macronix_nor_default_init,
>  	.late_init = macronix_nor_late_init,
>  };
>