[PATCH] mtd: nand: spi: Add initial support for Toshiba TC58CVG2S0H

[Schrempf Frieder]

Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.

Signed-off-by: Frieder Schrempf <***@kontron.de>
---
drivers/mtd/nand/spi/Makefile | 2 +-
drivers/mtd/nand/spi/core.c | 1 +
drivers/mtd/nand/spi/toshiba.c | 136 ++++++++++++++++++++++++++++++++++++
include/linux/mtd/spinand.h | 1 +
4 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index b74e074..be5f735 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
-spinand-objs := core.o macronix.o micron.o winbond.o
+spinand-objs := core.o macronix.o micron.o toshiba.o winbond.o
obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 30f8364..87bdf2a 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -766,6 +766,7 @@ static const struct nand_ops spinand_ops = {
static const struct spinand_manufacturer *spinand_manufacturers[] = {
&macronix_spinand_manufacturer,
&micron_spinand_manufacturer,
+ &toshiba_spinand_manufacturer,
&winbond_spinand_manufacturer,
};

diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
new file mode 100644
index 0000000..294bcf6
--- /dev/null
+++ b/drivers/mtd/nand/spi/toshiba.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 exceet electronics GmbH
+ * Copyright (c) 2018 Kontron Electronics GmbH
+ *
+ * Author: Frieder Schrempf <***@kontron.de>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_TOSHIBA 0x98
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+ SPINAND_PAGE_READ_FROM_CACHE_X4_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(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+ SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 128 + 16 * section;
+ region->length = 16;
+
+
+ return 0;
+}
+
+static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 2 + 16 * section;
+ region->length = 14;
+
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops tc58cvg2s0h_ooblayout = {
+ .ecc = tc58cvg2s0h_ooblayout_ecc,
+ .free = tc58cvg2s0h_ooblayout_free,
+};
+
+static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ struct nand_device *nand = spinand_to_nand(spinand);
+ u8 mbf = 0;
+ struct spi_mem_op op = SPINAND_GET_FEATURE_OP(0x30, &mbf);
+
+ switch (status & STATUS_ECC_MASK) {
+ case STATUS_ECC_NO_BITFLIPS:
+ return 0;
+
+ case STATUS_ECC_UNCOR_ERROR:
+ return -EBADMSG;
+
+ case STATUS_ECC_HAS_BITFLIPS:
+ /*
+ * Let's try to retrieve the real maximum number of bitflips
+ * in order to avoid forcing the wear-leveling layer to move
+ * data around if it's not necessary.
+ */
+ if (spi_mem_exec_op(spinand->spimem, &op))
+ return nand->eccreq.strength;
+
+ mbf >>= 4;
+
+ if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
+ return nand->eccreq.strength;
+
+ return mbf;
+
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct spinand_info toshiba_spinand_table[] = {
+ SPINAND_INFO("TC58CVG2S0H", 0xCD,
+ NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
+ NAND_ECCREQ(8, 512),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_HAS_QE_BIT,
+ SPINAND_ECCINFO(&tc58cvg2s0h_ooblayout,
+ tc58cvg2s0h_ecc_get_status)),
+};
+
+static int toshiba_spinand_detect(struct spinand_device *spinand)
+{
+ u8 *id = spinand->id.data;
+ int ret;
+
+ /*
+ * Toshiba SPI NAND read ID needs a dummy byte,
+ * so the first byte in id is garbage.
+ */
+ if (id[1] != SPINAND_MFR_TOSHIBA)
+ return 0;
+
+ ret = spinand_match_and_init(spinand, toshiba_spinand_table,
+ ARRAY_SIZE(toshiba_spinand_table),
+ id[2]);
+ if (ret)
+ return ret;
+
+ return 1;
+}
+
+static const struct spinand_manufacturer_ops toshiba_spinand_manuf_ops = {
+ .detect = toshiba_spinand_detect,
+};
+
+const struct spinand_manufacturer toshiba_spinand_manufacturer = {
+ .id = SPINAND_MFR_TOSHIBA,
+ .name = "Toshiba",
+ .ops = &toshiba_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 088ff96..816c4b0 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -196,6 +196,7 @@ struct spinand_manufacturer {
/* SPI NAND manufacturers */
extern const struct spinand_manufacturer macronix_spinand_manufacturer;
extern const struct spinand_manufacturer micron_spinand_manufacturer;
+extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
extern const struct spinand_manufacturer winbond_spinand_manufacturer;

/**

[Boris Brezillon]

Nitpick: subject prefix should be "mtd: spinand" not
"mtd: nand: spi" :-).

On Thu, 8 Nov 2018 08:32:11 +0000

Post by Schrempf Frieder
Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.
---
drivers/mtd/nand/spi/Makefile | 2 +-
drivers/mtd/nand/spi/core.c | 1 +
drivers/mtd/nand/spi/toshiba.c | 136 ++++++++++++++++++++++++++++++++++++
include/linux/mtd/spinand.h | 1 +
4 files changed, 139 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index b74e074..be5f735 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
-spinand-objs := core.o macronix.o micron.o winbond.o
+spinand-objs := core.o macronix.o micron.o toshiba.o winbond.o
obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 30f8364..87bdf2a 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -766,6 +766,7 @@ static const struct nand_ops spinand_ops = {
static const struct spinand_manufacturer *spinand_manufacturers[] = {
&macronix_spinand_manufacturer,
&micron_spinand_manufacturer,
+ &toshiba_spinand_manufacturer,
&winbond_spinand_manufacturer,
};
diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
new file mode 100644
index 0000000..294bcf6
--- /dev/null
+++ b/drivers/mtd/nand/spi/toshiba.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 exceet electronics GmbH
+ * Copyright (c) 2018 Kontron Electronics GmbH
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_TOSHIBA 0x98
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+ SPINAND_PAGE_READ_FROM_CACHE_X4_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(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+ SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 128 + 16 * section;
+ region->length = 16;
+
+
+ return 0;
+}
+
+static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 2 + 16 * section;
+ region->length = 14;
+
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops tc58cvg2s0h_ooblayout = {
+ .ecc = tc58cvg2s0h_ooblayout_ecc,
+ .free = tc58cvg2s0h_ooblayout_free,
+};
+
+static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ struct nand_device *nand = spinand_to_nand(spinand);
+ u8 mbf = 0;
+ struct spi_mem_op op = SPINAND_GET_FEATURE_OP(0x30, &mbf);
+
+ switch (status & STATUS_ECC_MASK) {
+ return 0;
+
+ return -EBADMSG;
+
+ /*
+ * Let's try to retrieve the real maximum number of bitflips
+ * in order to avoid forcing the wear-leveling layer to move
+ * data around if it's not necessary.
+ */
+ if (spi_mem_exec_op(spinand->spimem, &op))
+ return nand->eccreq.strength;
+
+ mbf >>= 4;
+
+ if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
+ return nand->eccreq.strength;
+
+ return mbf;
+
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct spinand_info toshiba_spinand_table[] = {
+ SPINAND_INFO("TC58CVG2S0H", 0xCD,
+ NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
+ NAND_ECCREQ(8, 512),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_HAS_QE_BIT,
+ SPINAND_ECCINFO(&tc58cvg2s0h_ooblayout,
+ tc58cvg2s0h_ecc_get_status)),
+};
+
+static int toshiba_spinand_detect(struct spinand_device *spinand)
+{
+ u8 *id = spinand->id.data;
+ int ret;
+
+ /*
+ * Toshiba SPI NAND read ID needs a dummy byte,
+ * so the first byte in id is garbage.
+ */
+ if (id[1] != SPINAND_MFR_TOSHIBA)
+ return 0;
+
+ ret = spinand_match_and_init(spinand, toshiba_spinand_table,
+ ARRAY_SIZE(toshiba_spinand_table),
+ id[2]);
+ if (ret)
+ return ret;
+
+ return 1;
+}
+
+static const struct spinand_manufacturer_ops toshiba_spinand_manuf_ops = {
+ .detect = toshiba_spinand_detect,
+};
+
+const struct spinand_manufacturer toshiba_spinand_manufacturer = {
+ .id = SPINAND_MFR_TOSHIBA,
+ .name = "Toshiba",
+ .ops = &toshiba_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 088ff96..816c4b0 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -196,6 +196,7 @@ struct spinand_manufacturer {
/* SPI NAND manufacturers */
extern const struct spinand_manufacturer macronix_spinand_manufacturer;
extern const struct spinand_manufacturer micron_spinand_manufacturer;
+extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
extern const struct spinand_manufacturer winbond_spinand_manufacturer;
/**

[Miquel Raynal]

Hi Schrempf,

Post by Schrempf Frieder
Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.
---

With "mtd: spinand:" as prefix, applied to nand/next.

Thanks,
Miquèl

[Schrempf Frieder]

Hi Miquèl,

Post by Miquel Raynal
Hi Schrempf,

Post by Schrempf Frieder
Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.
---

With "mtd: spinand:" as prefix, applied to nand/next.

Clément suggested some fixes/improvements for this patch. You can find
them in this patch: https://patchwork.ozlabs.org/patch/1004501/.

When these changes are approved, you can decide if you apply this on top
of the initial patch or if you can/want to squash both patches.

Thanks,
Frieder

[Schrempf Frieder]

+Clément Péron

Hi Clément,

FYI, this has already been merged to nand/next.

Regards,
Frieder

Post by Schrempf Frieder
Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.
---
drivers/mtd/nand/spi/Makefile | 2 +-
drivers/mtd/nand/spi/core.c | 1 +
drivers/mtd/nand/spi/toshiba.c | 136 ++++++++++++++++++++++++++++++++++++
include/linux/mtd/spinand.h | 1 +
4 files changed, 139 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index b74e074..be5f735 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
-spinand-objs := core.o macronix.o micron.o winbond.o
+spinand-objs := core.o macronix.o micron.o toshiba.o winbond.o
obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 30f8364..87bdf2a 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -766,6 +766,7 @@ static const struct nand_ops spinand_ops = {
static const struct spinand_manufacturer *spinand_manufacturers[] = {
&macronix_spinand_manufacturer,
&micron_spinand_manufacturer,
+ &toshiba_spinand_manufacturer,
&winbond_spinand_manufacturer,
};
diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
new file mode 100644
index 0000000..294bcf6
--- /dev/null
+++ b/drivers/mtd/nand/spi/toshiba.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 exceet electronics GmbH
+ * Copyright (c) 2018 Kontron Electronics GmbH
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_TOSHIBA 0x98
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+ SPINAND_PAGE_READ_FROM_CACHE_X4_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(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+ SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 128 + 16 * section;
+ region->length = 16;
+
+
+ return 0;
+}
+
+static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 2 + 16 * section;
+ region->length = 14;
+
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops tc58cvg2s0h_ooblayout = {
+ .ecc = tc58cvg2s0h_ooblayout_ecc,
+ .free = tc58cvg2s0h_ooblayout_free,
+};
+
+static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ struct nand_device *nand = spinand_to_nand(spinand);
+ u8 mbf = 0;
+ struct spi_mem_op op = SPINAND_GET_FEATURE_OP(0x30, &mbf);
+
+ switch (status & STATUS_ECC_MASK) {
+ return 0;
+
+ return -EBADMSG;
+
+ /*
+ * Let's try to retrieve the real maximum number of bitflips
+ * in order to avoid forcing the wear-leveling layer to move
+ * data around if it's not necessary.
+ */
+ if (spi_mem_exec_op(spinand->spimem, &op))
+ return nand->eccreq.strength;
+
+ mbf >>= 4;
+
+ if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
+ return nand->eccreq.strength;
+
+ return mbf;
+
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct spinand_info toshiba_spinand_table[] = {
+ SPINAND_INFO("TC58CVG2S0H", 0xCD,
+ NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
+ NAND_ECCREQ(8, 512),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_HAS_QE_BIT,
+ SPINAND_ECCINFO(&tc58cvg2s0h_ooblayout,
+ tc58cvg2s0h_ecc_get_status)),
+};
+
+static int toshiba_spinand_detect(struct spinand_device *spinand)
+{
+ u8 *id = spinand->id.data;
+ int ret;
+
+ /*
+ * Toshiba SPI NAND read ID needs a dummy byte,
+ * so the first byte in id is garbage.
+ */
+ if (id[1] != SPINAND_MFR_TOSHIBA)
+ return 0;
+
+ ret = spinand_match_and_init(spinand, toshiba_spinand_table,
+ ARRAY_SIZE(toshiba_spinand_table),
+ id[2]);
+ if (ret)
+ return ret;
+
+ return 1;
+}
+
+static const struct spinand_manufacturer_ops toshiba_spinand_manuf_ops = {
+ .detect = toshiba_spinand_detect,
+};
+
+const struct spinand_manufacturer toshiba_spinand_manufacturer = {
+ .id = SPINAND_MFR_TOSHIBA,
+ .name = "Toshiba",
+ .ops = &toshiba_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 088ff96..816c4b0 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -196,6 +196,7 @@ struct spinand_manufacturer {
/* SPI NAND manufacturers */
extern const struct spinand_manufacturer macronix_spinand_manufacturer;
extern const struct spinand_manufacturer micron_spinand_manufacturer;
+extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
extern const struct spinand_manufacturer winbond_spinand_manufacturer;
/**

______________________________________________________
Linux MTD discussion mailing list

[Clément Péron]

Hi Frieder,

On Tue, 27 Nov 2018 at 16:08, Schrempf Frieder

Post by Schrempf Frieder
+Clément Péron
Hi Clément,
FYI, this has already been merged to nand/next.

Just want to point the difference that I made when I try to introduce my driver.
The datasheet I used is this one :
https://media.digikey.com/pdf/Data%20Sheets/Toshiba%20PDFs/TC58CVG2S0HxAIx_Rev1.1_2016-11-08.pdf

Post by Schrempf Frieder
Regards,
Frieder

Post by Schrempf Frieder
Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.
---
drivers/mtd/nand/spi/Makefile | 2 +-
drivers/mtd/nand/spi/core.c | 1 +
drivers/mtd/nand/spi/toshiba.c | 136 ++++++++++++++++++++++++++++++++++++
include/linux/mtd/spinand.h | 1 +
4 files changed, 139 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index b74e074..be5f735 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
-spinand-objs := core.o macronix.o micron.o winbond.o
+spinand-objs := core.o macronix.o micron.o toshiba.o winbond.o
obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 30f8364..87bdf2a 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -766,6 +766,7 @@ static const struct nand_ops spinand_ops = {
static const struct spinand_manufacturer *spinand_manufacturers[] = {
&macronix_spinand_manufacturer,
&micron_spinand_manufacturer,
+ &toshiba_spinand_manufacturer,
&winbond_spinand_manufacturer,
};
diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
new file mode 100644
index 0000000..294bcf6
--- /dev/null
+++ b/drivers/mtd/nand/spi/toshiba.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 exceet electronics GmbH
+ * Copyright (c) 2018 Kontron Electronics GmbH
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_TOSHIBA 0x98
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+ SPINAND_PAGE_READ_FROM_CACHE_X4_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(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+ SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 128 + 16 * section;
+ region->length = 16;

Here you expose the ECC bits has 8 sections of 16 bytes.
But regarding the datasheet this should not be accessed page 32.
"The ECC parity code generated by internal ECC is stored in column
addresses 4224-4351 and the users cannot access to these specific
addresses when internal ECC is enabled."

Post by Schrempf Frieder

Post by Schrempf Frieder
+
+
+ return 0;
+}
+
+static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 2 + 16 * section;
+ region->length = 14;

This reserved 2 bytes for BBM for each section.
But maybe we could declare this as 1 section of 128bytes:

if (section)
return -ERANGE;

region->offset = 2;
region->length = 126;

Post by Schrempf Frieder

Post by Schrempf Frieder
+
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops tc58cvg2s0h_ooblayout = {
+ .ecc = tc58cvg2s0h_ooblayout_ecc,
+ .free = tc58cvg2s0h_ooblayout_free,
+};
+
+static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ struct nand_device *nand = spinand_to_nand(spinand);
+ u8 mbf = 0;
+ struct spi_mem_op op = SPINAND_GET_FEATURE_OP(0x30, &mbf);
+
+ switch (status & STATUS_ECC_MASK) {
+ return 0;
+
+ return -EBADMSG;
+
+ /*
+ * Let's try to retrieve the real maximum number of bitflips
+ * in order to avoid forcing the wear-leveling layer to move
+ * data around if it's not necessary.
+ */
+ if (spi_mem_exec_op(spinand->spimem, &op))
+ return nand->eccreq.strength;
+
+ mbf >>= 4;
+
+ if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
+ return nand->eccreq.strength;
+
+ return mbf;
+

These take take care of 0, 0x1 and 0x2 but not 0x3 which is valid
page 26 of the datasheet :

ECC status bits indicate the status of internal ECC operation
00b: No bit flips were detected in previous page read.
01b: Bit flips were detected and corrected. Bit flip count did not
exceed the bit flip detection threshold. The threshold is set by bits
[7:4] in address 10h in the feature table.
10b: Multiple bit flips were detected and not corrected.
11b: Bit flips were detected and corrected. Bit flip count exceeded
the bit flip detection threshold. The threshold is set by bits [7:4]
in address 10h in the feature table

Regards,
Clement

Post by Schrempf Frieder

Post by Schrempf Frieder
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct spinand_info toshiba_spinand_table[] = {
+ SPINAND_INFO("TC58CVG2S0H", 0xCD,
+ NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
+ NAND_ECCREQ(8, 512),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_HAS_QE_BIT,
+ SPINAND_ECCINFO(&tc58cvg2s0h_ooblayout,
+ tc58cvg2s0h_ecc_get_status)),
+};
+
+static int toshiba_spinand_detect(struct spinand_device *spinand)
+{
+ u8 *id = spinand->id.data;
+ int ret;
+
+ /*
+ * Toshiba SPI NAND read ID needs a dummy byte,
+ * so the first byte in id is garbage.
+ */
+ if (id[1] != SPINAND_MFR_TOSHIBA)
+ return 0;
+
+ ret = spinand_match_and_init(spinand, toshiba_spinand_table,
+ ARRAY_SIZE(toshiba_spinand_table),
+ id[2]);
+ if (ret)
+ return ret;
+
+ return 1;
+}
+
+static const struct spinand_manufacturer_ops toshiba_spinand_manuf_ops = {
+ .detect = toshiba_spinand_detect,
+};
+
+const struct spinand_manufacturer toshiba_spinand_manufacturer = {
+ .id = SPINAND_MFR_TOSHIBA,
+ .name = "Toshiba",
+ .ops = &toshiba_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 088ff96..816c4b0 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -196,6 +196,7 @@ struct spinand_manufacturer {
/* SPI NAND manufacturers */
extern const struct spinand_manufacturer macronix_spinand_manufacturer;
extern const struct spinand_manufacturer micron_spinand_manufacturer;
+extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
extern const struct spinand_manufacturer winbond_spinand_manufacturer;
/**

[Schrempf Frieder]

Hi Clément,

Post by Clément Péron
Hi Frieder,
On Tue, 27 Nov 2018 at 16:08, Schrempf Frieder

Post by Schrempf Frieder
+Clément Péron
Hi Clément,
FYI, this has already been merged to nand/next.

Just want to point the difference that I made when I try to introduce my driver.
https://media.digikey.com/pdf/Data%20Sheets/Toshiba%20PDFs/TC58CVG2S0HxAIx_Rev1.1_2016-11-08.pdf

Post by Schrempf Frieder
Regards,
Frieder

Post by Schrempf Frieder
Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.
---
drivers/mtd/nand/spi/Makefile | 2 +-
drivers/mtd/nand/spi/core.c | 1 +
drivers/mtd/nand/spi/toshiba.c | 136 ++++++++++++++++++++++++++++++++++++
include/linux/mtd/spinand.h | 1 +
4 files changed, 139 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index b74e074..be5f735 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
-spinand-objs := core.o macronix.o micron.o winbond.o
+spinand-objs := core.o macronix.o micron.o toshiba.o winbond.o
obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 30f8364..87bdf2a 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -766,6 +766,7 @@ static const struct nand_ops spinand_ops = {
static const struct spinand_manufacturer *spinand_manufacturers[] = {
&macronix_spinand_manufacturer,
&micron_spinand_manufacturer,
+ &toshiba_spinand_manufacturer,
&winbond_spinand_manufacturer,
};
diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
new file mode 100644
index 0000000..294bcf6
--- /dev/null
+++ b/drivers/mtd/nand/spi/toshiba.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 exceet electronics GmbH
+ * Copyright (c) 2018 Kontron Electronics GmbH
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_TOSHIBA 0x98
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+ SPINAND_PAGE_READ_FROM_CACHE_X4_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(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+ SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 128 + 16 * section;
+ region->length = 16;

Here you expose the ECC bits has 8 sections of 16 bytes.
But regarding the datasheet this should not be accessed page 32.
"The ECC parity code generated by internal ECC is stored in column
addresses 4224-4351 and the users cannot access to these specific
addresses when internal ECC is enabled."

This is just to let the other layers know, where the bytes used for ECC
are. As long as the driver uses the on-chip ECC it won't write to this
area. So this is correct unless I misunderstood this concept. All the
other supported SPI NAND chips use the same approach.

Post by Clément Péron

Post by Schrempf Frieder

Post by Schrempf Frieder
+
+
+ return 0;
+}
+
+static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 2 + 16 * section;
+ region->length = 14;

This reserved 2 bytes for BBM for each section.
if (section)
return -ERANGE;
region->offset = 2;
region->length = 126;

The datasheet (p. 32) describes that the OOB data of a page is divided
into 8 sections. So why should we not reflect this in the software?
Probably it would be sufficient to reserve two bytes for the bad block
marker at the beginning, instead of two bytes in each section. But I'm
not sure about this and it doesn't really hurt.

Post by Clément Péron

Post by Schrempf Frieder

Post by Schrempf Frieder
+
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops tc58cvg2s0h_ooblayout = {
+ .ecc = tc58cvg2s0h_ooblayout_ecc,
+ .free = tc58cvg2s0h_ooblayout_free,
+};
+
+static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ struct nand_device *nand = spinand_to_nand(spinand);
+ u8 mbf = 0;
+ struct spi_mem_op op = SPINAND_GET_FEATURE_OP(0x30, &mbf);
+
+ switch (status & STATUS_ECC_MASK) {
+ return 0;
+
+ return -EBADMSG;
+
+ /*
+ * Let's try to retrieve the real maximum number of bitflips
+ * in order to avoid forcing the wear-leveling layer to move
+ * data around if it's not necessary.
+ */
+ if (spi_mem_exec_op(spinand->spimem, &op))
+ return nand->eccreq.strength;
+
+ mbf >>= 4;
+
+ if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
+ return nand->eccreq.strength;
+
+ return mbf;
+

These take take care of 0, 0x1 and 0x2 but not 0x3 which is valid

Hm, you're right. We don't care if it is 0x1 or 0x3 (both mean that
bitflips were corrected), but we currently only check for 0x1.

I will send a fix for this tomorrow.

Thanks,
Frieder

Post by Clément Péron
ECC status bits indicate the status of internal ECC operation
00b: No bit flips were detected in previous page read.
01b: Bit flips were detected and corrected. Bit flip count did not
exceed the bit flip detection threshold. The threshold is set by bits
[7:4] in address 10h in the feature table.
10b: Multiple bit flips were detected and not corrected.
11b: Bit flips were detected and corrected. Bit flip count exceeded
the bit flip detection threshold. The threshold is set by bits [7:4]
in address 10h in the feature table
Regards,
Clement

Post by Schrempf Frieder

Post by Schrempf Frieder
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct spinand_info toshiba_spinand_table[] = {
+ SPINAND_INFO("TC58CVG2S0H", 0xCD,
+ NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
+ NAND_ECCREQ(8, 512),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_HAS_QE_BIT,
+ SPINAND_ECCINFO(&tc58cvg2s0h_ooblayout,
+ tc58cvg2s0h_ecc_get_status)),
+};
+
+static int toshiba_spinand_detect(struct spinand_device *spinand)
+{
+ u8 *id = spinand->id.data;
+ int ret;
+
+ /*
+ * Toshiba SPI NAND read ID needs a dummy byte,
+ * so the first byte in id is garbage.
+ */
+ if (id[1] != SPINAND_MFR_TOSHIBA)
+ return 0;
+
+ ret = spinand_match_and_init(spinand, toshiba_spinand_table,
+ ARRAY_SIZE(toshiba_spinand_table),
+ id[2]);
+ if (ret)
+ return ret;
+
+ return 1;
+}
+
+static const struct spinand_manufacturer_ops toshiba_spinand_manuf_ops = {
+ .detect = toshiba_spinand_detect,
+};
+
+const struct spinand_manufacturer toshiba_spinand_manufacturer = {
+ .id = SPINAND_MFR_TOSHIBA,
+ .name = "Toshiba",
+ .ops = &toshiba_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 088ff96..816c4b0 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -196,6 +196,7 @@ struct spinand_manufacturer {
/* SPI NAND manufacturers */
extern const struct spinand_manufacturer macronix_spinand_manufacturer;
extern const struct spinand_manufacturer micron_spinand_manufacturer;
+extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
extern const struct spinand_manufacturer winbond_spinand_manufacturer;
/**

[Clément Péron]

Hi Frieder,

On Tue, 27 Nov 2018 at 17:42, Schrempf Frieder

Post by Schrempf Frieder
Hi Clément,

Post by Clément Péron
Hi Frieder,
On Tue, 27 Nov 2018 at 16:08, Schrempf Frieder

Post by Schrempf Frieder
+Clément Péron
Hi Clément,
FYI, this has already been merged to nand/next.

Just want to point the difference that I made when I try to introduce my driver.
https://media.digikey.com/pdf/Data%20Sheets/Toshiba%20PDFs/TC58CVG2S0HxAIx_Rev1.1_2016-11-08.pdf

Post by Schrempf Frieder
Regards,
Frieder

Post by Schrempf Frieder
Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.
---
drivers/mtd/nand/spi/Makefile | 2 +-
drivers/mtd/nand/spi/core.c | 1 +
drivers/mtd/nand/spi/toshiba.c | 136 ++++++++++++++++++++++++++++++++++++
include/linux/mtd/spinand.h | 1 +
4 files changed, 139 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index b74e074..be5f735 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
-spinand-objs := core.o macronix.o micron.o winbond.o
+spinand-objs := core.o macronix.o micron.o toshiba.o winbond.o
obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 30f8364..87bdf2a 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -766,6 +766,7 @@ static const struct nand_ops spinand_ops = {
static const struct spinand_manufacturer *spinand_manufacturers[] = {
&macronix_spinand_manufacturer,
&micron_spinand_manufacturer,
+ &toshiba_spinand_manufacturer,
&winbond_spinand_manufacturer,
};
diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
new file mode 100644
index 0000000..294bcf6
--- /dev/null
+++ b/drivers/mtd/nand/spi/toshiba.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 exceet electronics GmbH
+ * Copyright (c) 2018 Kontron Electronics GmbH
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_TOSHIBA 0x98
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+ SPINAND_PAGE_READ_FROM_CACHE_X4_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(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+ SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 128 + 16 * section;
+ region->length = 16;

Here you expose the ECC bits has 8 sections of 16 bytes.
But regarding the datasheet this should not be accessed page 32.
"The ECC parity code generated by internal ECC is stored in column
addresses 4224-4351 and the users cannot access to these specific
addresses when internal ECC is enabled."

This is just to let the other layers know, where the bytes used for ECC
are. As long as the driver uses the on-chip ECC it won't write to this
area. So this is correct unless I misunderstood this concept. All the
other supported SPI NAND chips use the same approach.

Ok for not write, but are you sure that if we try to read them the SPI
will respond something logic and not some garbage ?
When I read the datasheet it looks like that the read cmd will stop or
send some random value when trying to read these columns.

Post by Schrempf Frieder

Post by Clément Péron

Post by Schrempf Frieder

Post by Schrempf Frieder
+
+
+ return 0;
+}
+
+static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 2 + 16 * section;
+ region->length = 14;

This reserved 2 bytes for BBM for each section.
if (section)
return -ERANGE;
region->offset = 2;
region->length = 126;

The datasheet (p. 32) describes that the OOB data of a page is divided
into 8 sections. So why should we not reflect this in the software?
Probably it would be sufficient to reserve two bytes for the bad block
marker at the beginning, instead of two bytes in each section. But I'm
not sure about this and it doesn't really hurt.

If the OOB are used one day (I think it's not the case actually), It
will be more efficient to do only one request.

Regards, Clement

Post by Schrempf Frieder

Post by Clément Péron

Post by Schrempf Frieder

Post by Schrempf Frieder
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops tc58cvg2s0h_ooblayout = {
+ .ecc = tc58cvg2s0h_ooblayout_ecc,
+ .free = tc58cvg2s0h_ooblayout_free,
+};
+
+static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ struct nand_device *nand = spinand_to_nand(spinand);
+ u8 mbf = 0;
+ struct spi_mem_op op = SPINAND_GET_FEATURE_OP(0x30, &mbf);
+
+ switch (status & STATUS_ECC_MASK) {
+ return 0;
+
+ return -EBADMSG;
+
+ /*
+ * Let's try to retrieve the real maximum number of bitflips
+ * in order to avoid forcing the wear-leveling layer to move
+ * data around if it's not necessary.
+ */
+ if (spi_mem_exec_op(spinand->spimem, &op))
+ return nand->eccreq.strength;
+
+ mbf >>= 4;
+
+ if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
+ return nand->eccreq.strength;
+
+ return mbf;
+

These take take care of 0, 0x1 and 0x2 but not 0x3 which is valid

Hm, you're right. We don't care if it is 0x1 or 0x3 (both mean that
bitflips were corrected), but we currently only check for 0x1.
I will send a fix for this tomorrow.
Thanks,
Frieder

Post by Clément Péron
ECC status bits indicate the status of internal ECC operation
00b: No bit flips were detected in previous page read.
01b: Bit flips were detected and corrected. Bit flip count did not
exceed the bit flip detection threshold. The threshold is set by bits
[7:4] in address 10h in the feature table.
10b: Multiple bit flips were detected and not corrected.
11b: Bit flips were detected and corrected. Bit flip count exceeded
the bit flip detection threshold. The threshold is set by bits [7:4]
in address 10h in the feature table
Regards,
Clement

Post by Schrempf Frieder

Post by Schrempf Frieder
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct spinand_info toshiba_spinand_table[] = {
+ SPINAND_INFO("TC58CVG2S0H", 0xCD,
+ NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
+ NAND_ECCREQ(8, 512),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_HAS_QE_BIT,
+ SPINAND_ECCINFO(&tc58cvg2s0h_ooblayout,
+ tc58cvg2s0h_ecc_get_status)),
+};
+
+static int toshiba_spinand_detect(struct spinand_device *spinand)
+{
+ u8 *id = spinand->id.data;
+ int ret;
+
+ /*
+ * Toshiba SPI NAND read ID needs a dummy byte,
+ * so the first byte in id is garbage.
+ */
+ if (id[1] != SPINAND_MFR_TOSHIBA)
+ return 0;
+
+ ret = spinand_match_and_init(spinand, toshiba_spinand_table,
+ ARRAY_SIZE(toshiba_spinand_table),
+ id[2]);
+ if (ret)
+ return ret;
+
+ return 1;
+}
+
+static const struct spinand_manufacturer_ops toshiba_spinand_manuf_ops = {
+ .detect = toshiba_spinand_detect,
+};
+
+const struct spinand_manufacturer toshiba_spinand_manufacturer = {
+ .id = SPINAND_MFR_TOSHIBA,
+ .name = "Toshiba",
+ .ops = &toshiba_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 088ff96..816c4b0 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -196,6 +196,7 @@ struct spinand_manufacturer {
/* SPI NAND manufacturers */
extern const struct spinand_manufacturer macronix_spinand_manufacturer;
extern const struct spinand_manufacturer micron_spinand_manufacturer;
+extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
extern const struct spinand_manufacturer winbond_spinand_manufacturer;
/**

[Schrempf Frieder]

Hi Clément,

Post by Clément Péron
Hi Frieder,
On Tue, 27 Nov 2018 at 17:42, Schrempf Frieder

Post by Schrempf Frieder
Hi Clément,

Post by Clément Péron
Hi Frieder,
On Tue, 27 Nov 2018 at 16:08, Schrempf Frieder

Post by Schrempf Frieder
+Clément Péron
Hi Clément,
FYI, this has already been merged to nand/next.

Just want to point the difference that I made when I try to introduce my driver.
https://media.digikey.com/pdf/Data%20Sheets/Toshiba%20PDFs/TC58CVG2S0HxAIx_Rev1.1_2016-11-08.pdf

Post by Schrempf Frieder
Regards,
Frieder

Post by Schrempf Frieder
Add minimal support for the Toshiba TC58CVG2S0H SPI NAND chip.
---
drivers/mtd/nand/spi/Makefile | 2 +-
drivers/mtd/nand/spi/core.c | 1 +
drivers/mtd/nand/spi/toshiba.c | 136 ++++++++++++++++++++++++++++++++++++
include/linux/mtd/spinand.h | 1 +
4 files changed, 139 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index b74e074..be5f735 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
-spinand-objs := core.o macronix.o micron.o winbond.o
+spinand-objs := core.o macronix.o micron.o toshiba.o winbond.o
obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 30f8364..87bdf2a 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -766,6 +766,7 @@ static const struct nand_ops spinand_ops = {
static const struct spinand_manufacturer *spinand_manufacturers[]= {
&macronix_spinand_manufacturer,
&micron_spinand_manufacturer,
+ &toshiba_spinand_manufacturer,
&winbond_spinand_manufacturer,
};
diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
new file mode 100644
index 0000000..294bcf6
--- /dev/null
+++ b/drivers/mtd/nand/spi/toshiba.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 exceet electronics GmbH
+ * Copyright (c) 2018 Kontron Electronics GmbH
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_TOSHIBA 0x98
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+ SPINAND_PAGE_READ_FROM_CACHE_X4_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(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+ SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 128 + 16 * section;
+ region->length = 16;

Here you expose the ECC bits has 8 sections of 16 bytes.
But regarding the datasheet this should not be accessed page 32.
"The ECC parity code generated by internal ECC is stored in column
addresses 4224-4351 and the users cannot access to these specific
addresses when internal ECC is enabled."

This is just to let the other layers know, where the bytes used for ECC
are. As long as the driver uses the on-chip ECC it won't write to this
area. So this is correct unless I misunderstood this concept. All the
other supported SPI NAND chips use the same approach.

Ok for not write, but are you sure that if we try to read them the SPI
will respond something logic and not some garbage ?
When I read the datasheet it looks like that the read cmd will stop or
send some random value when trying to read these columns.

Maybe you are right. The datasheet says we shouldn't *access* ECC bytes,
which includes read and write.
I guess it won't hurt to block access to the ECC bytes as they are
currently of no use anyway and I just saw that the Macronix driver is
doing the same.

Post by Clément Péron

Post by Schrempf Frieder

Post by Clément Péron

Post by Schrempf Frieder

Post by Schrempf Frieder
+
+
+ return 0;
+}
+
+static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 2 + 16 * section;
+ region->length = 14;

This reserved 2 bytes for BBM for each section.
if (section)
return -ERANGE;
region->offset = 2;
region->length = 126;

The datasheet (p. 32) describes that the OOB data of a page is divided
into 8 sections. So why should we not reflect this in the software?
Probably it would be sufficient to reserve two bytes for the bad block
marker at the beginning, instead of two bytes in each section. But I'm
not sure about this and it doesn't really hurt.

If the OOB are used one day (I think it's not the case actually), It
will be more efficient to do only one request.

Ok, and even more so we already have a default OOB layout in the core
driver, that could be reused for this case and also in other SPI NAND
drivers. So we could actually get rid of a few lines of code.

Thanks,
Frieder

Post by Clément Péron

Post by Schrempf Frieder

Post by Clément Péron

Post by Schrempf Frieder

Post by Schrempf Frieder
+
+ return 0;
+}
+
+static const struct mtd_ooblayout_ops tc58cvg2s0h_ooblayout = {
+ .ecc = tc58cvg2s0h_ooblayout_ecc,
+ .free = tc58cvg2s0h_ooblayout_free,
+};
+
+static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
+ u8 status)
+{
+ struct nand_device *nand = spinand_to_nand(spinand);
+ u8 mbf = 0;
+ struct spi_mem_op op = SPINAND_GET_FEATURE_OP(0x30, &mbf);
+
+ switch (status & STATUS_ECC_MASK) {
+ return 0;
+
+ return -EBADMSG;
+
+ /*
+ * Let's try to retrieve the real maximum number of bitflips
+ * in order to avoid forcing the wear-leveling layer tomove
+ * data around if it's not necessary.
+ */
+ if (spi_mem_exec_op(spinand->spimem, &op))
+ return nand->eccreq.strength;
+
+ mbf >>= 4;
+
+ if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
+ return nand->eccreq.strength;
+
+ return mbf;
+

These take take care of 0, 0x1 and 0x2 but not 0x3 which is valid

Hm, you're right. We don't care if it is 0x1 or 0x3 (both mean that
bitflips were corrected), but we currently only check for 0x1.
I will send a fix for this tomorrow.
Thanks,
Frieder

Post by Clément Péron
ECC status bits indicate the status of internal ECC operation
00b: No bit flips were detected in previous page read.
01b: Bit flips were detected and corrected. Bit flip count did not
exceed the bit flip detection threshold. The threshold is set by bits
[7:4] in address 10h in the feature table.
10b: Multiple bit flips were detected and not corrected.
11b: Bit flips were detected and corrected. Bit flip count exceeded
the bit flip detection threshold. The threshold is set by bits [7:4]
in address 10h in the feature table
Regards,
Clement

Post by Schrempf Frieder

Post by Schrempf Frieder
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct spinand_info toshiba_spinand_table[] = {
+ SPINAND_INFO("TC58CVG2S0H", 0xCD,
+ NAND_MEMORG(1, 4096, 256, 64, 2048, 1, 1, 1),
+ NAND_ECCREQ(8, 512),
+ SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+ &write_cache_variants,
+ &update_cache_variants),
+ SPINAND_HAS_QE_BIT,
+ SPINAND_ECCINFO(&tc58cvg2s0h_ooblayout,
+ tc58cvg2s0h_ecc_get_status)),
+};
+
+static int toshiba_spinand_detect(struct spinand_device *spinand)
+{
+ u8 *id = spinand->id.data;
+ int ret;
+
+ /*
+ * Toshiba SPI NAND read ID needs a dummy byte,
+ * so the first byte in id is garbage.
+ */
+ if (id[1] != SPINAND_MFR_TOSHIBA)
+ return 0;
+
+ ret = spinand_match_and_init(spinand, toshiba_spinand_table,
+ ARRAY_SIZE(toshiba_spinand_table),
+ id[2]);
+ if (ret)
+ return ret;
+
+ return 1;
+}
+
+static const struct spinand_manufacturer_ops toshiba_spinand_manuf_ops = {
+ .detect = toshiba_spinand_detect,
+};
+
+const struct spinand_manufacturer toshiba_spinand_manufacturer = {
+ .id = SPINAND_MFR_TOSHIBA,
+ .name = "Toshiba",
+ .ops = &toshiba_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 088ff96..816c4b0 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -196,6 +196,7 @@ struct spinand_manufacturer {
/* SPI NAND manufacturers */
extern const struct spinand_manufacturer macronix_spinand_manufacturer;
extern const struct spinand_manufacturer micron_spinand_manufacturer;
+extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
extern const struct spinand_manufacturer winbond_spinand_manufacturer;
/**

[Boris Brezillon]

On Tue, 27 Nov 2018 16:41:56 +0000

Post by Schrempf Frieder

Post by Clément Péron

Post by Schrempf Frieder
+static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 128 + 16 * section;
+ region->length = 16;

Here you expose the ECC bits has 8 sections of 16 bytes.
But regarding the datasheet this should not be accessed page 32.
"The ECC parity code generated by internal ECC is stored in column
addresses 4224-4351 and the users cannot access to these specific
addresses when internal ECC is enabled."

'when internal ECC is enabled' means that those bytes can be accessed
when it's disabled. We should keep exposing the ECC byte sections. Note
that even if ECC sections are not exposed, the core will read those
bytes. They're probably filled with garbage in this case, but we don't
care since we won't use them.

Post by Schrempf Frieder
This is just to let the other layers know, where the bytes used for ECC
are. As long as the driver uses the on-chip ECC it won't write to this
area. So this is correct unless I misunderstood this concept. All the
other supported SPI NAND chips use the same approach.

Yes, and that's the right thing to do. We want to know where the ECC
bytes are, especially when doing raw accesses.

Post by Schrempf Frieder

Post by Clément Péron

Post by Schrempf Frieder
+
+
+ return 0;
+}
+
+static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 2 + 16 * section;
+ region->length = 14;

This reserved 2 bytes for BBM for each section.
if (section)
return -ERANGE;
region->offset = 2;
region->length = 126;

I agree with this suggestion: if the free bytes are contiguous, it's
better to expose them as a single section.

[Schrempf Frieder]

Hi Boris,

Post by Boris Brezillon
On Tue, 27 Nov 2018 16:41:56 +0000

Post by Schrempf Frieder

Post by Clément Péron

Post by Schrempf Frieder
+static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 128 + 16 * section;
+ region->length = 16;

Here you expose the ECC bits has 8 sections of 16 bytes.
But regarding the datasheet this should not be accessed page 32.
"The ECC parity code generated by internal ECC is stored in column
addresses 4224-4351 and the users cannot access to these specific
addresses when internal ECC is enabled."

'when internal ECC is enabled' means that those bytes can be accessed
when it's disabled. We should keep exposing the ECC byte sections. Note
that even if ECC sections are not exposed, the core will read those
bytes. They're probably filled with garbage in this case, but we don't
care since we won't use them.

Post by Schrempf Frieder
This is just to let the other layers know, where the bytes used for ECC
are. As long as the driver uses the on-chip ECC it won't write to this
area. So this is correct unless I misunderstood this concept. All the
other supported SPI NAND chips use the same approach.

Yes, and that's the right thing to do. We want to know where the ECC
bytes are, especially when doing raw accesses.

Thank you for clarifying this. I will send a v2 of my patch and revert
this change.

Thanks,
Frieder

Post by Boris Brezillon

Post by Schrempf Frieder

Post by Clément Péron

Post by Schrempf Frieder
+
+
+ return 0;
+}
+
+static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *region)
+{
+ if (section > 7)
+ return -ERANGE;
+
+ region->offset = 2 + 16 * section;
+ region->length = 14;

This reserved 2 bytes for BBM for each section.
if (section)
return -ERANGE;
region->offset = 2;
region->length = 126;

I agree with this suggestion: if the free bytes are contiguous, it's
better to expose them as a single section.