| Message ID | 20220408161904.94383-1-felix@matouschek.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] mtd: spinand: Add support for XTX XT26G0xA | expand |
Hi Felix, felix@matouschek.org wrote on Fri, 8 Apr 2022 18:19:04 +0200: > Add support for XTX Technology XT26G01AXXXXX, XTX26G02AXXXXX and > XTX26G04AXXXXX SPI NAND. > > These are 3V, 1G/2G/4Gbit serial SLC NAND flash devices with on-die ECC > (8bit strength per 512bytes). > > Tested on Teltonika RUTX10 flashed with OpenWrt. > > Links: > - http://www.xtxtech.com/download/?AId=225 > - https://datasheet.lcsc.com/szlcsc/2005251034_XTX-XT26G01AWSEGA_C558841.pdf > Signed-off-by: Felix Matouschek <felix@matouschek.org> > --- > drivers/mtd/nand/spi/Makefile | 2 +- > drivers/mtd/nand/spi/core.c | 1 + > drivers/mtd/nand/spi/xtx.c | 122 ++++++++++++++++++++++++++++++++++ > include/linux/mtd/spinand.h | 1 + > 4 files changed, 125 insertions(+), 1 deletion(-) > create mode 100644 drivers/mtd/nand/spi/xtx.c > > diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile > index 9662b9c1d5a9..80dabe6ff0f3 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 gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o > +spinand-objs := core.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o xtx.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 ff8336870bc0..d5b685d1605e 100644 > --- a/drivers/mtd/nand/spi/core.c > +++ b/drivers/mtd/nand/spi/core.c > @@ -933,6 +933,7 @@ static const struct spinand_manufacturer *spinand_manufacturers[] = { > ¶gon_spinand_manufacturer, > &toshiba_spinand_manufacturer, > &winbond_spinand_manufacturer, > + &xtx_spinand_manufacturer, > }; > > static int spinand_manufacturer_match(struct spinand_device *spinand, > diff --git a/drivers/mtd/nand/spi/xtx.c b/drivers/mtd/nand/spi/xtx.c > new file mode 100644 > index 000000000000..3c408337fa92 > --- /dev/null > +++ b/drivers/mtd/nand/spi/xtx.c > @@ -0,0 +1,122 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Author: > + * Felix Matouschek <felix@matouschek.org> > + */ > + > +#include <linux/device.h> > +#include <linux/kernel.h> > +#include <linux/mtd/spinand.h> > + > +#define SPINAND_MFR_XTX 0x0B > + > +#define XT26G0XA_STATUS_ECC_MASK GENMASK(5, 2) > +#define XT26G0XA_STATUS_ECC_NO_DETECTED (0 << 2) > +#define XT26G0XA_STATUS_ECC_8_CORRECTED (3 << 4) > +#define XT26G0XA_STATUS_ECC_UNCOR_ERROR (2 << 4) > + > +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 xt26g0xa_ooblayout_ecc(struct mtd_info *mtd, int section, > + struct mtd_oob_region *region) > +{ > + if (section) > + return -ERANGE; > + > + region->offset = 48; > + region->length = 16; > + > + return 0; > +} > + > +static int xt26g0xa_ooblayout_free(struct mtd_info *mtd, int section, > + struct mtd_oob_region *region) > +{ > + if (section) > + return -ERANGE; > + > + region->offset = 1; > + region->length = 47; > + > + return 0; > +} > + > +static const struct mtd_ooblayout_ops xt26g0xa_ooblayout = { > + .ecc = xt26g0xa_ooblayout_ecc, > + .free = xt26g0xa_ooblayout_free, > +}; > + > +static int xt26g0xa_ecc_get_status(struct spinand_device *spinand, > + u8 status) > +{ > + switch (status & XT26G0XA_STATUS_ECC_MASK) { > + case XT26G0XA_STATUS_ECC_NO_DETECTED: > + return 0; > + case XT26G0XA_STATUS_ECC_8_CORRECTED: > + return 8; > + case XT26G0XA_STATUS_ECC_UNCOR_ERROR: > + return -EBADMSG; > + default: /* (1 << 2) through (7 << 2) are 1-7 corrected errors */ > + return (status & XT26G0XA_STATUS_ECC_MASK) >> 2; > + } > + > + return -EINVAL; You'll never end here, right? Perhaps there are values that you cannot handle correctly in the switch and you should add a bit more logic to return -EINVAL if something is wrong? Otherwise you will return 0 above, which might be correct (I'll let you verify but in this case the final "return -EINVAL" seems useless. > +} > + > +static const struct spinand_info xtx_spinand_table[] = { > + SPINAND_INFO("XT26G01A", > + SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xE1), > + NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 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(&xt26g0xa_ooblayout, > + xt26g0xa_ecc_get_status)), > + SPINAND_INFO("XT26G02A", > + SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xE2), > + NAND_MEMORG(1, 2048, 64, 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(&xt26g0xa_ooblayout, > + xt26g0xa_ecc_get_status)), > + SPINAND_INFO("XT26G04A", > + SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xE3), > + NAND_MEMORG(1, 2048, 64, 128, 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(&xt26g0xa_ooblayout, > + xt26g0xa_ecc_get_status)), > +}; > + > +static const struct spinand_manufacturer_ops xtx_spinand_manuf_ops = { > +}; > + > +const struct spinand_manufacturer xtx_spinand_manufacturer = { > + .id = SPINAND_MFR_XTX, > + .name = "XTX", > + .chips = xtx_spinand_table, > + .nchips = ARRAY_SIZE(xtx_spinand_table), > + .ops = &xtx_spinand_manuf_ops, > +}; > diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h > index 3aa28240a77f..5584d3bb6556 100644 > --- a/include/linux/mtd/spinand.h > +++ b/include/linux/mtd/spinand.h > @@ -266,6 +266,7 @@ extern const struct spinand_manufacturer micron_spinand_manufacturer; > extern const struct spinand_manufacturer paragon_spinand_manufacturer; > extern const struct spinand_manufacturer toshiba_spinand_manufacturer; > extern const struct spinand_manufacturer winbond_spinand_manufacturer; > +extern const struct spinand_manufacturer xtx_spinand_manufacturer; > > /** > * struct spinand_op_variants - SPI NAND operation variants Thanks, Miquèl
Hi Miquel, Am 11.04.22 um 09:23 schrieb Miquel Raynal: >> +static int xt26g0xa_ecc_get_status(struct spinand_device *spinand, >> + u8 status) >> +{ >> + switch (status & XT26G0XA_STATUS_ECC_MASK) { >> + case XT26G0XA_STATUS_ECC_NO_DETECTED: >> + return 0; >> + case XT26G0XA_STATUS_ECC_8_CORRECTED: >> + return 8; >> + case XT26G0XA_STATUS_ECC_UNCOR_ERROR: >> + return -EBADMSG; >> + default: /* (1 << 2) through (7 << 2) are 1-7 corrected errors */ >> + return (status & XT26G0XA_STATUS_ECC_MASK) >> 2; >> + } >> + >> + return -EINVAL; > > You'll never end here, right? Perhaps there are values that you cannot > handle correctly in the switch and you should add a bit more logic to > return -EINVAL if something is wrong? Otherwise you will return 0 > above, which might be correct (I'll let you verify but in this > case the final "return -EINVAL" seems useless. According to the data-sheet (section 7, page 27) the switch should be able to handle all values of the status register. So should I remove the final "return -EINVAL"? Thank you, Felix
Hi Felix, felix@matouschek.org wrote on Mon, 11 Apr 2022 21:53:06 +0200: > Hi Miquel, > > Am 11.04.22 um 09:23 schrieb Miquel Raynal: > >> +static int xt26g0xa_ecc_get_status(struct spinand_device *spinand, > >> + u8 status) > >> +{ > >> + switch (status & XT26G0XA_STATUS_ECC_MASK) { > >> + case XT26G0XA_STATUS_ECC_NO_DETECTED: > >> + return 0; > >> + case XT26G0XA_STATUS_ECC_8_CORRECTED: > >> + return 8; > >> + case XT26G0XA_STATUS_ECC_UNCOR_ERROR: > >> + return -EBADMSG; > >> + default: /* (1 << 2) through (7 << 2) are 1-7 corrected errors */ > >> + return (status & XT26G0XA_STATUS_ECC_MASK) >> 2; > >> + } > >> + > >> + return -EINVAL; > > > > You'll never end here, right? Perhaps there are values that you cannot > > handle correctly in the switch and you should add a bit more logic to > > return -EINVAL if something is wrong? Otherwise you will return 0 > > above, which might be correct (I'll let you verify but in this > > case the final "return -EINVAL" seems useless. > > According to the data-sheet (section 7, page 27) the switch should be > able to handle all values of the status register. So should I remove the > final "return -EINVAL"? I think so, yes. Thanks, Miquèl
Hi Miquel, Am 11.04.22 um 22:03 schrieb Miquel Raynal: >> According to the data-sheet (section 7, page 27) the switch should be >> able to handle all values of the status register. So should I remove the >> final "return -EINVAL"? > > I think so, yes. I noticed that there are actually some possible values for the ECC bits in the status register, that are not covered by the switch case. According to the data-sheet they should not happen, but would it be wise to catch them anyways? Maybe if something goes wrong? Thank you, Felix
Hi Felix, felix@matouschek.org wrote on Wed, 13 Apr 2022 17:38:28 +0200: > Hi Miquel, > > Am 11.04.22 um 22:03 schrieb Miquel Raynal: > >> According to the data-sheet (section 7, page 27) the switch should be > >> able to handle all values of the status register. So should I remove the > >> final "return -EINVAL"? > > > > I think so, yes. > > I noticed that there are actually some possible values for the ECC bits in the > status register, that are not covered by the switch case. According to the > data-sheet they should not happen, but would it be wise to catch them anyways? > Maybe if something goes wrong? Yes of course, that was my advice in the first place :) > > Thank you, > Felix Thanks, Miquèl
Hi Miquel, Am 13.04.22 um 17:56 schrieb Miquel Raynal: >> I noticed that there are actually some possible values for the ECC bits in the >> status register, that are not covered by the switch case. According to the >> data-sheet they should not happen, but would it be wise to catch them anyways? >> Maybe if something goes wrong? > > Yes of course, that was my advice in the first place :) Pardon my stupidity, I sent a v3. :) Thank you, Felix
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile index 9662b9c1d5a9..80dabe6ff0f3 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 gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o +spinand-objs := core.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o xtx.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 ff8336870bc0..d5b685d1605e 100644 --- a/drivers/mtd/nand/spi/core.c +++ b/drivers/mtd/nand/spi/core.c @@ -933,6 +933,7 @@ static const struct spinand_manufacturer *spinand_manufacturers[] = { ¶gon_spinand_manufacturer, &toshiba_spinand_manufacturer, &winbond_spinand_manufacturer, + &xtx_spinand_manufacturer, }; static int spinand_manufacturer_match(struct spinand_device *spinand, diff --git a/drivers/mtd/nand/spi/xtx.c b/drivers/mtd/nand/spi/xtx.c new file mode 100644 index 000000000000..3c408337fa92 --- /dev/null +++ b/drivers/mtd/nand/spi/xtx.c @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Author: + * Felix Matouschek <felix@matouschek.org> + */ + +#include <linux/device.h> +#include <linux/kernel.h> +#include <linux/mtd/spinand.h> + +#define SPINAND_MFR_XTX 0x0B + +#define XT26G0XA_STATUS_ECC_MASK GENMASK(5, 2) +#define XT26G0XA_STATUS_ECC_NO_DETECTED (0 << 2) +#define XT26G0XA_STATUS_ECC_8_CORRECTED (3 << 4) +#define XT26G0XA_STATUS_ECC_UNCOR_ERROR (2 << 4) + +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 xt26g0xa_ooblayout_ecc(struct mtd_info *mtd, int section, + struct mtd_oob_region *region) +{ + if (section) + return -ERANGE; + + region->offset = 48; + region->length = 16; + + return 0; +} + +static int xt26g0xa_ooblayout_free(struct mtd_info *mtd, int section, + struct mtd_oob_region *region) +{ + if (section) + return -ERANGE; + + region->offset = 1; + region->length = 47; + + return 0; +} + +static const struct mtd_ooblayout_ops xt26g0xa_ooblayout = { + .ecc = xt26g0xa_ooblayout_ecc, + .free = xt26g0xa_ooblayout_free, +}; + +static int xt26g0xa_ecc_get_status(struct spinand_device *spinand, + u8 status) +{ + switch (status & XT26G0XA_STATUS_ECC_MASK) { + case XT26G0XA_STATUS_ECC_NO_DETECTED: + return 0; + case XT26G0XA_STATUS_ECC_8_CORRECTED: + return 8; + case XT26G0XA_STATUS_ECC_UNCOR_ERROR: + return -EBADMSG; + default: /* (1 << 2) through (7 << 2) are 1-7 corrected errors */ + return (status & XT26G0XA_STATUS_ECC_MASK) >> 2; + } + + return -EINVAL; +} + +static const struct spinand_info xtx_spinand_table[] = { + SPINAND_INFO("XT26G01A", + SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xE1), + NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 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(&xt26g0xa_ooblayout, + xt26g0xa_ecc_get_status)), + SPINAND_INFO("XT26G02A", + SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xE2), + NAND_MEMORG(1, 2048, 64, 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(&xt26g0xa_ooblayout, + xt26g0xa_ecc_get_status)), + SPINAND_INFO("XT26G04A", + SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xE3), + NAND_MEMORG(1, 2048, 64, 128, 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(&xt26g0xa_ooblayout, + xt26g0xa_ecc_get_status)), +}; + +static const struct spinand_manufacturer_ops xtx_spinand_manuf_ops = { +}; + +const struct spinand_manufacturer xtx_spinand_manufacturer = { + .id = SPINAND_MFR_XTX, + .name = "XTX", + .chips = xtx_spinand_table, + .nchips = ARRAY_SIZE(xtx_spinand_table), + .ops = &xtx_spinand_manuf_ops, +}; diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h index 3aa28240a77f..5584d3bb6556 100644 --- a/include/linux/mtd/spinand.h +++ b/include/linux/mtd/spinand.h @@ -266,6 +266,7 @@ extern const struct spinand_manufacturer micron_spinand_manufacturer; extern const struct spinand_manufacturer paragon_spinand_manufacturer; extern const struct spinand_manufacturer toshiba_spinand_manufacturer; extern const struct spinand_manufacturer winbond_spinand_manufacturer; +extern const struct spinand_manufacturer xtx_spinand_manufacturer; /** * struct spinand_op_variants - SPI NAND operation variants
Add support for XTX Technology XT26G01AXXXXX, XTX26G02AXXXXX and XTX26G04AXXXXX SPI NAND. These are 3V, 1G/2G/4Gbit serial SLC NAND flash devices with on-die ECC (8bit strength per 512bytes). Tested on Teltonika RUTX10 flashed with OpenWrt. Links: - http://www.xtxtech.com/download/?AId=225 - https://datasheet.lcsc.com/szlcsc/2005251034_XTX-XT26G01AWSEGA_C558841.pdf Signed-off-by: Felix Matouschek <felix@matouschek.org> --- drivers/mtd/nand/spi/Makefile | 2 +- drivers/mtd/nand/spi/core.c | 1 + drivers/mtd/nand/spi/xtx.c | 122 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spinand.h | 1 + 4 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/spi/xtx.c