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;
 	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,
 };
 
-- 
2.25.1

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

Hi Tudor,

Tudor Ambarus <tudor.ambarus@linaro.org> 於 2025年4月2日 週三 下午8:10寫道：
>
>
>
> 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.

Yes, this part indeed only requires adding the late_init fixup
for older versions of JESD216 flash, rather than a manufacturer-level
late_init. I will make the necessary modifications and send out v2.
Thank you for your correction!

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

Thanks,
Cheng Ming Lin