[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20121003192044.GB27502@parrot.com>
Date: Wed, 3 Oct 2012 21:20:45 +0200
From: Ivan Djelic <ivan.djelic@...rot.com>
To: "Philip, Avinash" <avinashphilip@...com>
CC: "dwmw2@...radead.org" <dwmw2@...radead.org>,
"artem.bityutskiy@...ux.intel.com" <artem.bityutskiy@...ux.intel.com>,
"tony@...mide.com" <tony@...mide.com>,
"afzal@...com" <afzal@...com>,
"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"devicetree-discuss@...ts.ozlabs.org"
<devicetree-discuss@...ts.ozlabs.org>
Subject: Re: [PATCH 4/4] mtd: nand: omap2: Add data correction support
On Wed, Oct 03, 2012 at 03:29:49PM +0100, Philip, Avinash wrote:
> ELM module can be used for error correction of BCH 4 & 8 bit. Also
> support read & write page in one shot by adding custom read_page &
> write_page methods. This helps in optimizing code.
>
> New structure member "is_elm_used" is added to know the status of
> whether the ELM module is used for error correction or not.
>
> Note:
> ECC layout of BCH8 uses 14 bytes for 512 byte of data to make compatible
> with RBL ECC layout, even though the requirement was only 13 byte. This
> results a common ecc layout across RBL, U-boot & Linux.
>
See a few comments below,
(...)
> +static int omap_elm_correct_data(struct mtd_info *mtd, u_char *dat,
> + u_char *read_ecc, u_char *calc_ecc)
> +{
> + struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
> + mtd);
> + int eccsteps = info->nand.ecc.steps;
> + int i , j, stat = 0;
> + int eccsize, eccflag, size;
> + struct elm_errorvec err_vec[ERROR_VECTOR_MAX];
> + u_char *ecc_vec = calc_ecc;
> + enum bch_ecc type;
> + bool is_error_reported = false;
> +
> + /* initialize elm error vector to zero */
> + memset(err_vec, 0, sizeof(err_vec));
> + if (info->nand.ecc.strength == BCH8_MAX_ERROR) {
> + size = BCH8_SIZE;
> + eccsize = BCH8_ECC_OOB_BYTES;
> + type = BCH8_ECC;
> + } else {
> + size = BCH4_SIZE;
> + eccsize = BCH4_SIZE;
> + type = BCH4_ECC;
> + }
> +
> + for (i = 0; i < eccsteps ; i++) {
> + eccflag = 0; /* initialize eccflag */
> +
> + for (j = 0; (j < eccsize); j++) {
> + if (read_ecc[j] != 0xFF) {
> + eccflag = 1; /* data area is flashed */
Just a reminder: this way of checking if a page has been programmed is not robust to bitflips,
so you may get into trouble with UBIFS on a fairly recent device.
(...)
> @@ -1039,14 +1180,45 @@ static void omap3_enable_hwecc_bch(struct mtd_info *mtd, int mode)
>
> nerrors = info->nand.ecc.strength;
> dev_width = (chip->options & NAND_BUSWIDTH_16) ? 1 : 0;
> +#ifdef CONFIG_MTD_NAND_OMAP_BCH
> + if (info->is_elm_used) {
> + /*
> + * Program GPMC to perform correction on (steps * 512) byte
> + * sector at a time.
> + */
> + gpmc_enable_hwecc_bch(info->gpmc_cs, mode, dev_width,
> + info->nand.ecc.steps, nerrors);
> + return;
> + }
> +#endif
> /*
> - * Program GPMC to perform correction on one 512-byte sector at a time.
> - * Using 4 sectors at a time (i.e. ecc.size = 2048) is also possible and
> - * gives a slight (5%) performance gain (but requires additional code).
> + * Program GPMC to perform correction on one 512-byte sector at
> + * a time.
Why removing the comment about 4-sector perf gain ? :-)
(...)
> @@ -1146,35 +1402,62 @@ static int omap3_init_bch(struct mtd_info *mtd, int ecc_opt)
> goto fail;
> }
>
> - /* initialize GPMC BCH engine */
> - ret = gpmc_init_hwecc_bch(info->gpmc_cs, 1, max_errors);
> - if (ret)
> - goto fail;
> -
> - /* software bch library is only used to detect and locate errors */
> - info->bch = init_bch(13, max_errors, 0x201b /* hw polynomial */);
> - if (!info->bch)
> - goto fail;
> + info->nand.ecc.size = 512;
> + info->nand.ecc.hwctl = omap3_enable_hwecc_bch;
> + info->nand.ecc.mode = NAND_ECC_HW;
> + info->nand.ecc.strength = hw_errors;
>
> - info->nand.ecc.size = 512;
> - info->nand.ecc.hwctl = omap3_enable_hwecc_bch;
> - info->nand.ecc.correct = omap3_correct_data_bch;
> - info->nand.ecc.mode = NAND_ECC_HW;
> + if (info->is_elm_used && (mtd->writesize <= 4096)) {
> + enum bch_ecc bch_type;
>
> - /*
> - * The number of corrected errors in an ecc block that will trigger
> - * block scrubbing defaults to the ecc strength (4 or 8).
> - * Set mtd->bitflip_threshold here to define a custom threshold.
> - */
> + if (hw_errors == BCH8_MAX_ERROR) {
> + bch_type = BCH8_ECC;
> + info->nand.ecc.bytes = BCH8_SIZE;
> + } else {
> + bch_type = BCH4_ECC;
> + info->nand.ecc.bytes = BCH4_SIZE;
> + }
>
> - if (max_errors == 8) {
> - info->nand.ecc.strength = 8;
> - info->nand.ecc.bytes = 13;
> - info->nand.ecc.calculate = omap3_calculate_ecc_bch8;
> + info->nand.ecc.correct = omap_elm_correct_data;
> + info->nand.ecc.calculate = omap3_calculate_ecc_bch;
> + info->nand.ecc.read_page = omap_read_page_bch;
> + info->nand.ecc.write_page = omap_write_page_bch;
> + info->elm_dev = elm_request(bch_type);
> + if (!info->elm_dev) {
> + pr_err("Request to elm module failed\n");
> + goto fail;
> + }
> } else {
> - info->nand.ecc.strength = 4;
> - info->nand.ecc.bytes = 7;
> - info->nand.ecc.calculate = omap3_calculate_ecc_bch4;
> +
> + /* initialize GPMC BCH engine */
> + ret = gpmc_init_hwecc_bch(info->gpmc_cs, 1, max_errors);
> + if (ret)
> + goto fail;
> +
> + /*
> + * software bch library is only used to detect and
> + * locateerrors
s/locateerrors/locate errors/
BTW, did you check that your patch does not break the software BCH case (i.e. no ELM) ?
BR,
--
Ivan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists