Skip to contentSkip to conversation

mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

PATCH In review Marek Vasut 3 participants 7 messages Aug 30, 2012 – Sep 18, 2012

Subsystems (2)
Memory technology devices (MTD) NAND flash
lore
Share & tools
lore

Marek Vasut

author
#0 lore raw

PATCH 1/1 mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

This is based on Linux kernel -next:

commit a1256b0e087ed3cdb584c683acb966ee885f733c

Author: Brian Norris <[email protected]>
Date:   Fri Jul 13 09:28:24 2012 -0700

mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

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; it was only here to prevent certain options from accidentally being set by the driver. But the original thought turns out to be a bad idea occasionally. Thus, kill it.

Note, this patch fixes some major gpmi-nand breakage.

Signed-off-by Marek Vasut <[email protected]>
Cc Brian Norris <[email protected]>
Cc Eric Nelson <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Otavio Salvador <[email protected]>
Cc Scott Wood <[email protected]>
---
 drivers/mtd/nand/nand_base.c |    7 ++-----
 include/linux/mtd/nand.h     |    3 ---
 2 files changed, 2 insertions(+), 8 deletions(-)

NOTE: Please test before applying, I'd like to know it doesn't cause breakage.

drivers/mtd/nand/nand_base.c +2 -5
index bfd668f..658b3ad 100644
@@ -2578,9 +2578,7 @@ 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;
-	chip->options |= (NAND_NO_READRDY |
-			NAND_NO_AUTOINCR) & NAND_CHIPOPTIONS_MSK;
+	chip->options |= NAND_NO_READRDY | NAND_NO_AUTOINCR;
 
 	return 1;
 }

@@ -2752,8 +2750,7 @@ static const struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 		}
 	}
 	/* Get chip options, preserve non chip based options */
-	chip->options &= ~NAND_CHIPOPTIONS_MSK;
-	chip->options |= type->options & NAND_CHIPOPTIONS_MSK;
+	chip->options |= type->options;
 
 	/* Check if chip is a not a samsung device. Do not clear the
 	 * options for chips which are not having an extended id.
include/linux/mtd/nand.h +0 -3
index 82704de..0a5aa64 100644
@@ -205,9 +205,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 & ~NAND_NO_AUTOINCR)
-
 /* Non chip related options */
 /*
  * Use a flash based bad block table. OOB identifier is saved in OOB area.
Signature · 1 line
1.7.10.4
author
#1 lore raw

PATCH 1/1 mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

Dear Marek Vasut,

Quoted 27 lines

This is based on Linux kernel -next:

commit a1256b0e087ed3cdb584c683acb966ee885f733c

Author: Brian Norris <[email protected]>
Date:   Fri Jul 13 09:28:24 2012 -0700

mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

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; it was only here to prevent certain options from accidentally being set by the driver. But
the original thought turns out to be a bad idea occasionally. Thus, kill it.

Note, this patch fixes some major gpmi-nand breakage.

Signed-off-by Marek Vasut <[email protected]>
Cc Brian Norris <[email protected]>
Cc Eric Nelson <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Otavio Salvador <[email protected]>
Cc Scott Wood <[email protected]>
Quoted 7 lines
---
 drivers/mtd/nand/nand_base.c |    7 ++-----
 include/linux/mtd/nand.h     |    3 ---
 2 files changed, 2 insertions(+), 8 deletions(-)

NOTE: Please test before applying, I'd like to know it doesn't cause breakage.

NOTE2: This fixes UBI on MX28, MX6Q and soon-to-be MX233 GPMI NAND. Thanks again, Brian!

Quoted 30 lines
drivers/mtd/nand/nand_base.c +2 -5
index bfd668f..658b3ad 100644
@@ -2578,9 +2578,7 @@ 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;
-	chip->options |= (NAND_NO_READRDY |
-			NAND_NO_AUTOINCR) & NAND_CHIPOPTIONS_MSK;
+	chip->options |= NAND_NO_READRDY | NAND_NO_AUTOINCR;
 
 	return 1;
 }

@@ -2752,8 +2750,7 @@ static const struct nand_flash_dev
 *nand_get_flash_type(struct mtd_info *mtd, }
 	}
 	/* Get chip options, preserve non chip based options */
-	chip->options &= ~NAND_CHIPOPTIONS_MSK;
-	chip->options |= type->options & NAND_CHIPOPTIONS_MSK;
+	chip->options |= type->options;
 
 	/* Check if chip is a not a samsung device. Do not clear the
 	 * options for chips which are not having an extended id.
include/linux/mtd/nand.h +0 -0
index 82704de..0a5aa64 100644

include/linux/mtd/nand.h typedef enum { · line 210

Quoted 9 lines
include/linux/mtd/nand.h +0 -3
@@ -205,9 +205,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 & ~NAND_NO_AUTOINCR)
-
 /* Non chip related options */
 /*
  * Use a flash based bad block table. OOB identifier is saved in OOB area.

Best regards,
Marek Vasut

#2 lore raw

PATCH 1/1 mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

In reply to Marek Vasut
Quoted 20 lines · Marek Vasut

This is based on Linux kernel -next:

commit a1256b0e087ed3cdb584c683acb966ee885f733c

Author: Brian Norris <[email protected]>
Date:   Fri Jul 13 09:28:24 2012 -0700

mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

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; it was only here to prevent certain options from accidentally being set by the driver. But the original thought turns out to be a bad idea occasionally. Thus, kill it.

Note, this patch fixes some major gpmi-nand breakage.

Quoted 7 lines
Signed-off-by Marek Vasut <[email protected]>
Cc Brian Norris <[email protected]>
Cc Eric Nelson <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Otavio Salvador <[email protected]>
Cc Scott Wood <[email protected]>

---

Hi Marek,

drivers/mtd/nand/nand_base.c

Quoted 8 lines
 drivers/mtd/nand/nand_base.c |    7 ++-----
 include/linux/mtd/nand.h     |    3 ---
 2 files changed, 2 insertions(+), 8 deletions(-)

NOTE: Please test before applying, I'd like to know it doesn't cause breakage.

drivers/mtd/nand/nand_base.c +0 -0
index bfd668f..658b3ad 100644

This fixes the same issue on the ea20 (davinci), and drop the painful NAND_CHIPOPTIONS_MSK. Thanks !

Regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
author
#3 lore raw

PATCH 1/1 mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

Dear Stefano Babic,

Quoted 36 lines · Marek Vasut
Quoted 21 lines

This is based on Linux kernel -next:

commit a1256b0e087ed3cdb584c683acb966ee885f733c

Author: Brian Norris <[email protected]>
Date:   Fri Jul 13 09:28:24 2012 -0700

mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

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; it was only here to prevent certain options from accidentally being set by the driver. But the original thought turns out to be a bad idea occasionally. Thus, kill it.

Note, this patch fixes some major gpmi-nand breakage.

Quoted 7 lines
Signed-off-by Marek Vasut <[email protected]>
Cc Brian Norris <[email protected]>
Cc Eric Nelson <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Otavio Salvador <[email protected]>
Cc Scott Wood <[email protected]>

---

Hi Marek,

Quoted 4 lines
 drivers/mtd/nand/nand_base.c |    7 ++-----
 include/linux/mtd/nand.h     |    3 ---
 2 files changed, 2 insertions(+), 8 deletions(-)
Quoted 8 lines

drivers/mtd/nand/nand_base.c

Quoted 5 lines

NOTE: Please test before applying, I'd like to know it doesn't cause breakage.

drivers/mtd/nand/nand_base.c +0 -0
index bfd668f..658b3ad 100644

This fixes the same issue on the ea20 (davinci), and drop the painful NAND_CHIPOPTIONS_MSK. Thanks !

I'm glad. I hope I didn't infringe anything with this patch and that I properly followed this "porting Linux patches" procedure.

Quoted 2 lines

Regards,
Stefano

Best regards,
Marek Vasut

#4 lore raw
In reply to Marek Vasut
Quoted 18 lines · Marek Vasut

This is based on Linux kernel -next:

commit a1256b0e087ed3cdb584c683acb966ee885f733c

Author: Brian Norris <[email protected]>
Date:   Fri Jul 13 09:28:24 2012 -0700

mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

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; it was only here to prevent certain options from accidentally being set by the driver. But the original thought turns out to be a bad idea occasionally. Thus, kill it.

Quoted 8 lines

Note, this patch fixes some major gpmi-nand breakage.

Signed-off-by Marek Vasut <[email protected]>
Cc Brian Norris <[email protected]>
Cc Eric Nelson <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Otavio Salvador <[email protected]>
Cc Scott Wood <[email protected]>

Applied to u-boot-nand-flash with the SHA1 updated based on current
linux-next: 14f44abf1dafc20ba42ce8616a8fc8fbd1b3712b

-Scott

author
#5 lore raw

Dear Scott Wood,

Quoted 29 lines · Marek Vasut
Quoted 19 lines

This is based on Linux kernel -next:

commit a1256b0e087ed3cdb584c683acb966ee885f733c

Author: Brian Norris <[email protected]>
Date:   Fri Jul 13 09:28:24 2012 -0700

mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

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; it was only here to prevent certain options from accidentally being set by the driver. But the original thought turns out to be a bad idea occasionally. Thus, kill it.

Quoted 8 lines

Note, this patch fixes some major gpmi-nand breakage.

Signed-off-by Marek Vasut <[email protected]>
Cc Brian Norris <[email protected]>
Cc Eric Nelson <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Otavio Salvador <[email protected]>
Cc Scott Wood <[email protected]>
Quoted 2 lines

Applied to u-boot-nand-flash with the SHA1 updated based on current
linux-next: 14f44abf1dafc20ba42ce8616a8fc8fbd1b3712b

Thanks ... I think the sha1 is irrelevant as next is being constantly rebased anyway :/

btw. this should definitelly go to current release.

Best regards,
Marek Vasut

#6 lore raw
Quoted 37 lines · Marek Vasut

Dear Scott Wood,

Quoted 10 lines · Marek Vasut
Quoted 9 lines

This is based on Linux kernel -next:

commit a1256b0e087ed3cdb584c683acb966ee885f733c

Author: Brian Norris <[email protected]>
Date:   Fri Jul 13 09:28:24 2012 -0700

mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

The NAND_CHIPOPTIONS_MSK has limited utility and is causing

real

Quoted 1 line
Quoted 1 line

bugs. It silently masks off at least one flag that might be

set by

Quoted 1 line
Quoted 1 line

the driver (NAND_NO_SUBPAGE_WRITE). This breaks the GPMI NAND

driver

Quoted 3 lines
Quoted 3 lines

and possibly others.

Really, as long as driver writers exercise a small amount of

care

Quoted 1 line
Quoted 1 line

with NAND_* options, this mask is not necessary at all; it

was only

Quoted 1 line
Quoted 1 line

here to prevent certain options from accidentally being set

by the

Quoted 12 lines
Quoted 11 lines

driver. But the original thought turns out to be a bad idea occasionally. Thus, kill it.

Note, this patch fixes some major gpmi-nand breakage.

Signed-off-by Marek Vasut <[email protected]>
Cc Brian Norris <[email protected]>
Cc Eric Nelson <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Otavio Salvador <[email protected]>
Cc Scott Wood <[email protected]>
Quoted 6 lines
Quoted 2 lines

Applied to u-boot-nand-flash with the SHA1 updated based on current
linux-next: 14f44abf1dafc20ba42ce8616a8fc8fbd1b3712b

Thanks ... I think the sha1 is irrelevant as next is being constantly rebased
anyway :/

They're at -rc6 or so, so maybe this'll be the final one. :-)

The SHA1 should only change if the tree it was pulled from got rebased.

Quoted 1 line

btw. this should definitelly go to current release.

Yes.

-Scott