On Tue, Feb 2, 2016 at 7:28 AM, Markus Pargmann <mpa@xxxxxxxxxxxxxx> wrote: > Hi, > > On Saturday, January 23, 2016 03:01:47 PM Brian Norris wrote: >> + Markus Pargmann >> >> On Wed, Dec 02, 2015 at 04:47:45PM -0600, Han Xu wrote: >> > i.MX6QP and i.MX7D BCH module integrated a new feature to detect the >> > bitflip number for erased NAND page. So for these two platform, set the >> > erase threshold to ecc_strength and if bitflip detected, GPMI driver will >> > correct the data to all 0xFF. >> >> Looks like Marcus is also trying to add SW correction for the same >> issue: >> >> http://patchwork.ozlabs.org/patch/559557/ >> >> How do those 2 interact (if at all)? > > Thanks for CCing. > >> >> Brian >> >> > Signed-off-by: Han Xu <b45815@xxxxxxxxxxxxx> >> > --- >> > drivers/mtd/nand/gpmi-nand/bch-regs.h | 10 ++++++++++ >> > drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 5 +++++ >> > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 24 +++++++++++++++++++++++- >> > drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 5 ++++- >> > 4 files changed, 42 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h >> > index 53e58bc..a84d72b 100644 >> > --- a/drivers/mtd/nand/gpmi-nand/bch-regs.h >> > +++ b/drivers/mtd/nand/gpmi-nand/bch-regs.h >> > @@ -30,7 +30,13 @@ >> > #define BM_BCH_CTRL_COMPLETE_IRQ (1 << 0) >> > >> > #define HW_BCH_STATUS0 0x00000010 >> > + >> > #define HW_BCH_MODE 0x00000020 >> > +#define BP_BCH_MODE_ERASE_THRESHOLD 0 >> > +#define BM_BCH_MODE_ERASE_THRESHOLD (0xff << BP_BCH_MODE_ERASE_THRESHOLD) >> > +#define BF_BCH_MODE_ERASE_THRESHOLD(v) \ >> > + (((v) << BP_BCH_MODE_ERASE_THRESHOLD) & BM_BCH_MODE_ERASE_THRESHOLD) >> > + >> > #define HW_BCH_ENCODEPTR 0x00000030 >> > #define HW_BCH_DATAPTR 0x00000040 >> > #define HW_BCH_METAPTR 0x00000050 >> > @@ -125,4 +131,8 @@ >> > ) >> > >> > #define HW_BCH_VERSION 0x00000160 >> > +#define HW_BCH_DEBUG1 0x00000170 >> > +#define BP_BCH_DEBUG1_ERASED_ZERO_COUNT 0 >> > +#define BM_BCH_DEBUG1_ERASED_ZERO_COUNT \ >> > + (0x1ff << BP_BCH_DEBUG1_ERASED_ZERO_COUNT) >> > #endif >> > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c >> > index 1f26a79..0548d84 100644 >> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c >> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c >> > @@ -298,6 +298,11 @@ int bch_set_geometry(struct gpmi_nand_data *this) >> > | BF_BCH_FLASH0LAYOUT1_DATAN_SIZE(block_size, this), >> > r->bch_regs + HW_BCH_FLASH0LAYOUT1); >> > >> > + /* Set erase threshold to ecc_strength for mx6qp and mx7 */ >> > + if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this)) >> > + writel(BF_BCH_MODE_ERASE_THRESHOLD(ecc_strength), >> > + r->bch_regs + HW_BCH_MODE); > > This is probably different on imx6qp and imx7?! > On the other imx6 processors, setting this will instruct the BCH engine > to allow 'ecc_strength' number of bitflips in an erased area without a > reported ECC error. Unfortunately it does not fix the bitflips. > > So what happens on imx6q,dl,s in this case: > 1) BCH detects bitflips in the erased page, but ignores them due to the > ERASE_THRESHOLD setting up to 'ecc_strength' bitflips. > 2) The gpmi driver does not notice any errors as BCH did not set that > there is any ECC error in the data. So the driver assumes correct data > and does not fix the existing bitflips in the erased page. > 3) UBI does not see any ECC errors and does not check the page. > 4) UBIFS does check the content of erased pages at some points and > breaks at this point. The filesystem can't be mounted anymore. > > This is why I added my patch a software check in case of an > uncorrectable ECC error. The ERASE_THRESHOLD has to be 0 for this to > work. > > As this patch only affects imx6qp and imx7, I assume that there is a > better mechanism now. IC team add this new feature in i.mx6qp and i.mx7 for bitflip, which provide a easy way to fix the issue, while we still need some SW check for other i.mx6 platforms. So these patches don't conflict with others. Han Xu > > Best Regards, > > Markus > >> > + >> > /* Set *all* chip selects to use layout 0. */ >> > writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT); >> > >> > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> > index 9f67f0f..9dea56e 100644 >> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> > @@ -71,6 +71,12 @@ static const struct gpmi_devdata gpmi_devdata_imx6q = { >> > .max_chain_delay = 12, >> > }; >> > >> > +static const struct gpmi_devdata gpmi_devdata_imx6qp = { >> > + .type = IS_MX6QP, >> > + .bch_max_ecc_strength = 40, >> > + .max_chain_delay = 12, >> > +}; >> > + >> > static const struct gpmi_devdata gpmi_devdata_imx6sx = { >> > .type = IS_MX6SX, >> > .bch_max_ecc_strength = 62, >> > @@ -1010,6 +1016,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip, >> > { >> > struct gpmi_nand_data *this = chip->priv; >> > struct bch_geometry *nfc_geo = &this->bch_geometry; >> > + void __iomem *bch_regs = this->resources.bch_regs; >> > void *payload_virt; >> > dma_addr_t payload_phys; >> > void *auxiliary_virt; >> > @@ -1018,6 +1025,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip, >> > unsigned char *status; >> > unsigned int max_bitflips = 0; >> > int ret; >> > + int flag = 0; >> > >> > dev_dbg(this->dev, "page number is : %d\n", page); >> > ret = read_page_prepare(this, buf, nfc_geo->payload_size, >> > @@ -1050,9 +1058,16 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip, >> > status = auxiliary_virt + nfc_geo->auxiliary_status_offset; >> > >> > for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) { >> > - if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED)) >> > + if (*status == STATUS_GOOD) >> > continue; >> > >> > + if (*status == STATUS_ERASED) { >> > + if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this)) >> > + if (readl(bch_regs + HW_BCH_DEBUG1)) >> > + flag = 1; >> > + continue; >> > + } >> > + >> > if (*status == STATUS_UNCORRECTABLE) { >> > mtd->ecc_stats.failed++; >> > continue; >> > @@ -1081,6 +1096,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip, >> > nfc_geo->payload_size, >> > payload_virt, payload_phys); >> > >> > + /* if bitflip occurred in erased page, change data to all 0xff */ >> > + if (flag) >> > + memset(buf, 0xff, nfc_geo->payload_size); >> > + >> > return max_bitflips; >> > } >> > >> > @@ -1990,6 +2009,9 @@ static const struct of_device_id gpmi_nand_id_table[] = { >> > .compatible = "fsl,imx6q-gpmi-nand", >> > .data = &gpmi_devdata_imx6q, >> > }, { >> > + .compatible = "fsl,imx6qp-gpmi-nand", >> > + .data = (void *)&gpmi_devdata_imx6qp, >> > + }, { >> > .compatible = "fsl,imx6sx-gpmi-nand", >> > .data = &gpmi_devdata_imx6sx, >> > }, { >> > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h >> > index 58b3d69..149a442 100644 >> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h >> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h >> > @@ -123,6 +123,7 @@ enum gpmi_type { >> > IS_MX23, >> > IS_MX28, >> > IS_MX6Q, >> > + IS_MX6QP, >> > IS_MX6SX, >> > IS_MX7D, >> > }; >> > @@ -306,9 +307,11 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off, >> > #define GPMI_IS_MX23(x) ((x)->devdata->type == IS_MX23) >> > #define GPMI_IS_MX28(x) ((x)->devdata->type == IS_MX28) >> > #define GPMI_IS_MX6Q(x) ((x)->devdata->type == IS_MX6Q) >> > +#define GPMI_IS_MX6QP(x) ((x)->devdata->type == IS_MX6QP) >> > #define GPMI_IS_MX6SX(x) ((x)->devdata->type == IS_MX6SX) >> > #define GPMI_IS_MX7D(x) ((x)->devdata->type == IS_MX7D) >> > >> > -#define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x)) >> > +#define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6QP(x)\ >> > + || GPMI_IS_MX6SX(x)) >> > #define GPMI_IS_MX7(x) (GPMI_IS_MX7D(x)) >> > #endif >> > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
