From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from co202.xi-lite.net ([149.6.83.202]) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1Sw6xa-0001At-R1 for linux-mtd@lists.infradead.org; Tue, 31 Jul 2012 07:33:43 +0000 Message-ID: <50178A45.3000406@parrot.com> Date: Tue, 31 Jul 2012 09:33:25 +0200 From: Matthieu CASTET MIME-Version: 1.0 To: Brian Norris Subject: Re: [PATCH RESEND] mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver References: <1343696537-2564-1-git-send-email-computersforpeace@gmail.com> In-Reply-To: <1343696537-2564-1-git-send-email-computersforpeace@gmail.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 8bit Cc: Scott Wood , Marek Vasut , David Woodhouse , "linux-mtd@lists.infradead.org" , Artem Bityutskiy List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi, for ONFI flash (like this micron one) the information should be extracted form the ONFI table (programs_per_page IIRC) This should be better than relying on the SOC driver for setting this flags. Does the gpmi driver set this flag because it do not support partial write ? In this case why it doesn't set chip->ecc.steps to 1 ? Matthieu Brian Norris a écrit : > The NAND_CHIPOPTIONS_MSK has limited utility and is causing real bugs. It > silently masks off at least one flag that might be set by the driver > (NAND_NO_SUBPAGE_WRITE). This breaks the GPMI NAND driver and possibly others. > > Really, as long as driver writers exercise a small amount of care with NAND_* > options, this mask is not necessary at all. Thus, kill it. > >>>From Huang Shijie: > > "I tested this patch on imx6q-arm2 board with Micron MT29F32G08QAA. > it works fine, thanks." > > Signed-off-by: Brian Norris > Tested-by: Huang Shijie > Cc: Marek Vasut > --- > Hello Artem/David, > > GPMI NAND has needed this patch for some time, and I think it was agreed > on by a few. I'm resending to get acknowledgment from a maintainer. > > drivers/mtd/nand/nand_base.c | 7 ++----- > include/linux/mtd/nand.h | 3 --- > 2 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index ead301a..996cc48 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -2909,8 +2909,6 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip, > if (le16_to_cpu(p->features) & 1) > *busw = NAND_BUSWIDTH_16; > > - chip->options &= ~NAND_CHIPOPTIONS_MSK; > - > pr_info("ONFI flash detected\n"); > return 1; > } > @@ -3074,9 +3072,8 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd, > mtd->erasesize <<= ((id_data[3] & 0x03) << 1); > } > } > - /* Get chip options, preserve non chip based options */ > - chip->options &= ~NAND_CHIPOPTIONS_MSK; > - chip->options |= type->options & NAND_CHIPOPTIONS_MSK; > + /* Get chip options */ > + chip->options |= type->options; > > /* > * Check if chip is not a Samsung device. Do not clear the > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 6dce5a7..eeb7015 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -206,9 +206,6 @@ typedef enum { > #define NAND_SUBPAGE_READ(chip) ((chip->ecc.mode == NAND_ECC_SOFT) \ > && (chip->page_shift > 9)) > > -/* Mask to zero out the chip options, which come from the id table */ > -#define NAND_CHIPOPTIONS_MSK 0x0000ffff > - > /* Non chip related options */ > /* This option skips the bbt scan during initialization. */ > #define NAND_SKIP_BBTSCAN 0x00010000