From patchwork Mon Aug 29 03:58:13 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "RogerCC.Lin" X-Patchwork-Id: 9303043 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 77896601C0 for ; Mon, 29 Aug 2016 03:58:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6207328799 for ; Mon, 29 Aug 2016 03:58:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 569CD287A1; Mon, 29 Aug 2016 03:58:48 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 1DBE428799 for ; Mon, 29 Aug 2016 03:58:45 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1beDie-0005aA-1P; Mon, 29 Aug 2016 03:58:44 +0000 Received: from [210.61.82.184] (helo=mailgw02.mediatek.com) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1beDib-00053r-5A; Mon, 29 Aug 2016 03:58:42 +0000 Received: from mtkhts09.mediatek.inc [(172.21.101.70)] by mailgw02.mediatek.com (envelope-from ) (mhqrelay.mediatek.com ESMTP with TLS) with ESMTP id 579656537; Mon, 29 Aug 2016 11:58:14 +0800 Received: from [172.21.77.33] (172.21.77.33) by mtkhts09.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 14.3.266.1; Mon, 29 Aug 2016 11:58:13 +0800 Message-ID: <1472443093.27061.4.camel@mtkswgap22> Subject: [Patch] mtd: fix writing incorrect ECC parity data in OOB region. From: mtk04561 To: Date: Mon, 29 Aug 2016 11:58:13 +0800 X-Mailer: Evolution 3.2.3-0ubuntu6 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160828_205841_549292_894FA10D X-CRM114-Status: GOOD ( 13.25 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: robh@kernel.org, daniel.thompson@linaro.org, steven.liu@mediatek.com, linux-mtd@lists.infradead.org, matthias.bgg@gmail.com, linux-mediatek@lists.infradead.org, xiaolei.li@mediatek.com, computersforpeace@gmail.com, dwmw2@infradead.org, blogic@openwrt.org Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+patchwork-linux-mediatek=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Boris, I found a problem in current Mediatek upstream NAND driver (of nand/next branch). When testing the upstream driver with UBIFS file system, it has some chance to create incorrect ECC data in the second subpage, this makes UBIFS failed to mount. Below patch fixes this problem. Please help to review the patch and upstream. Thank you. Best regards, Roger Description: When mtk_ecc_encode() is writing the ECC parity data to the OOB region, because each register is 4 bytes in length, but the len's unit is in bytes, the operation in the for loop will cross the ECC's boundary. And when mtk_nfc_do_write_page() comparing the sector number, because the sector number field is at the 12th-bit position of NFI_BYTELEN register, the masked register should be shifted 12 bits before being compared. The result of this bug may cause the second subpage has incomplete ECC parity bytes. Test: The patch passed the test of UBIFS file-system read/write on Mediatek's RFB. The tested driver is checked-out from LEDE OpenWRT project's upstream driver, which is pretty much same as nand/next branch upstream driver(git clone https://git.lede-project.org/source.git). Signed-off-by: RogerCC Lin --- if (ret) dev_err(dev, "hwecc write timeout\n"); @@ -902,8 +902,8 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, dev_warn(nfc->dev, "read ahb/dma done timeout\n"); rc = readl_poll_timeout_atomic(nfc->regs + NFI_BYTELEN, reg, - (reg & CNTR_MASK) >= sectors, 10, - MTK_TIMEOUT); + ((reg & CNTR_MASK) >> 12) >= sectors, + 10, MTK_TIMEOUT); if (rc < 0) { dev_err(nfc->dev, "subpage done timeout\n"); bitflips = -EIO; diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c index 25a4fbd..0a7ce8d 100644 --- a/drivers/mtd/nand/mtk_ecc.c +++ b/drivers/mtd/nand/mtk_ecc.c @@ -366,7 +366,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config, u8 *data, u32 bytes) { dma_addr_t addr; - u32 *p, len, i; + u8 *p; + u32 len, i, val; int ret = 0; addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE); @@ -395,8 +396,11 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config, p = (u32 *)(data + bytes); /* write the parity bytes generated by the ECC back to the OOB region */ - for (i = 0; i < len; i++) - p[i] = readl(ecc->regs + ECC_ENCPAR(i)); + for (i = 0; i < len; i++) { + if ((i % 4) == 0) + val = readl(ecc->regs + ECC_ENCPAR(i >> 2)); + p[i] = *((u8 *)&val + (i % 4)); + } timeout: dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE); diff --git a/drivers/mtd/nand/mtk_nand.c b/drivers/mtd/nand/mtk_nand.c index ddaa2ac..9c49121 100644 --- a/drivers/mtd/nand/mtk_nand.c +++ b/drivers/mtd/nand/mtk_nand.c @@ -699,8 +699,8 @@ static int mtk_nfc_do_write_page(struct mtd_info *mtd, struct nand_chip *chip, } ret = readl_poll_timeout_atomic(nfc->regs + NFI_ADDRCNTR, reg, - (reg & CNTR_MASK) >= chip->ecc.steps, - 10, MTK_TIMEOUT); + ((reg & CNTR_MASK) >> 12) >= + chip->ecc.steps, 10, MTK_TIMEOUT);