All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] pcm058: fix NAND flash not using badblock table
@ 2018-12-05 12:29 Harald Seiler
  2018-12-07  8:45 ` Stefano Babic
  0 siblings, 1 reply; 9+ messages in thread
From: Harald Seiler @ 2018-12-05 12:29 UTC (permalink / raw)
  To: u-boot

Without setting this flag, U-Boot will silently use a memory
only badblock table.  This is not only bad because it means badblock
info is ignored, but also leads to other issues if a driver ontop
then tries to read the badblock table as filesystem info (eg. UBI).

Signed-off-by: Harald Seiler <hws@denx.de>
---
 include/configs/pcm058.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/configs/pcm058.h b/include/configs/pcm058.h
index 49048c163f..b9bc08b388 100644
--- a/include/configs/pcm058.h
+++ b/include/configs/pcm058.h
@@ -55,6 +55,7 @@
 #define CONFIG_SYS_NAND_BASE		0x40000000
 #define CONFIG_SYS_NAND_5_ADDR_CYCLE
 #define CONFIG_SYS_NAND_ONFI_DETECTION
+#define CONFIG_SYS_NAND_USE_FLASH_BBT
 #endif
 
 /* DMA stuff, needed for GPMI/MXS NAND support */
-- 
2.17.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] pcm058: fix NAND flash not using badblock table
  2018-12-05 12:29 [U-Boot] [PATCH] pcm058: fix NAND flash not using badblock table Harald Seiler
@ 2018-12-07  8:45 ` Stefano Babic
  2018-12-07  9:19   ` [U-Boot] [PATCH v2] " Harald Seiler
  0 siblings, 1 reply; 9+ messages in thread
From: Stefano Babic @ 2018-12-07  8:45 UTC (permalink / raw)
  To: u-boot

On 05/12/18 13:29, Harald Seiler wrote:
> Without setting this flag, U-Boot will silently use a memory
> only badblock table.  This is not only bad because it means badblock
> info is ignored, but also leads to other issues if a driver ontop
> then tries to read the badblock table as filesystem info (eg. UBI).
> 

Comment is not consistent and does not really explain what happens - can
you formulate it again ? Thanks !

Best regards,
Stefano Babic

> Signed-off-by: Harald Seiler <hws@denx.de>
> ---
>  include/configs/pcm058.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/configs/pcm058.h b/include/configs/pcm058.h
> index 49048c163f..b9bc08b388 100644
> --- a/include/configs/pcm058.h
> +++ b/include/configs/pcm058.h
> @@ -55,6 +55,7 @@
>  #define CONFIG_SYS_NAND_BASE		0x40000000
>  #define CONFIG_SYS_NAND_5_ADDR_CYCLE
>  #define CONFIG_SYS_NAND_ONFI_DETECTION
> +#define CONFIG_SYS_NAND_USE_FLASH_BBT
>  #endif
>  
>  /* DMA stuff, needed for GPMI/MXS NAND support */
> 


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
=====================================================================

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] pcm058: fix NAND flash not using badblock table
  2018-12-07  8:45 ` Stefano Babic
@ 2018-12-07  9:19   ` Harald Seiler
  2018-12-07 11:48     ` Marek Vasut
  0 siblings, 1 reply; 9+ messages in thread
From: Harald Seiler @ 2018-12-07  9:19 UTC (permalink / raw)
  To: u-boot

Currently, U-Boot ignores the BBT stored in the last 4 blocks of NAND
flash because the NAND_BBT_USE_FLASH flag is not set.  This leads to
two issues:

* U-Boot silently uses a memory-only BBT which is initialized with all
  blocks marked as good.  This means, actual bad blocks are marked good
  and U-Boot might try writing to or reading from them.
* The BBT in flash, which will be created once Linux boots up, is not
  off limits for a driver ontop, like UBI.  While it does not seem to
  consistently produce an error, sometimes UBI will fail to attach
  because the BBT blocks obviously don't contain valid UBI data.

To fix this, this patch sets the CONFIG_SYS_NAND_USE_FLASH_BBT option,
which is used in ./drivers/mtd/nand/raw/mxs_nand.c to decide whether
a BBT in flash is used.

Signed-off-by: Harald Seiler <hws@denx.de>
---
 include/configs/pcm058.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/configs/pcm058.h b/include/configs/pcm058.h
index 49048c163f..b9bc08b388 100644
--- a/include/configs/pcm058.h
+++ b/include/configs/pcm058.h
@@ -55,6 +55,7 @@
 #define CONFIG_SYS_NAND_BASE		0x40000000
 #define CONFIG_SYS_NAND_5_ADDR_CYCLE
 #define CONFIG_SYS_NAND_ONFI_DETECTION
+#define CONFIG_SYS_NAND_USE_FLASH_BBT
 #endif
 
 /* DMA stuff, needed for GPMI/MXS NAND support */
-- 
2.17.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] pcm058: fix NAND flash not using badblock table
  2018-12-07  9:19   ` [U-Boot] [PATCH v2] " Harald Seiler
@ 2018-12-07 11:48     ` Marek Vasut
  2018-12-07 12:15       ` Harald Seiler
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2018-12-07 11:48 UTC (permalink / raw)
  To: u-boot

On 12/07/2018 10:19 AM, Harald Seiler wrote:
> Currently, U-Boot ignores the BBT stored in the last 4 blocks of NAND
> flash because the NAND_BBT_USE_FLASH flag is not set.  This leads to
> two issues:
> 
> * U-Boot silently uses a memory-only BBT which is initialized with all
>   blocks marked as good.  This means, actual bad blocks are marked good
>   and U-Boot might try writing to or reading from them.
> * The BBT in flash, which will be created once Linux boots up, is not
>   off limits for a driver ontop, like UBI.  While it does not seem to
>   consistently produce an error, sometimes UBI will fail to attach
>   because the BBT blocks obviously don't contain valid UBI data.
> 
> To fix this, this patch sets the CONFIG_SYS_NAND_USE_FLASH_BBT option,
> which is used in ./drivers/mtd/nand/raw/mxs_nand.c to decide whether
> a BBT in flash is used.
> 
> Signed-off-by: Harald Seiler <hws@denx.de>

V2 Changelog is missing.

> ---
>  include/configs/pcm058.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/configs/pcm058.h b/include/configs/pcm058.h
> index 49048c163f..b9bc08b388 100644
> --- a/include/configs/pcm058.h
> +++ b/include/configs/pcm058.h
> @@ -55,6 +55,7 @@
>  #define CONFIG_SYS_NAND_BASE		0x40000000
>  #define CONFIG_SYS_NAND_5_ADDR_CYCLE
>  #define CONFIG_SYS_NAND_ONFI_DETECTION
> +#define CONFIG_SYS_NAND_USE_FLASH_BBT

Shouldn't this be enabled on all boards with GPMI NAND ?

>  #endif
>  
>  /* DMA stuff, needed for GPMI/MXS NAND support */
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] pcm058: fix NAND flash not using badblock table
  2018-12-07 11:48     ` Marek Vasut
@ 2018-12-07 12:15       ` Harald Seiler
  2018-12-07 12:18         ` Marek Vasut
  0 siblings, 1 reply; 9+ messages in thread
From: Harald Seiler @ 2018-12-07 12:15 UTC (permalink / raw)
  To: u-boot

Hello Marek,

On Fri, 2018-12-07 at 12:48 +0100, Marek Vasut wrote:
> On 12/07/2018 10:19 AM, Harald Seiler wrote:
> > Currently, U-Boot ignores the BBT stored in the last 4 blocks of NAND
> > flash because the NAND_BBT_USE_FLASH flag is not set.  This leads to
> > two issues:
> > 
> > * U-Boot silently uses a memory-only BBT which is initialized with all
> >   blocks marked as good.  This means, actual bad blocks are marked good
> >   and U-Boot might try writing to or reading from them.
> > * The BBT in flash, which will be created once Linux boots up, is not
> >   off limits for a driver ontop, like UBI.  While it does not seem to
> >   consistently produce an error, sometimes UBI will fail to attach
> >   because the BBT blocks obviously don't contain valid UBI data.
> > 
> > To fix this, this patch sets the CONFIG_SYS_NAND_USE_FLASH_BBT option,
> > which is used in ./drivers/mtd/nand/raw/mxs_nand.c to decide whether
> > a BBT in flash is used.
> > 
> > Signed-off-by: Harald Seiler <hws@denx.de>
> 
> V2 Changelog is missing.
> 
> > ---
> >  include/configs/pcm058.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/configs/pcm058.h b/include/configs/pcm058.h
> > index 49048c163f..b9bc08b388 100644
> > --- a/include/configs/pcm058.h
> > +++ b/include/configs/pcm058.h
> > @@ -55,6 +55,7 @@
> >  #define CONFIG_SYS_NAND_BASE		0x40000000
> >  #define CONFIG_SYS_NAND_5_ADDR_CYCLE
> >  #define CONFIG_SYS_NAND_ONFI_DETECTION
> > +#define CONFIG_SYS_NAND_USE_FLASH_BBT
> 
> Shouldn't this be enabled on all boards with GPMI NAND ?
> 

I looked at other boards and they all defined this config, so I
assumed this was the way to go ... But yes, as far as I understand,
it would make sense to have it enabled most of the time.  Although
there is some code which makes this configuration from the
devicetree, which might be an even better solution.

> >  #endif
> >  
> >  /* DMA stuff, needed for GPMI/MXS NAND support */
> > 
> 
> 

-- 
Harald

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62  Fax: +49-8142-66989-80   Email: hws at denx.de 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] pcm058: fix NAND flash not using badblock table
  2018-12-07 12:15       ` Harald Seiler
@ 2018-12-07 12:18         ` Marek Vasut
  2018-12-07 18:55           ` Stefano Babic
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2018-12-07 12:18 UTC (permalink / raw)
  To: u-boot

On 12/07/2018 01:15 PM, Harald Seiler wrote:
> Hello Marek,

Hi,

> On Fri, 2018-12-07 at 12:48 +0100, Marek Vasut wrote:
>> On 12/07/2018 10:19 AM, Harald Seiler wrote:
>>> Currently, U-Boot ignores the BBT stored in the last 4 blocks of NAND
>>> flash because the NAND_BBT_USE_FLASH flag is not set.  This leads to
>>> two issues:
>>>
>>> * U-Boot silently uses a memory-only BBT which is initialized with all
>>>   blocks marked as good.  This means, actual bad blocks are marked good
>>>   and U-Boot might try writing to or reading from them.
>>> * The BBT in flash, which will be created once Linux boots up, is not
>>>   off limits for a driver ontop, like UBI.  While it does not seem to
>>>   consistently produce an error, sometimes UBI will fail to attach
>>>   because the BBT blocks obviously don't contain valid UBI data.
>>>
>>> To fix this, this patch sets the CONFIG_SYS_NAND_USE_FLASH_BBT option,
>>> which is used in ./drivers/mtd/nand/raw/mxs_nand.c to decide whether
>>> a BBT in flash is used.
>>>
>>> Signed-off-by: Harald Seiler <hws@denx.de>
>>
>> V2 Changelog is missing.
>>
>>> ---
>>>  include/configs/pcm058.h | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/configs/pcm058.h b/include/configs/pcm058.h
>>> index 49048c163f..b9bc08b388 100644
>>> --- a/include/configs/pcm058.h
>>> +++ b/include/configs/pcm058.h
>>> @@ -55,6 +55,7 @@
>>>  #define CONFIG_SYS_NAND_BASE		0x40000000
>>>  #define CONFIG_SYS_NAND_5_ADDR_CYCLE
>>>  #define CONFIG_SYS_NAND_ONFI_DETECTION
>>> +#define CONFIG_SYS_NAND_USE_FLASH_BBT
>>
>> Shouldn't this be enabled on all boards with GPMI NAND ?
>>
> 
> I looked at other boards and they all defined this config, so I
> assumed this was the way to go ... But yes, as far as I understand,
> it would make sense to have it enabled most of the time.  Although
> there is some code which makes this configuration from the
> devicetree, which might be an even better solution.

Cool, if this can be fixed in a more general manner, that'd be awesome
and would help others too. Better yet, it'd get rid of possibly
hazardous duplication. Can you research it ?

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] pcm058: fix NAND flash not using badblock table
  2018-12-07 12:18         ` Marek Vasut
@ 2018-12-07 18:55           ` Stefano Babic
  2018-12-07 21:18             ` Harald Seiler
  0 siblings, 1 reply; 9+ messages in thread
From: Stefano Babic @ 2018-12-07 18:55 UTC (permalink / raw)
  To: u-boot

Hi Harald,

On 07/12/18 13:18, Marek Vasut wrote:
> On 12/07/2018 01:15 PM, Harald Seiler wrote:
>> Hello Marek,
> 
> Hi,
> 
>> On Fri, 2018-12-07 at 12:48 +0100, Marek Vasut wrote:
>>> On 12/07/2018 10:19 AM, Harald Seiler wrote:
>>>> Currently, U-Boot ignores the BBT stored in the last 4 blocks of NAND
>>>> flash because the NAND_BBT_USE_FLASH flag is not set.  This leads to
>>>> two issues:
>>>>
>>>> * U-Boot silently uses a memory-only BBT which is initialized with all
>>>>   blocks marked as good.  This means, actual bad blocks are marked good
>>>>   and U-Boot might try writing to or reading from them.
>>>> * The BBT in flash, which will be created once Linux boots up, is not
>>>>   off limits for a driver ontop, like UBI.  While it does not seem to
>>>>   consistently produce an error, sometimes UBI will fail to attach
>>>>   because the BBT blocks obviously don't contain valid UBI data.
>>>>
>>>> To fix this, this patch sets the CONFIG_SYS_NAND_USE_FLASH_BBT option,
>>>> which is used in ./drivers/mtd/nand/raw/mxs_nand.c to decide whether
>>>> a BBT in flash is used.
>>>>
>>>> Signed-off-by: Harald Seiler <hws@denx.de>
>>>
>>> V2 Changelog is missing.
>>>
>>>> ---
>>>>  include/configs/pcm058.h | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/include/configs/pcm058.h b/include/configs/pcm058.h
>>>> index 49048c163f..b9bc08b388 100644
>>>> --- a/include/configs/pcm058.h
>>>> +++ b/include/configs/pcm058.h
>>>> @@ -55,6 +55,7 @@
>>>>  #define CONFIG_SYS_NAND_BASE		0x40000000
>>>>  #define CONFIG_SYS_NAND_5_ADDR_CYCLE
>>>>  #define CONFIG_SYS_NAND_ONFI_DETECTION
>>>> +#define CONFIG_SYS_NAND_USE_FLASH_BBT
>>>
>>> Shouldn't this be enabled on all boards with GPMI NAND ?
>>>
>>
>> I looked at other boards and they all defined this config, so I
>> assumed this was the way to go ...

Let me understand. If Isearch for CONFIG_NAND_MXS, I get 27 boards using
this drivers, with different SOC (mx28, mx6[Dual|Quad|Solo], mx6sx,
mx6ull). But none of them is setting  CONFIG_SYS_NAND_USE_FLASH_BBT.

When does it happen the issue ? It should happen if we create a UBI
container and its volumes in U-Boot. If UBI is generated in linux, this
should not happen. Is it the case or does it happen in any condition ?

I think the issue happens because there is a disalignment between kernel
and u-boot. Kernel mainline for this board (file
imx6qdl-phytec-phycore-som.dtsi) sets "nand-on-flash-bbt", while U-Boot
not. That mean that this can always happen if kernel and U-Boot does not
use the same setup for the driver.

Maybe with previous kernel versions there was no problem because Linux
used the same setup as U-Boot.

Anyway, this discourage setting CONFIG_SYS_NAND_USE_FLASH_BBT
unconditionally for all boards with GPMI driver.

> But yes, as far as I understand,
>> it would make sense to have it enabled most of the time.  Although
>> there is some code which makes this configuration from the
>> devicetree, which might be an even better solution.

Device tree has already this option as I see in
drivers/mtd/nand/raw/nand_base.c. The driver enforces this if no DT (as
in this case) is used.

> 
> Cool, if this can be fixed in a more general manner, that'd be awesome
> and would help others too. Better yet, it'd get rid of possibly
> hazardous duplication. Can you research it ?
> 

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
=====================================================================

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] pcm058: fix NAND flash not using badblock table
  2018-12-07 18:55           ` Stefano Babic
@ 2018-12-07 21:18             ` Harald Seiler
  2018-12-08 18:44               ` Stefano Babic
  0 siblings, 1 reply; 9+ messages in thread
From: Harald Seiler @ 2018-12-07 21:18 UTC (permalink / raw)
  To: u-boot

Hello Stefano,

On Fri, 2018-12-07 at 19:55 +0100, Stefano Babic wrote:
> Hi Harald,
> 
> On 07/12/18 13:18, Marek Vasut wrote:
> > On 12/07/2018 01:15 PM, Harald Seiler wrote:
> > > Hello Marek,
> > 
> > Hi,
> > 
> > > On Fri, 2018-12-07 at 12:48 +0100, Marek Vasut wrote:
> > > > On 12/07/2018 10:19 AM, Harald Seiler wrote:
> > > > > Currently, U-Boot ignores the BBT stored in the last 4 blocks of NAND
> > > > > flash because the NAND_BBT_USE_FLASH flag is not set.  This leads to
> > > > > two issues:
> > > > > 
> > > > > * U-Boot silently uses a memory-only BBT which is initialized with all
> > > > >   blocks marked as good.  This means, actual bad blocks are marked good
> > > > >   and U-Boot might try writing to or reading from them.
> > > > > * The BBT in flash, which will be created once Linux boots up, is not
> > > > >   off limits for a driver ontop, like UBI.  While it does not seem to
> > > > >   consistently produce an error, sometimes UBI will fail to attach
> > > > >   because the BBT blocks obviously don't contain valid UBI data.
> > > > > 
> > > > > To fix this, this patch sets the CONFIG_SYS_NAND_USE_FLASH_BBT option,
> > > > > which is used in ./drivers/mtd/nand/raw/mxs_nand.c to decide whether
> > > > > a BBT in flash is used.
> > > > > 
> > > > > Signed-off-by: Harald Seiler <hws@denx.de>
> > > > 
> > > > V2 Changelog is missing.
> > > > 
> > > > > ---
> > > > >  include/configs/pcm058.h | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/include/configs/pcm058.h b/include/configs/pcm058.h
> > > > > index 49048c163f..b9bc08b388 100644
> > > > > --- a/include/configs/pcm058.h
> > > > > +++ b/include/configs/pcm058.h
> > > > > @@ -55,6 +55,7 @@
> > > > >  #define CONFIG_SYS_NAND_BASE		0x40000000
> > > > >  #define CONFIG_SYS_NAND_5_ADDR_CYCLE
> > > > >  #define CONFIG_SYS_NAND_ONFI_DETECTION
> > > > > +#define CONFIG_SYS_NAND_USE_FLASH_BBT
> > > > 
> > > > Shouldn't this be enabled on all boards with GPMI NAND ?
> > > > 
> > > 
> > > I looked at other boards and they all defined this config, so I
> > > assumed this was the way to go ...
> 
> Let me understand. If Isearch for CONFIG_NAND_MXS, I get 27 boards using
> this drivers, with different SOC (mx28, mx6[Dual|Quad|Solo], mx6sx,
> mx6ull). But none of them is setting  CONFIG_SYS_NAND_USE_FLASH_BBT.
> 
> When does it happen the issue ? It should happen if we create a UBI
> container and its volumes in U-Boot. If UBI is generated in linux, this
> should not happen. Is it the case or does it happen in any condition ?

Linux will write the badblock table to the last 4 blocks by default which
U-Boot ignores at the moment.  So the issue happens if you use the default
kernel config (which you pointed out below).

> I think the issue happens because there is a disalignment between kernel
> and u-boot. Kernel mainline for this board (file
> imx6qdl-phytec-phycore-som.dtsi) sets "nand-on-flash-bbt", while U-Boot
> not. That mean that this can always happen if kernel and U-Boot does not
> use the same setup for the driver.
> 
> Maybe with previous kernel versions there was no problem because Linux
> used the same setup as U-Boot.

This is exactly what I think is going on ...

> Anyway, this discourage setting CONFIG_SYS_NAND_USE_FLASH_BBT
> unconditionally for all boards with GPMI driver.
> 

I agree, it only makes sense in cases where Linux also does it this way.
Although, not doing it this way means not having any persistent badblock
table at all, which I feel is never a good idea ...

> > But yes, as far as I understand,
> > > it would make sense to have it enabled most of the time.  Although
> > > there is some code which makes this configuration from the
> > > devicetree, which might be an even better solution.
> 
> Device tree has already this option as I see in
> drivers/mtd/nand/raw/nand_base.c. The driver enforces this if no DT (as
> in this case) is used.

What do you mean by enforce? If no devicetree is used, the code does not
set any flags as far as I understand it, which also aligns with the results
of my experimentation ... (Currently bbt_options is 0)

> > Cool, if this can be fixed in a more general manner, that'd be awesome
> > and would help others too. Better yet, it'd get rid of possibly
> > hazardous duplication. Can you research it ?
> > 
> 
> Best regards,
> Stefano
> 

-- 
Harald

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62  Fax: +49-8142-66989-80   Email: hws at denx.de 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH v2] pcm058: fix NAND flash not using badblock table
  2018-12-07 21:18             ` Harald Seiler
@ 2018-12-08 18:44               ` Stefano Babic
  0 siblings, 0 replies; 9+ messages in thread
From: Stefano Babic @ 2018-12-08 18:44 UTC (permalink / raw)
  To: u-boot

Hi Harald,

On 07/12/18 22:18, Harald Seiler wrote:
> Hello Stefano,
> 
> On Fri, 2018-12-07 at 19:55 +0100, Stefano Babic wrote:
>> Hi Harald,
>>
>> On 07/12/18 13:18, Marek Vasut wrote:
>>> On 12/07/2018 01:15 PM, Harald Seiler wrote:
>>>> Hello Marek,
>>>
>>> Hi,
>>>
>>>> On Fri, 2018-12-07 at 12:48 +0100, Marek Vasut wrote:
>>>>> On 12/07/2018 10:19 AM, Harald Seiler wrote:
>>>>>> Currently, U-Boot ignores the BBT stored in the last 4 blocks of NAND
>>>>>> flash because the NAND_BBT_USE_FLASH flag is not set.  This leads to
>>>>>> two issues:
>>>>>>
>>>>>> * U-Boot silently uses a memory-only BBT which is initialized with all
>>>>>>   blocks marked as good.  This means, actual bad blocks are marked good
>>>>>>   and U-Boot might try writing to or reading from them.
>>>>>> * The BBT in flash, which will be created once Linux boots up, is not
>>>>>>   off limits for a driver ontop, like UBI.  While it does not seem to
>>>>>>   consistently produce an error, sometimes UBI will fail to attach
>>>>>>   because the BBT blocks obviously don't contain valid UBI data.
>>>>>>
>>>>>> To fix this, this patch sets the CONFIG_SYS_NAND_USE_FLASH_BBT option,
>>>>>> which is used in ./drivers/mtd/nand/raw/mxs_nand.c to decide whether
>>>>>> a BBT in flash is used.
>>>>>>
>>>>>> Signed-off-by: Harald Seiler <hws@denx.de>
>>>>>
>>>>> V2 Changelog is missing.
>>>>>
>>>>>> ---
>>>>>>  include/configs/pcm058.h | 1 +
>>>>>>  1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/include/configs/pcm058.h b/include/configs/pcm058.h
>>>>>> index 49048c163f..b9bc08b388 100644
>>>>>> --- a/include/configs/pcm058.h
>>>>>> +++ b/include/configs/pcm058.h
>>>>>> @@ -55,6 +55,7 @@
>>>>>>  #define CONFIG_SYS_NAND_BASE		0x40000000
>>>>>>  #define CONFIG_SYS_NAND_5_ADDR_CYCLE
>>>>>>  #define CONFIG_SYS_NAND_ONFI_DETECTION
>>>>>> +#define CONFIG_SYS_NAND_USE_FLASH_BBT
>>>>>
>>>>> Shouldn't this be enabled on all boards with GPMI NAND ?
>>>>>
>>>>
>>>> I looked at other boards and they all defined this config, so I
>>>> assumed this was the way to go ...
>>
>> Let me understand. If Isearch for CONFIG_NAND_MXS, I get 27 boards using
>> this drivers, with different SOC (mx28, mx6[Dual|Quad|Solo], mx6sx,
>> mx6ull). But none of them is setting  CONFIG_SYS_NAND_USE_FLASH_BBT.
>>
>> When does it happen the issue ? It should happen if we create a UBI
>> container and its volumes in U-Boot. If UBI is generated in linux, this
>> should not happen. Is it the case or does it happen in any condition ?
> 
> Linux will write the badblock table to the last 4 blocks by default which
> U-Boot ignores at the moment.  So the issue happens if you use the default
> kernel config (which you pointed out below).

Right, or in any case where there is a mismatch in the configuration
between U-Boot and Linux.

> 
>> I think the issue happens because there is a disalignment between kernel
>> and u-boot. Kernel mainline for this board (file
>> imx6qdl-phytec-phycore-som.dtsi) sets "nand-on-flash-bbt", while U-Boot
>> not. That mean that this can always happen if kernel and U-Boot does not
>> use the same setup for the driver.
>>
>> Maybe with previous kernel versions there was no problem because Linux
>> used the same setup as U-Boot.
> 
> This is exactly what I think is going on ...

Right.

> 
>> Anyway, this discourage setting CONFIG_SYS_NAND_USE_FLASH_BBT
>> unconditionally for all boards with GPMI driver.
>>
> 
> I agree, it only makes sense in cases where Linux also does it this way.
> Although, not doing it this way means not having any persistent badblock
> table at all, which I feel is never a good idea ...
> 
>>> But yes, as far as I understand,
>>>> it would make sense to have it enabled most of the time.  Although
>>>> there is some code which makes this configuration from the
>>>> devicetree, which might be an even better solution.
>>
>> Device tree has already this option as I see in
>> drivers/mtd/nand/raw/nand_base.c. The driver enforces this if no DT (as
>> in this case) is used.
> 
> What do you mean by enforce? 

Without DT, behavior is decided at compile time.

> If no devicetree is used, the code does not
> set any flags as far as I understand it, which also aligns with the results
> of my experimentation ... (Currently bbt_options is 0)

Ok - we agree that this change cannot be done for all boards because it
depends if Linux is using or not, and we do not know the common use
cases for thoese boards. It must be decided any time by the board
maintainer (well, in this case me..). Use case for this board is running
kernel mainline, so it makes sense to apply this patch.

Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
=====================================================================

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-12-08 18:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-05 12:29 [U-Boot] [PATCH] pcm058: fix NAND flash not using badblock table Harald Seiler
2018-12-07  8:45 ` Stefano Babic
2018-12-07  9:19   ` [U-Boot] [PATCH v2] " Harald Seiler
2018-12-07 11:48     ` Marek Vasut
2018-12-07 12:15       ` Harald Seiler
2018-12-07 12:18         ` Marek Vasut
2018-12-07 18:55           ` Stefano Babic
2018-12-07 21:18             ` Harald Seiler
2018-12-08 18:44               ` Stefano Babic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.