* [PATCH 1/2] mtd: rawnand: Use the ->init_data_interface() hook [not found] <richard@nod.at;> @ 2020-05-14 9:13 ` Rickard Andersson 2020-05-14 9:13 ` [PATCH 2/2] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 Rickard Andersson 2020-05-19 9:04 ` [PATCH 1/2] mtd: rawnand: Use the ->init_data_interface() hook Miquel Raynal 0 siblings, 2 replies; 4+ messages in thread From: Rickard Andersson @ 2020-05-14 9:13 UTC (permalink / raw) To: miquel.raynal, linux-mtd; +Cc: rickaran From: Rickard x Andersson <rickaran@axis.com> Check if the NAND vendor provided hook works and check that the NAND controller can handle the timings. Signed-off-by: Rickard x Andersson <rickaran@axis.com> --- drivers/mtd/nand/raw/nand_base.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 8744f0033f78..974050a04f23 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -955,6 +955,30 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr) return ret; } +static int nand_try_init_data_interface(struct nand_chip *chip) +{ + int ret = chip->ops.init_data_interface(chip); + + if (!ret) { + /* + * Pass NAND_DATA_IFACE_CHECK_ONLY to only check if the + * controller supports the requested timings. + */ + ret = chip->controller->ops->setup_data_interface(chip, + NAND_DATA_IFACE_CHECK_ONLY, + &chip->data_interface); + } + + if (ret) { + /* The provided data interface timings did not work */ + chip->ops.init_data_interface = NULL; + memset(&chip->data_interface, 0, + sizeof(struct nand_data_interface)); + } + + return ret; +} + /** * nand_choose_data_interface - find the best data interface and timings * @chip: The NAND chip @@ -980,8 +1004,11 @@ static int nand_choose_data_interface(struct nand_chip *chip) * ->init_data_interface() is expected to update the entire chip's * nand_data_interface structure. */ - if (nand_has_init_data_interface(chip)) - return chip->ops.init_data_interface(chip); + if (nand_has_init_data_interface(chip)) { + ret = nand_try_init_data_interface(chip); + if (!ret) + return 0; + } /* * First try to identify the best timings from ONFI parameters and -- 2.11.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 2020-05-14 9:13 ` [PATCH 1/2] mtd: rawnand: Use the ->init_data_interface() hook Rickard Andersson @ 2020-05-14 9:13 ` Rickard Andersson 2020-05-19 12:08 ` Miquel Raynal 2020-05-19 9:04 ` [PATCH 1/2] mtd: rawnand: Use the ->init_data_interface() hook Miquel Raynal 1 sibling, 1 reply; 4+ messages in thread From: Rickard Andersson @ 2020-05-14 9:13 UTC (permalink / raw) To: miquel.raynal, linux-mtd; +Cc: rickaran From: Rickard x Andersson <rickaran@axis.com> The Kioxia/Toshiba TH58NVG2S3HBAI4 NAND memory is not a ONFI compliant memory. The timings of the memory is quite close to ONFI mode 4 but is breaking that spec. Erase block read speed is increased from 6910 KiB/s to 13490 KiB/s. Erase block write speed is increased from 3350 KiB/s to 4410 KiB/s. Tested on IMX6SX which has a NAND controller supporting EDO mode. Signed-off-by: Rickard x Andersson <rickaran@axis.com> --- drivers/mtd/nand/raw/nand_ids.c | 3 ++ drivers/mtd/nand/raw/nand_toshiba.c | 61 +++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c index e0dbc2e316c7..8b676e8b481b 100644 --- a/drivers/mtd/nand/raw/nand_ids.c +++ b/drivers/mtd/nand/raw/nand_ids.c @@ -52,6 +52,9 @@ struct nand_flash_dev nand_flash_ids[] = { { .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} }, SZ_8K, SZ_8K, SZ_2M, NAND_NEED_SCRAMBLING, 6, 640, NAND_ECC_INFO(40, SZ_1K), 4 }, + {"TH58NVG2S3HBAI4 4G 3.3V 8-bit", + { .id = {0x98, 0xdc, 0x91, 0x15, 0x76} }, + SZ_2K, SZ_512, SZ_128K, 0, 5, 128, NAND_ECC_INFO(8, SZ_512) }, LEGACY_ID_NAND("NAND 4MiB 5V 8-bit", 0x6B, 4, SZ_8K, SP_OPTIONS), LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 4, SZ_8K, SP_OPTIONS), diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c index b6efaf5195bb..380499cfa491 100644 --- a/drivers/mtd/nand/raw/nand_toshiba.c +++ b/drivers/mtd/nand/raw/nand_toshiba.c @@ -26,6 +26,52 @@ /* Max ECC Steps for BENAND */ #define TOSHIBA_NAND_MAX_ECC_STEPS 8 +static const struct nand_data_interface th58nvg2s3hbai4_timings = { + .type = NAND_SDR_IFACE, + .timings.mode = 0, + .timings.sdr = { + .tPROG_max = 700000000, + .tBERS_max = 5000000000, + .tCCS_min = 500000, + .tR_max = 200000000, + .tADL_min = 400000, + .tALH_min = 5000, + .tALS_min = 12000, + .tAR_min = 10000, + .tCEA_max = 25000, + .tCEH_min = 20000, + .tCH_min = 5000, + .tCHZ_max = 20000, + .tCLH_min = 5000, + .tCLR_min = 10000, + .tCLS_min = 12000, + .tCOH_min = 0, + .tCS_min = 20000, + .tDH_min = 5000, + .tDS_min = 12000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 25000, + .tREA_max = 20000, + .tREH_min = 10000, + .tRHOH_min = 25000, + .tRHW_min = 30000, + .tRHZ_max = 60000, + .tRLOH_min = 5000, + .tRP_min = 12000, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tWC_min = 25000, + .tWH_min = 10000, + .tWHR_min = 60000, + .tWP_min = 12000, + .tWW_min = 100000, + } +}; + + static int toshiba_nand_benand_read_eccstatus_op(struct nand_chip *chip, u8 *buf) { @@ -194,6 +240,13 @@ static void toshiba_nand_decode_id(struct nand_chip *chip) } } +static int th58nvg2s3hbai4_init_data_interface(struct nand_chip *chip) +{ + chip->data_interface = th58nvg2s3hbai4_timings; + + return 0; +} + static int tc58teg5dclta00_init(struct nand_chip *chip) { struct mtd_info *mtd = nand_to_mtd(chip); @@ -205,6 +258,12 @@ static int tc58teg5dclta00_init(struct nand_chip *chip) return 0; } +static int th58nvg2s3hbai4_init(struct nand_chip *chip) +{ + chip->ops.init_data_interface = th58nvg2s3hbai4_init_data_interface; + return 0; +} + static int toshiba_nand_init(struct nand_chip *chip) { if (nand_is_slc(chip)) @@ -217,6 +276,8 @@ static int toshiba_nand_init(struct nand_chip *chip) if (!strcmp("TC58TEG5DCLTA00", chip->parameters.model)) tc58teg5dclta00_init(chip); + if (!strncmp("TH58NVG2S3HBAI4", chip->parameters.model, 15)) + th58nvg2s3hbai4_init(chip); return 0; } -- 2.11.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 2020-05-14 9:13 ` [PATCH 2/2] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 Rickard Andersson @ 2020-05-19 12:08 ` Miquel Raynal 0 siblings, 0 replies; 4+ messages in thread From: Miquel Raynal @ 2020-05-19 12:08 UTC (permalink / raw) To: Rickard Andersson; +Cc: linux-mtd Hi Rickard, Rickard Andersson <rickaran@axis.com> wrote on Thu, 14 May 2020 11:13:42 +0200: > From: Rickard x Andersson <rickaran@axis.com> > > The Kioxia/Toshiba TH58NVG2S3HBAI4 NAND memory is not a > ONFI compliant memory. The timings of the memory is quite > close to ONFI mode 4 but is breaking that spec. > > Erase block read speed is increased from 6910 KiB/s to > 13490 KiB/s. Erase block write speed is increased from > 3350 KiB/s to 4410 KiB/s. > > Tested on IMX6SX which has a NAND controller supporting > EDO mode. > > Signed-off-by: Rickard x Andersson <rickaran@axis.com> > --- > drivers/mtd/nand/raw/nand_ids.c | 3 ++ > drivers/mtd/nand/raw/nand_toshiba.c | 61 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c > index e0dbc2e316c7..8b676e8b481b 100644 > --- a/drivers/mtd/nand/raw/nand_ids.c > +++ b/drivers/mtd/nand/raw/nand_ids.c > @@ -52,6 +52,9 @@ struct nand_flash_dev nand_flash_ids[] = { > { .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} }, > SZ_8K, SZ_8K, SZ_2M, NAND_NEED_SCRAMBLING, 6, 640, > NAND_ECC_INFO(40, SZ_1K), 4 }, > + {"TH58NVG2S3HBAI4 4G 3.3V 8-bit", > + { .id = {0x98, 0xdc, 0x91, 0x15, 0x76} }, > + SZ_2K, SZ_512, SZ_128K, 0, 5, 128, NAND_ECC_INFO(8, SZ_512) }, > > LEGACY_ID_NAND("NAND 4MiB 5V 8-bit", 0x6B, 4, SZ_8K, SP_OPTIONS), > LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 4, SZ_8K, SP_OPTIONS), > diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c > index b6efaf5195bb..380499cfa491 100644 > --- a/drivers/mtd/nand/raw/nand_toshiba.c > +++ b/drivers/mtd/nand/raw/nand_toshiba.c > @@ -26,6 +26,52 @@ > /* Max ECC Steps for BENAND */ > #define TOSHIBA_NAND_MAX_ECC_STEPS 8 > > +static const struct nand_data_interface th58nvg2s3hbai4_timings = { > + .type = NAND_SDR_IFACE, > + .timings.mode = 0, I just a branch with a new helper to fill this field, it is called onfi_find_equivalent_sdr_mode(). It is only compile tested so please verify it works. > + .timings.sdr = { > + .tPROG_max = 700000000, > + .tBERS_max = 5000000000, > + .tCCS_min = 500000, > + .tR_max = 200000000, > + .tADL_min = 400000, > + .tALH_min = 5000, > + .tALS_min = 12000, > + .tAR_min = 10000, > + .tCEA_max = 25000, > + .tCEH_min = 20000, > + .tCH_min = 5000, > + .tCHZ_max = 20000, > + .tCLH_min = 5000, > + .tCLR_min = 10000, > + .tCLS_min = 12000, > + .tCOH_min = 0, > + .tCS_min = 20000, > + .tDH_min = 5000, > + .tDS_min = 12000, > + .tFEAT_max = 1000000, > + .tIR_min = 0, > + .tITC_max = 1000000, > + .tRC_min = 25000, > + .tREA_max = 20000, > + .tREH_min = 10000, > + .tRHOH_min = 25000, > + .tRHW_min = 30000, > + .tRHZ_max = 60000, > + .tRLOH_min = 5000, > + .tRP_min = 12000, > + .tRR_min = 20000, > + .tRST_max = 500000000, > + .tWB_max = 100000, > + .tWC_min = 25000, > + .tWH_min = 10000, > + .tWHR_min = 60000, > + .tWP_min = 12000, > + .tWW_min = 100000, > + } > +}; > + > + > static int toshiba_nand_benand_read_eccstatus_op(struct nand_chip > *chip, u8 *buf) > { > @@ -194,6 +240,13 @@ static void toshiba_nand_decode_id(struct > nand_chip *chip) } > } > > +static int th58nvg2s3hbai4_init_data_interface(struct nand_chip I renamed the hook so the helper should be th58..._choose_data_interface() > *chip) +{ > + chip->data_interface = th58nvg2s3hbai4_timings; this data interface should be tested against the controller's capabilities and return an error otherwise. Please check that, in case of error (simulate it) it fallbacks to mode 0 and does not fail silently. > + > + return 0; > +} > + > static int tc58teg5dclta00_init(struct nand_chip *chip) > { > struct mtd_info *mtd = nand_to_mtd(chip); > @@ -205,6 +258,12 @@ static int tc58teg5dclta00_init(struct nand_chip > *chip) return 0; > } > > +static int th58nvg2s3hbai4_init(struct nand_chip *chip) > +{ > + chip->ops.init_data_interface = > th58nvg2s3hbai4_init_data_interface; > + return 0; > +} > + > static int toshiba_nand_init(struct nand_chip *chip) > { > if (nand_is_slc(chip)) > @@ -217,6 +276,8 @@ static int toshiba_nand_init(struct nand_chip > *chip) > if (!strcmp("TC58TEG5DCLTA00", chip->parameters.model)) > tc58teg5dclta00_init(chip); > + if (!strncmp("TH58NVG2S3HBAI4", chip->parameters.model, 15)) > + th58nvg2s3hbai4_init(chip); > > return 0; > } Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] mtd: rawnand: Use the ->init_data_interface() hook 2020-05-14 9:13 ` [PATCH 1/2] mtd: rawnand: Use the ->init_data_interface() hook Rickard Andersson 2020-05-14 9:13 ` [PATCH 2/2] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 Rickard Andersson @ 2020-05-19 9:04 ` Miquel Raynal 1 sibling, 0 replies; 4+ messages in thread From: Miquel Raynal @ 2020-05-19 9:04 UTC (permalink / raw) To: Rickard Andersson; +Cc: linux-mtd Hi Rickard, Rickard Andersson <rickaran@axis.com> wrote on Thu, 14 May 2020 11:13:41 +0200: > From: Rickard x Andersson <rickaran@axis.com> > > Check if the NAND vendor provided hook works and check that > the NAND controller can handle the timings. > > Signed-off-by: Rickard x Andersson <rickaran@axis.com> > --- > drivers/mtd/nand/raw/nand_base.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index 8744f0033f78..974050a04f23 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -955,6 +955,30 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr) > return ret; > } > > +static int nand_try_init_data_interface(struct nand_chip *chip) > +{ > + int ret = chip->ops.init_data_interface(chip); > + > + if (!ret) { > + /* > + * Pass NAND_DATA_IFACE_CHECK_ONLY to only check if the > + * controller supports the requested timings. > + */ > + ret = chip->controller->ops->setup_data_interface(chip, > + NAND_DATA_IFACE_CHECK_ONLY, > + &chip->data_interface); I would like this to happen in Toshiba's code. Imagine you can try several timings, the vendor code should be able to check by itself the controller supports the timings. I think you can drop this "try init data interface" and move the content to Toshiba's driver. > + } > + > + if (ret) { > + /* The provided data interface timings did not work */ > + chip->ops.init_data_interface = NULL; > + memset(&chip->data_interface, 0, > + sizeof(struct nand_data_interface)); > + } > + > + return ret; > +} > + > /** > * nand_choose_data_interface - find the best data interface and timings > * @chip: The NAND chip > @@ -980,8 +1004,11 @@ static int nand_choose_data_interface(struct nand_chip *chip) > * ->init_data_interface() is expected to update the entire chip's > * nand_data_interface structure. > */ > - if (nand_has_init_data_interface(chip)) > - return chip->ops.init_data_interface(chip); I renamed it "choose_data_interface" now, you can pull changes from Github. > + if (nand_has_init_data_interface(chip)) { > + ret = nand_try_init_data_interface(chip); > + if (!ret) > + return 0; I'm fine with a fallback on the regular ONFI research though, which in this case would choose timings mode 0 I guess. Can you validate? On my side I changed the code on Github and now a negative return code just silently fails so that we fallback on mode 0. > + } > > /* > * First try to identify the best timings from ONFI parameters and Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-19 12:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <richard@nod.at;>
2020-05-14 9:13 ` [PATCH 1/2] mtd: rawnand: Use the ->init_data_interface() hook Rickard Andersson
2020-05-14 9:13 ` [PATCH 2/2] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 Rickard Andersson
2020-05-19 12:08 ` Miquel Raynal
2020-05-19 9:04 ` [PATCH 1/2] mtd: rawnand: Use the ->init_data_interface() hook Miquel Raynal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).