[U-Boot] [U-boot] [Patch v2 3/3] k2hk: change default nand ecc layout

[Scott Wood]

On Wed, 2014-06-25 at 18:42 +0300, Ivan Khoronzhuk wrote:
> On 06/25/2014 02:25 AM, Scott Wood wrote:
> > On Mon, 2014-06-23 at 18:26 +0300, Ivan Khoronzhuk wrote:
> >> On 06/21/2014 02:40 AM, Scott Wood wrote:
> >>> On Sat, 2014-06-21 at 02:28 +0300, Ivan Khoronzhuk wrote:
> >>>> For keystyone k2hk board the default nand layout is different
> >>>> from davinci. So swich ecc layout at init in board file.
> >>>>
> >>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk at ti.com>
> >>>> ---
> >>>>    board/ti/k2hk_evm/board.c | 11 +++++++++++
> >>>>    1 file changed, 11 insertions(+)
> >>>>
> >>>> diff --git a/board/ti/k2hk_evm/board.c b/board/ti/k2hk_evm/board.c
> >>>> index ef90f9d..baa6ab7 100644
> >>>> --- a/board/ti/k2hk_evm/board.c
> >>>> +++ b/board/ti/k2hk_evm/board.c
> >>>> @@ -11,6 +11,7 @@
> >>>>    #include <exports.h>
> >>>>    #include <fdt_support.h>
> >>>>    #include <libfdt.h>
> >>>> +#include <nand.h>
> >>>>    
> >>>>    #include <asm/arch/hardware.h>
> >>>>    #include <asm/arch/clock.h>
> >>>> @@ -19,6 +20,7 @@
> >>>>    #include <asm/arch/emac_defs.h>
> >>>>    #include <asm/arch/psc_defs.h>
> >>>>    #include <asm/ti-common/ti-aemif.h>
> >>>> +#include <asm/ti-common/davinci_nand.h>
> >>>>    
> >>>>    DECLARE_GLOBAL_DATA_PTR;
> >>>>    
> >>>> @@ -147,6 +149,15 @@ int cpu_to_bus(u32 *ptr, u32 length)
> >>>>    	return 0;
> >>>>    }
> >>>>    
> >>>> +int board_nand_init(struct nand_chip *chip)
> >>>> +{
> >>>> +	davinci_nand_init(chip);
> >>>> +	chip->ecc.layout =
> >>>> +		board_nand_get_ecclayout(NAND_KEYSTONE_RBL_4BIT_LAYOUT);
> >>>> +
> >>>> +	return 0;
> >>>> +}
> >>> Shouldn't you be calling board_nand_set_ecclayout()?  How will oobavail
> >>> get set?
> >> There is no reason to set oobavail here, I suppose that's done while int
> >> nand_scan_tail().
> > But nand_scan_tail() has already run at this point.  oobavail would be
> > set based on the wrong layout.
> 
> Where did you get.. I've checked it again. It was called after 
> board_nand_init().
> 
> nand_init_chip
>          -> board_nand_init
>                  -> nand_scan
>                          -> nand_scan_tail

Never mind, I was thinking of the flow when using
CONFIG_SYS_NAND_SELF_INIT.

Still, it's probably not great to introduce a mechanism that is
incompatible with transitioning to self-init.

> If I correctly understand you, what if while we are writing data a new 
> bad block is found?
> Then we try to update BBT wich was written using a different layout....
> 
> Well probably I should rewrite BBT when layout is switched..
> Will see if it can be done correctly.

Could you clarify the use case here?  Are you changing the layout at
runtime so that different portions of the chip use different ECC schemes
(e.g. because booting requires something different)?  If so, then you
don't want to rewrite the BBT based on this, and a better approach than
a command line switcher would be to be able to define different layouts
for different offset ranges.

If this is meant to be a permanent change that affects the entire chip,
then the entire chip (not just the BBT, but excluding factory bad block
markers) needs to be wiped when you change, and you should be recording
the default in the environment (e.g. hwconfig) rather than switching
with a command line option.

-Scott