| Message ID | 1346369978-28137-1-git-send-email-marex@denx.de |
|---|---|
| State | Accepted |
| Commit | 9c790a748f6b61e36f7aaf0c8d5ed35c9b09f454 |
| Delegated to: | Scott Wood |
| Headers | show |
Dear Marek Vasut, > This is based on Linux kernel -next: > > commit a1256b0e087ed3cdb584c683acb966ee885f733c > Author: Brian Norris <computersforpeace@gmail.com> > 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 <marex@denx.de> > Cc: Brian Norris <computersforpeace@gmail.com> > Cc: Eric Nelson <eric.nelson@boundarydevices.com> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: Otavio Salvador <otavio@ossystems.com.br> > Cc: Scott Wood <scottwood@freescale.com> > --- > 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! > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index bfd668f..658b3ad 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -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. > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 82704de..0a5aa64 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -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
On 31/08/2012 01:39, Marek Vasut wrote: > This is based on Linux kernel -next: > > commit a1256b0e087ed3cdb584c683acb966ee885f733c > Author: Brian Norris <computersforpeace@gmail.com> > 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 <marex@denx.de> > Cc: Brian Norris <computersforpeace@gmail.com> > Cc: Eric Nelson <eric.nelson@boundarydevices.com> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: Otavio Salvador <otavio@ossystems.com.br> > Cc: Scott Wood <scottwood@freescale.com> > --- Hi Marek, > 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. > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index bfd668f..658b3ad 100644 This fixes the same issue on the ea20 (davinci), and drop the painful NAND_CHIPOPTIONS_MSK. Thanks ! Regards, Stefano
Dear Stefano Babic, > On 31/08/2012 01:39, Marek Vasut wrote: > > This is based on Linux kernel -next: > > > > commit a1256b0e087ed3cdb584c683acb966ee885f733c > > Author: Brian Norris <computersforpeace@gmail.com> > > 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 <marex@denx.de> > > Cc: Brian Norris <computersforpeace@gmail.com> > > Cc: Eric Nelson <eric.nelson@boundarydevices.com> > > Cc: Fabio Estevam <festevam@gmail.com> > > Cc: Otavio Salvador <otavio@ossystems.com.br> > > Cc: Scott Wood <scottwood@freescale.com> > > --- > > Hi Marek, > > > 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. > > > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > > 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. > Regards, > Stefano Best regards, Marek Vasut
On Thu, Aug 30, 2012 at 01:39:38PM -0000, Marek Vasut wrote: > This is based on Linux kernel -next: > > commit a1256b0e087ed3cdb584c683acb966ee885f733c > Author: Brian Norris <computersforpeace@gmail.com> > 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 <marex@denx.de> > Cc: Brian Norris <computersforpeace@gmail.com> > Cc: Eric Nelson <eric.nelson@boundarydevices.com> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: Otavio Salvador <otavio@ossystems.com.br> > Cc: Scott Wood <scottwood@freescale.com> Applied to u-boot-nand-flash with the SHA1 updated based on current linux-next: 14f44abf1dafc20ba42ce8616a8fc8fbd1b3712b -Scott
Dear Scott Wood, > On Thu, Aug 30, 2012 at 01:39:38PM -0000, Marek Vasut wrote: > > This is based on Linux kernel -next: > > > > commit a1256b0e087ed3cdb584c683acb966ee885f733c > > Author: Brian Norris <computersforpeace@gmail.com> > > 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 <marex@denx.de> > > Cc: Brian Norris <computersforpeace@gmail.com> > > Cc: Eric Nelson <eric.nelson@boundarydevices.com> > > Cc: Fabio Estevam <festevam@gmail.com> > > Cc: Otavio Salvador <otavio@ossystems.com.br> > > Cc: Scott Wood <scottwood@freescale.com> > > 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
On 09/17/2012 07:01:39 PM, Marek Vasut wrote: > Dear Scott Wood, > > > On Thu, Aug 30, 2012 at 01:39:38PM -0000, Marek Vasut wrote: > > > This is based on Linux kernel -next: > > > > > > commit a1256b0e087ed3cdb584c683acb966ee885f733c > > > Author: Brian Norris <computersforpeace@gmail.com> > > > 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 <marex@denx.de> > > > Cc: Brian Norris <computersforpeace@gmail.com> > > > Cc: Eric Nelson <eric.nelson@boundarydevices.com> > > > Cc: Fabio Estevam <festevam@gmail.com> > > > Cc: Otavio Salvador <otavio@ossystems.com.br> > > > Cc: Scott Wood <scottwood@freescale.com> > > > > 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. > btw. this should definitelly go to current release. Yes. -Scott
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index bfd668f..658b3ad 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -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. diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 82704de..0a5aa64 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -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.
This is based on Linux kernel -next: commit a1256b0e087ed3cdb584c683acb966ee885f733c Author: Brian Norris <computersforpeace@gmail.com> 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 <marex@denx.de> Cc: Brian Norris <computersforpeace@gmail.com> Cc: Eric Nelson <eric.nelson@boundarydevices.com> Cc: Fabio Estevam <festevam@gmail.com> Cc: Otavio Salvador <otavio@ossystems.com.br> Cc: Scott Wood <scottwood@freescale.com> --- 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.