From patchwork Tue Oct 23 20:36:54 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juha Kuikka X-Patchwork-Id: 1633451 Return-Path: X-Original-To: patchwork-linux-omap@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id AE5B83FD85 for ; Tue, 23 Oct 2012 20:36:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964782Ab2JWUg4 (ORCPT ); Tue, 23 Oct 2012 16:36:56 -0400 Received: from mail-ob0-f174.google.com ([209.85.214.174]:53882 "EHLO mail-ob0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933834Ab2JWUgz (ORCPT ); Tue, 23 Oct 2012 16:36:55 -0400 Received: by mail-ob0-f174.google.com with SMTP id uo13so3973942obb.19 for ; Tue, 23 Oct 2012 13:36:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:content-type; bh=o9A56CHPDMaNJXLuAFynCqH/RuzmjM2anpKMhz6b+84=; b=Sh8yMfcnRPgxoY9fv+qCr22jabYr0PAbZx87RLyIcoCC9WI8gIpA87Ien/2onHE6rc EIIjPO+OS+4RY8b0o5YJGZTQeoq+WbHQJAy3QZInh6eyLEoN/lFDshzbCg7k6xdMYy3f NrJdmZRoi1szpquaUztXCHO34eiKOpweUUQprlLBYn+tCrq9Z3OgeOiZCdzuqa9Hf9FW 8AA8QhIKTl/JsbzSQ6oTwNbMa8kU9qQmPvqHHHB6SEObchvUvD89tFo3VsBJrLiWu489 rfWXeQsjDeZMbrfZqtKDlDvA2L0k+7YFbvD4dL5LuZ7cXiBilxwAlTp/l5Gpduam6CHz bhcQ== MIME-Version: 1.0 Received: by 10.182.0.1 with SMTP id 1mr10980085oba.18.1351024614762; Tue, 23 Oct 2012 13:36:54 -0700 (PDT) Received: by 10.76.113.200 with HTTP; Tue, 23 Oct 2012 13:36:54 -0700 (PDT) Date: Tue, 23 Oct 2012 13:36:54 -0700 Message-ID: Subject: OMAP NAND driver issue with subpage reads From: Juha Kuikka To: "linux-omap@vger.kernel.org Mailing List" Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org Hi, There seems to be an issue in the omap nand driver (drivers/mtd/nand/omap2.c) concerning sub-page reads (visible when using 16bit LP NAND, SOFT_ECC and UBI). Problem is caused by the omap_read_buf_pref() function using 32bit reads from the pre-fetch engine. It takes care of the mis-aligned length but not a mis-aligned buffer pointer. Combined with how the nand_read_subpage() function aligns the pointer and length to NAND width (8 or 16 bits) this can lead to situation where the handling of the mis-aligned length in omap_read_buf_pref() causes the pointer to not be aligned to 32bits and the data gets corrupted in the read. This of course leds to ECC issues. The way I see is there are several ways to fix this: 1. Change nand_read_subpage() to be more strict about alignment 2. Change omap_read_buf_pref() to handle any reads (len % 4) || (buf % 4) with omap_read_bufX() completely 3. Change omap_read_buf_pref() to use ioread16_rep() since buffer and length are already aligned for us. I'm leaning towards #2 because, assuming that the nand driver cannot make assumptions of alignment, we need to be able to handle them all anyway. The common case of reading big chunks of page data would still be fast (since reads are always sub-page aligned) but the special case of reading small OOB chunks would be handled gracefully. Something like this: /* configure and start prefetch transfer */ Comments? - Juha --- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index 5c8978e..8a929cf 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -334,13 +334,12 @@ static void omap_read_buf_pref(struct mtd_info *mtd, u_char *buf, int len) u32 *p = (u32 *)buf; /* take care of subpage reads */ - if (len % 4) { + if (len % 4 || (unsigned long) buf % 4) { if (info->nand.options & NAND_BUSWIDTH_16) omap_read_buf16(mtd, buf, len % 4); else omap_read_buf8(mtd, buf, len % 4); - p = (u32 *) (buf + len % 4); - len -= len % 4; + return; }