From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Han Xu <han.xu@nxp.com>
Cc: <richard@nod.at>, <dwmw2@infradead.org>,
<computersforpeace@gmail.com>, <robh+dt@kernel.org>,
<pawel.moll@arm.com>, <mark.rutland@arm.com>,
<ijc+devicetree@hellion.org.uk>, <galak@codeaurora.org>,
<devicetree@vger.kernel.org>, <linux-mtd@lists.infradead.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page
Date: Tue, 14 Jun 2016 21:14:01 +0200 [thread overview]
Message-ID: <20160614211401.40c26b37@bbrezillon> (raw)
In-Reply-To: <1465578094-8816-5-git-send-email-han.xu@nxp.com>
On Fri, 10 Jun 2016 12:01:32 -0500
Han Xu <han.xu@nxp.com> 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.
>
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
> 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 | 27 ++++++++++++++++++++++++++-
> 3 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> index 228142c..2c44b88 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 358ff5d..0b5666a 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);
> +
> /* 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 aedaff3..03bdb4d 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1043,6 +1043,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> {
> struct gpmi_nand_data *this = nand_get_controller_data(chip);
> 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;
> @@ -1051,6 +1052,8 @@ 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 bitflips = 0;
> + int bitflip_flag = 0;
Hm, I would choose another name, but I'm not even sure this is really
needed (see below).
>
> dev_dbg(this->dev, "page number is : %d\n", page);
> ret = read_page_prepare(this, buf, nfc_geo->payload_size,
> @@ -1088,8 +1091,20 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> payload_virt, payload_phys);
>
> 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)) {
> + bitflips = readl(bch_regs + HW_BCH_DEBUG1);
You could use ret here or define a flips variable locally to this code
(see what's done for the software correction).
BTW, why not memset-ing the data chunk here (as done for the SW
correction part). This way you wouldn't need the extra bitflips_flag
variable.
> + if (bitflips) {
> + bitflip_flag = 1;
> + max_bitflips = max_t(unsigned int,
> + max_bitflips,
> + bitflips);
> + }
> + }
> continue;
> + }
>
> if (*status == STATUS_UNCORRECTABLE) {
> int eccbits = nfc_geo->ecc_strength * nfc_geo->gf_len;
> @@ -1098,6 +1113,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> int eccbytes;
> int flips;
>
> + /* shortcut for i.MX7 and i.MX6QP */
> + if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))
You should increment mtd->ecc_stats.failed in this case.
> + continue;
> +
> /* Read ECC bytes into our internal raw_buffer */
> offset = nfc_geo->metadata_size * 8;
> offset += ((8 * nfc_geo->ecc_chunk_size) + eccbits) * (i + 1);
> @@ -1182,6 +1201,12 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> chip->oob_poi[0] = ((uint8_t *) auxiliary_virt)[0];
> }
>
> + /* if bitflip occurred in erased page, change data to all 0xff */
> + if (bitflip_flag) {
> + memset(buf, ~0, nfc_geo->payload_size);
> + memset(chip->oob_poi, ~0, mtd->oobsize);
> + }
> +
> return max_bitflips;
> }
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
next prev parent reply other threads:[~2016-06-14 19:14 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-10 17:01 [PATCH v2 0/6] support gpmi on i.MX6UL/7D and HW bitflip on i.MX6QP/7D Han Xu
2016-06-10 17:01 ` [PATCH v2 1/6] mtd: nand: gpmi: add GPMI NAND support for i.MX7D Han Xu
2016-06-14 20:05 ` Boris Brezillon
2016-06-10 17:01 ` [PATCH v2 2/6] mtd: nand: gpmi: document the clocks and clock-names in DT property Han Xu
2016-06-14 12:46 ` Rob Herring
2016-06-10 17:01 ` [PATCH v2 3/6] mtd: nand: gpmi: add GPMI NAND support for i.MX6QP Han Xu
2016-06-10 17:01 ` [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page Han Xu
2016-06-14 19:14 ` Boris Brezillon [this message]
2016-06-21 15:53 ` Han Xu
2016-06-21 16:18 ` Boris Brezillon
2016-06-10 17:01 ` [PATCH v2 5/6] mtd: nand: gpmi: support NAND on i.MX6UL Han Xu
2016-06-10 17:01 ` [PATCH v2 6/6] mtd: nand: gpmi: document the new supported chip in DT Han Xu
2016-06-14 12:48 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160614211401.40c26b37@bbrezillon \
--to=boris.brezillon@free-electrons.com \
--cc=computersforpeace@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=dwmw2@infradead.org \
--cc=galak@codeaurora.org \
--cc=han.xu@nxp.com \
--cc=ijc+devicetree@hellion.org.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=pawel.moll@arm.com \
--cc=richard@nod.at \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).