* [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG
@ 2019-05-10 12:17 lede
2019-05-13 7:02 ` Schrempf Frieder
2019-05-13 13:56 ` Schrempf Frieder
0 siblings, 2 replies; 6+ messages in thread
From: lede @ 2019-05-10 12:17 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut
Cc: linux-mtd, linux-kernel
From: Jeff Kletsky <git-commits@allycomm.com>
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.
To accommodate these changes, this patch also:
* Adds support for two-byte manufacturer IDs
* Adds #define-s for three-byte addressing for read ops
http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
Signed-off-by: Jeff Kletsky <git-commits@allycomm.com>
---
drivers/mtd/nand/spi/core.c | 2 +-
drivers/mtd/nand/spi/gigadevice.c | 79 +++++++++++++++++++++++++++++++--------
include/linux/mtd/spinand.h | 34 ++++++++++++++++-
3 files changed, 97 insertions(+), 18 deletions(-)
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index fa87ae28cdfe..a13154785dad 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -853,7 +853,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/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
index 0b49d8264bef..d6497ac4c5d8 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, 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;
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index b92e2aa955b6..8901ba272538 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), \
@@ -260,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;
@@ -415,7 +445,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.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG
2019-05-10 12:17 [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG lede
@ 2019-05-13 7:02 ` Schrempf Frieder
2019-05-13 13:56 ` Schrempf Frieder
1 sibling, 0 replies; 6+ messages in thread
From: Schrempf Frieder @ 2019-05-13 7:02 UTC (permalink / raw)
To: linux-mtd
Hi Jeff,
On 10.05.19 14:17, lede@allycomm.com wrote:
> From: Jeff Kletsky <git-commits@allycomm.com>
>
> 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.
>
> To accommodate these changes, this patch also:
>
> * Adds support for two-byte manufacturer IDs
> * Adds #define-s for three-byte addressing for read ops
>
> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
>
> Signed-off-by: Jeff Kletsky <git-commits@allycomm.com>
Maybe it would be better to split this patch into three parts:
* Add support for two-byte manufacturer IDs
* Add #define-s for three-byte addressing for read ops
* Add support for GD5F1GQ4UFxxG
Anyway the content looks good to me, so:
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
> drivers/mtd/nand/spi/core.c | 2 +-
> drivers/mtd/nand/spi/gigadevice.c | 79 +++++++++++++++++++++++++++++++--------
> include/linux/mtd/spinand.h | 34 ++++++++++++++++-
> 3 files changed, 97 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index fa87ae28cdfe..a13154785dad 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -853,7 +853,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/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
> index 0b49d8264bef..d6497ac4c5d8 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, 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;
>
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> index b92e2aa955b6..8901ba272538 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), \
> @@ -260,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;
> @@ -415,7 +445,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);
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG
2019-05-10 12:17 [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG lede
2019-05-13 7:02 ` Schrempf Frieder
@ 2019-05-13 13:56 ` Schrempf Frieder
[not found] ` <1023ba21-b188-1dcc-3ecc-c563d4cb8a67@allycomm.com>
1 sibling, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2019-05-13 13:56 UTC (permalink / raw)
To: lede, Boris Brezillon, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut
Cc: linux-mtd, linux-kernel
Hi Jeff,
I just noticed I hit the wrong button and my previous reply was only
sent to the MTD list, so I'm resending with fixed recipients...
On 10.05.19 14:17, lede@allycomm.com wrote:
> From: Jeff Kletsky <git-commits@allycomm.com>
>
> 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.
>
> To accommodate these changes, this patch also:
>
> * Adds support for two-byte manufacturer IDs
> * Adds #define-s for three-byte addressing for read ops
>
> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
>
> Signed-off-by: Jeff Kletsky <git-commits@allycomm.com>
Maybe it would be better to split this patch into three parts:
* Add support for two-byte device IDs
* Add #define-s for three-byte addressing for read ops
* Add support for GD5F1GQ4UFxxG
Anyway the content looks good to me, so:
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
> drivers/mtd/nand/spi/core.c | 2 +-
> drivers/mtd/nand/spi/gigadevice.c | 79 +++++++++++++++++++++++++++++++--------
> include/linux/mtd/spinand.h | 34 ++++++++++++++++-
> 3 files changed, 97 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index fa87ae28cdfe..a13154785dad 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -853,7 +853,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/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
> index 0b49d8264bef..d6497ac4c5d8 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, 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;
>
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> index b92e2aa955b6..8901ba272538 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), \
> @@ -260,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;
> @@ -415,7 +445,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);
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG
[not found] ` <1023ba21-b188-1dcc-3ecc-c563d4cb8a67@allycomm.com>
@ 2019-05-14 16:11 ` Schrempf Frieder
2019-05-14 20:01 ` Miquel Raynal
0 siblings, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2019-05-14 16:11 UTC (permalink / raw)
To: Jeff Kletsky, Boris Brezillon, Miquel Raynal, Richard Weinberger,
David Woodhouse, Brian Norris, Marek Vasut
Cc: linux-mtd, linux-kernel
Hi Jeff,
On 14.05.19 17:42, Jeff Kletsky wrote:
> On 5/13/19 6:56 AM, Schrempf Frieder wrote:
>
>> Hi Jeff,
>>
>> I just noticed I hit the wrong button and my previous reply was only
>> sent to the MTD list, so I'm resending with fixed recipients...
>>
>> On 10.05.19 14:17,lede@allycomm.com wrote:
>>> From: Jeff Kletsky<git-commits@allycomm.com>
>>>
>>> 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.
>>>
>>> To accommodate these changes, this patch also:
>>>
>>> * Adds support for two-byte manufacturer IDs
>>> * Adds #define-s for three-byte addressing for read ops
>>>
>>> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
>>>
>>> Signed-off-by: Jeff Kletsky<git-commits@allycomm.com>
>> Maybe it would be better to split this patch into three parts:
>> * Add support for two-byte device IDs
>> * Add #define-s for three-byte addressing for read ops
>> * Add support for GD5F1GQ4UFxxG
>>
>> Anyway the content looks good to me, so:
>>
>> Reviewed-by: Frieder Schrempf<frieder.schrempf@kontron.de>
>>
>> [...]
>
> Thanks for the time in review and good words!
You're welcome!
> My apologies for an incomplete git-send-email config that left
> me nameless in the headers.
No problem, I guessed your name from the Signed-off-by tag ;)
> I wasn't sure if that was direction to submit as three patches
> at this time, but would be happy to do so if the consensus is
> that it the direction to follow.
I think it's common to separate logical different changes. This makes it
easier to read.
Also the preparation changes only touch the SPI NAND core. I guess
that's another reason why they should be separated from the
chip-specific changes.
> At least for me, I feel that the other two don't really stand
> on their own without the context for their need.
I don't think that's a problem. Just add a note to the commit message
that these core changes are needed to prepare for the GD5F1GQ4UFxxG support.
Thanks,
Frieder
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG
2019-05-14 16:11 ` Schrempf Frieder
@ 2019-05-14 20:01 ` Miquel Raynal
2019-05-14 22:08 ` Jeff Kletsky
0 siblings, 1 reply; 6+ messages in thread
From: Miquel Raynal @ 2019-05-14 20:01 UTC (permalink / raw)
To: Schrempf Frieder
Cc: Jeff Kletsky, Boris Brezillon, Richard Weinberger, linux-kernel,
Marek Vasut, linux-mtd, Brian Norris, David Woodhouse
Hi Schrempf,
Schrempf Frieder <frieder.schrempf@kontron.de> wrote on Tue, 14 May
2019 16:11:28 +0000:
> Hi Jeff,
>
> On 14.05.19 17:42, Jeff Kletsky wrote:
> > On 5/13/19 6:56 AM, Schrempf Frieder wrote:
> >
> >> Hi Jeff,
> >>
> >> I just noticed I hit the wrong button and my previous reply was only
> >> sent to the MTD list, so I'm resending with fixed recipients...
> >>
> >> On 10.05.19 14:17,lede@allycomm.com wrote:
> >>> From: Jeff Kletsky<git-commits@allycomm.com>
> >>>
> >>> 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.
> >>>
> >>> To accommodate these changes, this patch also:
> >>>
> >>> * Adds support for two-byte manufacturer IDs
> >>> * Adds #define-s for three-byte addressing for read ops
> >>>
> >>> http://www.gigadevice.com/datasheet/gd5f1gq4xfxxg/
> >>>
> >>> Signed-off-by: Jeff Kletsky<git-commits@allycomm.com>
> >> Maybe it would be better to split this patch into three parts:
> >> * Add support for two-byte device IDs
> >> * Add #define-s for three-byte addressing for read ops
> >> * Add support for GD5F1GQ4UFxxG
> >>
> >> Anyway the content looks good to me, so:
> >>
> >> Reviewed-by: Frieder Schrempf<frieder.schrempf@kontron.de>
> >>
> >> [...]
> >
> > Thanks for the time in review and good words!
>
> You're welcome!
>
> > My apologies for an incomplete git-send-email config that left
> > me nameless in the headers.
>
> No problem, I guessed your name from the Signed-off-by tag ;)
>
> > I wasn't sure if that was direction to submit as three patches
> > at this time, but would be happy to do so if the consensus is
> > that it the direction to follow.
>
> I think it's common to separate logical different changes. This makes it
> easier to read.
> Also the preparation changes only touch the SPI NAND core. I guess
> that's another reason why they should be separated from the
> chip-specific changes.
>
> > At least for me, I feel that the other two don't really stand
> > on their own without the context for their need.
>
> I don't think that's a problem. Just add a note to the commit message
> that these core changes are needed to prepare for the GD5F1GQ4UFxxG support.
>
> Thanks,
> Frieder
I agree with Frieder, if you don't mind, please split this commit in
three.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG
2019-05-14 20:01 ` Miquel Raynal
@ 2019-05-14 22:08 ` Jeff Kletsky
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Kletsky @ 2019-05-14 22:08 UTC (permalink / raw)
To: Miquel Raynal, Schrempf Frieder
Cc: Boris Brezillon, Richard Weinberger, linux-kernel, Marek Vasut,
linux-mtd, Brian Norris, David Woodhouse
On 5/14/19 1:01 PM, Miquel Raynal wrote:
> Hi Schrempf,
>
> Schrempf Frieder <frieder.schrempf@kontron.de> wrote on Tue, 14 May
> 2019 16:11:28 +0000:
>
>> Hi Jeff,
>>
>> On 14.05.19 17:42, Jeff Kletsky wrote:
>>> On 5/13/19 6:56 AM, Schrempf Frieder wrote:
>>>
>>>> Hi Jeff,
>>>>
>>>> [...]
>>>> Maybe it would be better to split this patch into three parts:
>>>> * Add support for two-byte device IDs
>>>> * Add #define-s for three-byte addressing for read ops
>>>> * Add support for GD5F1GQ4UFxxG
>>>>
>>>> Anyway the content looks good to me, so:
>>>>
>>>> Reviewed-by: Frieder Schrempf<frieder.schrempf@kontron.de>
>>>>
>>>> [...]
>>> [...]
> I agree with Frieder, if you don't mind, please split this commit in
> three.
>
> Thanks,
> Miquèl
>
Split and superseded by
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=107874
Jeff
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-05-14 22:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-10 12:17 [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG lede
2019-05-13 7:02 ` Schrempf Frieder
2019-05-13 13:56 ` Schrempf Frieder
[not found] ` <1023ba21-b188-1dcc-3ecc-c563d4cb8a67@allycomm.com>
2019-05-14 16:11 ` Schrempf Frieder
2019-05-14 20:01 ` Miquel Raynal
2019-05-14 22:08 ` Jeff Kletsky
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).