From: AlvinZhou <alvinzhou@mxic.com.tw>

Adding manufacturer ID 0xC2 at the end of ID table
to allow manufacturer fixup to be applied for any
Macronix flashes instead of needing to list each
flash ID in the ID table.

Such as macronix_nor_set_octal_dtr function in the
manufacturer fixup can be applied to any Macronix
Octal Flashes without the need to add the specific
ID in the ID table.

Suggested-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
Signed-off-by: AlvinZhou <alvinzhou@mxic.com.tw>
---
 drivers/mtd/spi-nor/macronix.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index f039819a5252..1a8ccebdfe0e 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -200,7 +200,9 @@ static const struct flash_info macronix_nor_parts[] = {
 		.name = "mx25l3255e",
 		.size = SZ_4M,
 		.no_sfdp_flags = SECT_4K,
-	}
+	},
+	/* Need the manufacturer fixups, Keep this last */
+	{ .id = SNOR_ID(0xc2) }
 };
 
 static int macronix_nor_octal_dtr_en(struct spi_nor *nor)
-- 
2.25.1

On 26.09.2024 17:19, AlvinZhou wrote:
> From: AlvinZhou <alvinzhou@mxic.com.tw>
> 
> Adding manufacturer ID 0xC2 at the end of ID table
> to allow manufacturer fixup to be applied for any
> Macronix flashes instead of needing to list each
> flash ID in the ID table.
> 
> Such as macronix_nor_set_octal_dtr function in the
> manufacturer fixup can be applied to any Macronix
> Octal Flashes without the need to add the specific
> ID in the ID table.
> 
> Suggested-by: Michael Walle <mwalle@kernel.org>
> Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> Signed-off-by: AlvinZhou <alvinzhou@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/macronix.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index f039819a5252..1a8ccebdfe0e 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -200,7 +200,9 @@ static const struct flash_info macronix_nor_parts[] = {
>  		.name = "mx25l3255e",
>  		.size = SZ_4M,
>  		.no_sfdp_flags = SECT_4K,
> -	}
> +	},
> +	/* Need the manufacturer fixups, Keep this last */

you have a capital letter in the middle of the sentence.

I'll replace the comment with:

/*

 * This spares us of adding new flash entries for flashes that can be
 * initialized solely based on the SFDP data, but still need the
 * manufacturer hooks to set parameters that can't be discovered at SFDP
 * parsing time.
 */

Which brings me to why you really set this. I remember SFDP contains
tables with sequence of commands for enabling/disabling Octal DTR mode.
Would you please remember me, why you didn't use those SFDP tables and
implemented your own enable/disable methods?

> +	{ .id = SNOR_ID(0xc2) }
>  };
>  
>  static int macronix_nor_octal_dtr_en(struct spi_nor *nor)

Hi Tudor,

Tudor Ambarus <tudor.ambarus@linaro.org> 於 2024年10月2日 週三 下午3:45寫道：
>
>
>
> On 26.09.2024 17:19, AlvinZhou wrote:
> > From: AlvinZhou <alvinzhou@mxic.com.tw>
> >
> > Adding manufacturer ID 0xC2 at the end of ID table
> > to allow manufacturer fixup to be applied for any
> > Macronix flashes instead of needing to list each
> > flash ID in the ID table.
> >
> > Such as macronix_nor_set_octal_dtr function in the
> > manufacturer fixup can be applied to any Macronix
> > Octal Flashes without the need to add the specific
> > ID in the ID table.
> >
> > Suggested-by: Michael Walle <mwalle@kernel.org>
> > Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> > Signed-off-by: AlvinZhou <alvinzhou@mxic.com.tw>
> > ---
> >  drivers/mtd/spi-nor/macronix.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> > index f039819a5252..1a8ccebdfe0e 100644
> > --- a/drivers/mtd/spi-nor/macronix.c
> > +++ b/drivers/mtd/spi-nor/macronix.c
> > @@ -200,7 +200,9 @@ static const struct flash_info macronix_nor_parts[] = {
> >               .name = "mx25l3255e",
> >               .size = SZ_4M,
> >               .no_sfdp_flags = SECT_4K,
> > -     }
> > +     },
> > +     /* Need the manufacturer fixups, Keep this last */
>
> you have a capital letter in the middle of the sentence.
>
> I'll replace the comment with:
>
> /*
>
>  * This spares us of adding new flash entries for flashes that can be
>  * initialized solely based on the SFDP data, but still need the
>  * manufacturer hooks to set parameters that can't be discovered at SFDP
>  * parsing time.
>  */
>
> Which brings me to why you really set this. I remember SFDP contains
> tables with sequence of commands for enabling/disabling Octal DTR mode.
> Would you please remember me, why you didn't use those SFDP tables and
> implemented your own enable/disable methods?

While the SFDP does provide a sequence of commands to enable Octal
DDR mode, following this sequence forces the I/O driver strength to 50 ohms,
which causes I/O driver strength to be weak and and leads to read/write
issues, so we chose to use a fixup approach to enable/disable Octal DDR
mode.

Thanks,
Alvin