[PATCH 1/2] mtd: nand: spi: Only one dummy byte in QUADIO

[PATCH 1/2] mtd: nand: spi: Only one dummy byte in QUADIO

[Reto Schneider]

From: Reto Schneider <reto.schneider@husqvarnagroup.com>

The datasheet only lists one dummy byte in the 0xEH operation for the
following chips:
* GD5F1GQ4xBxxG
* GD5F1GQ4xExxG
* GD5F1GQ4xFxxG
* GD5F1GQ4UAYIG
* GD5F4GQ4UAYIG

This patch and its commit message reflects what has been done in Linux
and has not been tested by me.

Signed-off-by: Reto Schneider <reto.schneider@husqvarnagroup.com>
CC: Stefan Roese <sr@denx.de>
---
 drivers/mtd/nand/spi/gigadevice.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
index 0b228dcb5b..5de0ebbb7b 100644
--- a/drivers/mtd/nand/spi/gigadevice.c
+++ b/drivers/mtd/nand/spi/gigadevice.c
@@ -20,7 +20,7 @@
 #define GD5FXGQ4XEXXG_REG_STATUS2		0xf0
 
 static SPINAND_OP_VARIANTS(read_cache_variants,
-		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
 		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
 		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
 		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
-- 
2.29.2

[PATCH 2/2] mtd: nand: spi: Support GigaDevice GD5F1GQ5UExxG

[Reto Schneider]

From: Reto Schneider <reto.schneider@husqvarnagroup.com>

The relevant changes to the already existing GD5F1GQ4UExxG support has
been determined by consulting the GigaDevice product change notice
AN-0392-10, version 1.0 from November 30, 2020.

As the overlaps are huge, variable names have been generalized
accordingly.

Apart form the lowered ECC strength (4 instead of 8 bits per 512 bytes),
the new device ID, and the extra quad IO dummy byte, no changes had to
be taken into account.

New hardware features are not supported, namely:
 - Power on reset
 - Unique ID
 - Double transfer rate (DTR)
 - Parameter page
 - Random data quad IO

The inverted semantic of the "driver strength" register bits, defaulting
to 100% instead of 50% for the Q5 devices, got ignored as the driver has
never touched them anyway.

The no longer supported "read from cache during block erase"
functionality I do not know how to reflect.

Implementation has been tested on MediaTek MT7688 based GARDENA smart
Gateways using both, GigaDevice GD5F1GQ5UEYIG and GD5F1GQ4UBYIG.

Signed-off-by: Reto Schneider <reto.schneider@husqvarnagroup.com>
CC: Stefan Roese <sr@denx.de>
---
 drivers/mtd/nand/spi/gigadevice.c | 79 +++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
index 5de0ebbb7b..a2c93486f4 100644
--- a/drivers/mtd/nand/spi/gigadevice.c
+++ b/drivers/mtd/nand/spi/gigadevice.c
@@ -17,9 +17,13 @@
 #define GD5FXGQ4XA_STATUS_ECC_1_7_BITFLIPS	(1 << 4)
 #define GD5FXGQ4XA_STATUS_ECC_8_BITFLIPS	(3 << 4)
 
-#define GD5FXGQ4XEXXG_REG_STATUS2		0xf0
+#define GD5FXGQ5XE_STATUS_ECC_1_4_BITFLIPS	(1 << 4)
+#define GD5FXGQ5XE_STATUS_ECC_4_BITFLIPS	(3 << 4)
 
-static SPINAND_OP_VARIANTS(read_cache_variants,
+#define GD5FXGQXXEXXG_REG_STATUS2		0xf0
+
+/* Q4 devices, QUADIO: Dummy bytes valid for 1 and 2 GBit variants */
+static SPINAND_OP_VARIANTS(gd5fxgq4_read_cache_variants,
 		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
 		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
 		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
@@ -27,6 +31,15 @@ static SPINAND_OP_VARIANTS(read_cache_variants,
 		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
 		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
 
+/* Q5 devices, QUADIO: Dummy bytes only valid for 1 GBit variants */
+static SPINAND_OP_VARIANTS(gd5f1gq5_read_cache_variants,
+		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
+
 static SPINAND_OP_VARIANTS(write_cache_variants,
 		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
 		SPINAND_PROG_LOAD(true, 0, NULL, 0));
@@ -35,7 +48,7 @@ static SPINAND_OP_VARIANTS(update_cache_variants,
 		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
 		SPINAND_PROG_LOAD(false, 0, NULL, 0));
 
-static int gd5fxgq4xexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
+static int gd5fxgqxxexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
 				       struct mtd_oob_region *region)
 {
 	if (section)
@@ -47,7 +60,7 @@ static int gd5fxgq4xexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
 	return 0;
 }
 
-static int gd5fxgq4xexxg_ooblayout_free(struct mtd_info *mtd, int section,
+static int gd5fxgqxxexxg_ooblayout_free(struct mtd_info *mtd, int section,
 					struct mtd_oob_region *region)
 {
 	if (section)
@@ -64,7 +77,7 @@ static int gd5fxgq4xexxg_ecc_get_status(struct spinand_device *spinand,
 					u8 status)
 {
 	u8 status2;
-	struct spi_mem_op op = SPINAND_GET_FEATURE_OP(GD5FXGQ4XEXXG_REG_STATUS2,
+	struct spi_mem_op op = SPINAND_GET_FEATURE_OP(GD5FXGQXXEXXG_REG_STATUS2,
 						      &status2);
 	int ret;
 
@@ -102,21 +115,67 @@ static int gd5fxgq4xexxg_ecc_get_status(struct spinand_device *spinand,
 	return -EINVAL;
 }
 
-static const struct mtd_ooblayout_ops gd5fxgq4xexxg_ooblayout = {
-	.ecc = gd5fxgq4xexxg_ooblayout_ecc,
-	.rfree = gd5fxgq4xexxg_ooblayout_free,
+static int gd5fxgq5xexxg_ecc_get_status(struct spinand_device *spinand,
+					u8 status)
+{
+	u8 status2;
+	struct spi_mem_op op = SPINAND_GET_FEATURE_OP(GD5FXGQXXEXXG_REG_STATUS2,
+						      &status2);
+	int ret;
+
+	switch (status & STATUS_ECC_MASK) {
+	case STATUS_ECC_NO_BITFLIPS:
+		return 0;
+
+	case GD5FXGQ5XE_STATUS_ECC_1_4_BITFLIPS:
+		/*
+		 * Read status2 register to determine a more fine grained
+		 * bit error status
+		 */
+		ret = spi_mem_exec_op(spinand->slave, &op);
+		if (ret)
+			return ret;
+
+		/*
+		 * 1 ... 4 bits are flipped (and corrected)
+		 */
+		/* bits sorted this way (1...0): ECCSE1, ECCSE0 */
+		return ((status2 & STATUS_ECC_MASK) >> 4) + 1;
+
+	case STATUS_ECC_UNCOR_ERROR:
+		return -EBADMSG;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static const struct mtd_ooblayout_ops gd5fxgqxxexxg_ooblayout = {
+	.ecc = gd5fxgqxxexxg_ooblayout_ecc,
+	.rfree = gd5fxgqxxexxg_ooblayout_free,
 };
 
 static const struct spinand_info gigadevice_spinand_table[] = {
 	SPINAND_INFO("GD5F1GQ4UExxG", 0xd1,
 		     NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
 		     NAND_ECCREQ(8, 512),
-		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+		     SPINAND_INFO_OP_VARIANTS(&gd5fxgq4_read_cache_variants,
 					      &write_cache_variants,
 					      &update_cache_variants),
 		     0,
-		     SPINAND_ECCINFO(&gd5fxgq4xexxg_ooblayout,
+		     SPINAND_ECCINFO(&gd5fxgqxxexxg_ooblayout,
 				     gd5fxgq4xexxg_ecc_get_status)),
+	SPINAND_INFO("GD5F1GQ5UExxG", 0x51,
+		     NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
+		     NAND_ECCREQ(4, 512),
+		     SPINAND_INFO_OP_VARIANTS(&gd5f1gq5_read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&gd5fxgqxxexxg_ooblayout,
+				     gd5fxgq5xexxg_ecc_get_status)),
 };
 
 static int gigadevice_spinand_detect(struct spinand_device *spinand)
-- 
2.29.2

[PATCH 1/2] mtd: nand: spi: Only one dummy byte in QUADIO

[Stefan Roese]

On 10.02.21 19:36, Reto Schneider wrote:
> From: Reto Schneider <reto.schneider@husqvarnagroup.com>
> 
> The datasheet only lists one dummy byte in the 0xEH operation for the
> following chips:
> * GD5F1GQ4xBxxG
> * GD5F1GQ4xExxG
> * GD5F1GQ4xFxxG
> * GD5F1GQ4UAYIG
> * GD5F4GQ4UAYIG
> 
> This patch and its commit message reflects what has been done in Linux
> and has not been tested by me.
> 
> Signed-off-by: Reto Schneider <reto.schneider@husqvarnagroup.com>
> CC: Stefan Roese <sr@denx.de>

AFAICT, you are posting a Linux patch ported to U-Boot here. Which is
good of course. But if you didn't do substantial changes to this
patch, then please keep the original (Linux) authorship of Hauke for
this patch (From: ...). You might want to add something like:

Reto Schneider:
Linux patch ported to U-Boot

to the end of the commit text and add your Sob of course.

Other than this:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan


> ---
>   drivers/mtd/nand/spi/gigadevice.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
> index 0b228dcb5b..5de0ebbb7b 100644
> --- a/drivers/mtd/nand/spi/gigadevice.c
> +++ b/drivers/mtd/nand/spi/gigadevice.c
> @@ -20,7 +20,7 @@
>   #define GD5FXGQ4XEXXG_REG_STATUS2		0xf0
>   
>   static SPINAND_OP_VARIANTS(read_cache_variants,
> -		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
>   		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
>   		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
>   		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 2/2] mtd: nand: spi: Support GigaDevice GD5F1GQ5UExxG

[Stefan Roese]

On 10.02.21 19:36, Reto Schneider wrote:
> From: Reto Schneider <reto.schneider@husqvarnagroup.com>
> 
> The relevant changes to the already existing GD5F1GQ4UExxG support has
> been determined by consulting the GigaDevice product change notice
> AN-0392-10, version 1.0 from November 30, 2020.
> 
> As the overlaps are huge, variable names have been generalized
> accordingly.
> 
> Apart form the lowered ECC strength (4 instead of 8 bits per 512 bytes),
> the new device ID, and the extra quad IO dummy byte, no changes had to
> be taken into account.
> 
> New hardware features are not supported, namely:
>   - Power on reset
>   - Unique ID
>   - Double transfer rate (DTR)
>   - Parameter page
>   - Random data quad IO
> 
> The inverted semantic of the "driver strength" register bits, defaulting
> to 100% instead of 50% for the Q5 devices, got ignored as the driver has
> never touched them anyway.
> 
> The no longer supported "read from cache during block erase"
> functionality I do not know how to reflect.
> 
> Implementation has been tested on MediaTek MT7688 based GARDENA smart
> Gateways using both, GigaDevice GD5F1GQ5UEYIG and GD5F1GQ4UBYIG.
> 
> Signed-off-by: Reto Schneider <reto.schneider@husqvarnagroup.com>
> CC: Stefan Roese <sr@denx.de>

Looks good, so:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/mtd/nand/spi/gigadevice.c | 79 +++++++++++++++++++++++++++----
>   1 file changed, 69 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
> index 5de0ebbb7b..a2c93486f4 100644
> --- a/drivers/mtd/nand/spi/gigadevice.c
> +++ b/drivers/mtd/nand/spi/gigadevice.c
> @@ -17,9 +17,13 @@
>   #define GD5FXGQ4XA_STATUS_ECC_1_7_BITFLIPS	(1 << 4)
>   #define GD5FXGQ4XA_STATUS_ECC_8_BITFLIPS	(3 << 4)
>   
> -#define GD5FXGQ4XEXXG_REG_STATUS2		0xf0
> +#define GD5FXGQ5XE_STATUS_ECC_1_4_BITFLIPS	(1 << 4)
> +#define GD5FXGQ5XE_STATUS_ECC_4_BITFLIPS	(3 << 4)
>   
> -static SPINAND_OP_VARIANTS(read_cache_variants,
> +#define GD5FXGQXXEXXG_REG_STATUS2		0xf0
> +
> +/* Q4 devices, QUADIO: Dummy bytes valid for 1 and 2 GBit variants */
> +static SPINAND_OP_VARIANTS(gd5fxgq4_read_cache_variants,
>   		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
>   		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
>   		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
> @@ -27,6 +31,15 @@ static SPINAND_OP_VARIANTS(read_cache_variants,
>   		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
>   		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
>   
> +/* Q5 devices, QUADIO: Dummy bytes only valid for 1 GBit variants */
> +static SPINAND_OP_VARIANTS(gd5f1gq5_read_cache_variants,
> +		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> +
>   static SPINAND_OP_VARIANTS(write_cache_variants,
>   		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
>   		SPINAND_PROG_LOAD(true, 0, NULL, 0));
> @@ -35,7 +48,7 @@ static SPINAND_OP_VARIANTS(update_cache_variants,
>   		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
>   		SPINAND_PROG_LOAD(false, 0, NULL, 0));
>   
> -static int gd5fxgq4xexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
> +static int gd5fxgqxxexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
>   				       struct mtd_oob_region *region)
>   {
>   	if (section)
> @@ -47,7 +60,7 @@ static int gd5fxgq4xexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
>   	return 0;
>   }
>   
> -static int gd5fxgq4xexxg_ooblayout_free(struct mtd_info *mtd, int section,
> +static int gd5fxgqxxexxg_ooblayout_free(struct mtd_info *mtd, int section,
>   					struct mtd_oob_region *region)
>   {
>   	if (section)
> @@ -64,7 +77,7 @@ static int gd5fxgq4xexxg_ecc_get_status(struct spinand_device *spinand,
>   					u8 status)
>   {
>   	u8 status2;
> -	struct spi_mem_op op = SPINAND_GET_FEATURE_OP(GD5FXGQ4XEXXG_REG_STATUS2,
> +	struct spi_mem_op op = SPINAND_GET_FEATURE_OP(GD5FXGQXXEXXG_REG_STATUS2,
>   						      &status2);
>   	int ret;
>   
> @@ -102,21 +115,67 @@ static int gd5fxgq4xexxg_ecc_get_status(struct spinand_device *spinand,
>   	return -EINVAL;
>   }
>   
> -static const struct mtd_ooblayout_ops gd5fxgq4xexxg_ooblayout = {
> -	.ecc = gd5fxgq4xexxg_ooblayout_ecc,
> -	.rfree = gd5fxgq4xexxg_ooblayout_free,
> +static int gd5fxgq5xexxg_ecc_get_status(struct spinand_device *spinand,
> +					u8 status)
> +{
> +	u8 status2;
> +	struct spi_mem_op op = SPINAND_GET_FEATURE_OP(GD5FXGQXXEXXG_REG_STATUS2,
> +						      &status2);
> +	int ret;
> +
> +	switch (status & STATUS_ECC_MASK) {
> +	case STATUS_ECC_NO_BITFLIPS:
> +		return 0;
> +
> +	case GD5FXGQ5XE_STATUS_ECC_1_4_BITFLIPS:
> +		/*
> +		 * Read status2 register to determine a more fine grained
> +		 * bit error status
> +		 */
> +		ret = spi_mem_exec_op(spinand->slave, &op);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * 1 ... 4 bits are flipped (and corrected)
> +		 */
> +		/* bits sorted this way (1...0): ECCSE1, ECCSE0 */
> +		return ((status2 & STATUS_ECC_MASK) >> 4) + 1;
> +
> +	case STATUS_ECC_UNCOR_ERROR:
> +		return -EBADMSG;
> +
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct mtd_ooblayout_ops gd5fxgqxxexxg_ooblayout = {
> +	.ecc = gd5fxgqxxexxg_ooblayout_ecc,
> +	.rfree = gd5fxgqxxexxg_ooblayout_free,
>   };
>   
>   static const struct spinand_info gigadevice_spinand_table[] = {
>   	SPINAND_INFO("GD5F1GQ4UExxG", 0xd1,
>   		     NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
>   		     NAND_ECCREQ(8, 512),
> -		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +		     SPINAND_INFO_OP_VARIANTS(&gd5fxgq4_read_cache_variants,
>   					      &write_cache_variants,
>   					      &update_cache_variants),
>   		     0,
> -		     SPINAND_ECCINFO(&gd5fxgq4xexxg_ooblayout,
> +		     SPINAND_ECCINFO(&gd5fxgqxxexxg_ooblayout,
>   				     gd5fxgq4xexxg_ecc_get_status)),
> +	SPINAND_INFO("GD5F1GQ5UExxG", 0x51,
> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
> +		     NAND_ECCREQ(4, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&gd5f1gq5_read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&gd5fxgqxxexxg_ooblayout,
> +				     gd5fxgq5xexxg_ecc_get_status)),
>   };
>   
>   static int gigadevice_spinand_detect(struct spinand_device *spinand)
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de