| Message ID | 20240415075755.780653-1-ada@thorsis.com |
|---|---|
| State | Under Review |
| Delegated to: | Dario Binacchi |
| Headers | show |
| Series | mtd: nand: raw: Collected improvements | expand |
Hei hei, Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl: > From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems > like we have to wait tREA after rising RE# before sampling the data. > Thus pulse time must be at least tREA. > > Without this fix we got PMECC errors when reading, after switching to > ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip. > > The approach to set timings used before worked on sam9g20 and sama5d2 > with the same flash (S34ML02G1), probably because those have a slower > mck clock rate and thus the resolution of the timings setup is not as > tight as with sam9x60. > > The approach to fix the issue was carried over from at91bootstrap, and > has been successfully tested in at91bootstrap, U-Boot and Linux. > > Link: https://github.com/linux4sam/at91bootstrap/issues/174 > Cc: Li Bin <bin.li@microchip.com> > Signed-off-by: Alexander Dahl <ada@thorsis.com> > --- > > Notes: > v3: > - initial patch version (not present in v1 and v2) This patch was send as part of a series, which you wanted to have some more time to test. Besides that, has anyone looked into this particular fix? Maybe it can be applied separately? Greets Alex > > drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c > index bbafc88e44c..00ffeadd113 100644 > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c > @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > const struct nand_data_interface *conf, > struct atmel_smc_cs_conf *smcconf) > { > - u32 ncycles, totalcycles, timeps, mckperiodps; > + u32 ncycles, totalcycles, timeps, mckperiodps, pulse; > struct atmel_nand_controller *nc; > int ret; > > @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > ATMEL_SMC_MODE_TDFMODE_OPTIMIZED; > > /* > - * Read pulse timing directly matches tRP: > + * Read pulse timing would directly match tRP, > + * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF) > + * do not work properly in timing mode 3. > + * The workaround is to extend the SMC NRD pulse to meet tREA > + * timing. > * > - * NRD_PULSE = tRP > + * NRD_PULSE = max(tRP, tREA) > */ > - ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps); > + pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max); > + ncycles = DIV_ROUND_UP(pulse, mckperiodps); > totalcycles += ncycles; > ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT, > ncycles); > -- > 2.39.2 >
Hello, Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl: > Hei hei, > > Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl: > > From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems > > like we have to wait tREA after rising RE# before sampling the data. > > Thus pulse time must be at least tREA. > > > > Without this fix we got PMECC errors when reading, after switching to > > ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip. > > > > The approach to set timings used before worked on sam9g20 and sama5d2 > > with the same flash (S34ML02G1), probably because those have a slower > > mck clock rate and thus the resolution of the timings setup is not as > > tight as with sam9x60. > > > > The approach to fix the issue was carried over from at91bootstrap, and > > has been successfully tested in at91bootstrap, U-Boot and Linux. > > > > Link: https://github.com/linux4sam/at91bootstrap/issues/174 > > Cc: Li Bin <bin.li@microchip.com> > > Signed-off-by: Alexander Dahl <ada@thorsis.com> > > --- > > > > Notes: > > v3: > > - initial patch version (not present in v1 and v2) > > This patch was send as part of a series, which you wanted to have some > more time to test. Besides that, has anyone looked into this > particular fix? Maybe it can be applied separately? I'd kindly ask what the status of this patch series is from U-Boot NAND subsystem maintainers POV? Could you test it? Should I rebase and resend? Two of these three patches are specific to at91 family, what's the opinion of at91 maintainers on this? Link to the series discussion for reference: https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u Greets Alex > > Greets > Alex > > > > > drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c > > index bbafc88e44c..00ffeadd113 100644 > > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c > > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c > > @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > > const struct nand_data_interface *conf, > > struct atmel_smc_cs_conf *smcconf) > > { > > - u32 ncycles, totalcycles, timeps, mckperiodps; > > + u32 ncycles, totalcycles, timeps, mckperiodps, pulse; > > struct atmel_nand_controller *nc; > > int ret; > > > > @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > > ATMEL_SMC_MODE_TDFMODE_OPTIMIZED; > > > > /* > > - * Read pulse timing directly matches tRP: > > + * Read pulse timing would directly match tRP, > > + * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF) > > + * do not work properly in timing mode 3. > > + * The workaround is to extend the SMC NRD pulse to meet tREA > > + * timing. > > * > > - * NRD_PULSE = tRP > > + * NRD_PULSE = max(tRP, tREA) > > */ > > - ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps); > > + pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max); > > + ncycles = DIV_ROUND_UP(pulse, mckperiodps); > > totalcycles += ncycles; > > ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT, > > ncycles); > > -- > > 2.39.2 > > >
On 9/30/24 11:07, Alexander Dahl wrote: > Hello, > > Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl: >> Hei hei, >> >> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl: >>> From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems >>> like we have to wait tREA after rising RE# before sampling the data. >>> Thus pulse time must be at least tREA. >>> >>> Without this fix we got PMECC errors when reading, after switching to >>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip. >>> >>> The approach to set timings used before worked on sam9g20 and sama5d2 >>> with the same flash (S34ML02G1), probably because those have a slower >>> mck clock rate and thus the resolution of the timings setup is not as >>> tight as with sam9x60. >>> >>> The approach to fix the issue was carried over from at91bootstrap, and >>> has been successfully tested in at91bootstrap, U-Boot and Linux. >>> >>> Link: https://github.com/linux4sam/at91bootstrap/issues/174 >>> Cc: Li Bin <bin.li@microchip.com> >>> Signed-off-by: Alexander Dahl <ada@thorsis.com> >>> --- >>> >>> Notes: >>> v3: >>> - initial patch version (not present in v1 and v2) >> >> This patch was send as part of a series, which you wanted to have some >> more time to test. Besides that, has anyone looked into this >> particular fix? Maybe it can be applied separately? > > I'd kindly ask what the status of this patch series is from U-Boot > NAND subsystem maintainers POV? Could you test it? Should I rebase > and resend? > > Two of these three patches are specific to at91 family, what's the > opinion of at91 maintainers on this? Hello Alexander, From at91 perspective I have no objections. As these are very NAND specific, I defer the patches to the NAND maintainers. Eugen > > Link to the series discussion for reference: > > https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u > > Greets > Alex > >> >> Greets >> Alex >> >>> >>> drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++---- >>> 1 file changed, 9 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c >>> index bbafc88e44c..00ffeadd113 100644 >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c >>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, >>> const struct nand_data_interface *conf, >>> struct atmel_smc_cs_conf *smcconf) >>> { >>> - u32 ncycles, totalcycles, timeps, mckperiodps; >>> + u32 ncycles, totalcycles, timeps, mckperiodps, pulse; >>> struct atmel_nand_controller *nc; >>> int ret; >>> >>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, >>> ATMEL_SMC_MODE_TDFMODE_OPTIMIZED; >>> >>> /* >>> - * Read pulse timing directly matches tRP: >>> + * Read pulse timing would directly match tRP, >>> + * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF) >>> + * do not work properly in timing mode 3. >>> + * The workaround is to extend the SMC NRD pulse to meet tREA >>> + * timing. >>> * >>> - * NRD_PULSE = tRP >>> + * NRD_PULSE = max(tRP, tREA) >>> */ >>> - ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps); >>> + pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max); >>> + ncycles = DIV_ROUND_UP(pulse, mckperiodps); >>> totalcycles += ncycles; >>> ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT, >>> ncycles); >>> -- >>> 2.39.2 >>> >>
Hi On Tue, Oct 8, 2024 at 1:41 PM Eugen Hristev <eugen.hristev@collabora.com> wrote: > > On 9/30/24 11:07, Alexander Dahl wrote: > > Hello, > > > > Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl: > >> Hei hei, > >> > >> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl: > >>> From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems > >>> like we have to wait tREA after rising RE# before sampling the data. > >>> Thus pulse time must be at least tREA. > >>> > >>> Without this fix we got PMECC errors when reading, after switching to > >>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip. > >>> > >>> The approach to set timings used before worked on sam9g20 and sama5d2 > >>> with the same flash (S34ML02G1), probably because those have a slower > >>> mck clock rate and thus the resolution of the timings setup is not as > >>> tight as with sam9x60. > >>> > >>> The approach to fix the issue was carried over from at91bootstrap, and > >>> has been successfully tested in at91bootstrap, U-Boot and Linux. > >>> > >>> Link: https://github.com/linux4sam/at91bootstrap/issues/174 > >>> Cc: Li Bin <bin.li@microchip.com> > >>> Signed-off-by: Alexander Dahl <ada@thorsis.com> > >>> --- > >>> > >>> Notes: > >>> v3: > >>> - initial patch version (not present in v1 and v2) > >> > >> This patch was send as part of a series, which you wanted to have some > >> more time to test. Besides that, has anyone looked into this > >> particular fix? Maybe it can be applied separately? > > > > I'd kindly ask what the status of this patch series is from U-Boot > > NAND subsystem maintainers POV? Could you test it? Should I rebase > > and resend? > > > > Two of these three patches are specific to at91 family, what's the > > opinion of at91 maintainers on this? > > Hello Alexander, > > From at91 perspective I have no objections. > As these are very NAND specific, I defer the patches to the NAND maintainers. > > Eugen > > > > > Link to the series discussion for reference: > > > > https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u > > > > Greets > > Alex > > I will review this series Michael > >> > >> Greets > >> Alex > >> > >>> > >>> drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++---- > >>> 1 file changed, 9 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c > >>> index bbafc88e44c..00ffeadd113 100644 > >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c > >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c > >>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > >>> const struct nand_data_interface *conf, > >>> struct atmel_smc_cs_conf *smcconf) > >>> { > >>> - u32 ncycles, totalcycles, timeps, mckperiodps; > >>> + u32 ncycles, totalcycles, timeps, mckperiodps, pulse; > >>> struct atmel_nand_controller *nc; > >>> int ret; > >>> > >>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > >>> ATMEL_SMC_MODE_TDFMODE_OPTIMIZED; > >>> > >>> /* > >>> - * Read pulse timing directly matches tRP: > >>> + * Read pulse timing would directly match tRP, > >>> + * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF) > >>> + * do not work properly in timing mode 3. > >>> + * The workaround is to extend the SMC NRD pulse to meet tREA > >>> + * timing. > >>> * > >>> - * NRD_PULSE = tRP > >>> + * NRD_PULSE = max(tRP, tREA) > >>> */ > >>> - ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps); > >>> + pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max); > >>> + ncycles = DIV_ROUND_UP(pulse, mckperiodps); > >>> totalcycles += ncycles; > >>> ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT, > >>> ncycles); > >>> -- > >>> 2.39.2 > >>> > >> >
On 30/09/24 1:37 pm, Alexander Dahl wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hello, > > Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl: >> Hei hei, >> >> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl: >>> From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems >>> like we have to wait tREA after rising RE# before sampling the data. >>> Thus pulse time must be at least tREA. >>> >>> Without this fix we got PMECC errors when reading, after switching to >>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip. >>> >>> The approach to set timings used before worked on sam9g20 and sama5d2 >>> with the same flash (S34ML02G1), probably because those have a slower >>> mck clock rate and thus the resolution of the timings setup is not as >>> tight as with sam9x60. >>> >>> The approach to fix the issue was carried over from at91bootstrap, and >>> has been successfully tested in at91bootstrap, U-Boot and Linux. >>> >>> Link: https://github.com/linux4sam/at91bootstrap/issues/174 >>> Cc: Li Bin <bin.li@microchip.com> >>> Signed-off-by: Alexander Dahl <ada@thorsis.com> >>> --- >>> >>> Notes: >>> v3: >>> - initial patch version (not present in v1 and v2) >> >> This patch was send as part of a series, which you wanted to have some >> more time to test. Besides that, has anyone looked into this >> particular fix? Maybe it can be applied separately? > > I'd kindly ask what the status of this patch series is from U-Boot > NAND subsystem maintainers POV? Could you test it? Should I rebase > and resend? > > Two of these three patches are specific to at91 family, what's the > opinion of at91 maintainers on this? > > Link to the series discussion for reference: > > https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u > > Greets > Alex > Hi Alex, Apologies for the delay in response. I also faced the same kind of problem while testing our new sam7d65 board with MX30LF4G28AD nand flash. I just had a workaround to increase the pulse time by 5 nsecs just for testing. The issue has been reported to the validation team and an investigation is under progress. I would request a few more days for this patch alone. Thanks, Bala. >> >> Greets >> Alex >> >>> >>> drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++---- >>> 1 file changed, 9 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c >>> index bbafc88e44c..00ffeadd113 100644 >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c >>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, >>> const struct nand_data_interface *conf, >>> struct atmel_smc_cs_conf *smcconf) >>> { >>> - u32 ncycles, totalcycles, timeps, mckperiodps; >>> + u32 ncycles, totalcycles, timeps, mckperiodps, pulse; >>> struct atmel_nand_controller *nc; >>> int ret; >>> >>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, >>> ATMEL_SMC_MODE_TDFMODE_OPTIMIZED; >>> >>> /* >>> - * Read pulse timing directly matches tRP: >>> + * Read pulse timing would directly match tRP, >>> + * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF) >>> + * do not work properly in timing mode 3. >>> + * The workaround is to extend the SMC NRD pulse to meet tREA >>> + * timing. >>> * >>> - * NRD_PULSE = tRP >>> + * NRD_PULSE = max(tRP, tREA) >>> */ >>> - ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps); >>> + pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max); >>> + ncycles = DIV_ROUND_UP(pulse, mckperiodps); >>> totalcycles += ncycles; >>> ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT, >>> ncycles); >>> -- >>> 2.39.2 >>> >>
Hello Bala, Am Wed, Oct 09, 2024 at 04:45:02AM +0000 schrieb Balamanikandan.Gunasundar@microchip.com: > On 30/09/24 1:37 pm, Alexander Dahl wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Hello, > > > > Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl: > >> Hei hei, > >> > >> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl: > >>> From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems > >>> like we have to wait tREA after rising RE# before sampling the data. > >>> Thus pulse time must be at least tREA. > >>> > >>> Without this fix we got PMECC errors when reading, after switching to > >>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip. > >>> > >>> The approach to set timings used before worked on sam9g20 and sama5d2 > >>> with the same flash (S34ML02G1), probably because those have a slower > >>> mck clock rate and thus the resolution of the timings setup is not as > >>> tight as with sam9x60. > >>> > >>> The approach to fix the issue was carried over from at91bootstrap, and > >>> has been successfully tested in at91bootstrap, U-Boot and Linux. > >>> > >>> Link: https://github.com/linux4sam/at91bootstrap/issues/174 > >>> Cc: Li Bin <bin.li@microchip.com> > >>> Signed-off-by: Alexander Dahl <ada@thorsis.com> > >>> --- > >>> > >>> Notes: > >>> v3: > >>> - initial patch version (not present in v1 and v2) > >> > >> This patch was send as part of a series, which you wanted to have some > >> more time to test. Besides that, has anyone looked into this > >> particular fix? Maybe it can be applied separately? > > > > I'd kindly ask what the status of this patch series is from U-Boot > > NAND subsystem maintainers POV? Could you test it? Should I rebase > > and resend? > > > > Two of these three patches are specific to at91 family, what's the > > opinion of at91 maintainers on this? > > > > Link to the series discussion for reference: > > > > https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u > > > > Greets > > Alex > > > > Hi Alex, > > Apologies for the delay in response. I also faced the same kind of > problem while testing our new sam7d65 board with MX30LF4G28AD nand > flash. I just had a workaround to increase the pulse time by 5 nsecs > just for testing. The issue has been reported to the validation team and > an investigation is under progress. > > I would request a few more days for this patch alone. Did the validation team come to any conclusion on this? Greets Alex > > Thanks, > Bala. > > >> > >> Greets > >> Alex > >> > >>> > >>> drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++---- > >>> 1 file changed, 9 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c > >>> index bbafc88e44c..00ffeadd113 100644 > >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c > >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c > >>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > >>> const struct nand_data_interface *conf, > >>> struct atmel_smc_cs_conf *smcconf) > >>> { > >>> - u32 ncycles, totalcycles, timeps, mckperiodps; > >>> + u32 ncycles, totalcycles, timeps, mckperiodps, pulse; > >>> struct atmel_nand_controller *nc; > >>> int ret; > >>> > >>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > >>> ATMEL_SMC_MODE_TDFMODE_OPTIMIZED; > >>> > >>> /* > >>> - * Read pulse timing directly matches tRP: > >>> + * Read pulse timing would directly match tRP, > >>> + * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF) > >>> + * do not work properly in timing mode 3. > >>> + * The workaround is to extend the SMC NRD pulse to meet tREA > >>> + * timing. > >>> * > >>> - * NRD_PULSE = tRP > >>> + * NRD_PULSE = max(tRP, tREA) > >>> */ > >>> - ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps); > >>> + pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max); > >>> + ncycles = DIV_ROUND_UP(pulse, mckperiodps); > >>> totalcycles += ncycles; > >>> ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT, > >>> ncycles); > >>> -- > >>> 2.39.2 > >>> > >> >
Hello Michael, Am Tue, Oct 08, 2024 at 02:12:38PM +0200 schrieb Michael Nazzareno Trimarchi: > Hi > > On Tue, Oct 8, 2024 at 1:41 PM Eugen Hristev > <eugen.hristev@collabora.com> wrote: > > > > On 9/30/24 11:07, Alexander Dahl wrote: > > > Hello, > > > > > > Am Tue, May 28, 2024 at 12:32:44PM +0200 schrieb Alexander Dahl: > > >> Hei hei, > > >> > > >> Am Mon, Apr 15, 2024 at 09:57:55AM +0200 schrieb Alexander Dahl: > > >>> From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems > > >>> like we have to wait tREA after rising RE# before sampling the data. > > >>> Thus pulse time must be at least tREA. > > >>> > > >>> Without this fix we got PMECC errors when reading, after switching to > > >>> ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip. > > >>> > > >>> The approach to set timings used before worked on sam9g20 and sama5d2 > > >>> with the same flash (S34ML02G1), probably because those have a slower > > >>> mck clock rate and thus the resolution of the timings setup is not as > > >>> tight as with sam9x60. > > >>> > > >>> The approach to fix the issue was carried over from at91bootstrap, and > > >>> has been successfully tested in at91bootstrap, U-Boot and Linux. > > >>> > > >>> Link: https://github.com/linux4sam/at91bootstrap/issues/174 > > >>> Cc: Li Bin <bin.li@microchip.com> > > >>> Signed-off-by: Alexander Dahl <ada@thorsis.com> > > >>> --- > > >>> > > >>> Notes: > > >>> v3: > > >>> - initial patch version (not present in v1 and v2) > > >> > > >> This patch was send as part of a series, which you wanted to have some > > >> more time to test. Besides that, has anyone looked into this > > >> particular fix? Maybe it can be applied separately? > > > > > > I'd kindly ask what the status of this patch series is from U-Boot > > > NAND subsystem maintainers POV? Could you test it? Should I rebase > > > and resend? > > > > > > Two of these three patches are specific to at91 family, what's the > > > opinion of at91 maintainers on this? > > > > Hello Alexander, > > > > From at91 perspective I have no objections. > > As these are very NAND specific, I defer the patches to the NAND maintainers. > > > > Eugen > > > > > > > > Link to the series discussion for reference: > > > > > > https://lore.kernel.org/u-boot/20240415074547.779264-1-ada@thorsis.com/T/#u > > > > > > Greets > > > Alex > > > > > I will review this series This might have fallen through the cracks? The series was initially based on v2024.04, now we are on v2025.01-rc4 already. Still applies cleanly to master, though. I'm especially interested in the timing fix of patch 3. Could you please have a look? Greets Alex > > Michael > > > >> > > >> Greets > > >> Alex > > >> > > >>> > > >>> drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++---- > > >>> 1 file changed, 9 insertions(+), 4 deletions(-) > > >>> > > >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c > > >>> index bbafc88e44c..00ffeadd113 100644 > > >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c > > >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c > > >>> @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > > >>> const struct nand_data_interface *conf, > > >>> struct atmel_smc_cs_conf *smcconf) > > >>> { > > >>> - u32 ncycles, totalcycles, timeps, mckperiodps; > > >>> + u32 ncycles, totalcycles, timeps, mckperiodps, pulse; > > >>> struct atmel_nand_controller *nc; > > >>> int ret; > > >>> > > >>> @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, > > >>> ATMEL_SMC_MODE_TDFMODE_OPTIMIZED; > > >>> > > >>> /* > > >>> - * Read pulse timing directly matches tRP: > > >>> + * Read pulse timing would directly match tRP, > > >>> + * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF) > > >>> + * do not work properly in timing mode 3. > > >>> + * The workaround is to extend the SMC NRD pulse to meet tREA > > >>> + * timing. > > >>> * > > >>> - * NRD_PULSE = tRP > > >>> + * NRD_PULSE = max(tRP, tREA) > > >>> */ > > >>> - ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps); > > >>> + pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max); > > >>> + ncycles = DIV_ROUND_UP(pulse, mckperiodps); > > >>> totalcycles += ncycles; > > >>> ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT, > > >>> ncycles); > > >>> -- > > >>> 2.39.2 > > >>> > > >> > > > > > -- > Michael Nazzareno Trimarchi > Co-Founder & Chief Executive Officer > M. +39 347 913 2170 > michael@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > info@amarulasolutions.com > www.amarulasolutions.com
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c index bbafc88e44c..00ffeadd113 100644 --- a/drivers/mtd/nand/raw/atmel/nand-controller.c +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c @@ -1133,7 +1133,7 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, const struct nand_data_interface *conf, struct atmel_smc_cs_conf *smcconf) { - u32 ncycles, totalcycles, timeps, mckperiodps; + u32 ncycles, totalcycles, timeps, mckperiodps, pulse; struct atmel_nand_controller *nc; int ret; @@ -1259,11 +1259,16 @@ static int atmel_smc_nand_prepare_smcconf(struct atmel_nand *nand, ATMEL_SMC_MODE_TDFMODE_OPTIMIZED; /* - * Read pulse timing directly matches tRP: + * Read pulse timing would directly match tRP, + * but some NAND flash chips (S34ML01G2 and W29N02KVxxAF) + * do not work properly in timing mode 3. + * The workaround is to extend the SMC NRD pulse to meet tREA + * timing. * - * NRD_PULSE = tRP + * NRD_PULSE = max(tRP, tREA) */ - ncycles = DIV_ROUND_UP(conf->timings.sdr.tRP_min, mckperiodps); + pulse = max(conf->timings.sdr.tRP_min, conf->timings.sdr.tREA_max); + ncycles = DIV_ROUND_UP(pulse, mckperiodps); totalcycles += ncycles; ret = atmel_smc_cs_conf_set_pulse(smcconf, ATMEL_SMC_NRD_SHIFT, ncycles);
From reading the S34ML02G1 and the SAM9X60 datasheets again, it seems like we have to wait tREA after rising RE# before sampling the data. Thus pulse time must be at least tREA. Without this fix we got PMECC errors when reading, after switching to ONFI timing mode 3 on SAM9X60 SoC with S34ML02G1 raw NAND flash chip. The approach to set timings used before worked on sam9g20 and sama5d2 with the same flash (S34ML02G1), probably because those have a slower mck clock rate and thus the resolution of the timings setup is not as tight as with sam9x60. The approach to fix the issue was carried over from at91bootstrap, and has been successfully tested in at91bootstrap, U-Boot and Linux. Link: https://github.com/linux4sam/at91bootstrap/issues/174 Cc: Li Bin <bin.li@microchip.com> Signed-off-by: Alexander Dahl <ada@thorsis.com> --- Notes: v3: - initial patch version (not present in v1 and v2) drivers/mtd/nand/raw/atmel/nand-controller.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)