| Message ID | 20240926141956.2386374-7-alvinzhou.tw@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | afe1ea1344bbd3a40be5a2547ff1b13899c5a7fa |
| Headers | show |
| Series | Add octal DTR support for Macronix flash | expand |
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
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)