From patchwork Fri Mar 15 16:17:58 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Mosberger-Tang X-Patchwork-Id: 228084 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from merlin.infradead.org (merlin.infradead.org [IPv6:2001:4978:20e::2]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id EB0BF2C00E2 for ; Sat, 16 Mar 2013 03:19:26 +1100 (EST) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UGXKV-0000T4-3H; Fri, 15 Mar 2013 16:18:03 +0000 Received: from mail-qe0-f47.google.com ([209.85.128.47]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UGXKS-0000Sh-Dl for linux-mtd@lists.infradead.org; Fri, 15 Mar 2013 16:18:01 +0000 Received: by mail-qe0-f47.google.com with SMTP id q19so2033555qeb.34 for ; Fri, 15 Mar 2013 09:17:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:content-type; bh=BcXui8avfLK8IY+ze/k5EaShf7faHUw+KbRJbbQ39v8=; b=ar7pD8x3PSFvasKI8rtpLemeU2v/rrJFb22Ve/8JkY0U/68hWEb3Pk8SfJM0wPW8eC MMdD/4njkRHeK6Fxgl2u6lBaA8F9OAmO0++6hG56KBC3H6v46vJhWQGTTG2g3Xg02pI2 0kMgSLLyJ9PPaluoVfpkFk/68zMfpkWHfEfMELyioHv5PaJbnVhFC+TxY5Qv2HXyHgLz cjBZV9p7o/n+86rjep+LxJdRvLeWzq3lm4qDmDPAXvaEfN/jQ+uZXcSRrf9kmQPI4+PE 16LOJxT5C3FXU9gV5RfM6Ef9Ogtm5WLcFc/6K0WenmkqkxU49cKU9SgT391gEM9lEmqW xuCA== MIME-Version: 1.0 X-Received: by 10.229.106.27 with SMTP id v27mr1408701qco.8.1363364278375; Fri, 15 Mar 2013 09:17:58 -0700 (PDT) Received: by 10.49.63.66 with HTTP; Fri, 15 Mar 2013 09:17:58 -0700 (PDT) In-Reply-To: References: Date: Fri, 15 Mar 2013 10:17:58 -0600 Message-ID: Subject: Re: on-die ECC support From: David Mosberger-Tang To: linux-mtd@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130315_121800_564720_9A07157B X-CRM114-Status: GOOD ( 27.09 ) X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (dmosberger[at]gmail.com) -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.128.47 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org Hopefully this attachment comes through... On Fri, Mar 15, 2013 at 10:08 AM, David Mosberger-Tang wrote: > We need to be able to support 4-bit-correcting ECC with a > micro-controller that doesn't have hardware-support for that. We are > planning to use the on-die ECC controller supported by Micron flash > chips. I suppose we could use the BCH swecc support in the Linux > kernel, but I'm concerned about the performance implication of that > and we'd also have to add BCH ecc to our bootloader, which would mean > more development and testing. > > For backwards-compatibility, we need to be able to support a > subpage-size of 512 bytes. From the Micron data sheets, it's not > clear to me whether the on-die controller really supports reading 512 > byte subpages but the attached patch certainly *seems* to work fine so > far. > > I'd love to get some feedback as to whether I'm on the right track > here or whether I should pursue a different path. The patch does the > following: > > - Add NAND_ECC_HW_ON_DIE ecc mode > - Add nand_read_subpage_raw() to read a subpage without worrying about ECC > - Hack atmel_nand.c to use on-die ECC. > > Thanks, > > --david diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c index 92623ac..8043fde 100644 --- a/drivers/mtd/nand/atmel_nand.c +++ b/drivers/mtd/nand/atmel_nand.c @@ -1462,7 +1462,12 @@ static int __init atmel_nand_probe(struct platform_device *pdev) } nand_chip->ecc.mode = host->board.ecc_mode; +nand_chip->ecc.mode = NAND_ECC_HW_ON_DIE; // XXX +nand_chip->ecc.size = 512; // XXX +nand_chip->ecc.strength = 4; // XXX +printk("nand_chip->ecc.mode=%d\n", nand_chip->ecc.mode); nand_chip->chip_delay = 20; /* 20us command delay time */ + nand_chip->chip_delay = 70; // XXX t_R_ECC if (host->board.bus_width_16) /* 16-bit bus width */ nand_chip->options |= NAND_BUSWIDTH_16; diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 1a03b7f..88ee536 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -711,6 +711,29 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command, chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE); + if (1) { + u8 status; + + udelay(chip->chip_delay); + + chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1); + status = chip->read_byte(mtd); + + if (status & 0x01) { + printk("ERROR: on-die ECC read failure\n"); + } else if (status & 0x08) { + // bit flip + printk("ERROR: on-die ECC rewrite recommended\n" +); + } + + chip->cmd_ctrl(mtd, NAND_CMD_READ0, + NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE); + chip->cmd_ctrl(mtd, NAND_CMD_NONE, + NAND_NCE | NAND_CTRL_CHANGE); + return; + } + /* This applies to read commands */ default: /* @@ -1222,6 +1245,42 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, } /** + * nand_read_subpage_raw - [REPLACEABLE] raw sub-page read function + * @mtd: mtd info structure + * @chip: nand chip info structure + * @data_offs: offset of requested data within the page + * @readlen: data length + * @bufpoi: buffer to store read data + */ +static int nand_read_subpage_raw(struct mtd_info *mtd, struct nand_chip *chip, + uint32_t data_offs, uint32_t readlen, uint8_t *bufpoi) +{ + int start_step, end_step, num_steps; + uint8_t *p; + int data_col_addr; + int datafrag_len; + unsigned int max_bitflips = 0; + + /* Column address within the page aligned to ECC size */ + start_step = data_offs / chip->ecc.size; + end_step = (data_offs + readlen - 1) / chip->ecc.size; + num_steps = end_step - start_step + 1; + + /* Data size aligned to ECC ecc.size */ + datafrag_len = num_steps * chip->ecc.size; + + data_col_addr = start_step * chip->ecc.size; + /* If we read not a page aligned data */ + if (data_col_addr != 0) + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_col_addr, -1); + + p = bufpoi + data_col_addr; + chip->read_buf(mtd, p, datafrag_len); + + return max_bitflips; +} + +/** * nand_read_page_hwecc - [REPLACEABLE] hardware ECC based page read function * @mtd: mtd info structure * @chip: nand chip info structure @@ -3530,6 +3589,21 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.bytes * 8 / fls(8 * chip->ecc.size); break; + case NAND_ECC_HW_ON_DIE: + chip->ecc.read_page = nand_read_page_raw; + chip->ecc.read_subpage = nand_read_subpage_raw; + chip->ecc.write_page = nand_write_page_raw; + chip->ecc.read_oob = nand_read_oob_std; + chip->ecc.read_page_raw = nand_read_page_raw; + chip->ecc.write_page_raw = nand_write_page_raw; + chip->ecc.write_oob = nand_write_oob_std; + chip->ecc.bytes = 0; +/* + chip->ecc.size = mtd->writesize; + chip->ecc.strength = 0; +*/ + break; + case NAND_ECC_NONE: pr_warn("NAND_ECC_NONE selected by board driver. " "This is not recommended!\n"); @@ -3591,6 +3665,7 @@ int nand_scan_tail(struct mtd_info *mtd) break; } } +mtd->subpage_sft = 2; // XXX chip->subpagesize = mtd->writesize >> mtd->subpage_sft; /* Initialize state */ @@ -3603,7 +3678,9 @@ int nand_scan_tail(struct mtd_info *mtd) chip->pagebuf = -1; /* Large page NAND with SOFT_ECC should support subpage reads */ - if ((chip->ecc.mode == NAND_ECC_SOFT) && (chip->page_shift > 9)) + if (((chip->ecc.mode == NAND_ECC_SOFT) + || (chip->ecc.mode == NAND_ECC_HW_ON_DIE)) + && (chip->page_shift > 9)) chip->options |= NAND_SUBPAGE_READ; /* Fill in remaining MTD driver data */ diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c index a27ec94..79ea026 100644 --- a/drivers/of/of_mtd.c +++ b/drivers/of/of_mtd.c @@ -23,6 +23,7 @@ static const char *nand_ecc_modes[] = { [NAND_ECC_HW_SYNDROME] = "hw_syndrome", [NAND_ECC_HW_OOB_FIRST] = "hw_oob_first", [NAND_ECC_SOFT_BCH] = "soft_bch", + [NAND_ECC_HW_ON_DIE] = "hw_on_die", }; /** diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 24e9159..2c62b2a 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -143,6 +143,7 @@ typedef enum { NAND_ECC_HW_SYNDROME, NAND_ECC_HW_OOB_FIRST, NAND_ECC_SOFT_BCH, + NAND_ECC_HW_ON_DIE, } nand_ecc_modes_t; /*