| Message ID | 20240905055333.2363358-2-linchengming884@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | mtd: spi-nand: Add support for read retry | expand |
Hi Cheng Ming, linchengming884@gmail.com wrote on Thu, 5 Sep 2024 13:53:32 +0800: > From: Cheng Ming Lin <chengminglin@mxic.com.tw> > > Add fixups for support read retry: > - Initialize the NAND device maximum retry mode. > - Set feature on Special Read for Data Recovery register. > > The Special Read for Data Recovery operation is enabled by Set Feature > function. > > There are 5 modes for the user to recover the lost data. > > Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw> > --- > drivers/mtd/nand/spi/macronix.c | 79 ++++++++++++++++++++++++++------- > include/linux/mtd/spinand.h | 17 +++++++ > 2 files changed, 81 insertions(+), 15 deletions(-) > > diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c > index 3f9e9c572854..baca67ff1cd6 100644 > --- a/drivers/mtd/nand/spi/macronix.c > +++ b/drivers/mtd/nand/spi/macronix.c > @@ -9,6 +9,8 @@ > #include <linux/kernel.h> > #include <linux/mtd/spinand.h> > > +#define MACRONIX_NUM_READ_RETRY_MODES 6 You said 5 in the cover letter? > +#define MACRONIX_FEATURE_ADDR_READ_RETRY 0x70 Both definitions should probably come... > #define SPINAND_MFR_MACRONIX 0xC2 > #define MACRONIX_ECCSR_MASK 0x0F ...here > > @@ -100,6 +102,38 @@ static int mx35lf1ge4ab_ecc_get_status(struct spinand_device *spinand, > return -EINVAL; > } > > +/** > + * macronix_spinand_init_read_retry - Initialize read_retries > + * @spinand: SPI NAND device > + * > + * Return: the number of read retry modes > + */ > +static int macronix_spinand_init_read_retry(struct spinand_device *spinand) > +{ > + return MACRONIX_NUM_READ_RETRY_MODES; Does not sound very useful as a function? > +} > + > +/** > + * macronix_spinand_setup_read_retry - Set the retry mode > + * @spinand: SPI NAND device > + * @retry_mode: Specify which retry mode to set > + * > + * Return: 0 on success, -error otherwise , a negative error code otherwise. > + */ > +static int macronix_spinand_setup_read_retry(struct spinand_device *spinand, u8 retry_mode) > +{ > + struct spi_mem_op op = SPINAND_SET_FEATURE_OP(MACRONIX_FEATURE_ADDR_READ_RETRY, > + spinand->scratchbuf); > + > + *spinand->scratchbuf = retry_mode; > + return spi_mem_exec_op(spinand->spimem, &op); > +} > + > +static const struct spi_nand_fixups read_retry_fixups = { > + .init_read_retry = macronix_spinand_init_read_retry, > + .setup_read_retry = macronix_spinand_setup_read_retry, > +}; > + ... > @@ -325,7 +373,8 @@ static const struct spinand_info macronix_spinand_table[] = { > &update_cache_variants), > SPINAND_HAS_QE_BIT, > SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, > - mx35lf1ge4ab_ecc_get_status)), > + mx35lf1ge4ab_ecc_get_status), > + SPINAND_FIXUPS(&read_retry_fixups)), > SPINAND_INFO("MX35UF1GE4AC", > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x92, 0x01), > NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1), I expect a patch targeting the core first, and then the changes in the Macronix driver. > diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h > index 5c19ead60499..e567d00a2805 100644 > --- a/include/linux/mtd/spinand.h > +++ b/include/linux/mtd/spinand.h > @@ -354,6 +354,7 @@ struct spinand_info { > } op_variants; > int (*select_target)(struct spinand_device *spinand, > unsigned int target); > + const struct spi_nand_fixups *fixups; > }; > > #define SPINAND_ID(__method, ...) \ > @@ -379,6 +380,9 @@ struct spinand_info { > #define SPINAND_SELECT_TARGET(__func) \ > .select_target = __func, > > +#define SPINAND_FIXUPS(__func) \ > + .fixups = __func, I don't like the fixups naming. It feels like something is inherently wrong. > + > #define SPINAND_INFO(__model, __id, __memorg, __eccreq, __op_variants, \ > __flags, ...) \ > { \ > @@ -398,6 +402,16 @@ struct spinand_dirmap { > struct spi_mem_dirmap_desc *rdesc_ecc; > }; > > +/** > + * struct spi_nand_fixups - SPI NAND fixup hooks > + * @init_read_retry: initialize spinand->read_retries > + * @setup_read_retry: set the retry mode > + */ > +struct spi_nand_fixups { > + int (*init_read_retry)(struct spinand_device *spinand); > + int (*setup_read_retry)(struct spinand_device *spinand, u8 retry_mode); > +}; > + > /** > * struct spinand_device - SPI NAND device instance > * @base: NAND device instance > @@ -423,6 +437,7 @@ struct spinand_dirmap { > * the stack > * @manufacturer: SPI NAND manufacturer information > * @priv: manufacturer private data > + * @read_retries: the number of read retry modes supported > */ > struct spinand_device { > struct nand_device base; > @@ -449,8 +464,10 @@ struct spinand_device { > u8 *databuf; > u8 *oobbuf; > u8 *scratchbuf; > + const struct spinand_info *info; This looks like a leftover. > const struct spinand_manufacturer *manufacturer; > void *priv; > + int read_retries; Any reason to keep this variable signed? > }; > > /** Thanks, Miquèl
Hi Miquel, Miquel Raynal <miquel.raynal@bootlin.com> 於 2024年10月1日 週二 下午5:40寫道： > > Hi Cheng Ming, > > linchengming884@gmail.com wrote on Thu, 5 Sep 2024 13:53:32 +0800: > > > From: Cheng Ming Lin <chengminglin@mxic.com.tw> > > > > Add fixups for support read retry: > > - Initialize the NAND device maximum retry mode. > > - Set feature on Special Read for Data Recovery register. > > > > The Special Read for Data Recovery operation is enabled by Set Feature > > function. > > > > There are 5 modes for the user to recover the lost data. > > > > Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw> > > --- > > drivers/mtd/nand/spi/macronix.c | 79 ++++++++++++++++++++++++++------- > > include/linux/mtd/spinand.h | 17 +++++++ > > 2 files changed, 81 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c > > index 3f9e9c572854..baca67ff1cd6 100644 > > --- a/drivers/mtd/nand/spi/macronix.c > > +++ b/drivers/mtd/nand/spi/macronix.c > > @@ -9,6 +9,8 @@ > > #include <linux/kernel.h> > > #include <linux/mtd/spinand.h> > > > > +#define MACRONIX_NUM_READ_RETRY_MODES 6 > > You said 5 in the cover letter? Since the original mode is labeled as default in our datasheet, there are a total of six modes, including five additional modes numbered from Mode 1 to Mode 5. > > > +#define MACRONIX_FEATURE_ADDR_READ_RETRY 0x70 > > Both definitions should probably come... > > > #define SPINAND_MFR_MACRONIX 0xC2 > > #define MACRONIX_ECCSR_MASK 0x0F > > ...here Sure, thanks for your advice. > > > > > @@ -100,6 +102,38 @@ static int mx35lf1ge4ab_ecc_get_status(struct spinand_device *spinand, > > return -EINVAL; > > } > > > > +/** > > + * macronix_spinand_init_read_retry - Initialize read_retries > > + * @spinand: SPI NAND device > > + * > > + * Return: the number of read retry modes > > + */ > > +static int macronix_spinand_init_read_retry(struct spinand_device *spinand) > > +{ > > + return MACRONIX_NUM_READ_RETRY_MODES; > > Does not sound very useful as a function? > > > +} > > + > > +/** > > + * macronix_spinand_setup_read_retry - Set the retry mode > > + * @spinand: SPI NAND device > > + * @retry_mode: Specify which retry mode to set > > + * > > + * Return: 0 on success, -error otherwise > > , a negative error code otherwise. > > > + */ > > +static int macronix_spinand_setup_read_retry(struct spinand_device *spinand, u8 retry_mode) > > +{ > > + struct spi_mem_op op = SPINAND_SET_FEATURE_OP(MACRONIX_FEATURE_ADDR_READ_RETRY, > > + spinand->scratchbuf); > > + > > + *spinand->scratchbuf = retry_mode; > > + return spi_mem_exec_op(spinand->spimem, &op); > > +} > > + > > +static const struct spi_nand_fixups read_retry_fixups = { > > + .init_read_retry = macronix_spinand_init_read_retry, > > + .setup_read_retry = macronix_spinand_setup_read_retry, > > +}; > > + > > ... > > > @@ -325,7 +373,8 @@ static const struct spinand_info macronix_spinand_table[] = { > > &update_cache_variants), > > SPINAND_HAS_QE_BIT, > > SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, > > - mx35lf1ge4ab_ecc_get_status)), > > + mx35lf1ge4ab_ecc_get_status), > > + SPINAND_FIXUPS(&read_retry_fixups)), > > SPINAND_INFO("MX35UF1GE4AC", > > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x92, 0x01), > > NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1), > > I expect a patch targeting the core first, and then the changes in the > Macronix driver. Got it, so do you prefer that we switch to using flags instead? > > > diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h > > index 5c19ead60499..e567d00a2805 100644 > > --- a/include/linux/mtd/spinand.h > > +++ b/include/linux/mtd/spinand.h > > @@ -354,6 +354,7 @@ struct spinand_info { > > } op_variants; > > int (*select_target)(struct spinand_device *spinand, > > unsigned int target); > > + const struct spi_nand_fixups *fixups; > > }; > > > > #define SPINAND_ID(__method, ...) \ > > @@ -379,6 +380,9 @@ struct spinand_info { > > #define SPINAND_SELECT_TARGET(__func) \ > > .select_target = __func, > > > > +#define SPINAND_FIXUPS(__func) \ > > + .fixups = __func, > > I don't like the fixups naming. It feels like something is inherently > wrong. > > > + > > #define SPINAND_INFO(__model, __id, __memorg, __eccreq, __op_variants, \ > > __flags, ...) \ > > { \ > > @@ -398,6 +402,16 @@ struct spinand_dirmap { > > struct spi_mem_dirmap_desc *rdesc_ecc; > > }; > > > > +/** > > + * struct spi_nand_fixups - SPI NAND fixup hooks > > + * @init_read_retry: initialize spinand->read_retries > > + * @setup_read_retry: set the retry mode > > + */ > > +struct spi_nand_fixups { > > + int (*init_read_retry)(struct spinand_device *spinand); > > + int (*setup_read_retry)(struct spinand_device *spinand, u8 retry_mode); > > +}; > > + > > /** > > * struct spinand_device - SPI NAND device instance > > * @base: NAND device instance > > @@ -423,6 +437,7 @@ struct spinand_dirmap { > > * the stack > > * @manufacturer: SPI NAND manufacturer information > > * @priv: manufacturer private data > > + * @read_retries: the number of read retry modes supported > > */ > > struct spinand_device { > > struct nand_device base; > > @@ -449,8 +464,10 @@ struct spinand_device { > > u8 *databuf; > > u8 *oobbuf; > > u8 *scratchbuf; > > + const struct spinand_info *info; > > This looks like a leftover. > > > const struct spinand_manufacturer *manufacturer; > > void *priv; > > + int read_retries; > > Any reason to keep this variable signed? No, we can simply change from int to u8. > > > }; > > > > /** > > > Thanks, > Miquèl Thanks, Cheng Ming Lin
Hi Cheng Ming, > > > @@ -325,7 +373,8 @@ static const struct spinand_info macronix_spinand_table[] = { > > > &update_cache_variants), > > > SPINAND_HAS_QE_BIT, > > > SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, > > > - mx35lf1ge4ab_ecc_get_status)), > > > + mx35lf1ge4ab_ecc_get_status), > > > + SPINAND_FIXUPS(&read_retry_fixups)), > > > SPINAND_INFO("MX35UF1GE4AC", > > > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x92, 0x01), > > > NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1), > > > > I expect a patch targeting the core first, and then the changes in the > > Macronix driver. > > Got it, so do you prefer that we switch to using flags instead? Not necessarily, did I? ... > > > const struct spinand_manufacturer *manufacturer; > > > void *priv; > > > + int read_retries; > > > > Any reason to keep this variable signed? > > No, we can simply change from int to u8. Just unsigned int is fine. Thanks, Miquèl
Hi Miquel, Miquel Raynal <miquel.raynal@bootlin.com> 於 2024年10月7日 週一 下午4:33寫道： > > Hi Cheng Ming, > > > > > @@ -325,7 +373,8 @@ static const struct spinand_info macronix_spinand_table[] = { > > > > &update_cache_variants), > > > > SPINAND_HAS_QE_BIT, > > > > SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, > > > > - mx35lf1ge4ab_ecc_get_status)), > > > > + mx35lf1ge4ab_ecc_get_status), > > > > + SPINAND_FIXUPS(&read_retry_fixups)), > > > > SPINAND_INFO("MX35UF1GE4AC", > > > > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x92, 0x01), > > > > NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1), > > > > > > I expect a patch targeting the core first, and then the changes in the > > > Macronix driver. > > > > Got it, so do you prefer that we switch to using flags instead? > > Not necessarily, did I? > > ... > Using a flag instead of fixups allows this patch to target the core first, and reduces changes in the Macronix driver. > > > > const struct spinand_manufacturer *manufacturer; > > > > void *priv; > > > > + int read_retries; > > > > > > Any reason to keep this variable signed? > > > > No, we can simply change from int to u8. > > Just unsigned int is fine. > Sure, thanks! > Thanks, > Miquèl Thanks, Cheng Ming Lin
Hi, linchengming884@gmail.com wrote on Tue, 8 Oct 2024 14:25:25 +0800: > Hi Miquel, > > Miquel Raynal <miquel.raynal@bootlin.com> 於 2024年10月7日 週一 下午4:33寫道： > > > > Hi Cheng Ming, > > > > > > > @@ -325,7 +373,8 @@ static const struct spinand_info macronix_spinand_table[] = { > > > > > &update_cache_variants), > > > > > SPINAND_HAS_QE_BIT, > > > > > SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, > > > > > - mx35lf1ge4ab_ecc_get_status)), > > > > > + mx35lf1ge4ab_ecc_get_status), > > > > > + SPINAND_FIXUPS(&read_retry_fixups)), > > > > > SPINAND_INFO("MX35UF1GE4AC", > > > > > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x92, 0x01), > > > > > NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1), > > > > > > > > I expect a patch targeting the core first, and then the changes in the > > > > Macronix driver. > > > > > > Got it, so do you prefer that we switch to using flags instead? > > > > Not necessarily, did I? > > > > ... > > > > Using a flag instead of fixups allows this patch to target the core first, > and reduces changes in the Macronix driver. Propose what ever you think is best. You can also look at how it is done in raw NAND. But always include the core changes first, please. It is not related to how you implement it. > > > > > > const struct spinand_manufacturer *manufacturer; > > > > > void *priv; > > > > > + int read_retries; > > > > > > > > Any reason to keep this variable signed? > > > > > > No, we can simply change from int to u8. > > > > Just unsigned int is fine. > > > > Sure, thanks! > > > Thanks, > > Miquèl > > Thanks, > Cheng Ming Lin Thanks, Miquèl
Hi Miquel, Miquel Raynal <miquel.raynal@bootlin.com> 於 2024年10月8日 週二 下午4:55寫道： > > Hi, > > linchengming884@gmail.com wrote on Tue, 8 Oct 2024 14:25:25 +0800: > > > Hi Miquel, > > > > Miquel Raynal <miquel.raynal@bootlin.com> 於 2024年10月7日 週一 下午4:33寫道： > > > > > > Hi Cheng Ming, > > > > > > > > > @@ -325,7 +373,8 @@ static const struct spinand_info macronix_spinand_table[] = { > > > > > > &update_cache_variants), > > > > > > SPINAND_HAS_QE_BIT, > > > > > > SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, > > > > > > - mx35lf1ge4ab_ecc_get_status)), > > > > > > + mx35lf1ge4ab_ecc_get_status), > > > > > > + SPINAND_FIXUPS(&read_retry_fixups)), > > > > > > SPINAND_INFO("MX35UF1GE4AC", > > > > > > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x92, 0x01), > > > > > > NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1), > > > > > > > > > > I expect a patch targeting the core first, and then the changes in the > > > > > Macronix driver. > > > > > > > > Got it, so do you prefer that we switch to using flags instead? > > > > > > Not necessarily, did I? > > > > > > ... > > > > > > > Using a flag instead of fixups allows this patch to target the core first, > > and reduces changes in the Macronix driver. > > Propose what ever you think is best. You can also look at how it is > done in raw NAND. But always include the core changes first, please. > It is not related to how you implement it. > Thank you so much for your suggestion. I will ensure the core changes are addressed first, as you mentioned. Also, I did refer to how it's done in raw NAND when working on this patch. I really appreciate your guidance and will definitely keep your words in mind. > > > > > > > > const struct spinand_manufacturer *manufacturer; > > > > > > void *priv; > > > > > > + int read_retries; > > > > > > > > > > Any reason to keep this variable signed? > > > > > > > > No, we can simply change from int to u8. > > > > > > Just unsigned int is fine. > > > > > > > Sure, thanks! > > > > > Thanks, > > > Miquèl > > > > Thanks, > > Cheng Ming Lin > > > Thanks, > Miquèl Thanks, Cheng Ming Lin
diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c index 3f9e9c572854..baca67ff1cd6 100644 --- a/drivers/mtd/nand/spi/macronix.c +++ b/drivers/mtd/nand/spi/macronix.c @@ -9,6 +9,8 @@ #include <linux/kernel.h> #include <linux/mtd/spinand.h> +#define MACRONIX_NUM_READ_RETRY_MODES 6 +#define MACRONIX_FEATURE_ADDR_READ_RETRY 0x70 #define SPINAND_MFR_MACRONIX 0xC2 #define MACRONIX_ECCSR_MASK 0x0F @@ -100,6 +102,38 @@ static int mx35lf1ge4ab_ecc_get_status(struct spinand_device *spinand, return -EINVAL; } +/** + * macronix_spinand_init_read_retry - Initialize read_retries + * @spinand: SPI NAND device + * + * Return: the number of read retry modes + */ +static int macronix_spinand_init_read_retry(struct spinand_device *spinand) +{ + return MACRONIX_NUM_READ_RETRY_MODES; +} + +/** + * macronix_spinand_setup_read_retry - Set the retry mode + * @spinand: SPI NAND device + * @retry_mode: Specify which retry mode to set + * + * Return: 0 on success, -error otherwise + */ +static int macronix_spinand_setup_read_retry(struct spinand_device *spinand, u8 retry_mode) +{ + struct spi_mem_op op = SPINAND_SET_FEATURE_OP(MACRONIX_FEATURE_ADDR_READ_RETRY, + spinand->scratchbuf); + + *spinand->scratchbuf = retry_mode; + return spi_mem_exec_op(spinand->spimem, &op); +} + +static const struct spi_nand_fixups read_retry_fixups = { + .init_read_retry = macronix_spinand_init_read_retry, + .setup_read_retry = macronix_spinand_setup_read_retry, +}; + static const struct spinand_info macronix_spinand_table[] = { SPINAND_INFO("MX35LF1GE4AB", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x12), @@ -129,7 +163,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35LF4GE4AD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x37, 0x03), NAND_MEMORG(1, 4096, 128, 64, 2048, 40, 1, 1, 1), @@ -139,7 +174,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35LF1G24AD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x14, 0x03), NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1), @@ -148,7 +184,8 @@ static const struct spinand_info macronix_spinand_table[] = { &write_cache_variants, &update_cache_variants), SPINAND_HAS_QE_BIT, - SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL)), + SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35LF2G24AD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x24, 0x03), NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1), @@ -157,7 +194,8 @@ static const struct spinand_info macronix_spinand_table[] = { &write_cache_variants, &update_cache_variants), SPINAND_HAS_QE_BIT, - SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL)), + SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35LF2G24AD-Z4I8", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x64, 0x03), NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1), @@ -166,7 +204,8 @@ static const struct spinand_info macronix_spinand_table[] = { &write_cache_variants, &update_cache_variants), SPINAND_HAS_QE_BIT, - SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL)), + SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35LF4G24AD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x35, 0x03), NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 2, 1, 1), @@ -175,7 +214,8 @@ static const struct spinand_info macronix_spinand_table[] = { &write_cache_variants, &update_cache_variants), SPINAND_HAS_QE_BIT, - SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL)), + SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35LF4G24AD-Z4I8", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x75, 0x03), NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1), @@ -184,7 +224,8 @@ static const struct spinand_info macronix_spinand_table[] = { &write_cache_variants, &update_cache_variants), SPINAND_HAS_QE_BIT, - SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL)), + SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX31LF1GE4BC", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x1e), NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1), @@ -225,7 +266,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35UF4G24AD-Z4I8", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xf5, 0x03), NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1), @@ -235,7 +277,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35UF4GE4AD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xb7, 0x03), NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1), @@ -245,7 +288,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35UF2G14AC", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xa0), NAND_MEMORG(1, 2048, 64, 64, 2048, 40, 2, 1, 1), @@ -265,7 +309,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35UF2G24AD-Z4I8", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xe4, 0x03), NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1), @@ -275,7 +320,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35UF2GE4AD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xa6, 0x03), NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1), @@ -285,7 +331,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35UF2GE4AC", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xa2, 0x01), NAND_MEMORG(1, 2048, 64, 64, 2048, 40, 1, 1, 1), @@ -315,7 +362,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35UF1GE4AD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x96, 0x03), NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1), @@ -325,7 +373,8 @@ static const struct spinand_info macronix_spinand_table[] = { &update_cache_variants), SPINAND_HAS_QE_BIT, SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, - mx35lf1ge4ab_ecc_get_status)), + mx35lf1ge4ab_ecc_get_status), + SPINAND_FIXUPS(&read_retry_fixups)), SPINAND_INFO("MX35UF1GE4AC", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x92, 0x01), NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1), diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h index 5c19ead60499..e567d00a2805 100644 --- a/include/linux/mtd/spinand.h +++ b/include/linux/mtd/spinand.h @@ -354,6 +354,7 @@ struct spinand_info { } op_variants; int (*select_target)(struct spinand_device *spinand, unsigned int target); + const struct spi_nand_fixups *fixups; }; #define SPINAND_ID(__method, ...) \ @@ -379,6 +380,9 @@ struct spinand_info { #define SPINAND_SELECT_TARGET(__func) \ .select_target = __func, +#define SPINAND_FIXUPS(__func) \ + .fixups = __func, + #define SPINAND_INFO(__model, __id, __memorg, __eccreq, __op_variants, \ __flags, ...) \ { \ @@ -398,6 +402,16 @@ struct spinand_dirmap { struct spi_mem_dirmap_desc *rdesc_ecc; }; +/** + * struct spi_nand_fixups - SPI NAND fixup hooks + * @init_read_retry: initialize spinand->read_retries + * @setup_read_retry: set the retry mode + */ +struct spi_nand_fixups { + int (*init_read_retry)(struct spinand_device *spinand); + int (*setup_read_retry)(struct spinand_device *spinand, u8 retry_mode); +}; + /** * struct spinand_device - SPI NAND device instance * @base: NAND device instance @@ -423,6 +437,7 @@ struct spinand_dirmap { * the stack * @manufacturer: SPI NAND manufacturer information * @priv: manufacturer private data + * @read_retries: the number of read retry modes supported */ struct spinand_device { struct nand_device base; @@ -449,8 +464,10 @@ struct spinand_device { u8 *databuf; u8 *oobbuf; u8 *scratchbuf; + const struct spinand_info *info; const struct spinand_manufacturer *manufacturer; void *priv; + int read_retries; }; /**