[PATCH v3 1/2] mtd: rawnand: Add and use helper for testing data interface

[PATCH v3 1/2] mtd: rawnand: Add and use helper for testing data interface

[Rickard Andersson]

From: Rickard x Andersson <rickaran@axis.com>

This helper checks if the controller supports the current
data interface timings. If the timings are not supported
the vendor provided corresponding onfi mode will be tried,
and if that fails we will fall back to onfi mode 0.

The helper can also be used by NAND vendor driver for
testing different timings.

Signed-off-by: Rickard x Andersson <rickaran@axis.com>
---
 drivers/mtd/nand/raw/internals.h | 24 ++++++++++++++++++++++++
 drivers/mtd/nand/raw/nand_base.c | 28 ++++++++++++++++++----------
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
index 615677820338..6dacf110b559 100644
--- a/drivers/mtd/nand/raw/internals.h
+++ b/drivers/mtd/nand/raw/internals.h
@@ -148,6 +148,30 @@ static inline bool nand_can_choose_data_interface(struct nand_chip *chip)
 	return chip->ops.choose_data_interface;
 }
 
+/**
+ * nand_controller_supports_data_interface - Check if controller can handle
+ * the current timings.
+ *
+ * @chip: The NAND chip
+ */
+static inline int
+nand_controller_supports_data_interface(struct nand_chip *chip)
+{
+	int ret;
+	const struct nand_controller_ops *ops = chip->controller->ops;
+
+	/*
+	 * Pass NAND_DATA_IFACE_CHECK_ONLY to only check if the
+	 * controller supports the requested timings.
+	 */
+	ret = ops->setup_data_interface(chip,
+					NAND_DATA_IFACE_CHECK_ONLY,
+					&chip->data_interface);
+
+	return ret;
+}
+
+
 /* BBT functions */
 int nand_markbad_bbt(struct nand_chip *chip, loff_t offs);
 int nand_isreserved_bbt(struct nand_chip *chip, loff_t offs);
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c42cbeb7e446..e3c57649380b 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -982,8 +982,24 @@ static int nand_choose_data_interface(struct nand_chip *chip)
 	 */
 	if (nand_can_choose_data_interface(chip)) {
 		ret = chip->ops.choose_data_interface(chip);
+		if (ret)
+			return ret;
+
+		/*
+		 * Check if the controller can handle the NAND vendor specific
+		 * best timings.
+		 */
+		ret = nand_controller_supports_data_interface(chip);
 		if (!ret)
 			return ret;
+
+		/*
+		 * Specific timings did not work, try using corresponding
+		 * onfi mode provided by vendor.
+		 */
+		if (chip->data_interface.timings.mode)
+			chip->default_timing_mode =
+				chip->data_interface.timings.mode;
 	}
 
 	/*
@@ -994,9 +1010,6 @@ static int nand_choose_data_interface(struct nand_chip *chip)
 	if (chip->parameters.onfi) {
 		modes = chip->parameters.onfi->async_timing_mode;
 	} else {
-		if (!chip->default_timing_mode)
-			return 0;
-
 		modes = GENMASK(chip->default_timing_mode, 0);
 	}
 
@@ -1005,13 +1018,8 @@ static int nand_choose_data_interface(struct nand_chip *chip)
 		if (ret)
 			continue;
 
-		/*
-		 * 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);
+		/* Check if the controller supports the requested timings. */
+		ret = nand_controller_supports_data_interface(chip);
 		if (!ret) {
 			chip->default_timing_mode = mode;
 			break;
-- 
2.11.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v3 2/2] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4

[Rickard Andersson]

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 | 44 +++++++++++++++++++++++++++++++++++++
 2 files changed, 47 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..bea6bd1beece 100644
--- a/drivers/mtd/nand/raw/nand_toshiba.c
+++ b/drivers/mtd/nand/raw/nand_toshiba.c
@@ -194,6 +194,42 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
 	}
 }
 
+static int th58nvg2s3hbai4_choose_data_interface(struct nand_chip *chip)
+{
+	int ret;
+	struct nand_sdr_timings *timings = &chip->data_interface.timings.sdr;
+
+	/* Start with timings from the closest timing mode, mode 4. */
+	ret = onfi_fill_data_interface(chip, NAND_SDR_IFACE, 4);
+	if (ret)
+		return ret;
+
+	/* Patch timings that differ from mode 4. */
+	timings->tALS_min = 12000;
+	timings->tCHZ_max = 20000;
+	timings->tCLS_min = 12000;
+	timings->tCOH_min = 0;
+	timings->tDS_min = 12000;
+	timings->tRHOH_min = 25000;
+	timings->tRHW_min = 30000;
+	timings->tRHZ_max = 60000;
+	timings->tWHR_min = 60000;
+
+	/* Patch timings not part of onfi timing mode. */
+	timings->tPROG_max = 700000000;
+	timings->tBERS_max = 5000000000;
+
+	/*
+	 * Find the corresponding onfi timing mode that is safe to use. This
+	 * will only be used if the controller does not handle the specific
+	 * timings that were provided.
+	 */
+	chip->data_interface.timings.mode =
+		onfi_find_equivalent_sdr_mode(timings);
+
+	return ret;
+}
+
 static int tc58teg5dclta00_init(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
@@ -205,6 +241,12 @@ static int tc58teg5dclta00_init(struct nand_chip *chip)
 	return 0;
 }
 
+static int th58nvg2s3hbai4_init(struct nand_chip *chip)
+{
+	chip->ops.choose_data_interface = th58nvg2s3hbai4_choose_data_interface;
+	return 0;
+}
+
 static int toshiba_nand_init(struct nand_chip *chip)
 {
 	if (nand_is_slc(chip))
@@ -217,6 +259,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/

Re: [PATCH v3 1/2] mtd: rawnand: Add and use helper for testing data interface

[Miquel Raynal]

Hi Rickard,

Rickard Andersson <rickaran@axis.com> wrote on Mon, 25 May 2020
16:51:32 +0200:

> From: Rickard x Andersson <rickaran@axis.com>
> 
> This helper checks if the controller supports the current
> data interface timings. If the timings are not supported
> the vendor provided corresponding onfi mode will be tried,
> and if that fails we will fall back to onfi mode 0.
> 
> The helper can also be used by NAND vendor driver for
> testing different timings.
> 
> Signed-off-by: Rickard x Andersson <rickaran@axis.com>
> ---
>  drivers/mtd/nand/raw/internals.h | 24 ++++++++++++++++++++++++
>  drivers/mtd/nand/raw/nand_base.c | 28 ++++++++++++++++++----------
>  2 files changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index 615677820338..6dacf110b559 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -148,6 +148,30 @@ static inline bool nand_can_choose_data_interface(struct nand_chip *chip)
>  	return chip->ops.choose_data_interface;
>  }
>  
> +/**
> + * nand_controller_supports_data_interface - Check if controller can handle
> + * the current timings.
> + *
> + * @chip: The NAND chip
> + */
> +static inline int
> +nand_controller_supports_data_interface(struct nand_chip *chip)
> +{
> +	int ret;
> +	const struct nand_controller_ops *ops = chip->controller->ops;
> +
> +	/*
> +	 * Pass NAND_DATA_IFACE_CHECK_ONLY to only check if the
> +	 * controller supports the requested timings.
> +	 */
> +	ret = ops->setup_data_interface(chip,
> +					NAND_DATA_IFACE_CHECK_ONLY,
> +					&chip->data_interface);
> +
> +	return ret;
> +}
> +
> +
>  /* BBT functions */
>  int nand_markbad_bbt(struct nand_chip *chip, loff_t offs);
>  int nand_isreserved_bbt(struct nand_chip *chip, loff_t offs);
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index c42cbeb7e446..e3c57649380b 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -982,8 +982,24 @@ static int nand_choose_data_interface(struct nand_chip *chip)
>  	 */
>  	if (nand_can_choose_data_interface(chip)) {
>  		ret = chip->ops.choose_data_interface(chip);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Check if the controller can handle the NAND vendor specific
> +		 * best timings.
> +		 */
> +		ret = nand_controller_supports_data_interface(chip);
>  		if (!ret)
>  			return ret;
> +
> +		/*
> +		 * Specific timings did not work, try using corresponding
> +		 * onfi mode provided by vendor.
> +		 */
> +		if (chip->data_interface.timings.mode)
> +			chip->default_timing_mode =
> +				chip->data_interface.timings.mode;
>  	}
>  
>  	/*
> @@ -994,9 +1010,6 @@ static int nand_choose_data_interface(struct nand_chip *chip)
>  	if (chip->parameters.onfi) {
>  		modes = chip->parameters.onfi->async_timing_mode;
>  	} else {
> -		if (!chip->default_timing_mode)
> -			return 0;
> -
>  		modes = GENMASK(chip->default_timing_mode, 0);
>  	}
>  
> @@ -1005,13 +1018,8 @@ static int nand_choose_data_interface(struct nand_chip *chip)
>  		if (ret)
>  			continue;
>  
> -		/*
> -		 * 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);
> +		/* Check if the controller supports the requested timings. */
> +		ret = nand_controller_supports_data_interface(chip);
>  		if (!ret) {
>  			chip->default_timing_mode = mode;
>  			break;

Actually I think what Boris had in mind was a little bit different. As
the "preparation series" has not been sent over the mailing list yet,
if you don't mind, I will drop patch 1/2 which is not needed after my
latest changes and take your second patch, adapted to the latest
changes too. I am not taking the ownership of your series, just
proposing the big picture and provide a more usual way to review the
patches than a Github branch. Then, I'll might ask you to respin the
entire series as well if needed.


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

SV: [PATCH v3 1/2] mtd: rawnand: Add and use helper for testing data interface

[Rickard X Andersson]

> Actually I think what Boris had in mind was a little bit different. As
> the "preparation series" has not been sent over the mailing list yet,
> if you don't mind, I will drop patch 1/2 which is not needed after my
> latest changes and take your second patch, adapted to the latest
> changes too. I am not taking the ownership of your series, just
> proposing the big picture and provide a more usual way to review the
> patches than a Github branch. Then, I'll might ask you to respin the
> entire series as well if needed.
>
>
> Thanks,
> Miquèl

Sounds good! I just wanted to update my patches as much as possible with the information I had.

BR,
Rickard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/