2019-05-22 22:07:44

by Jeff Kletsky

[permalink] [raw]
Subject: [PATCH v4 0/3] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG

Addresses changes in macros and related data structures introduced by
commit 377e517b5fa5
mtd: nand: Add max_bad_eraseblocks_per_lun info to memorg

Resolves issue detected by kbuild test robot
Tue, 21 May 2019 17:28:09 +0800
Tue, 21 May 2019 18:17:28 +0800

GD5F1GQ4UFxxG data sheet on page 34 of gd5f1gq4xfxxg_v2.4_20190322.pdf
indicates "Minumum number of valid blocks (Nvb)" 1004 out of 1024 total.

Newly introduced "max bad blocks" parameter set to 20 (1024-1020).

Rebased on v5.2-rc1 and confirmed patch applies on master.


Patches 1/3 and 2/3 are the same as in the previous series.

Patch 3/3, mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG,
includes the additional parameter (compared here to v3 of the series):

SPINAND_INFO("GD5F1GQ4UFxxG", 0xb148,
- NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
+ NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
NAND_ECCREQ(8, 512),
SPINAND_INFO_OP_VARIANTS(&read_cache_variants_f,
&write_cache_variants,

R-b of Frieder Schrempf removed from [3/3] as a result this change.

Supersedes series:

mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=108868



Jeff



Cc: Miquel Raynal <[email protected]>
Cc: Schrempf Frieder <[email protected]>
Cc: Boris Brezillon <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]




2019-05-22 22:07:46

by Jeff Kletsky

[permalink] [raw]
Subject: [PATCH v4 1/3] mtd: spinand: Define macros for page-read ops with three-byte addresses

From: Jeff Kletsky <[email protected]>

The GigaDevice GD5F1GQ4UFxxG SPI NAND utilizes three-byte addresses
for its page-read ops.

http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/

Signed-off-by: Jeff Kletsky <[email protected]>

Reviewed-by: Frieder Schrempf <[email protected]>
---
include/linux/mtd/spinand.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 507f7e289bd1..8aa39ac41e8e 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -68,30 +68,60 @@
SPI_MEM_OP_DUMMY(ndummy, 1), \
SPI_MEM_OP_DATA_IN(len, buf, 1))

+#define SPINAND_PAGE_READ_FROM_CACHE_OP_3A(fast, addr, ndummy, buf, len) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(fast ? 0x0b : 0x03, 1), \
+ SPI_MEM_OP_ADDR(3, addr, 1), \
+ SPI_MEM_OP_DUMMY(ndummy, 1), \
+ SPI_MEM_OP_DATA_IN(len, buf, 1))
+
#define SPINAND_PAGE_READ_FROM_CACHE_X2_OP(addr, ndummy, buf, len) \
SPI_MEM_OP(SPI_MEM_OP_CMD(0x3b, 1), \
SPI_MEM_OP_ADDR(2, addr, 1), \
SPI_MEM_OP_DUMMY(ndummy, 1), \
SPI_MEM_OP_DATA_IN(len, buf, 2))

+#define SPINAND_PAGE_READ_FROM_CACHE_X2_OP_3A(addr, ndummy, buf, len) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(0x3b, 1), \
+ SPI_MEM_OP_ADDR(3, addr, 1), \
+ SPI_MEM_OP_DUMMY(ndummy, 1), \
+ SPI_MEM_OP_DATA_IN(len, buf, 2))
+
#define SPINAND_PAGE_READ_FROM_CACHE_X4_OP(addr, ndummy, buf, len) \
SPI_MEM_OP(SPI_MEM_OP_CMD(0x6b, 1), \
SPI_MEM_OP_ADDR(2, addr, 1), \
SPI_MEM_OP_DUMMY(ndummy, 1), \
SPI_MEM_OP_DATA_IN(len, buf, 4))

+#define SPINAND_PAGE_READ_FROM_CACHE_X4_OP_3A(addr, ndummy, buf, len) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(0x6b, 1), \
+ SPI_MEM_OP_ADDR(3, addr, 1), \
+ SPI_MEM_OP_DUMMY(ndummy, 1), \
+ SPI_MEM_OP_DATA_IN(len, buf, 4))
+
#define SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(addr, ndummy, buf, len) \
SPI_MEM_OP(SPI_MEM_OP_CMD(0xbb, 1), \
SPI_MEM_OP_ADDR(2, addr, 2), \
SPI_MEM_OP_DUMMY(ndummy, 2), \
SPI_MEM_OP_DATA_IN(len, buf, 2))

+#define SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP_3A(addr, ndummy, buf, len) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(0xbb, 1), \
+ SPI_MEM_OP_ADDR(3, addr, 2), \
+ SPI_MEM_OP_DUMMY(ndummy, 2), \
+ SPI_MEM_OP_DATA_IN(len, buf, 2))
+
#define SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(addr, ndummy, buf, len) \
SPI_MEM_OP(SPI_MEM_OP_CMD(0xeb, 1), \
SPI_MEM_OP_ADDR(2, addr, 4), \
SPI_MEM_OP_DUMMY(ndummy, 4), \
SPI_MEM_OP_DATA_IN(len, buf, 4))

+#define SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP_3A(addr, ndummy, buf, len) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(0xeb, 1), \
+ SPI_MEM_OP_ADDR(3, addr, 4), \
+ SPI_MEM_OP_DUMMY(ndummy, 4), \
+ SPI_MEM_OP_DATA_IN(len, buf, 4))
+
#define SPINAND_PROG_EXEC_OP(addr) \
SPI_MEM_OP(SPI_MEM_OP_CMD(0x10, 1), \
SPI_MEM_OP_ADDR(3, addr, 1), \
--
2.20.1

2019-05-22 22:07:48

by Jeff Kletsky

[permalink] [raw]
Subject: [PATCH v4 2/3] mtd: spinand: Add support for two-byte device IDs

From: Jeff Kletsky <[email protected]>

The GigaDevice GD5F1GQ4UFxxG SPI NAND utilizes two-byte device IDs.

http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/

Signed-off-by: Jeff Kletsky <[email protected]>

Reviewed-by: Frieder Schrempf <[email protected]>
---
drivers/mtd/nand/spi/core.c | 2 +-
include/linux/mtd/spinand.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 4c15bb58c623..556bfdb34455 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -845,7 +845,7 @@ spinand_select_op_variant(struct spinand_device *spinand,
*/
int spinand_match_and_init(struct spinand_device *spinand,
const struct spinand_info *table,
- unsigned int table_size, u8 devid)
+ unsigned int table_size, u16 devid)
{
struct nand_device *nand = spinand_to_nand(spinand);
unsigned int i;
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 8aa39ac41e8e..fbc0423bb4ae 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -290,7 +290,7 @@ struct spinand_ecc_info {
*/
struct spinand_info {
const char *model;
- u8 devid;
+ u16 devid;
u32 flags;
struct nand_memory_organization memorg;
struct nand_ecc_req eccreq;
@@ -452,7 +452,7 @@ static inline void spinand_set_of_node(struct spinand_device *spinand,

int spinand_match_and_init(struct spinand_device *dev,
const struct spinand_info *table,
- unsigned int table_size, u8 devid);
+ unsigned int table_size, u16 devid);

int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);
int spinand_select_target(struct spinand_device *spinand, unsigned int target);
--
2.20.1

2019-05-22 22:08:57

by Jeff Kletsky

[permalink] [raw]
Subject: [PATCH v4 3/3] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG

From: Jeff Kletsky <[email protected]>

The GigaDevice GD5F1GQ4UFxxG SPI NAND is in current production devices
and, while it has the same logical layout as the E-series devices,
it differs in the SPI interfacing in significant ways.

This support is contingent on previous commits to:

* Add support for two-byte device IDs
* Define macros for page-read ops with three-byte addresses

http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/

Signed-off-by: Jeff Kletsky <[email protected]>

Reported-by: kbuild test robot <[email protected]>
---
drivers/mtd/nand/spi/gigadevice.c | 79 +++++++++++++++++++++++++------
1 file changed, 64 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
index e5586390026a..b0c26eb5e8b6 100644
--- a/drivers/mtd/nand/spi/gigadevice.c
+++ b/drivers/mtd/nand/spi/gigadevice.c
@@ -9,11 +9,17 @@
#include <linux/mtd/spinand.h>

#define SPINAND_MFR_GIGADEVICE 0xC8
+
#define GD5FXGQ4XA_STATUS_ECC_1_7_BITFLIPS (1 << 4)
#define GD5FXGQ4XA_STATUS_ECC_8_BITFLIPS (3 << 4)

#define GD5FXGQ4UEXXG_REG_STATUS2 0xf0

+#define GD5FXGQ4UXFXXG_STATUS_ECC_MASK (7 << 4)
+#define GD5FXGQ4UXFXXG_STATUS_ECC_NO_BITFLIPS (0 << 4)
+#define GD5FXGQ4UXFXXG_STATUS_ECC_1_3_BITFLIPS (1 << 4)
+#define GD5FXGQ4UXFXXG_STATUS_ECC_UNCOR_ERROR (7 << 4)
+
static SPINAND_OP_VARIANTS(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),
@@ -22,6 +28,14 @@ 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));

+static SPINAND_OP_VARIANTS(read_cache_variants_f,
+ SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_X4_OP_3A(0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_X2_OP_3A(0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_OP_3A(true, 0, 1, NULL, 0),
+ SPINAND_PAGE_READ_FROM_CACHE_OP_3A(false, 0, 0, NULL, 0));
+
static SPINAND_OP_VARIANTS(write_cache_variants,
SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
SPINAND_PROG_LOAD(true, 0, NULL, 0));
@@ -59,6 +73,11 @@ static int gd5fxgq4xa_ooblayout_free(struct mtd_info *mtd, int section,
return 0;
}

+static const struct mtd_ooblayout_ops gd5fxgq4xa_ooblayout = {
+ .ecc = gd5fxgq4xa_ooblayout_ecc,
+ .free = gd5fxgq4xa_ooblayout_free,
+};
+
static int gd5fxgq4xa_ecc_get_status(struct spinand_device *spinand,
u8 status)
{
@@ -83,7 +102,7 @@ static int gd5fxgq4xa_ecc_get_status(struct spinand_device *spinand,
return -EINVAL;
}

-static int gd5fxgq4uexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
+static int gd5fxgq4_variant2_ooblayout_ecc(struct mtd_info *mtd, int section,
struct mtd_oob_region *region)
{
if (section)
@@ -95,7 +114,7 @@ static int gd5fxgq4uexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
return 0;
}

-static int gd5fxgq4uexxg_ooblayout_free(struct mtd_info *mtd, int section,
+static int gd5fxgq4_variant2_ooblayout_free(struct mtd_info *mtd, int section,
struct mtd_oob_region *region)
{
if (section)
@@ -108,6 +127,11 @@ static int gd5fxgq4uexxg_ooblayout_free(struct mtd_info *mtd, int section,
return 0;
}

+static const struct mtd_ooblayout_ops gd5fxgq4_variant2_ooblayout = {
+ .ecc = gd5fxgq4_variant2_ooblayout_ecc,
+ .free = gd5fxgq4_variant2_ooblayout_free,
+};
+
static int gd5fxgq4uexxg_ecc_get_status(struct spinand_device *spinand,
u8 status)
{
@@ -150,15 +174,25 @@ static int gd5fxgq4uexxg_ecc_get_status(struct spinand_device *spinand,
return -EINVAL;
}

-static const struct mtd_ooblayout_ops gd5fxgq4xa_ooblayout = {
- .ecc = gd5fxgq4xa_ooblayout_ecc,
- .free = gd5fxgq4xa_ooblayout_free,
-};
+static int gd5fxgq4ufxxg_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ switch (status & GD5FXGQ4UXFXXG_STATUS_ECC_MASK) {
+ case GD5FXGQ4UXFXXG_STATUS_ECC_NO_BITFLIPS:
+ return 0;

-static const struct mtd_ooblayout_ops gd5fxgq4uexxg_ooblayout = {
- .ecc = gd5fxgq4uexxg_ooblayout_ecc,
- .free = gd5fxgq4uexxg_ooblayout_free,
-};
+ case GD5FXGQ4UXFXXG_STATUS_ECC_1_3_BITFLIPS:
+ return 3;
+
+ case GD5FXGQ4UXFXXG_STATUS_ECC_UNCOR_ERROR:
+ return -EBADMSG;
+
+ default: /* (2 << 4) through (6 << 4) are 4-8 corrected errors */
+ return ((status & GD5FXGQ4UXFXXG_STATUS_ECC_MASK) >> 4) + 2;
+ }
+
+ return -EINVAL;
+}

static const struct spinand_info gigadevice_spinand_table[] = {
SPINAND_INFO("GD5F1GQ4xA", 0xF1,
@@ -195,25 +229,40 @@ static const struct spinand_info gigadevice_spinand_table[] = {
&write_cache_variants,
&update_cache_variants),
0,
- SPINAND_ECCINFO(&gd5fxgq4uexxg_ooblayout,
+ SPINAND_ECCINFO(&gd5fxgq4_variant2_ooblayout,
gd5fxgq4uexxg_ecc_get_status)),
+ SPINAND_INFO("GD5F1GQ4UFxxG", 0xb148,
+ NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
+ NAND_ECCREQ(8, 512),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants_f,
+ &write_cache_variants,
+ &update_cache_variants),
+ 0,
+ SPINAND_ECCINFO(&gd5fxgq4_variant2_ooblayout,
+ gd5fxgq4ufxxg_ecc_get_status)),
};

static int gigadevice_spinand_detect(struct spinand_device *spinand)
{
u8 *id = spinand->id.data;
+ u16 did;
int ret;

/*
- * For GD NANDs, There is an address byte needed to shift in before IDs
- * are read out, so the first byte in raw_id is dummy.
+ * Earlier GDF5-series devices (A,E) return [0][MID][DID]
+ * Later (F) devices return [MID][DID1][DID2]
*/
- if (id[1] != SPINAND_MFR_GIGADEVICE)
+
+ if (id[0] == SPINAND_MFR_GIGADEVICE)
+ did = (id[1] << 8) + id[2];
+ else if (id[0] == 0 && id[1] == SPINAND_MFR_GIGADEVICE)
+ did = id[2];
+ else
return 0;

ret = spinand_match_and_init(spinand, gigadevice_spinand_table,
ARRAY_SIZE(gigadevice_spinand_table),
- id[2]);
+ did);
if (ret)
return ret;

--
2.20.1

2019-05-23 06:44:04

by Frieder Schrempf

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG

On 23.05.19 00:05, Jeff Kletsky wrote:
> From: Jeff Kletsky <[email protected]>
>
> The GigaDevice GD5F1GQ4UFxxG SPI NAND is in current production devices
> and, while it has the same logical layout as the E-series devices,
> it differs in the SPI interfacing in significant ways.
>
> This support is contingent on previous commits to:
>
> * Add support for two-byte device IDs
> * Define macros for page-read ops with three-byte addresses
>
> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
>
> Signed-off-by: Jeff Kletsky <[email protected]>

Reviewed-by: Frieder Schrempf <[email protected]>

>
> Reported-by: kbuild test robot <[email protected]>

I dont't think that this Reported-by tag should be used here. The bot
reported build errors caused by your patch and you fixed it in a new
version. As far as I understand this tag, it references someone who
reported a flaw/bug that led to this change in the first place.
The version history of the changes won't be visible in the git history
later, but the tag will be and would be rather confusing.

> ---
> drivers/mtd/nand/spi/gigadevice.c | 79 +++++++++++++++++++++++++------
> 1 file changed, 64 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
> index e5586390026a..b0c26eb5e8b6 100644
> --- a/drivers/mtd/nand/spi/gigadevice.c
> +++ b/drivers/mtd/nand/spi/gigadevice.c
> @@ -9,11 +9,17 @@
> #include <linux/mtd/spinand.h>
>
> #define SPINAND_MFR_GIGADEVICE 0xC8
> +
> #define GD5FXGQ4XA_STATUS_ECC_1_7_BITFLIPS (1 << 4)
> #define GD5FXGQ4XA_STATUS_ECC_8_BITFLIPS (3 << 4)
>
> #define GD5FXGQ4UEXXG_REG_STATUS2 0xf0
>
> +#define GD5FXGQ4UXFXXG_STATUS_ECC_MASK (7 << 4)
> +#define GD5FXGQ4UXFXXG_STATUS_ECC_NO_BITFLIPS (0 << 4)
> +#define GD5FXGQ4UXFXXG_STATUS_ECC_1_3_BITFLIPS (1 << 4)
> +#define GD5FXGQ4UXFXXG_STATUS_ECC_UNCOR_ERROR (7 << 4)
> +
> static SPINAND_OP_VARIANTS(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),
> @@ -22,6 +28,14 @@ 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));
>
> +static SPINAND_OP_VARIANTS(read_cache_variants_f,
> + SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_X4_OP_3A(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_X2_OP_3A(0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_OP_3A(true, 0, 1, NULL, 0),
> + SPINAND_PAGE_READ_FROM_CACHE_OP_3A(false, 0, 0, NULL, 0));
> +
> static SPINAND_OP_VARIANTS(write_cache_variants,
> SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
> SPINAND_PROG_LOAD(true, 0, NULL, 0));
> @@ -59,6 +73,11 @@ static int gd5fxgq4xa_ooblayout_free(struct mtd_info *mtd, int section,
> return 0;
> }
>
> +static const struct mtd_ooblayout_ops gd5fxgq4xa_ooblayout = {
> + .ecc = gd5fxgq4xa_ooblayout_ecc,
> + .free = gd5fxgq4xa_ooblayout_free,
> +};
> +
> static int gd5fxgq4xa_ecc_get_status(struct spinand_device *spinand,
> u8 status)
> {
> @@ -83,7 +102,7 @@ static int gd5fxgq4xa_ecc_get_status(struct spinand_device *spinand,
> return -EINVAL;
> }
>
> -static int gd5fxgq4uexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
> +static int gd5fxgq4_variant2_ooblayout_ecc(struct mtd_info *mtd, int section,
> struct mtd_oob_region *region)
> {
> if (section)
> @@ -95,7 +114,7 @@ static int gd5fxgq4uexxg_ooblayout_ecc(struct mtd_info *mtd, int section,
> return 0;
> }
>
> -static int gd5fxgq4uexxg_ooblayout_free(struct mtd_info *mtd, int section,
> +static int gd5fxgq4_variant2_ooblayout_free(struct mtd_info *mtd, int section,
> struct mtd_oob_region *region)
> {
> if (section)
> @@ -108,6 +127,11 @@ static int gd5fxgq4uexxg_ooblayout_free(struct mtd_info *mtd, int section,
> return 0;
> }
>
> +static const struct mtd_ooblayout_ops gd5fxgq4_variant2_ooblayout = {
> + .ecc = gd5fxgq4_variant2_ooblayout_ecc,
> + .free = gd5fxgq4_variant2_ooblayout_free,
> +};
> +
> static int gd5fxgq4uexxg_ecc_get_status(struct spinand_device *spinand,
> u8 status)
> {
> @@ -150,15 +174,25 @@ static int gd5fxgq4uexxg_ecc_get_status(struct spinand_device *spinand,
> return -EINVAL;
> }
>
> -static const struct mtd_ooblayout_ops gd5fxgq4xa_ooblayout = {
> - .ecc = gd5fxgq4xa_ooblayout_ecc,
> - .free = gd5fxgq4xa_ooblayout_free,
> -};
> +static int gd5fxgq4ufxxg_ecc_get_status(struct spinand_device *spinand,
> + u8 status)
> +{
> + switch (status & GD5FXGQ4UXFXXG_STATUS_ECC_MASK) {
> + case GD5FXGQ4UXFXXG_STATUS_ECC_NO_BITFLIPS:
> + return 0;
>
> -static const struct mtd_ooblayout_ops gd5fxgq4uexxg_ooblayout = {
> - .ecc = gd5fxgq4uexxg_ooblayout_ecc,
> - .free = gd5fxgq4uexxg_ooblayout_free,
> -};
> + case GD5FXGQ4UXFXXG_STATUS_ECC_1_3_BITFLIPS:
> + return 3;
> +
> + case GD5FXGQ4UXFXXG_STATUS_ECC_UNCOR_ERROR:
> + return -EBADMSG;
> +
> + default: /* (2 << 4) through (6 << 4) are 4-8 corrected errors */
> + return ((status & GD5FXGQ4UXFXXG_STATUS_ECC_MASK) >> 4) + 2;
> + }
> +
> + return -EINVAL;
> +}
>
> static const struct spinand_info gigadevice_spinand_table[] = {
> SPINAND_INFO("GD5F1GQ4xA", 0xF1,
> @@ -195,25 +229,40 @@ static const struct spinand_info gigadevice_spinand_table[] = {
> &write_cache_variants,
> &update_cache_variants),
> 0,
> - SPINAND_ECCINFO(&gd5fxgq4uexxg_ooblayout,
> + SPINAND_ECCINFO(&gd5fxgq4_variant2_ooblayout,
> gd5fxgq4uexxg_ecc_get_status)),
> + SPINAND_INFO("GD5F1GQ4UFxxG", 0xb148,
> + NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
> + NAND_ECCREQ(8, 512),
> + SPINAND_INFO_OP_VARIANTS(&read_cache_variants_f,
> + &write_cache_variants,
> + &update_cache_variants),
> + 0,
> + SPINAND_ECCINFO(&gd5fxgq4_variant2_ooblayout,
> + gd5fxgq4ufxxg_ecc_get_status)),
> };
>
> static int gigadevice_spinand_detect(struct spinand_device *spinand)
> {
> u8 *id = spinand->id.data;
> + u16 did;
> int ret;
>
> /*
> - * For GD NANDs, There is an address byte needed to shift in before IDs
> - * are read out, so the first byte in raw_id is dummy.
> + * Earlier GDF5-series devices (A,E) return [0][MID][DID]
> + * Later (F) devices return [MID][DID1][DID2]
> */
> - if (id[1] != SPINAND_MFR_GIGADEVICE)
> +
> + if (id[0] == SPINAND_MFR_GIGADEVICE)
> + did = (id[1] << 8) + id[2];
> + else if (id[0] == 0 && id[1] == SPINAND_MFR_GIGADEVICE)
> + did = id[2];
> + else
> return 0;
>
> ret = spinand_match_and_init(spinand, gigadevice_spinand_table,
> ARRAY_SIZE(gigadevice_spinand_table),
> - id[2]);
> + did);
> if (ret)
> return ret;
>
>

2019-05-23 06:59:12

by Frieder Schrempf

[permalink] [raw]
Subject: Re: [PATCH v4 0/3] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG

Hi Jeff,

On 23.05.19 00:05, Jeff Kletsky wrote:
> Addresses changes in macros and related data structures introduced by
> commit 377e517b5fa5
> mtd: nand: Add max_bad_eraseblocks_per_lun info to memorg
>
> Resolves issue detected by kbuild test robot
> Tue, 21 May 2019 17:28:09 +0800
> Tue, 21 May 2019 18:17:28 +0800
>
> GD5F1GQ4UFxxG data sheet on page 34 of gd5f1gq4xfxxg_v2.4_20190322.pdf
> indicates "Minumum number of valid blocks (Nvb)" 1004 out of 1024 total.
>
> Newly introduced "max bad blocks" parameter set to 20 (1024-1020).
>
> Rebased on v5.2-rc1 and confirmed patch applies on master.
>
>
> Patches 1/3 and 2/3 are the same as in the previous series.
>
> Patch 3/3, mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG,
> includes the additional parameter (compared here to v3 of the series):
>
> SPINAND_INFO("GD5F1GQ4UFxxG", 0xb148,
> - NAND_MEMORG(1, 2048, 128, 64, 1024, 1, 1, 1),
> + NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
> NAND_ECCREQ(8, 512),
> SPINAND_INFO_OP_VARIANTS(&read_cache_variants_f,
> &write_cache_variants,
>
> R-b of Frieder Schrempf removed from [3/3] as a result this change.

As to what I know, this would not have been necessary in this case. I
missed the missing parameter while reviewing and you fixed it with this
new version, so you obviously improved the patch with a minor change and
it wouldn't be working without this anyway. So I think in such cases you
would typically keep the R-b tags.

Also if you drop the R-b tag from one of the patches in your set, you
should instead CC the reviewer for the whole set. Otherwise
get_maintainer will only CC the reviewer for those patches that contain
his tag. In this case I wasn't CCed for patch 3/3.

Thanks,
Frieder

>
> Supersedes series:
>
> mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG
> https://patchwork.ozlabs.org/project/linux-mtd/list/?series=108868
>
>
>
> Jeff
>
>
>
> Cc: Miquel Raynal <[email protected]>
> Cc: Schrempf Frieder <[email protected]>
> Cc: Boris Brezillon <[email protected]>
> Cc: Richard Weinberger <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Cc: Brian Norris <[email protected]>
> Cc: Marek Vasut <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
>
>

2019-05-24 00:15:31

by Jeff Kletsky

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG

(reduced direct addressees, though still on lists)

On 5/22/19 11:42 PM, Schrempf Frieder wrote:

> On 23.05.19 00:05, Jeff Kletsky wrote:
>> From: Jeff Kletsky <[email protected]>
>>
>> The GigaDevice GD5F1GQ4UFxxG SPI NAND is in current production devices
>> and, while it has the same logical layout as the E-series devices,
>> it differs in the SPI interfacing in significant ways.
>>
>> This support is contingent on previous commits to:
>>
>> * Add support for two-byte device IDs
>> * Define macros for page-read ops with three-byte addresses
>>
>> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
>>
>> Signed-off-by: Jeff Kletsky <[email protected]>
> Reviewed-by: Frieder Schrempf <[email protected]>
>
>> Reported-by: kbuild test robot <[email protected]>
> I dont't think that this Reported-by tag should be used here. The bot
> reported build errors caused by your patch and you fixed it in a new
> version. As far as I understand this tag, it references someone who
> reported a flaw/bug that led to this change in the first place.
> The version history of the changes won't be visible in the git history
> later, but the tag will be and would be rather confusing.

Thank you for your patience and explanations. I've been being conservative
as I'm not a "seasoned, Linux professional" and am still getting my
git send-email config / command line for Linux properly straightened out.

Should I send another patch set with the `kbuild...` tag removed,
or would it be removed in the process of an appropriate member
of the Linux MTD team adding their tag for approval, if and when
that happens?

Jeff

2019-05-27 06:37:29

by Frieder Schrempf

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG

Hi Jeff,

On 24.05.19 02:12, Jeff Kletsky wrote:
> (reduced direct addressees, though still on lists)
>
> On 5/22/19 11:42 PM, Schrempf Frieder wrote:
>
>> On 23.05.19 00:05, Jeff Kletsky wrote:
>>> From: Jeff Kletsky <[email protected]>
>>>
>>> The GigaDevice GD5F1GQ4UFxxG SPI NAND is in current production devices
>>> and, while it has the same logical layout as the E-series devices,
>>> it differs in the SPI interfacing in significant ways.
>>>
>>> This support is contingent on previous commits to:
>>>
>>>     * Add support for two-byte device IDs
>>>     * Define macros for page-read ops with three-byte addresses
>>>
>>> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
>>>
>>> Signed-off-by: Jeff Kletsky <[email protected]>
>> Reviewed-by: Frieder Schrempf <[email protected]>
>>
>>> Reported-by: kbuild test robot <[email protected]>
>> I dont't think that this Reported-by tag should be used here. The bot
>> reported build errors caused by your patch and you fixed it in a new
>> version. As far as I understand this tag, it references someone who
>> reported a flaw/bug that led to this change in the first place.
>> The version history of the changes won't be visible in the git history
>> later, but the tag will be and would be rather confusing.
>
> Thank you for your patience and explanations. I've been being conservative
> as I'm not a "seasoned, Linux professional" and am still getting my
> git send-email config / command line for Linux properly straightened out.

Being conservative in such cases is not a fault at all. I'm not an
expert either. I'm just recommending what I think might be the "correct"
way to do it.

> Should I send another patch set with the `kbuild...` tag removed,
> or would it be removed in the process of an appropriate member
> of the Linux MTD team adding their tag for approval, if and when
> that happens?

I don't think that's necessary. Miquèl is the one to pick up the patch,
so he could probably drop the "Reported-by: kbuild" when he applies it.

Regards,
Frieder

2019-05-27 09:29:51

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG

Hi Schrempf,

Schrempf Frieder <[email protected]> wrote on Mon, 27 May
2019 06:35:59 +0000:

> Hi Jeff,
>
> On 24.05.19 02:12, Jeff Kletsky wrote:
> > (reduced direct addressees, though still on lists)
> >
> > On 5/22/19 11:42 PM, Schrempf Frieder wrote:
> >
> >> On 23.05.19 00:05, Jeff Kletsky wrote:
> >>> From: Jeff Kletsky <[email protected]>
> >>>
> >>> The GigaDevice GD5F1GQ4UFxxG SPI NAND is in current production devices
> >>> and, while it has the same logical layout as the E-series devices,
> >>> it differs in the SPI interfacing in significant ways.
> >>>
> >>> This support is contingent on previous commits to:
> >>>
> >>>     * Add support for two-byte device IDs
> >>>     * Define macros for page-read ops with three-byte addresses
> >>>
> >>> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
> >>>
> >>> Signed-off-by: Jeff Kletsky <[email protected]>
> >> Reviewed-by: Frieder Schrempf <[email protected]>
> >>
> >>> Reported-by: kbuild test robot <[email protected]>
> >> I dont't think that this Reported-by tag should be used here. The bot
> >> reported build errors caused by your patch and you fixed it in a new
> >> version. As far as I understand this tag, it references someone who
> >> reported a flaw/bug that led to this change in the first place.
> >> The version history of the changes won't be visible in the git history
> >> later, but the tag will be and would be rather confusing.
> >
> > Thank you for your patience and explanations. I've been being conservative
> > as I'm not a "seasoned, Linux professional" and am still getting my
> > git send-email config / command line for Linux properly straightened out.
>
> Being conservative in such cases is not a fault at all. I'm not an
> expert either. I'm just recommending what I think might be the "correct"
> way to do it.
>
> > Should I send another patch set with the `kbuild...` tag removed,
> > or would it be removed in the process of an appropriate member
> > of the Linux MTD team adding their tag for approval, if and when
> > that happens?
>
> I don't think that's necessary. Miquèl is the one to pick up the patch,
> so he could probably drop the "Reported-by: kbuild" when he applies it.

I will drop it.

Also, please do not add an empty line between tags, they should be a
single block. I will also modify your commits for this time.

Thanks,
Miquèl

2019-06-03 08:05:15

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG

On Wed, 2019-05-22 at 22:05:55 UTC, Jeff Kletsky wrote:
> From: Jeff Kletsky <[email protected]>
>
> The GigaDevice GD5F1GQ4UFxxG SPI NAND is in current production devices
> and, while it has the same logical layout as the E-series devices,
> it differs in the SPI interfacing in significant ways.
>
> This support is contingent on previous commits to:
>
> * Add support for two-byte device IDs
> * Define macros for page-read ops with three-byte addresses
>
> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
>
> Signed-off-by: Jeff Kletsky <[email protected]>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

2019-06-03 08:05:28

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] mtd: spinand: Add support for two-byte device IDs

On Wed, 2019-05-22 at 22:05:54 UTC, Jeff Kletsky wrote:
> From: Jeff Kletsky <[email protected]>
>
> The GigaDevice GD5F1GQ4UFxxG SPI NAND utilizes two-byte device IDs.
>
> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
>
> Signed-off-by: Jeff Kletsky <[email protected]>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

2019-06-03 10:41:20

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] mtd: spinand: Define macros for page-read ops with three-byte addresses

On Wed, 2019-05-22 at 22:05:53 UTC, Jeff Kletsky wrote:
> From: Jeff Kletsky <[email protected]>
>
> The GigaDevice GD5F1GQ4UFxxG SPI NAND utilizes three-byte addresses
> for its page-read ops.
>
> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
>
> Signed-off-by: Jeff Kletsky <[email protected]>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel