From: Boris Brezillon <boris.brezillon@bootlin.com>
To: Stefan Agner <stefan@agner.ch>
Cc: Dmitry Osipenko <digetx@gmail.com>,
dwmw2@infradead.org, computersforpeace@gmail.com,
marek.vasut@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com,
thierry.reding@gmail.com, dev@lynxeye.de,
miquel.raynal@bootlin.com, richard@nod.at, marcel@ziswiler.com,
krzk@kernel.org, benjamin.lindqvist@endian.se,
jonathanh@nvidia.com, pdeschrijver@nvidia.com,
pgaikwad@nvidia.com, mirza.krak@gmail.com, gaireg@gaireg.de,
linux-mtd@lists.infradead.org, linux-tegra@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
Date: Tue, 12 Jun 2018 10:13:19 +0200 [thread overview]
Message-ID: <20180612101319.528da20c@bbrezillon> (raw)
In-Reply-To: <fd15fcac50d25281fa8f7d97a738bf6a@agner.ch>
On Tue, 12 Jun 2018 10:02:12 +0200
Stefan Agner <stefan@agner.ch> wrote:
> >> +static int tegra_nand_read_page_hwecc(struct mtd_info *mtd,
> >> + struct nand_chip *chip,
> >> + uint8_t *buf, int oob_required, int page)
> >> +{
> >> + struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
> >> + struct tegra_nand_chip *nand = to_tegra_chip(chip);
> >> + void *oob_buf = oob_required ? chip->oob_poi : 0;
> >> + u32 dec_stat, max_corr_cnt;
> >> + unsigned long fail_sec_flag;
> >> + int ret;
> >> +
> >> + tegra_nand_hw_ecc(ctrl, chip, true);
> >> + ret = tegra_nand_page_xfer(mtd, chip, buf, oob_buf, nand->tag.length,
> >> + page, true);
> >> + tegra_nand_hw_ecc(ctrl, chip, false);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* No correctable or un-correctable errors, page must have 0 bitflips */
> >> + if (!ctrl->last_read_error)
> >> + return 0;
> >> +
> >> + /*
> >> + * Correctable or un-correctable errors occurred. Use DEC_STAT_BUF
> >> + * which contains information for all ECC selections.
> >> + *
> >> + * Note that since we do not use Command Queues DEC_RESULT does not
> >> + * state the number of pages we can read from the DEC_STAT_BUF. But
> >> + * since CORRFAIL_ERR did occur during page read we do have a valid
> >> + * result in DEC_STAT_BUF.
> >> + */
> >> + ctrl->last_read_error = false;
> >> + dec_stat = readl_relaxed(ctrl->regs + DEC_STAT_BUF);
> >> +
> >> + fail_sec_flag = (dec_stat & DEC_STAT_BUF_FAIL_SEC_FLAG_MASK) >>
> >> + DEC_STAT_BUF_FAIL_SEC_FLAG_SHIFT;
> >> +
> >> + max_corr_cnt = (dec_stat & DEC_STAT_BUF_MAX_CORR_CNT_MASK) >>
> >> + DEC_STAT_BUF_MAX_CORR_CNT_SHIFT;
> >> +
> >> + if (fail_sec_flag) {
> >> + int bit, max_bitflips = 0;
> >> +
> >> + /*
> >> + * Check if all sectors in a page failed. If only some failed
> >> + * its definitly not an erased page and we can return error
> >> + * stats right away.
> >> + *
> >> + * E.g. controller might return fail_sec_flag with 0x4, which
> >> + * would mean only the third sector failed to correct.
That works because you have NAND_NO_SUBPAGE_WRITE set (i.e. no partial
page programming), probably something you should state here.
> >> + */
> >> + if (fail_sec_flag ^ GENMASK(chip->ecc.steps - 1, 0)) {
> >> + mtd->ecc_stats.failed += hweight8(fail_sec_flag);
> >> + return max_corr_cnt;
> >> + }
> >> +
> >> + /*
> >> + * All sectors failed to correct, but the ECC isn't smart
> >> + * enough to figure out if a page is really completely erased.
> >> + * We check the read data here to figure out if it's a
> >> + * legitimate ECC error or only an erased page.
> >> + */
> >> + for_each_set_bit(bit, &fail_sec_flag, chip->ecc.steps) {
> >> + u8 *data = buf + (chip->ecc.size * bit);
> >> +
> >> + ret = nand_check_erased_ecc_chunk(data, chip->ecc.size,
> >> + NULL, 0,
You should also check that the ECC bytes are 0xff here, otherwise you
won't detect corruption of pages almost filled 0xff but with a few bits
set to 0.
When you use nand_check_erased_ecc_chunk(), it's important to always
pass the data along with its associated ECC bytes.
> >> + NULL, 0,
If you support writing extra OOB bytes, you should also pass them here.
> >> + chip->ecc.strength);
> >> + if (ret < 0)
> >> + mtd->ecc_stats.failed++;
> >> + else
> >> + max_bitflips = max(ret, max_bitflips);
> >> + }
> >> +
> >> + return max_t(unsigned int, max_corr_cnt, max_bitflips);
> >> + } else {
> >> + int corr_sec_flag;
> >> +
> >> + corr_sec_flag = (dec_stat & DEC_STAT_BUF_CORR_SEC_FLAG_MASK) >>
> >> + DEC_STAT_BUF_CORR_SEC_FLAG_SHIFT;
> >> +
> >> + /*
> >> + * The value returned in the register is the maximum of
> >> + * bitflips encountered in any of the ECC regions. As there is
> >> + * no way to get the number of bitflips in a specific regions
> >> + * we are not able to deliver correct stats but instead
> >> + * overestimate the number of corrected bitflips by assuming
> >> + * that all regions where errors have been corrected
> >> + * encountered the maximum number of bitflips.
> >> + */
> >> + mtd->ecc_stats.corrected += max_corr_cnt * hweight8(corr_sec_flag);
> >> +
> >> + return max_corr_cnt;
> >> + }
> >> +
next prev parent reply other threads:[~2018-06-12 8:13 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-11 20:52 [PATCH v4 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
2018-06-11 20:52 ` [PATCH v4 1/6] mtd: rawnand: add Reed-Solomon error correction algorithm Stefan Agner
2018-06-12 18:37 ` Rob Herring
2018-06-11 20:52 ` [PATCH v4 2/6] mtd: rawnand: add an option to specify NAND chip as a boot device Stefan Agner
2018-06-11 20:52 ` [PATCH v4 3/6] mtd: rawnand: tegra: add devicetree binding Stefan Agner
2018-06-11 20:52 ` [PATCH v4 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver Stefan Agner
2018-06-11 23:08 ` kbuild test robot
2018-06-11 23:32 ` Dmitry Osipenko
2018-06-12 8:02 ` Stefan Agner
2018-06-12 8:13 ` Boris Brezillon [this message]
2018-06-17 16:57 ` Stefan Agner
2018-06-12 0:03 ` Dmitry Osipenko
2018-06-12 8:06 ` Stefan Agner
2018-06-12 8:27 ` Boris Brezillon
2018-06-12 9:17 ` Stefan Agner
2018-06-12 9:34 ` Boris Brezillon
2018-06-12 15:24 ` Jens Axboe
2018-06-12 20:20 ` Stefan Agner
2018-06-12 21:20 ` Boris Brezillon
2018-06-12 21:24 ` Jens Axboe
2018-06-12 21:30 ` Boris Brezillon
2018-06-12 12:17 ` Dmitry Osipenko
2018-06-11 20:52 ` [PATCH v4 5/6] ARM: dts: tegra: add Tegra20 NAND flash controller node Stefan Agner
2018-06-11 20:52 ` [PATCH v4 6/6] ARM: dts: tegra: enable NAND flash on Colibri T20 Stefan Agner
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=20180612101319.528da20c@bbrezillon \
--to=boris.brezillon@bootlin.com \
--cc=benjamin.lindqvist@endian.se \
--cc=computersforpeace@gmail.com \
--cc=dev@lynxeye.de \
--cc=devicetree@vger.kernel.org \
--cc=digetx@gmail.com \
--cc=dwmw2@infradead.org \
--cc=gaireg@gaireg.de \
--cc=jonathanh@nvidia.com \
--cc=krzk@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=linux-tegra@vger.kernel.org \
--cc=marcel@ziswiler.com \
--cc=marek.vasut@gmail.com \
--cc=mark.rutland@arm.com \
--cc=miquel.raynal@bootlin.com \
--cc=mirza.krak@gmail.com \
--cc=pdeschrijver@nvidia.com \
--cc=pgaikwad@nvidia.com \
--cc=richard@nod.at \
--cc=robh+dt@kernel.org \
--cc=stefan@agner.ch \
--cc=thierry.reding@gmail.com \
/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).