| Message ID | 1334316061-26019-5-git-send-email-timo@exertus.fi |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On 04/13/2012 06:20 AM, Timo Ketola wrote: > First two bytes of the first OOB of erase block are reserved for factory > bad block marking, usually. > > Signed-off-by: Timo Ketola <timo@exertus.fi> > --- > drivers/mtd/nand/mxc_nand.c | 35 +++++++++++++++++++++++++++++++++++ > 1 files changed, 35 insertions(+), 0 deletions(-) So what happened before? The default is at offset 8, which doesn't conflict with the bad block marker. It seems the actual issue is a conflict with ECC? And NAND_USE_FLASH_BBT wasn't defined before, so a better subject line for this patch would be "nand/mxc: support flash-based BBT". > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c > index 35e89a0..73813a2 100644 > --- a/drivers/mtd/nand/mxc_nand.c > +++ b/drivers/mtd/nand/mxc_nand.c > @@ -1302,12 +1302,47 @@ static void mxc_setup_config1(void) > #define mxc_setup_config1() > #endif > > +#ifdef CONFIG_SYS_NAND_USE_FLASH_BBT > + > +static u8 bbt_pattern[] = {'B', 'b', 't', '0' }; > +static u8 mirror_pattern[] = {'1', 't', 'b', 'B' }; > + > +static struct nand_bbt_descr bbt_main_descr = { > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE | > + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP, > + .offs = 2, > + .len = 4, > + .veroffs = 6, > + .maxblocks = 4, > + .pattern = bbt_pattern, > +}; > + > +static struct nand_bbt_descr bbt_mirror_descr = { > + .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE | > + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP, > + .offs = 2, > + .len = 4, > + .veroffs = 6, > + .maxblocks = 4, > + .pattern = mirror_pattern, > +}; > + > +#endif > + Won't veroffs = 6 conflict with ECC in the MXC_NFC_V1 case? What about 8-bit small page support, in which case the bad block marker is at offset 5? > int board_nand_init(struct nand_chip *this) > { > struct mtd_info *mtd; > uint16_t tmp; > int err = 0; > > +#ifdef CONFIG_SYS_NAND_USE_FLASH_BBT > + > + this->options = NAND_USE_FLASH_BBT; > + this->bbt_td = &bbt_main_descr; > + this->bbt_md = &bbt_mirror_descr; > + > +#endif Please remove those blank lines inside the ifdef. -Scott
On 13.04.2012 20:19, Scott Wood wrote: > On 04/13/2012 06:20 AM, Timo Ketola wrote: >> First two bytes of the first OOB of erase block are reserved for factory >> bad block marking, usually. >> >> Signed-off-by: Timo Ketola<timo@exertus.fi> >> --- >> drivers/mtd/nand/mxc_nand.c | 35 +++++++++++++++++++++++++++++++++++ >> 1 files changed, 35 insertions(+), 0 deletions(-) > > So what happened before? The default is at offset 8, which doesn't > conflict with the bad block marker. It seems the actual issue is a > conflict with ECC? You seem to be right. I think I was badly confused with the kernel behaviour. > And NAND_USE_FLASH_BBT wasn't defined before, so a better subject line > for this patch would be "nand/mxc: support flash-based BBT". Most probably right too. > Won't veroffs = 6 conflict with ECC in the MXC_NFC_V1 case? Seems to. > What about 8-bit small page support, in which case the bad block marker > is at offset 5? What about putting into the block #if defined(MXC_NFC_V1) #ifndef CONFIG_SYS_NAND_LARGEPAGE defines for pattern and version offsets and use them in bbt_*_descr initializations? Or should they be in board configuration file? >> +#ifdef CONFIG_SYS_NAND_USE_FLASH_BBT >> + >> + this->options = NAND_USE_FLASH_BBT; >> + this->bbt_td =&bbt_main_descr; >> + this->bbt_md =&bbt_mirror_descr; >> + >> +#endif > > Please remove those blank lines inside the ifdef. Ok -- Timo
On 04/13/2012 01:12 PM, Timo Ketola wrote: > On 13.04.2012 20:19, Scott Wood wrote: >> On 04/13/2012 06:20 AM, Timo Ketola wrote: >>> First two bytes of the first OOB of erase block are reserved for factory >>> bad block marking, usually. >>> >>> Signed-off-by: Timo Ketola<timo@exertus.fi> >>> --- >>> drivers/mtd/nand/mxc_nand.c | 35 +++++++++++++++++++++++++++++++++++ >>> 1 files changed, 35 insertions(+), 0 deletions(-) >> >> So what happened before? The default is at offset 8, which doesn't >> conflict with the bad block marker. It seems the actual issue is a >> conflict with ECC? > > You seem to be right. I think I was badly confused with the kernel > behaviour. It looks like Linux wants the BBT to be at offset zero. Is there any plan to fix that? The two really should match... >> What about 8-bit small page support, in which case the bad block marker >> is at offset 5? > > What about putting into the block > > #if defined(MXC_NFC_V1) > #ifndef CONFIG_SYS_NAND_LARGEPAGE > > defines for pattern and version offsets and use them in bbt_*_descr > initializations? Sure. > Or should they be in board configuration file? I don't think it belongs in the board config file (unless there's existing behavior that has to be matched for compatibility on a specific board). -Scott
On 13.04.2012 21:17, Scott Wood wrote:
> It looks like Linux wants the BBT to be at offset zero.
I have not dug too deeply into the BBT logic in kernel but maybe it could be
possible to place BBT patterns over the factory markers. Then, when the code
scans for BBT blocks, it should ignore factory markers and react only on BBT
patterns. But I don't really know if this is the idea in kernel.
--
Timo
On 13.04.2012 21:17, Scott Wood wrote: > It looks like Linux wants the BBT to be at offset zero. Is there any > plan to fix that? The two really should match... Somewhere in the process I got an impression that BBT couldn't be placed over the area where factory markers would be. But you made me think and I reverted the offset in my kernel back to zero and changed U-Boot accordingly. It seems to work perfectly like that. So I think now that the offset at zero is right. Should I post v3 patchset now or only this one again or minimize noise and wait for more comments? -- Timo
On 04/16/2012 01:41 AM, Timo Ketola wrote: > Should I post v3 patchset now or only this one again or minimize noise > and wait for more comments? Whichever you'd prefer. -Scott
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c index 35e89a0..73813a2 100644 --- a/drivers/mtd/nand/mxc_nand.c +++ b/drivers/mtd/nand/mxc_nand.c @@ -1302,12 +1302,47 @@ static void mxc_setup_config1(void) #define mxc_setup_config1() #endif +#ifdef CONFIG_SYS_NAND_USE_FLASH_BBT + +static u8 bbt_pattern[] = {'B', 'b', 't', '0' }; +static u8 mirror_pattern[] = {'1', 't', 'b', 'B' }; + +static struct nand_bbt_descr bbt_main_descr = { + .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE | + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP, + .offs = 2, + .len = 4, + .veroffs = 6, + .maxblocks = 4, + .pattern = bbt_pattern, +}; + +static struct nand_bbt_descr bbt_mirror_descr = { + .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE | + NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP, + .offs = 2, + .len = 4, + .veroffs = 6, + .maxblocks = 4, + .pattern = mirror_pattern, +}; + +#endif + int board_nand_init(struct nand_chip *this) { struct mtd_info *mtd; uint16_t tmp; int err = 0; +#ifdef CONFIG_SYS_NAND_USE_FLASH_BBT + + this->options = NAND_USE_FLASH_BBT; + this->bbt_td = &bbt_main_descr; + this->bbt_md = &bbt_mirror_descr; + +#endif + /* structures must be linked */ mtd = &host->mtd; mtd->priv = this;
First two bytes of the first OOB of erase block are reserved for factory bad block marking, usually. Signed-off-by: Timo Ketola <timo@exertus.fi> --- drivers/mtd/nand/mxc_nand.c | 35 +++++++++++++++++++++++++++++++++++ 1 files changed, 35 insertions(+), 0 deletions(-)