[RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Kyungmin Park]

Hi,

It's the preparation patch for the MLC NAND/OneNAND flash.
The current jffs2 implementation at NAND case uses the NOP 2, cleanmarker and data each page 0. It breaks the NOP 1 limitation in MLC flash.

With this patch, we don't write the cleanmarker at oob area.
I'm not sure it should use the cleanmarker at data area as NOR does.
Apart from the cleanmarker, there's serious POR issue, paired page at MLC.

The paired page problem is that if the some page is broken, the paired page is also broken even though it is written correctly at previous time.
That's the reason the cleanmarker is not big issue at this time.

Now we support the NOP 1 and later we have to address this paired page issue. I'm not sure when this problem is addressed.

Any comments are welcome.

Thank you,
Kyungmin Park

P.S., you can check the NOP 1 easily, just s/MTD_CAP_NANDFLASH/MTD_CAP_MLC_NANDFLASH/ .
---
diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
index a1db918..65d7dd7 100644
--- a/fs/jffs2/erase.c
+++ b/fs/jffs2/erase.c
@@ -415,6 +415,7 @@ static void jffs2_mark_erased_block(struct jffs2_sb_info *c, struct jffs2_eraseb
 	/* Cleanmarker in oob area or no cleanmarker at all ? */
 	if (jffs2_cleanmarker_oob(c) || c->cleanmarker_size == 0) {
 
+		/* We only write cleanmarker in case of SLC NAND */
 		if (jffs2_cleanmarker_oob(c)) {
 			if (jffs2_write_nand_cleanmarker(c, jeb))
 				goto filebad;
diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index ee192af..aa18eea 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -656,7 +656,9 @@ void jffs2_gc_release_page(struct jffs2_sb_info *c,
 static int jffs2_flash_setup(struct jffs2_sb_info *c) {
 	int ret = 0;
 
-	if (jffs2_cleanmarker_oob(c)) {
+	if (c->mtd->type == MTD_NANDFLASH) {
+		if (c->mtd->flags & MTD_MULTI_LEVEL_CHIP)
+			printk(KERN_INFO "JFFS2: Multi Level Chip support\n");
 		/* NAND flash... do setup accordingly */
 		ret = jffs2_nand_flash_setup(c);
 		if (ret)
@@ -689,7 +691,7 @@ static int jffs2_flash_setup(struct jffs2_sb_info *c) {
 
 void jffs2_flash_cleanup(struct jffs2_sb_info *c) {
 
-	if (jffs2_cleanmarker_oob(c)) {
+	if (c->mtd->type == MTD_NANDFLASH) {
 		jffs2_nand_flash_cleanup(c);
 	}
 
diff --git a/fs/jffs2/os-linux.h b/fs/jffs2/os-linux.h
index bf64686..70881bc 100644
--- a/fs/jffs2/os-linux.h
+++ b/fs/jffs2/os-linux.h
@@ -110,7 +110,7 @@ static inline void jffs2_init_inode_info(struct jffs2_inode_info *f)
 #define jffs2_can_mark_obsolete(c) (c->mtd->flags & (MTD_BIT_WRITEABLE))
 #endif
 
-#define jffs2_cleanmarker_oob(c) (c->mtd->type == MTD_NANDFLASH)
+#define jffs2_cleanmarker_oob(c) (c->mtd->type == MTD_NANDFLASH && !(c->mtd->flags & MTD_MULTI_LEVEL_CHIP))
 
 #define jffs2_flash_write_oob(c, ofs, len, retlen, buf) ((c)->mtd->write_oob((c)->mtd, ofs, len, retlen, buf))
 #define jffs2_flash_read_oob(c, ofs, len, retlen, buf) ((c)->mtd->read_oob((c)->mtd, ofs, len, retlen, buf))
diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
diff --git a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index f71dac4..80334d7 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -30,12 +30,14 @@ struct mtd_oob_buf {
 #define MTD_BIT_WRITEABLE	0x800	/* Single bits can be flipped */
 #define MTD_NO_ERASE		0x1000	/* No erase necessary */
 #define MTD_STUPID_LOCK		0x2000	/* Always locked after reset */
+#define MTD_MULTI_LEVEL_CHIP	0x4000	/* Multi Level Chip */
 
 // Some common devices / combinations of capabilities
 #define MTD_CAP_ROM		0
 #define MTD_CAP_RAM		(MTD_WRITEABLE | MTD_BIT_WRITEABLE | MTD_NO_ERASE)
 #define MTD_CAP_NORFLASH	(MTD_WRITEABLE | MTD_BIT_WRITEABLE)
 #define MTD_CAP_NANDFLASH	(MTD_WRITEABLE)
+#define MTD_CAP_MLC_NANDFLASH	(MTD_WRITEABLE | MTD_MULTI_LEVEL_CHIP)
 
 /* ECC byte placement */
 #define MTD_NANDECC_OFF		0	// Switch off ECC (Not recommended)

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Jörn Engel]

On Tue, 27 November 2007 16:36:16 +0900, Kyungmin Park wrote:
> diff --git a/fs/jffs2/os-linux.h b/fs/jffs2/os-linux.h
> index bf64686..70881bc 100644
> --- a/fs/jffs2/os-linux.h
> +++ b/fs/jffs2/os-linux.h
> @@ -110,7 +110,7 @@ static inline void jffs2_init_inode_info(struct jffs2_inode_info *f)
>  #define jffs2_can_mark_obsolete(c) (c->mtd->flags & (MTD_BIT_WRITEABLE))
>  #endif
>  
> -#define jffs2_cleanmarker_oob(c) (c->mtd->type == MTD_NANDFLASH)
> +#define jffs2_cleanmarker_oob(c) (c->mtd->type == MTD_NANDFLASH && !(c->mtd->flags & MTD_MULTI_LEVEL_CHIP))
>  
>  #define jffs2_flash_write_oob(c, ofs, len, retlen, buf) ((c)->mtd->write_oob((c)->mtd, ofs, len, retlen, buf))
>  #define jffs2_flash_read_oob(c, ofs, len, retlen, buf) ((c)->mtd->read_oob((c)->mtd, ofs, len, retlen, buf))
> diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
> diff --git a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c
> diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
> index f71dac4..80334d7 100644
> --- a/include/mtd/mtd-abi.h
> +++ b/include/mtd/mtd-abi.h
> @@ -30,12 +30,14 @@ struct mtd_oob_buf {
>  #define MTD_BIT_WRITEABLE	0x800	/* Single bits can be flipped */
>  #define MTD_NO_ERASE		0x1000	/* No erase necessary */
>  #define MTD_STUPID_LOCK		0x2000	/* Always locked after reset */
> +#define MTD_MULTI_LEVEL_CHIP	0x4000	/* Multi Level Chip */
>  
>  // Some common devices / combinations of capabilities
>  #define MTD_CAP_ROM		0
>  #define MTD_CAP_RAM		(MTD_WRITEABLE | MTD_BIT_WRITEABLE | MTD_NO_ERASE)
>  #define MTD_CAP_NORFLASH	(MTD_WRITEABLE | MTD_BIT_WRITEABLE)
>  #define MTD_CAP_NANDFLASH	(MTD_WRITEABLE)
> +#define MTD_CAP_MLC_NANDFLASH	(MTD_WRITEABLE | MTD_MULTI_LEVEL_CHIP)

NAK.  This does not describe the limitations of your chip, but a random
name.  In fact, the limitations appear to be identical to
(MTD_NORFLASH & !MTD_BIT_WRITEABLE).

Maybe something like ALLOW_SEPERATE_OOB_WRITE with reversed logic?

Jörn

-- 
It does not require a majority to prevail, but rather an irate,
tireless minority keen to set brush fires in people's minds.
-- Samuel Adams

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Kyungmin Park]

> > diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
> > index f71dac4..80334d7 100644
> > --- a/include/mtd/mtd-abi.h
> > +++ b/include/mtd/mtd-abi.h
> > @@ -30,12 +30,14 @@ struct mtd_oob_buf {
> >  #define MTD_BIT_WRITEABLE    0x800   /* Single bits can be flipped */
> >  #define MTD_NO_ERASE         0x1000  /* No erase necessary */
> >  #define MTD_STUPID_LOCK              0x2000  /* Always locked after reset */
> > +#define MTD_MULTI_LEVEL_CHIP 0x4000  /* Multi Level Chip */
> >
> >  // Some common devices / combinations of capabilities
> >  #define MTD_CAP_ROM          0
> >  #define MTD_CAP_RAM          (MTD_WRITEABLE | MTD_BIT_WRITEABLE | MTD_NO_ERASE)
> >  #define MTD_CAP_NORFLASH     (MTD_WRITEABLE | MTD_BIT_WRITEABLE)
> >  #define MTD_CAP_NANDFLASH    (MTD_WRITEABLE)
> > +#define MTD_CAP_MLC_NANDFLASH        (MTD_WRITEABLE | MTD_MULTI_LEVEL_CHIP)
>
> NAK.  This does not describe the limitations of your chip, but a random
> name.  In fact, the limitations appear to be identical to
> (MTD_NORFLASH & !MTD_BIT_WRITEABLE).
>
> Maybe something like ALLOW_SEPERATE_OOB_WRITE with reversed logic?
>

However MTD_NORFLASH doesn't have the write buffer. It means it can be
write one more.
Maybe you only think the non-oob case. But MLC NAND should have the
write buffer for NOP 1 limitation.

In summary,
NOR: no oob, no problem to write bytes.
SLC NAND: has oob, NOP is allowed at least 2 and more.
MCL NAND:  has oob, but NOP is 1.

Thank you,
Kyungmin Park

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Artem Bityutskiy]

On Tue, 2007-11-27 at 20:48 +0900, Kyungmin Park wrote:
> In summary,
> NOR: no oob, no problem to write bytes.
> SLC NAND: has oob, NOP is allowed at least 2 and more.
> MCL NAND:  has oob, but NOP is 1.

Hi Kyungmin,

in MTD we call this stuff "sub-pages", so I'd suggest you to use this
term instead of NOP. People are not accustomed to NOP. Use whatever term
in the driver, but try to use "sub-pages" term when you go up to the
generic layer.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Jörn Engel]

On Tue, 27 November 2007 20:48:33 +0900, Kyungmin Park wrote:
> >
> > NAK.  This does not describe the limitations of your chip, but a random
> > name.  In fact, the limitations appear to be identical to
> > (MTD_NORFLASH & !MTD_BIT_WRITEABLE).
> >
> > Maybe something like ALLOW_SEPERATE_OOB_WRITE with reversed logic?
> >
> 
> However MTD_NORFLASH doesn't have the write buffer. It means it can be
> write one more.

Some NOR flashes _do_ require write buffers.  The ones I know must be
written in aligned multiples of 8, 16 or 1024 bytes, respectively.  In
spite of being NOR, that sounds quite similar to what you try to achieve
for NAND.

Jörn

-- 
The wise man seeks everything in himself; the ignorant man tries to get
everything from somebody else.
-- unknown

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Kyungmin Park]

On 11/27/07, Artem Bityutskiy <dedekind@infradead.org> wrote:
> On Tue, 2007-11-27 at 20:48 +0900, Kyungmin Park wrote:
> > In summary,
> > NOR: no oob, no problem to write bytes.
> > SLC NAND: has oob, NOP is allowed at least 2 and more.
> > MCL NAND:  has oob, but NOP is 1.
>
> Hi Kyungmin,
>
> in MTD we call this stuff "sub-pages", so I'd suggest you to use this
> term instead of NOP. People are not accustomed to NOP. Use whatever term
> in the driver, but try to use "sub-pages" term when you go up to the
> generic layer.

Hi Artem,

Umm, I'm not sure it's the right term. "sub-page" doesn't cover the
oob area. It's only meanful at data area.
Anyway it should be sub-page is 0 in MLC case.

Thank you,
Kyungmin Park

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Jörn Engel]

On Wed, 28 November 2007 08:28:56 +0900, Kyungmin Park wrote:
> 
> Umm, I'm not sure it's the right term. "sub-page" doesn't cover the
> oob area. It's only meanful at data area.
> Anyway it should be sub-page is 0 in MLC case.

Maybe we need a good description of the problem before we proceed.  This
argument about NOP vs. sub-page sounds as if not everyone has the same
definition for those words.  Without agreeing on those definitions, we
could argue for weeks without useful results.

Kyungmin, can you describe the problem again, including definitions of
potentially confusing terms?

Jörn

-- 
Data expands to fill the space available for storage.
-- Parkinson's Law

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Kyungmin Park]

> On Tue, 27 November 2007 20:48:33 +0900, Kyungmin Park wrote:
> > >
> > > NAK.  This does not describe the limitations of your chip, but a random
> > > name.  In fact, the limitations appear to be identical to
> > > (MTD_NORFLASH & !MTD_BIT_WRITEABLE).
> > >
> > > Maybe something like ALLOW_SEPERATE_OOB_WRITE with reversed logic?
> > >
> >
> > However MTD_NORFLASH doesn't have the write buffer. It means it can be
> > write one more.
>
> Some NOR flashes _do_ require write buffers.  The ones I know must be
> written in aligned multiples of 8, 16 or 1024 bytes, respectively.  In
> spite of being NOR, that sounds quite similar to what you try to achieve
> for NAND.
>

Hi Jörn,

You probably mean the "Intel Sibley NOR".
Yes, the concept is same. It want to use write buffer but oob in NAND.
I hope to modify the code with small modification to work the MLC NAND.
As written in previous mail, we only update the mtd->flags and sub
page value at driver level.

Thank you,
Kyungmin Park

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Kyungmin Park]

On 11/28/07, Jörn Engel <joern@logfs.org> wrote:
> On Wed, 28 November 2007 08:28:56 +0900, Kyungmin Park wrote:
> >
> > Umm, I'm not sure it's the right term. "sub-page" doesn't cover the
> > oob area. It's only meanful at data area.
> > Anyway it should be sub-page is 0 in MLC case.
>
> Maybe we need a good description of the problem before we proceed.  This
> argument about NOP vs. sub-page sounds as if not everyone has the same
> definition for those words.  Without agreeing on those definitions, we
> could argue for weeks without useful results.
>
> Kyungmin, can you describe the problem again, including definitions of
> potentially confusing terms?
>

sub-page: possible write size(?)
MTD usually write data with 'page size' (1KiB, 2KiB, and 4KiB) but
some upper layer such as UBI can write it with 'sub-page size (sector
size)'.

Number Of Program (NOP): How many times flash can be programed.
In general SLC NAND has 4 but this value will be smaller as the
technology is advanced and MLC NAND has 1.

The problem is that current JFFS2 implementation uses the NOP 2, data
area and oob area in NAND case. It breaks the NOP 1 limitation in MLC
case.

If the description is wrong, please let me know.

Thank you,
Kyungmin Park

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Jörn Engel]

On Wed, 28 November 2007 11:08:30 +0900, Kyungmin Park wrote:
> 
> sub-page: possible write size(?)
> MTD usually write data with 'page size' (1KiB, 2KiB, and 4KiB) but
> some upper layer such as UBI can write it with 'sub-page size (sector
> size)'.
> 
> Number Of Program (NOP): How many times flash can be programed.
> In general SLC NAND has 4 but this value will be smaller as the
> technology is advanced and MLC NAND has 1.
> 
> The problem is that current JFFS2 implementation uses the NOP 2, data
> area and oob area in NAND case. It breaks the NOP 1 limitation in MLC
> case.
> 
> If the description is wrong, please let me know.

Sounds plausible.  And reading up on the subpage code I start to doubt
its robustness wrt. newer SLC flashes as well.  If the NOP is lower than
the number of ECC steps, trouble is brewing.

But back to your original patch, you want JFFS2 to behave on MLC flashes
just as it already behaves on Sibley and those dreaded STMicro NORs.
And I claim that you should not only reach identical behaviour, but also
share the same code.  There is no point in having two sets of "special"
initializations with the exact same effect.

Maybe add a flag MTD_OOB_WRITEABLE (or some better name) to SLC flashes
and have the nand initializations in JFFS2 depend on those.  Without
this flag, the nor_wbuf_* code is called.  "nor_wbuf" could use a better
name as well, since it extends to more than just NOR flash.

Jörn

-- 
The grand essentials of happiness are: something to do, something to
love, and something to hope for.
-- Allan K. Chalmers

Re: [RFC][PATCH][JFFS2] JFFS2 support for NOP 1

[Kyungmin Park]

On Nov 28, 2007 11:47 AM, Jörn Engel <joern@logfs.org> wrote:
> On Wed, 28 November 2007 11:08:30 +0900, Kyungmin Park wrote:
> >
> > sub-page: possible write size(?)
> > MTD usually write data with 'page size' (1KiB, 2KiB, and 4KiB) but
> > some upper layer such as UBI can write it with 'sub-page size (sector
> > size)'.
> >
> > Number Of Program (NOP): How many times flash can be programed.
> > In general SLC NAND has 4 but this value will be smaller as the
> > technology is advanced and MLC NAND has 1.
> >
> > The problem is that current JFFS2 implementation uses the NOP 2, data
> > area and oob area in NAND case. It breaks the NOP 1 limitation in MLC
> > case.
> >
> > If the description is wrong, please let me know.
>
> Sounds plausible.  And reading up on the subpage code I start to doubt
> its robustness wrt. newer SLC flashes as well.  If the NOP is lower than
> the number of ECC steps, trouble is brewing.
>
> But back to your original patch, you want JFFS2 to behave on MLC flashes
> just as it already behaves on Sibley and those dreaded STMicro NORs.
> And I claim that you should not only reach identical behaviour, but also
> share the same code.  There is no point in having two sets of "special"
> initializations with the exact same effect.
>
> Maybe add a flag MTD_OOB_WRITEABLE (or some better name) to SLC flashes
> and have the nand initializations in JFFS2 depend on those.  Without
> this flag, the nor_wbuf_* code is called.  "nor_wbuf" could use a better
> name as well, since it extends to more than just NOR flash.
>

Okay, I will add the MTD_OOB_WRITABLE as NOR.

Are there more opinions or comments?
If not, I will send the updated patch.

Thank you,
Kyungmin Park