[U-Boot] [PATCH] nand: reinstate lazy bad block scanning

[U-Boot] [PATCH] nand: reinstate lazy bad block scanning

[Rostislav Lisovy]

Commit ff94bc40af3481d47546595ba73c136de6af6929
("mtd, ubi, ubifs: resync with Linux-3.14")
accidentally reverted part of the commit
13f0fd94e3cae6f8a0d9fba5d367e311edc8ebde
("NAND: Scan bad blocks lazily.").

Reinstate the change as by commit
fb49454b1b6c7c6e238ac3c0b1e302e73eb1a1ea
("nand: reinstate lazy bad block scanning")

Signed-off-by: Rostislav Lisovy <lisovy@merica.cz>
---
 drivers/mtd/nand/nand_base.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 0b6e7ee..70e780c 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -634,6 +634,11 @@ static int nand_block_checkbad(struct mtd_info *mtd, loff_t ofs, int getchip,
 {
 	struct nand_chip *chip = mtd->priv;
 
+	if (!(chip->options & NAND_BBT_SCANNED)) {
+		chip->scan_bbt(mtd);
+		chip->options |= NAND_BBT_SCANNED;
+	}
+
 	if (!chip->bbt)
 		return chip->block_bad(mtd, ofs, getchip);
 
@@ -4322,10 +4327,9 @@ int nand_scan_tail(struct mtd_info *mtd)
 
 	/* Check, if we should skip the bad block table scan */
 	if (chip->options & NAND_SKIP_BBTSCAN)
-		return 0;
+		chip->options |= NAND_BBT_SCANNED;
 
-	/* Build bad block table */
-	return chip->scan_bbt(mtd);
+	return 0;
 }
 EXPORT_SYMBOL(nand_scan_tail);
 
-- 
1.9.1

[U-Boot] [PATCH] nand: reinstate lazy bad block scanning

[Scott Wood]

On Wed, 2014-10-22 at 13:40 +0200, Rostislav Lisovy wrote:
> Commit ff94bc40af3481d47546595ba73c136de6af6929
> ("mtd, ubi, ubifs: resync with Linux-3.14")
> accidentally reverted part of the commit
> 13f0fd94e3cae6f8a0d9fba5d367e311edc8ebde
> ("NAND: Scan bad blocks lazily.").
> 
> Reinstate the change as by commit
> fb49454b1b6c7c6e238ac3c0b1e302e73eb1a1ea
> ("nand: reinstate lazy bad block scanning")
> 
> Signed-off-by: Rostislav Lisovy <lisovy@merica.cz>
> ---
>  drivers/mtd/nand/nand_base.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

Thanks for catching this.  

Heiko, this is the sort of thing I was concerned about with the "resync
from scratch" approach.

-Scott

[U-Boot] [PATCH] nand: reinstate lazy bad block scanning

[Masahiro Yamada]

Hi Scott, Rostislav,

On Mon, 3 Nov 2014 15:42:29 -0600
Scott Wood <scottwood@freescale.com> wrote:

> On Wed, 2014-10-22 at 13:40 +0200, Rostislav Lisovy wrote:
> > Commit ff94bc40af3481d47546595ba73c136de6af6929
> > ("mtd, ubi, ubifs: resync with Linux-3.14")
> > accidentally reverted part of the commit
> > 13f0fd94e3cae6f8a0d9fba5d367e311edc8ebde
> > ("NAND: Scan bad blocks lazily.").
> > 
> > Reinstate the change as by commit
> > fb49454b1b6c7c6e238ac3c0b1e302e73eb1a1ea
> > ("nand: reinstate lazy bad block scanning")
> > 
> > Signed-off-by: Rostislav Lisovy <lisovy@merica.cz>
> > ---
> >  drivers/mtd/nand/nand_base.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> Thanks for catching this.  
> 
> Heiko, this is the sort of thing I was concerned about with the "resync
> from scratch" approach.
> 

I do not believe resync is a bad idea,
of course we should be very careful not to break existing features.

I recommend to surround this code with "#ifdef __UBOOT__  ... #endif"
as we have done for the other parts.



BTW, we attempt to probe NAND devices during the boot sequence
just for displaying the device size, right?

NAND:  **** MiB


If so, can we postpone the whole of nand_scan
until we use it?  I am not sure about this, though.





Best Regards
Masahiro Yamada

[U-Boot] [PATCH] nand: reinstate lazy bad block scanning

[Scott Wood]

On Wed, 2014-11-05 at 12:40 +0900, Masahiro Yamada wrote:
> Hi Scott, Rostislav,
> 
> On Mon, 3 Nov 2014 15:42:29 -0600
> Scott Wood <scottwood@freescale.com> wrote:
> 
> > On Wed, 2014-10-22 at 13:40 +0200, Rostislav Lisovy wrote:
> > > Commit ff94bc40af3481d47546595ba73c136de6af6929
> > > ("mtd, ubi, ubifs: resync with Linux-3.14")
> > > accidentally reverted part of the commit
> > > 13f0fd94e3cae6f8a0d9fba5d367e311edc8ebde
> > > ("NAND: Scan bad blocks lazily.").
> > > 
> > > Reinstate the change as by commit
> > > fb49454b1b6c7c6e238ac3c0b1e302e73eb1a1ea
> > > ("nand: reinstate lazy bad block scanning")
> > > 
> > > Signed-off-by: Rostislav Lisovy <lisovy@merica.cz>
> > > ---
> > >  drivers/mtd/nand/nand_base.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > Thanks for catching this.  
> > 
> > Heiko, this is the sort of thing I was concerned about with the "resync
> > from scratch" approach.
> > 
> 
> I do not believe resync is a bad idea,
> of course we should be very careful not to break existing features.

I'm not saying don't sync -- I'm saying do it by generating a diff based
on the last sync (which is what all the previous NAND updates did),
rather than throwing out the U-Boot code and replacing it with Linux
files, and then fixing any problems encountered (which may be a subset
of the problems existing).

> I recommend to surround this code with "#ifdef __UBOOT__  ... #endif"
> as we have done for the other parts.

I recommend not making such a mess, for reasons I've described
previously.

> BTW, we attempt to probe NAND devices during the boot sequence
> just for displaying the device size, right?
> 
> NAND:  **** MiB
> 
> 
> If so, can we postpone the whole of nand_scan
> until we use it?  I am not sure about this, though.

Possibly.  IIRC someone once posted a patch trying to do this, but it
was missing an initialization check on some paths where NAND could be
accessed, and the patchset never got respun.

-Scott

[U-Boot] [PATCH] nand: reinstate lazy bad block scanning

[Masahiro Yamada]

Hi Scott,

On Tue, 4 Nov 2014 23:49:36 -0600
Scott Wood <scottwood@freescale.com> wrote:

> On Wed, 2014-11-05 at 12:40 +0900, Masahiro Yamada wrote:
> > Hi Scott, Rostislav,
> > 
> > On Mon, 3 Nov 2014 15:42:29 -0600
> > Scott Wood <scottwood@freescale.com> wrote:
> > 
> > > On Wed, 2014-10-22 at 13:40 +0200, Rostislav Lisovy wrote:
> > > > Commit ff94bc40af3481d47546595ba73c136de6af6929
> > > > ("mtd, ubi, ubifs: resync with Linux-3.14")
> > > > accidentally reverted part of the commit
> > > > 13f0fd94e3cae6f8a0d9fba5d367e311edc8ebde
> > > > ("NAND: Scan bad blocks lazily.").
> > > > 
> > > > Reinstate the change as by commit
> > > > fb49454b1b6c7c6e238ac3c0b1e302e73eb1a1ea
> > > > ("nand: reinstate lazy bad block scanning")
> > > > 
> > > > Signed-off-by: Rostislav Lisovy <lisovy@merica.cz>
> > > > ---
> > > >  drivers/mtd/nand/nand_base.c | 10 +++++++---
> > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > Thanks for catching this.  
> > > 
> > > Heiko, this is the sort of thing I was concerned about with the "resync
> > > from scratch" approach.
> > > 
> > 
> > I do not believe resync is a bad idea,
> > of course we should be very careful not to break existing features.
> 
> I'm not saying don't sync -- I'm saying do it by generating a diff based
> on the last sync (which is what all the previous NAND updates did),
> rather than throwing out the U-Boot code and replacing it with Linux
> files, and then fixing any problems encountered (which may be a subset
> of the problems existing).
> 
> > I recommend to surround this code with "#ifdef __UBOOT__  ... #endif"
> > as we have done for the other parts.
> 
> I recommend not making such a mess, for reasons I've described
> previously.
> 
> > BTW, we attempt to probe NAND devices during the boot sequence
> > just for displaying the device size, right?
> > 
> > NAND:  **** MiB
> > 
> > 
> > If so, can we postpone the whole of nand_scan
> > until we use it?  I am not sure about this, though.
> 
> Possibly.  IIRC someone once posted a patch trying to do this, but it
> was missing an initialization check on some paths where NAND could be
> accessed, and the patchset never got respun.
> 

In Simon's driver model, devices are probed when they are actually used.
Perhaps we can change this when we introduce driver model to NAND.



Best Regards
Masahiro Yamada

[U-Boot] [PATCH] nand: reinstate lazy bad block scanning

[Heiko Schocher]

Hello Scott, Rostislav,

Am 03.11.2014 22:42, schrieb Scott Wood:
> On Wed, 2014-10-22 at 13:40 +0200, Rostislav Lisovy wrote:
>> Commit ff94bc40af3481d47546595ba73c136de6af6929
>> ("mtd, ubi, ubifs: resync with Linux-3.14")
>> accidentally reverted part of the commit
>> 13f0fd94e3cae6f8a0d9fba5d367e311edc8ebde
>> ("NAND: Scan bad blocks lazily.").
>>
>> Reinstate the change as by commit
>> fb49454b1b6c7c6e238ac3c0b1e302e73eb1a1ea
>> ("nand: reinstate lazy bad block scanning")
>>
>> Signed-off-by: Rostislav Lisovy<lisovy@merica.cz>
>> ---
>>   drivers/mtd/nand/nand_base.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> Thanks for catching this.

Yep! I tested it on the siemens am335x boards so

Acked-by: Heiko Schocher <hs@denx.de>

> Heiko, this is the sort of thing I was concerned about with the "resync
> from scratch" approach.

I know, but the rebase was also error prone (at least in my attempts ...)

As we have now a clear defined base, maybe a resync with the help from
git works now in future?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] nand: reinstate lazy bad block scanning

[Scott Wood]

commit 2a8e0fc8b3dc31a3c571e439fbf04b882c8986be ("nand: Merge changes
from Linux nand driver") accidentally reverted commit
13f0fd94e3cae6f8a0d9fba5d367e311edc8ebde ("NAND: Scan bad blocks
lazily.").

Reinstate the change, as amended by commit
ff49ea8977b56916edd5b1766d9939010e30b181 ("NAND: Mark the BBT as scanned
prior to calling scan_bbt.").

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 drivers/mtd/nand/nand_base.c |   10 +++++++---
 include/linux/mtd/nand.h     |    2 ++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 8ab8303..44f7b91 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -479,6 +479,11 @@ static int nand_block_checkbad(struct mtd_info *mtd, loff_t ofs, int getchip,
 {
 	struct nand_chip *chip = mtd->priv;
 
+	if (!(chip->options & NAND_BBT_SCANNED)) {
+		chip->options |= NAND_BBT_SCANNED;
+		chip->scan_bbt(mtd);
+	}
+
 	if (!chip->bbt)
 		return chip->block_bad(mtd, ofs, getchip);
 
@@ -3166,10 +3171,9 @@ int nand_scan_tail(struct mtd_info *mtd)
 
 	/* Check, if we should skip the bad block table scan */
 	if (chip->options & NAND_SKIP_BBTSCAN)
-		return 0;
+		chip->options |= NAND_BBT_SCANNED;
 
-	/* Build bad block table */
-	return chip->scan_bbt(mtd);
+	return 0;
 }
 
 /**
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 99668d5..da6fa18 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -232,6 +232,8 @@ typedef enum {
 #define NAND_CREATE_EMPTY_BBT		0x01000000
 
 /* Options set by nand scan */
+/* bbt has already been read */
+#define NAND_BBT_SCANNED	0x40000000
 /* Nand scan has allocated controller struct */
 #define NAND_CONTROLLER_ALLOC	0x80000000
 
-- 
1.7.7.rc3.4.g8d714

[U-Boot] [PATCH] nand: reinstate lazy bad block scanning

[Heiko Schocher]

Hello Scott,

Scott Wood wrote:
> commit 2a8e0fc8b3dc31a3c571e439fbf04b882c8986be ("nand: Merge changes
> from Linux nand driver") accidentally reverted commit
> 13f0fd94e3cae6f8a0d9fba5d367e311edc8ebde ("NAND: Scan bad blocks
> lazily.").
> 
> Reinstate the change, as amended by commit
> ff49ea8977b56916edd5b1766d9939010e30b181 ("NAND: Mark the BBT as scanned
> prior to calling scan_bbt.").
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
>  drivers/mtd/nand/nand_base.c |   10 +++++++---
>  include/linux/mtd/nand.h     |    2 ++
>  2 files changed, 9 insertions(+), 3 deletions(-)

Thanks!

Tested on the enbw_cmc board.

Acked-by: Heiko Schocher <hs@denx.de>
Tested-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany