mtd: spinand: Add support for Etron EM73D044VCx
Subsystems (2)
PATCH v2 1/1 mtd: spinand: Add support for Etron EM73D044VCx
This adds a new vendor Etron, and support for a 2Gb chip.
The datasheet is available at
https://www.etron.com/cn/products/EM73%5B8%5DC%5BD_E_F%5DVC%20SPI%20NAND%20Flash_Promotion_Rev%201_00A.pdf
---
v2:
- Made ooblayout_free/_ecc depend on chip-specific parameters, instead of hardcoded to this 2Gb chip only
- Fixed manufacturer ordering
- Fixed minor formatting issues as reported
- Removed debug comment
drivers/mtd/nand/spi/Makefile | 2 +- drivers/mtd/nand/spi/core.c | 1 + drivers/mtd/nand/spi/etron.c | 104 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spinand.h | 1 + 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/spi/etron.c
drivers/mtd/nand/spi/Makefile +1 -1
@@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 -spinand-objs := core.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o +spinand-objs := core.o etron.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
drivers/mtd/nand/spi/core.c +1 -0
@@ -894,6 +894,7 @@ static const struct nand_ops spinand_ops = { };static const struct spinand_manufacturer *spinand_manufacturers[] = { + &etron_spinand_manufacturer, &gigadevice_spinand_manufacturer, ¯onix_spinand_manufacturer, µn_spinand_manufacturer,
drivers/mtd/nand/spi/etron.c new file +104 -0
104 lines not on this page · raw message
include/linux/mtd/spinand.h +1 -0
@@ -260,6 +260,7 @@ struct spinand_manufacturer { };/* SPI NAND manufacturers */ +extern const struct spinand_manufacturer etron_spinand_manufacturer;extern const struct spinand_manufacturer gigadevice_spinand_manufacturer;extern const struct spinand_manufacturer macronix_spinand_manufacturer;extern const struct spinand_manufacturer micron_spinand_manufacturer;
Signature · 1 line
2.25.1
Miquel Raynal
maintainerHi Bert,
Richard, a question for you below!
[email protected] wrote on Wed, 8 Sep 2021 22:16:19 +0200:
Quoted 20 lines
This adds a new vendor Etron, and support for a 2Gb chip.
The datasheet is available at
https://www.etron.com/cn/products/EM73%5B8%5DC%5BD_E_F%5DVC%20SPI%20NAND%20Flash_Promotion_Rev%201_00A.pdfSigned-off-by Bert Vermeulen <[email protected]>---
v2:
- Made ooblayout_free/_ecc depend on chip-specific parameters, instead of hardcoded to this 2Gb chip only
- Fixed manufacturer ordering
- Fixed minor formatting issues as reported
- Removed debug commentdrivers/mtd/nand/spi/Makefile | 2 +- drivers/mtd/nand/spi/core.c | 1 + drivers/mtd/nand/spi/etron.c | 104 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spinand.h | 1 + 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/spi/etron.c
[...]
drivers/mtd/nand/spi/etron.c line 70
Quoted 9 lines
drivers/mtd/nand/spi/etron.c new file +9
-0+static int etron_ecc_get_status (struct spinand_device *spinand,u8 status) +{ +switch (status &STATUS_ECC_MASK ) { +case STATUS_ECC_NO_BITFLIPS : +return 0 ; + +case STATUS_ECC_HAS_BITFLIPS : +/* Between 1-7 bitflips were corrected */ +return 7 ;
Mmmh this is a bit problematic, having no intermediate value means a single bitflip will trigger UBI to move the data around as its threshold will be reached. Richard, any feedback on this?
drivers/mtd/nand/spi/etron.c line 81
Quoted 11 lines
drivers/mtd/nand/spi/etron.c new file +11
-0+ +case STATUS_ECC_MASK : +/* Maximum bitflips were corrected */ +return 8 ; + +case STATUS_ECC_UNCOR_ERROR : +return -EBADMSG ; + } + +return -EINVAL ; +}
Thanks,
Miquèl
Richard Weinberger
maintainer----- Ursprüngliche Mail -----
Quoted 9 lines
Von: "Miquel Raynal" <[email protected]>
An: "richard" <[email protected]>
CC: "Bert Vermeulen" <[email protected]>, "Vignesh Raghavendra" <[email protected]>, "Patrice Chotard" <[email protected]>, "Boris Brezillon" <[email protected]>, "Christophe Kerello" <[email protected]>, "Mark Brown" <[email protected]>, "Alexander Lobakin" <[email protected]>, "linux-kernel" <[email protected]>, "linux-mtd" <[email protected]>
Gesendet: Dienstag, 14. September 2021 19:31:08
Betreff: Re: [PATCH v2] mtd: spinand: Add support for Etron EM73D044VCx
Quoted 3 lines
Hi Bert,
Richard, a question for you below!
:)
Quoted 25 lines
[email protected] wrote on Wed, 8 Sep 2021 22:16:19 +0200:
Quoted 20 lines
This adds a new vendor Etron, and support for a 2Gb chip.
The datasheet is available at
https://www.etron.com/cn/products/EM73%5B8%5DC%5BD_E_F%5DVC%20SPI%20NAND%20Flash_Promotion_Rev%201_00A.pdfSigned-off-by Bert Vermeulen <[email protected]>---
v2:
- Made ooblayout_free/_ecc depend on chip-specific parameters, instead of hardcoded to this 2Gb chip only
- Fixed manufacturer ordering
- Fixed minor formatting issues as reported
- Removed debug commentdrivers/mtd/nand/spi/Makefile | 2 +- drivers/mtd/nand/spi/core.c | 1 + drivers/mtd/nand/spi/etron.c | 104 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spinand.h | 1 + 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/spi/etron.c[...]
Quoted 13 lines
drivers/mtd/nand/spi/etron.c line 70
Quoted 9 lines
drivers/mtd/nand/spi/etron.c new file +9
-0+static int etron_ecc_get_status (struct spinand_device *spinand,u8 status) +{ +switch (status &STATUS_ECC_MASK ) { +case STATUS_ECC_NO_BITFLIPS : +return 0 ; + +case STATUS_ECC_HAS_BITFLIPS : +/* Between 1-7 bitflips were corrected */ +return 7 ;Mmmh this is a bit problematic, having no intermediate value means a single bitflip will trigger UBI to move the data around as its threshold will be reached. Richard, any feedback on this?
So, the NAND controller can only report "no bitflips", "some bitflips", "maximum biflips" and "no way to fix"? If so, yes, this is problematic for UBI because it will trigger wear-leveling way too often. On a medium aged NAND I'd expect to see STATUS_ECC_HAS_BITFLIPS almost always set. :-(
Thanks,
//richard
Quoted 40 lines · Richard Weinberger
----- Ursprüngliche Mail -----
Quoted 25 lines
[email protected] wrote on Wed, 8 Sep 2021 22:16:19 +0200:
Quoted 20 lines
This adds a new vendor Etron, and support for a 2Gb chip.
The datasheet is available at
https://www.etron.com/cn/products/EM73%5B8%5DC%5BD_E_F%5DVC%20SPI%20NAND%20Flash_Promotion_Rev%201_00A.pdfSigned-off-by Bert Vermeulen <[email protected]>---
v2:
- Made ooblayout_free/_ecc depend on chip-specific parameters, instead of hardcoded to this 2Gb chip only
- Fixed manufacturer ordering
- Fixed minor formatting issues as reported
- Removed debug commentdrivers/mtd/nand/spi/Makefile | 2 +- drivers/mtd/nand/spi/core.c | 1 + drivers/mtd/nand/spi/etron.c | 104 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spinand.h | 1 + 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/spi/etron.c[...]
Quoted 13 lines
drivers/mtd/nand/spi/etron.c line 70
Quoted 9 lines
drivers/mtd/nand/spi/etron.c new file +9
-0+static int etron_ecc_get_status (struct spinand_device *spinand,u8 status) +{ +switch (status &STATUS_ECC_MASK ) { +case STATUS_ECC_NO_BITFLIPS : +return 0 ; + +case STATUS_ECC_HAS_BITFLIPS : +/* Between 1-7 bitflips were corrected */ +return 7 ;Mmmh this is a bit problematic, having no intermediate value means a single bitflip will trigger UBI to move the data around as its threshold will be reached. Richard, any feedback on this?
Quoted 3 lines
So, the NAND controller can only report "no bitflips", "some bitflips", "maximum biflips" and "no way to fix"? If so, yes, this is problematic for UBI because it will trigger wear-leveling way too often. On a medium aged NAND I'd expect to see
STATUS_ECC_HAS_BITFLIPSalmost always set. :-(
Yes, that's all there is according to the datasheet. Can't be _that_ unusual, since that's all the STATUS_ECC_* flags cover.
Incidentally I'm abusing STATUS_ECC_MASK here, since the 0b11 pattern is missing from those flags.
Signature · 2 lines
Bert Vermeulen [email protected]
Miquel Raynal
maintainerHi Bert,
[email protected] wrote on Tue, 14 Sep 2021 22:03:49 +0200:
Quoted 41 lines · Richard Weinberger
Quoted 40 lines
----- Ursprüngliche Mail -----
Quoted 38 lines
[email protected] wrote on Wed, 8 Sep 2021 22:16:19 +0200:
Quoted 20 lines
This adds a new vendor Etron, and support for a 2Gb chip.
The datasheet is available at
https://www.etron.com/cn/products/EM73%5B8%5DC%5BD_E_F%5DVC%20SPI%20NAND%20Flash_Promotion_Rev%201_00A.pdfSigned-off-by Bert Vermeulen <[email protected]>---
v2:
- Made ooblayout_free/_ecc depend on chip-specific parameters, instead of hardcoded to this 2Gb chip only
- Fixed manufacturer ordering
- Fixed minor formatting issues as reported
- Removed debug commentdrivers/mtd/nand/spi/Makefile | 2 +- drivers/mtd/nand/spi/core.c | 1 + drivers/mtd/nand/spi/etron.c | 104 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spinand.h | 1 + 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/spi/etron.c[...]
drivers/mtd/nand/spi/etron.c line 70
Quoted 9 lines
drivers/mtd/nand/spi/etron.c new file +9
-0+static int etron_ecc_get_status (struct spinand_device *spinand,u8 status) +{ +switch (status &STATUS_ECC_MASK ) { +case STATUS_ECC_NO_BITFLIPS : +return 0 ; + +case STATUS_ECC_HAS_BITFLIPS : +/* Between 1-7 bitflips were corrected */ +return 7 ;Mmmh this is a bit problematic, having no intermediate value means a single bitflip will trigger UBI to move the data around as its threshold will be reached. Richard, any feedback on this?
Quoted 6 lines
Quoted 3 lines
So, the NAND controller can only report "no bitflips", "some bitflips", "maximum biflips" and "no way to fix"? If so, yes, this is problematic for UBI because it will trigger wear-leveling way too often. On a medium aged NAND I'd expect to see
STATUS_ECC_HAS_BITFLIPSalmost always set. :-(Yes, that's all there is according to the datasheet. Can't be _that_ unusual, since that's all the STATUS_ECC_* flags cover.
I forgot about that part which was reminded to me by Richard:
https://elixir.bootlin.com/linux/latest/source/drivers/mtd/mtdcore.c#L627
/* default value if not set by driver */ if (mtd->bitflip_threshold == 0) mtd->bitflip_threshold = mtd->ecc_strength;
So this is fine.
Thanks,
Miquèl
Hi Bert,
Quoted 110 lines · Bert Vermeulen
This adds a new vendor Etron, and support for a 2Gb chip.
Signed-off-by Bert Vermeulen <[email protected]>---
v2:
- Made ooblayout_free/_ecc depend on chip-specific parameters, instead of hardcoded to this 2Gb chip only
- Fixed manufacturer ordering
- Fixed minor formatting issues as reported
- Removed debug commentdrivers/mtd/nand/spi/Makefile | 2 +- drivers/mtd/nand/spi/core.c | 1 + drivers/mtd/nand/spi/etron.c | 104 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spinand.h | 1 + 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/spi/etron.cdrivers/mtd/nand/spi/Makefile +1
-1@@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 -spinand-objs := core.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o +spinand-objs := core.o etron.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o obj-$(CONFIG_MTD_SPI_NAND) += spinand.odrivers/mtd/nand/spi/core.c +1
-0@@ -894,6 +894,7 @@ static const struct nand_ops spinand_ops = { };static const struct spinand_manufacturer *spinand_manufacturers[] = { + &etron_spinand_manufacturer, &gigadevice_spinand_manufacturer, ¯onix_spinand_manufacturer, µn_spinand_manufacturer,drivers/mtd/nand/spi/etron.c new file +62
-0@@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/device.h> +#include <linux/kernel.h> +#include <linux/mtd/spinand.h> + +#define SPINAND_MFR_ETRON 0xd5 + +static SPINAND_OP_VARIANTS(read_cache_variants, + SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0), +SPINAND_PAGE_READ_FROM_CACHE_X4_OP (0 ,1 ,NULL ,0 ), +SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP (0 ,1 ,NULL ,0 ), +SPINAND_PAGE_READ_FROM_CACHE_X2_OP (0 ,1 ,NULL ,0 ), +SPINAND_PAGE_READ_FROM_CACHE_OP (true,0 ,1 ,NULL ,0 ), + SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0)); + +static SPINAND_OP_VARIANTS(write_cache_variants, + SPINAND_PROG_LOAD_X4(true, 0, NULL, 0), + SPINAND_PROG_LOAD(true, 0, NULL, 0)); + +static SPINAND_OP_VARIANTS(update_cache_variants, + SPINAND_PROG_LOAD_X4(false, 0, NULL, 0), + SPINAND_PROG_LOAD(false, 0, NULL, 0)); + +static int etron_oob_spare(struct mtd_info *mtd) +{ + if (mtd->size < 0x10000000) + return mtd->oobsize / 2; + else + return mtd->oobsize / 2 + mtd->oobsize / 16; +} + +static int etron_ooblayout_ecc (struct mtd_info *mtd,int section, +struct mtd_oob_region *oobregion) +{ +if (section) +return -ERANGE ; + + oobregion->offset =etron_oob_spare (mtd); + oobregion->length = mtd->oobsize - oobregion->offset; + +return 0 ; +} + +static int etron_ooblayout_free (struct mtd_info *mtd,int section, +struct mtd_oob_region *oobregion) +{ +if (section) +return -ERANGE ; + + oobregion->offset =2 ; + oobregion->length =etron_oob_spare (mtd) -2 ; + +return 0 ; +} + +static const struct mtd_ooblayout_ops etron_ooblayout = { + .ecc = etron_ooblayout_ecc, + .free = etron_ooblayout_free, +}; + +static int etron_ecc_get_status (struct spinand_device *spinand,u8 status)
drivers/mtd/nand/spi/etron.c line 72
Quoted 10 lines
drivers/mtd/nand/spi/etron.c new file +10
-0+{ +switch (status &STATUS_ECC_MASK ) { +case STATUS_ECC_NO_BITFLIPS : +return 0 ; + +case STATUS_ECC_HAS_BITFLIPS : +/* Between 1-7 bitflips were corrected */ +return 7 ; + +case STATUS_ECC_MASK :
I'm not sure if using STATUS_ECC_MASK here to check for the 0b11 pattern is really a good idea. I'd rather introduce a vendor-specific define for this as other drivers do (see toshiba.c, paragon.c or gigadevice.c). As this seems to be a common pattern among vendors we could also think about introducing a new common flag like STATUS_ECC_HAS_MAX_BITFLIPS or something like that and use this instead.
drivers/mtd/nand/spi/etron.c line 85
Quoted 13 lines
drivers/mtd/nand/spi/etron.c new file +13
-0+/* Maximum bitflips were corrected */ +return 8 ; + +case STATUS_ECC_UNCOR_ERROR : +return -EBADMSG ; + } + +return -EINVAL ; +} + +static const struct spinand_info etron_spinand_table[] = { + SPINAND_INFO("EM73D044VCx", + SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x1f),
According to the datasheet, the device id of the EM73D044VCL is 0x2e and not 0x1f. Am I missing something?
Best regards
Frieder
Quoted 32 lines
drivers/mtd/nand/spi/etron.c new file +19
-0+NAND_MEMORG (1 ,2048 ,128 ,64 ,2048 ,40 ,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(&etron_ooblayout, etron_ecc_get_status)), +}; + +static const struct spinand_manufacturer_ops etron_spinand_manuf_ops = { +}; + +const struct spinand_manufacturer etron_spinand_manufacturer = { + .id =SPINAND_MFR_ETRON , + .name ="Etron" , + .chips = etron_spinand_table, + .nchips =ARRAY_SIZE (etron_spinand_table), + .ops = &etron_spinand_manuf_ops, +};include/linux/mtd/spinand.h +1
-0@@ -260,6 +260,7 @@ struct spinand_manufacturer { };/* SPI NAND manufacturers */ +extern const struct spinand_manufacturer etron_spinand_manufacturer;extern const struct spinand_manufacturer gigadevice_spinand_manufacturer;extern const struct spinand_manufacturer macronix_spinand_manufacturer;extern const struct spinand_manufacturer micron_spinand_manufacturer;