Re: [patch 01/14] mtd: Flex-OneNAND support

Re: [patch 01/14] mtd: Flex-OneNAND support

[apgmoorthy]

Hi Artem , 

On Sat May 23 05:35:19 EDT 2009 dedekind@infradead.org wrote :

>I get the following warning when compile this:

>drivers/mtd/onenand/onenand_base.c:3263: warning: 'flexonenand_setup'
>defined but not used 

I tried on the MTD Git  and have seen the warning as you mentioned.
'#ifndef MODULE' around the function 'flexonenand_setup' should help to get away with that warning.
We will post a patch.

Against which Fork , the patch should be ? 

With Regards
  Moorthy

Re: [patch 01/14] mtd: Flex-OneNAND support

[Artem Bityutskiy]

Hi,

On Mon, 2009-05-25 at 16:22 +0530, apgmoorthy wrote:
> On Sat May 23 05:35:19 EDT 2009 dedekind@infradead.org wrote :
> 
> >I get the following warning when compile this:
> 
> >drivers/mtd/onenand/onenand_base.c:3263: warning: 'flexonenand_setup'
> >defined but not used 
> 
> I tried on the MTD Git  and have seen the warning as you mentioned.
> '#ifndef MODULE' around the function 'flexonenand_setup' should help to get away with that warning.
> We will post a patch.
> 
> Against which Fork , the patch should be ? 

Dunno. Probably 2.6.30-rc7? Also, I think the loooong printk's
lines in the patch are ugly. If you could make them nicer,
it'd be cool. But whole MTD is full of printk's like this, so
I did not complain about that.

I wanted to take your patch to my l2-mtd-2.6.git tree, and then
ping dwmw2 to pull from it. But because of the warning and
these ugly printk's I decided that I will rather not do this
so far.

Also, I'm not sure about #ifndef MODULE. AFAIK, the whole
'__setup()' thing is not supposed to be used in drivers.
But I'm not sure. I'll leave this for Andrew to comment on :-)

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

RE: [patch 01/14] mtd: Flex-OneNAND support

[apgmoorthy]

Hi Artem,

On Monday, May 25, 2009 5:12 PM dedekind@infradead.org wrote :

>> 
>> I tried on the MTD Git  and have seen the warning as you mentioned.
>> '#ifndef MODULE' around the function 'flexonenand_setup' should help to get away with that warning.
>> We will post a patch.
>> 
>> Against which Fork , the patch should be ? 

>Dunno. Probably 2.6.30-rc7?
        - Then is it on linus-fallBack fork of MTD 2.6 Git ?
          You tried on Which Git/Fork ? 
    
> Also, I think the loooong printk's lines in the patch are ugly. If you could make them nicer, it'd be cool. 
> But whole MTD is full of printk's like this, so I did not complain about that.
        - Got your concern , but felt the printk's convey to their purpose.
 
>I wanted to take your patch to my l2-mtd-2.6.git tree, and then ping dwmw2 to pull from it. But because of the warning and these ugly printk's I decided >that I will rather not do this so far.
        
>Also, I'm not sure about #ifndef MODULE. AFAIK, the whole '__setup()' thing is not supposed to be used in drivers.     
>But I'm not sure. I'll leave this for Andrew to comment on :-)
           - OK , But this is the way we see to make the Boundary Setting User Configurable , without compiling the Kernel.

With Regards
 Moorthy
         

Re: [patch 01/14] mtd: Flex-OneNAND support

[Artem Bityutskiy]

On Mon, 2009-05-25 at 16:22 +0530, apgmoorthy wrote:
> Hi Artem , 
> 
> On Sat May 23 05:35:19 EDT 2009 dedekind@infradead.org wrote :
> 
> >I get the following warning when compile this:
> 
> >drivers/mtd/onenand/onenand_base.c:3263: warning: 'flexonenand_setup'
> >defined but not used 
> 
> I tried on the MTD Git  and have seen the warning as you mentioned.
> '#ifndef MODULE' around the function 'flexonenand_setup' should help to get away with that warning.
> We will post a patch.

Does this mean that if I build OneNAND as a module I have no
possibility to define FlexOneNAND boundary?

Sorry if I missed the discussion, but why you cannot use something
like 'module_param_array()' ?
-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [patch 01/14] mtd: Flex-OneNAND support

[David Woodhouse]

On Wed, 2009-05-27 at 16:55 +0300, Artem Bityutskiy wrote:
> On Mon, 2009-05-25 at 16:22 +0530, apgmoorthy wrote:
> > I tried on the MTD Git  and have seen the warning as you mentioned.
> > '#ifndef MODULE' around the function 'flexonenand_setup' should help
> > to get away with that warning.
> > We will post a patch.
> 
> Does this mean that if I build OneNAND as a module I have no
> possibility to define FlexOneNAND boundary?
> 
> Sorry if I missed the discussion, but why you cannot use something
> like 'module_param_array()' ?

That certainly seems like it would be the best answer.
Can we have a patch to do that please?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [patch 01/14] mtd: Flex-OneNAND support

[Amit Kumar Sharma]

Hi David & Artem,

We will take care comments.

Thanks
Amit


----- Original Message ----- 
From: "David Woodhouse" <dwmw2@infradead.org>
To: <dedekind@infradead.org>
Cc: <vishak.g@samsung.com>; "apgmoorthy" 
<moorthy.apg@samsung.com>; <amul.saha@samsung.com>; 
<kyungmin.park@samsung.com>; 
<linux-mtd@lists.infradead.org>; <akpm@linux-foundation.org>
Sent: Tuesday, June 09, 2009 7:08 PM
Subject: Re: [patch 01/14] mtd: Flex-OneNAND support


> On Wed, 2009-05-27 at 16:55 +0300, Artem Bityutskiy wrote:
>> On Mon, 2009-05-25 at 16:22 +0530, apgmoorthy wrote:
>> > I tried on the MTD Git  and have seen the warning as 
>> > you mentioned.
>> > '#ifndef MODULE' around the function 
>> > 'flexonenand_setup' should help
>> > to get away with that warning.
>> > We will post a patch.
>>
>> Does this mean that if I build OneNAND as a module I have 
>> no
>> possibility to define FlexOneNAND boundary?
>>
>> Sorry if I missed the discussion, but why you cannot use 
>> something
>> like 'module_param_array()' ?
>
> That certainly seems like it would be the best answer.
> Can we have a patch to do that please?
>
> -- 
> David Woodhouse                            Open Source 
> Technology Centre
> David.Woodhouse@intel.com 
> Intel Corporation
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/ 

Re: [patch 01/14] mtd: Flex-OneNAND support

[Amul Saha]

>2009/6/9 David Woodhouse <dwmw2@infradead.org>:
>> On Wed, 2009-05-27 at 16:55 +0300, Artem Bityutskiy wrote:
>>
>> Does this mean that if I build OneNAND as a module I have no
>> possibility to define FlexOneNAND boundary?
>>
>> Sorry if I missed the discussion, but why you cannot use something
>> like 'module_param_array()' ?

We have made use of module_param for getting the parameters for Boundary
Setting.

>
> That certainly seems like it would be the best answer.

Yes, we too see that.

> Can we have a patch to do that please?

Sending the Patch, for supporting Flex-OneNAND as a module, taking Boundary setting parameters.

Regards,
Amul Kumar Saha

Re: [patch 01/14] mtd: Flex-OneNAND support

[Amul Saha]

This patch now adds support for Flex-OneNAND to be used as a module,
it also supports Boundary setting at module insertion time

Signed-off-by: Amul Kumar Saha <amul.saha at samsung.com>
Signed-off-by: Vishak G <vishak.g at samsung.com>
---
 onenand_base.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 8d4c9c2..a91133b 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -20,6 +20,7 @@

 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/delay.h>
@@ -31,6 +32,18 @@

 #include <asm/io.h>

+#ifdef MODULE
+static char *flex_bdry_info;
+
+module_param(flex_bdry_info, charp, 0400);
+MODULE_PARM_DESC(flex_bdry_info, "SLC Boundary information for Flex-OneNAND"
+				 "Syntax:flex_bdry_info=DIE_BDRY,LOCK,..."
+				 "DIE_BDRY: SLC boundary of the die"
+				 "LOCK: Locking information for SLC boundary"
+				 "    : 0->Set boundary in unlocked status"
+				 "    : 1->Set boundary in locked status");
+#endif
+
 /* Default Flex-OneNAND boundary and lock respectively */
 static int flex_bdry[MAX_DIES * 2] = { -1, 0, -1, 0 };

@@ -3271,7 +3284,9 @@ static int flexonenand_setup(char *s)
 	return 1;
 }

+#ifndef MODULE
 __setup("onenand.bdry=", flexonenand_setup);
+#endif

 /**
  * onenand_probe - [OneNAND Interface] Probe the OneNAND device
@@ -3456,6 +3471,10 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
 	if (onenand_probe(mtd))
 		return -ENXIO;

+#ifdef MODULE
+	flexonenand_setup(flex_bdry_info);
+#endif
+
 	/* Set Sync. Burst Read after probing */
 	if (this->mmcontrol) {
 		printk(KERN_INFO "OneNAND Sync. Burst Read support\n");

Re: [patch 01/14] mtd: Flex-OneNAND support

[Artem Bityutskiy]

On Thu, 2009-06-11 at 14:53 +0530, Amul Saha wrote:
> This patch now adds support for Flex-OneNAND to be used as a module,
> it also supports Boundary setting at module insertion time
> 
> Signed-off-by: Amul Kumar Saha <amul.saha at samsung.com>
> Signed-off-by: Vishak G <vishak.g at samsung.com>
> ---
>  onenand_base.c |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
> index 8d4c9c2..a91133b 100644
> --- a/drivers/mtd/onenand/onenand_base.c
> +++ b/drivers/mtd/onenand/onenand_base.c
> @@ -20,6 +20,7 @@
> 
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/moduleparam.h>
>  #include <linux/init.h>
>  #include <linux/sched.h>
>  #include <linux/delay.h>
> @@ -31,6 +32,18 @@
> 
>  #include <asm/io.h>
> 
> +#ifdef MODULE
> +static char *flex_bdry_info;
> +
> +module_param(flex_bdry_info, charp, 0400);
> +MODULE_PARM_DESC(flex_bdry_info, "SLC Boundary information for Flex-OneNAND"
> +				 "Syntax:flex_bdry_info=DIE_BDRY,LOCK,..."
> +				 "DIE_BDRY: SLC boundary of the die"
> +				 "LOCK: Locking information for SLC boundary"
> +				 "    : 0->Set boundary in unlocked status"
> +				 "    : 1->Set boundary in locked status");
> +#endif
> +
>  /* Default Flex-OneNAND boundary and lock respectively */
>  static int flex_bdry[MAX_DIES * 2] = { -1, 0, -1, 0 };
> 
> @@ -3271,7 +3284,9 @@ static int flexonenand_setup(char *s)
>  	return 1;
>  }
> 
> +#ifndef MODULE
>  __setup("onenand.bdry=", flexonenand_setup);
> +#endif

Why you still need this? Module parameters still work if your stuff
is compiled in - you just pass the "onenand.flex_bdry_info=" kernel
parameter.

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

Re: [patch 01/14] mtd: Flex-OneNAND support

[Amul Saha]

>> +#ifndef MODULE
>>  __setup("onenand.bdry=", flexonenand_setup);
>> +#endif
>
> Why you still need this? Module parameters still work if your stuff
> is compiled in - you just pass the "onenand.flex_bdry_info=" kernel
> parameter.
>
OK.
Sending across the updated patch.

Regards,
Amul Kumar Saha 

Re: [patch 01/14] mtd: Flex-OneNAND support

[Amul Saha]

This patch now adds support for Flex-OneNAND to be used as a module,
it also supports Boundary setting at module insertion time

Signed-off-by: Amul Kumar Saha <amul.saha at samsung.com>
Signed-off-by: Vishak G <vishak.g at samsung.com>
---
 onenand_base.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 8d4c9c2..6d2086f 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -20,6 +20,7 @@

 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/delay.h>
@@ -31,6 +32,16 @@

 #include <asm/io.h>

+static char *flex_bdry_info;
+
+module_param(flex_bdry_info, charp, 0400);
+MODULE_PARM_DESC(flex_bdry_info, "SLC Boundary information for Flex-OneNAND"
+				 "Syntax:flex_bdry_info=DIE_BDRY,LOCK,..."
+				 "DIE_BDRY: SLC boundary of the die"
+				 "LOCK: Locking information for SLC boundary"
+				 "    : 0->Set boundary in unlocked status"
+				 "    : 1->Set boundary in locked status");
+
 /* Default Flex-OneNAND boundary and lock respectively */
 static int flex_bdry[MAX_DIES * 2] = { -1, 0, -1, 0 };

@@ -3456,6 +3467,10 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
 	if (onenand_probe(mtd))
 		return -ENXIO;

+#ifdef MODULE
+	flexonenand_setup(flex_bdry_info);
+#endif
+
 	/* Set Sync. Burst Read after probing */
 	if (this->mmcontrol) {
 		printk(KERN_INFO "OneNAND Sync. Burst Read support\n");

Re: [patch 01/14] mtd: Flex-OneNAND support

[Artem Bityutskiy]

On Fri, 2009-06-12 at 15:56 +0530, Amul Saha wrote:
> This patch now adds support for Flex-OneNAND to be used as a module,
> it also supports Boundary setting at module insertion time
> 
> Signed-off-by: Amul Kumar Saha <amul.saha at samsung.com>
> Signed-off-by: Vishak G <vishak.g at samsung.com>
> ---
>  onenand_base.c |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
> index 8d4c9c2..6d2086f 100644
> --- a/drivers/mtd/onenand/onenand_base.c
> +++ b/drivers/mtd/onenand/onenand_base.c
> @@ -20,6 +20,7 @@
> 
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/moduleparam.h>
>  #include <linux/init.h>
>  #include <linux/sched.h>
>  #include <linux/delay.h>
> @@ -31,6 +32,16 @@
> 
>  #include <asm/io.h>
> 
> +static char *flex_bdry_info;
> +
> +module_param(flex_bdry_info, charp, 0400);
> +MODULE_PARM_DESC(flex_bdry_info, "SLC Boundary information for Flex-OneNAND"
> +				 "Syntax:flex_bdry_info=DIE_BDRY,LOCK,..."
> +				 "DIE_BDRY: SLC boundary of the die"
> +				 "LOCK: Locking information for SLC boundary"
> +				 "    : 0->Set boundary in unlocked status"
> +				 "    : 1->Set boundary in locked status");
> +
>  /* Default Flex-OneNAND boundary and lock respectively */
>  static int flex_bdry[MAX_DIES * 2] = { -1, 0, -1, 0 };
> 
> @@ -3456,6 +3467,10 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
>  	if (onenand_probe(mtd))
>  		return -ENXIO;
> 
> +#ifdef MODULE
> +	flexonenand_setup(flex_bdry_info);
> +#endif

Why do you need this ifdef? What is the fundamental difference between
onenand.ko as a module and onenand compiled-in?

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

Re: [patch 01/14] mtd: Flex-OneNAND support

[Amul Saha]

Hi Artem,

>> +#ifdef MODULE
>> + flexonenand_setup(flex_bdry_info);
>> +#endif
>
> Why do you need this ifdef? What is the fundamental difference between
> onenand.ko as a module and onenand compiled-in?
>

flexonenand_setup( ) need not be called, when OneNAND is built-in.
This function-call will cause overhead unwantedly on every boot, during OneNAND scan.

flexonenand_setup( ) call is needed only when it has been built as a module.

Regards,
Amul Kumar Saha 

Re: [patch 01/14] mtd: Flex-OneNAND support

[Artem Bityutskiy]

On Fri, 2009-06-12 at 17:27 +0530, Amul Saha wrote:
> Hi Artem,
> 
> >> +#ifdef MODULE
> >> + flexonenand_setup(flex_bdry_info);
> >> +#endif
> >
> > Why do you need this ifdef? What is the fundamental difference between
> > onenand.ko as a module and onenand compiled-in?
> >
> 
> flexonenand_setup( ) need not be called, when OneNAND is built-in.
> This function-call will cause overhead unwantedly on every boot, during OneNAND scan.
> 
> flexonenand_setup( ) call is needed only when it has been built as a module.

Why?

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

Re: [patch 01/14] mtd: Flex-OneNAND support

[David Woodhouse]

On Fri, 2009-06-12 at 15:03 +0300, Artem Bityutskiy wrote:
> 
> > flexonenand_setup( ) call is needed only when it has been built as a
> module.
> 
> Why?

Because they're adding a 'flex_bdry_info' module parameter which is a
string, that they then process just the same way as the original command
line option.

I think that's the wrong approach -- please use module_param_array()
instead, so it sets the contents of the flex_bdry[] array directly and
you can remove the flexonenand_setup() function completely.

Please ensure that your email addresses in the Signed-off-by: line are
correct, too. None of this 'user at domain' nonsense.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [patch 01/14] mtd: Flex-OneNAND support

[Amul Saha]

>> >> +#ifdef MODULE
>> >> + flexonenand_setup(flex_bdry_info);
>> >> +#endif
>> >
>> > Why do you need this ifdef? What is the fundamental difference between
>> > onenand.ko as a module and onenand compiled-in?
>> >
>>
>> flexonenand_setup( ) need not be called, when OneNAND is built-in.
>> This function-call will cause overhead unwantedly on every boot, during OneNAND scan.
>>
>> flexonenand_setup( ) call is needed only when it has been built as a module.
>
> Why?
>

When Flex-OneNAND is built-in, the SLC boundary can be set by kernel command line.
During the boot up time, flexonenand_setup() gets invoked by kernel, on parsing the kernel
command line.

But when Flex-OneNAND is built as a module, SLC boundary information is passed as module
parameter.
So in this case flexonenand_setup() has to be called explicitly during insomd time
(module_init),  to set the desired boundary.
So without a compilation macro (#ifdef MODULE) flexonenand_setup() is invoked in onenand_scan,
even when it is built-in which is not needed.

I am not getting your point, could you please clarify?

Regards,
Amul Kumar Saha 

Re: [patch 01/14] mtd: Flex-OneNAND support

[David Woodhouse]

On Fri, 2009-06-12 at 18:46 +0530, Amul Saha wrote:
> >> >> +#ifdef MODULE
> >> >> + flexonenand_setup(flex_bdry_info);
> >> >> +#endif
> >> >
> >> > Why do you need this ifdef? What is the fundamental difference between
> >> > onenand.ko as a module and onenand compiled-in?
> >> >
> >>
> >> flexonenand_setup( ) need not be called, when OneNAND is built-in.
> >> This function-call will cause overhead unwantedly on every boot, during OneNAND scan.
> >>
> >> flexonenand_setup( ) call is needed only when it has been built as a module.
> >
> > Why?
> >
> 
> When Flex-OneNAND is built-in, the SLC boundary can be set by kernel command line.
> During the boot up time, flexonenand_setup() gets invoked by kernel, on parsing the kernel
> command line.
> 
> But when Flex-OneNAND is built as a module, SLC boundary information is passed as module
> parameter.
> So in this case flexonenand_setup() has to be called explicitly during insomd time
> (module_init),  to set the desired boundary.
> So without a compilation macro (#ifdef MODULE) flexonenand_setup() is invoked in onenand_scan,
> even when it is built-in which is not needed.
> 
> I am not getting your point, could you please clarify?

You can remove flexonenand_setup() and the __setup("onenand_brdy=")
completely, and use _only_ "module_param_array()". That will work both
for modules and for the built-in case.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [patch 01/14] mtd: Flex-OneNAND support

[Amul Saha]

Hi David,

>
> You can remove flexonenand_setup() and the __setup("onenand_brdy=")
> completely, and use _only_ "module_param_array()". That will work both
> for modules and for the built-in case.
>

OK , Got your point.
Working towards it, will come up with the updated patch.

Regards
Amul Kumar Saha 

Re: [patch 01/14] mtd: Flex-OneNAND support

[Amul Saha]

This patch now adds support for Flex-OneNAND to be used as a module,
it also supports Boundary setting at module insertion time

Signed-off-by: Amul Kumar Saha <amul.saha@samsung.com>
Signed-off-by: Vishak G <vishak.g@samsung.com>
---
 onenand_base.c |   28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 8d4c9c2..5efb25b 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -20,6 +20,7 @@

 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/delay.h>
@@ -34,6 +35,14 @@
 /* Default Flex-OneNAND boundary and lock respectively */
 static int flex_bdry[MAX_DIES * 2] = { -1, 0, -1, 0 };

+module_param_array(flex_bdry, int, NULL, 0400);
+MODULE_PARM_DESC(flex_bdry,	"SLC Boundary information for Flex-OneNAND"
+				"Syntax:flex_bdry=DIE_BDRY,LOCK,..."
+				"DIE_BDRY: SLC boundary of the die"
+				"LOCK: Locking information for SLC boundary"
+				"    : 0->Set boundary in unlocked status"
+				"    : 1->Set boundary in locked status");
+
 /**
  *  onenand_oob_128 - oob info for Flex-Onenand with 4KB page
  *  For now, we expose only 64 out of 80 ecc bytes
@@ -3255,25 +3264,6 @@ out:
 }

 /**
- * flexonenand_setup - 	capture Flex-OneNAND boundary and lock
- * 			values  passed as kernel parameters
- * @param s	kernel parameter string
- */
-static int flexonenand_setup(char *s)
-{
-	int ints[5], i;
-
-	s = get_options(s, 5, ints);
-
-	for (i = 0; i < ints[0]; i++)
-		flex_bdry[i] = ints[i + 1];
-
-	return 1;
-}
-
-__setup("onenand.bdry=", flexonenand_setup);
-
-/**
  * onenand_probe - [OneNAND Interface] Probe the OneNAND device
  * @param mtd		MTD device structure
  *

Re: MLC Support in JFFS2

[David Woodhouse]

On Fri, 2009-06-19 at 12:20 +0530, apgmoorthy wrote:
> Hi David,
> 
> Currenlty , Is there a way to use JFFS2 with MLC Memories ( NOP == 1 )?
> 
> If not , Felt that any one of the following patch can be of use
> 
> 1. http://lists.infradead.org/pipermail/linux-mtd/2007-December/020047.html
>           - Which removes the Clean Marker.

There was a reason for the cleanmarker -- it saved us from using
partly-erased blocks for data. Do you have some other way to guarantee
that this won't happen?

(Sorry, I think we may have had this discussion before, but I've
forgotten the conclusion, if there was one. It was academic at the time,
since we didn't have any solution for the paired-page insanity anyway. I
was mostly just burying my head in the sand and hoping this stupid 'MLC'
crap would go away.)

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

RE: MLC Support in JFFS2

[apgmoorthy]

Hi David,  

On Tuesday, June 23, 2009 12:45 AM David Woodhouse wrote :
 
>> If not , Felt that any one of the following patch can be of use
>> 
>> 1. http://lists.infradead.org/pipermail/linux-mtd/2007-December/020047.html
>>           - Which removes the Clean Marker.

>There was a reason for the cleanmarker -- it saved us from using partly-erased blocks for data. Do you have some other way to
guarantee that this won't 
>happen?
- How about the second One (Refer Below Links), which leaves 1page/block for Cleanmarker.
   http://lists.infradead.org/pipermail/linux-mtd/2008-September/023101.html
   http://lists.infradead.org/pipermail/linux-mtd/2008-September/023044.html 
  
>(Sorry, I think we may have had this discussion before, but I've forgotten the conclusion, if there was one. It was academic at the
time, since we didn't >have any solution for the paired-page insanity anyway. 
- Right , I dont see any detailed discussion on this regard in the list.
  

With Regards
 Moorthy

RE: MLC Support in JFFS2

[David Woodhouse]

Your mail program is behaving very strangely -- your quotes are quite
hard to read. Is there any chance you could use a better one? I've tried
to clean it up, but it tends to make your replies hard to read. Some of
your replies are lacking correct References: headers too, so the
threading is broken.

On Tue, 2009-06-23 at 18:22 +0530, apgmoorthy wrote:
> >There was a reason for the cleanmarker -- it saved us from using 
> > partly-erased blocks for data. Do you have some other way to
> > guarantee that this won't happen?
>
> - How about the second One (Refer Below Links), which leaves
> 1page/block for Cleanmarker.
> 
> http://lists.infradead.org/pipermail/linux-mtd/2008-September/023101.html   http://lists.infradead.org/pipermail/linux-mtd/2008-September/023044.html 

Using one page per block for the cleanmarker would work, but seems
wasteful. I can think of one more approach which we might take -- erase
blocks only as we need them.

So instead of erasing blocks without cleanmarker _immediately_ on
startup, we just put them on a 'to be erased' list. Whenever the
empty_list is getting short of blocks, we erase another block from the
'to be erased' list and move it to the empty list.

It does mean that if you keep rebooting over and over again, you're
probably going to increase your erase count. But with a long-lived
system, it's likely to be more efficient than the large cleanmarker?

What do you think?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

RE: MLC Support in JFFS2

[apgmoorthy]

 
On Tuesday, June 23, 2009 6:50 PM David Woodhouse wrote:
 
> Your mail program is behaving very strangely -- your quotes are quite
> hard to read. Is there any chance you could use a better one? 

- Earlier Mailer had some issues , Now using a different one.
   
> So instead of erasing blocks without cleanmarker _immediately_ on
> startup, we just put them on a 'to be erased' list. Whenever the
> empty_list is getting short of blocks, we erase another block from the
> 'to be erased' list and move it to the empty list.
> 
> It does mean that if you keep rebooting over and over again, you're
> probably going to increase your erase count. But with a long-lived
> system, it's likely to be more efficient than the large cleanmarker?
> 
> What do you think?
> 

- Yes , the solution looks a better tradeoff.
  Will work in that direction and get a pacth.
  Later we can look into a way to optimize erase count. 

With Regards
 Moorthy

Re: MLC Support in JFFS2

[Amul Kumar Saha]

Hi David,

We have come up with an alternate solution to support MLC devices on JFFS2.

>>
>> Currenlty , Is there a way to use JFFS2 with MLC Memories ( NOP == 1 )?
>>
>> If not , Felt that any one of the following patch can be of use
>>
>> 1. http://lists.infradead.org/pipermail/linux-mtd/2007-December/020047.html
>>           - Which removes the Clean Marker.
>
> There was a reason for the cleanmarker -- it saved us from using
> partly-erased blocks for data. Do you have some other way to guarantee
> that this won't happen?
>
> (Sorry, I think we may have had this discussion before, but I've
> forgotten the conclusion, if there was one. It was academic at the time,
> since we didn't have any solution for the paired-page insanity anyway. I
> was mostly just burying my head in the sand and hoping this stupid 'MLC'
> crap would go away.)

New approach:-

We will have something known as the TOKEN_BLOCK.

TOKEN_BLOCK: A block used to store all the ready_to_use blocks related info.

ready_to_use blocks: These are blocks which are erased and ready to use,
 but don't have a usual CLEANMARKER on them. To save NOP for
 the 1st page of each block.

How it works:
1. Ask GC to provide a block, and use it as TOKEN_BLOCK on need basis.
2. In the provided block, we store all ready_to_use block info.
3. On remount; this info needs to be retreived.
    So, we shall make use of MAGIC_NUMBER signifying TOKEN_BLOCK,
    to identify the block containing the info.

Scan has to be modified, to identify the TOKEN_BLOCK and act accordingly.

Handling TOKEN_BLOCK
1. Make request for a block to be used as TOKEN_BLOCK.
2. List all the ready_to_use block info on the TOKEN_BLOCK contiguously.
3. Write the TOKEN_BLOCK's MAGIC_NUMBER in the OOB Area alongwith the
 ready_to_use info, in the Main Area.
4. On next update, ask GC to provide a fresh block.
5. Update this block accordingly as TOKEN_BLOCK.
6. Discard the previous TOKEN_BLOCK, and put it on dirty_list.

Let me know, about the feasibility of this design from your perspective.

This is just a base plan, and obvisouly there are a lot of things that have to be looked
into, before getting into the real imlpementation.

Regards,
Amul Kumar Saha

Re: MLC Support in JFFS2

[massimo cirillo]

Hi Amul,

> 3. Write the TOKEN_BLOCK's MAGIC_NUMBER in the OOB Area alongwith the
>  ready_to_use info, in the Main Area.
I think you should protect each ready_to_use block info in the TOKEN_BLOCK
with a CRC - as it was a node of JFFS2 - against power loss corruption.

> 4. On next update, ask GC to provide a fresh block.
What do you mean ? Each time a block is erased do you
update the TOKEN_BLOCK by using a new one ?

Re: MLC Support in JFFS2

[Amul Kumar Saha]

Hi,

>> 3. Write the TOKEN_BLOCK's MAGIC_NUMBER in the OOB Area alongwith the
>>  ready_to_use info, in the Main Area.
> I think you should protect each ready_to_use block info in the TOKEN_BLOCK
> with a CRC - as it was a node of JFFS2 - against power loss corruption.
>

Sure, I'll take that into account.

>> 4. On next update, ask GC to provide a fresh block.
> What do you mean ? Each time a block is erased do you
> update the TOKEN_BLOCK by using a new one ?
>

No.
That would be done only during a proper unmount or idle time.
As this block just contains ready_to_use block info,
that can be reconstructed, on remount.
Because any updation on fixed intervals will not be of use.
Neither will it be feasbale to update the TOKEN_BLOCK on every erase.
Please do comment.

Regards,
Amul 

Re: MLC Support in JFFS2

[massimo cirillo]

Ok,
but at the remount how do you verify if the TOKEN_BLOCK
is updated or not (in case of power loss) ? I suggest using
a commit mark (one page wasted) at the end of the update
procedure.
So you'll have in the TOKEN_BLOCK the following layout:

MAGIC_NUMBER of the TOKEN_BLOCK
ready_to_use info
commit mark

What do you think ?

2010/4/5 Amul Kumar Saha <amul.saha@samsung.com>:
> Hi,
>
>>> 3. Write the TOKEN_BLOCK's MAGIC_NUMBER in the OOB Area alongwith the
>>>  ready_to_use info, in the Main Area.
>> I think you should protect each ready_to_use block info in the TOKEN_BLOCK
>> with a CRC - as it was a node of JFFS2 - against power loss corruption.
>>
>
> Sure, I'll take that into account.
>
>>> 4. On next update, ask GC to provide a fresh block.
>> What do you mean ? Each time a block is erased do you
>> update the TOKEN_BLOCK by using a new one ?
>>
>
> No.
> That would be done only during a proper unmount or idle time.
> As this block just contains ready_to_use block info,
> that can be reconstructed, on remount.
> Because any updation on fixed intervals will not be of use.
> Neither will it be feasbale to update the TOKEN_BLOCK on every erase.
> Please do comment.
>
> Regards,
> Amul
>
>
>

Re: MLC Support in JFFS2

[Amul Kumar Saha]

Hi,


>Ok,
>but at the remount how do you verify if the TOKEN_BLOCK
>is updated or not (in case of power loss) ? I suggest using
>a commit mark (one page wasted) at the end of the update
>procedure.
>So you'll have in the TOKEN_BLOCK the following layout:
>
>MAGIC_NUMBER of the TOKEN_BLOCK
>ready_to_use info
>commit mark
>
>What do you think ?
>

Yes.
However, the TOKEN_BLOCK update would then
only be limited to mount and unmount.
As writing a commit mark while the system is still running,
would validate an invalid block on abrupt power-off.

Regards,
Amul 

Re: MLC Support in JFFS2

[massimo cirillo]

Hi,
I agree with you, and the following is my thinking.
Let's suppose we have a flash with 2KB page, 128KB block,
4096 blocks (512MB as size of the flash).
The cleanmarker would take one page for every block, so the
wasted space is 2KB*4096=8MB. Instead if we put the erased
info in a single block (TOKEN_BLOCK) we waste only 128KB.
But we need to consider the overhead in the management of
such a block. We need to store information for 4096 blocks,
for example an array of 4096 bit (1=erased, 0=not erased),
protected by ECC (against read error) and CRC (against
power loss in writing the array on the block). We need 2 pages
for each instance of the array. The question is: how often do we
need to update this array? If we choose to update each time it is
needed (that is when a block is  just erased and just before a block
is taken from the free list), at the 32nd update we should erase the
TOKEN_BLOCK and allocate a new one. IMO this overhead is
unacceptable in terms of time performance. So we update the info
only at the unmount and when the system is idle.
Moreover, there is the problem of power loss between two updates:
how to recognize at the mount that the array info is updated? In my
opinion the solution is to write an invalidation mark (one page with
all zeros) just after the instance of the array when the instance
becomes invalid.

Bye

2010/4/9 Amul Kumar Saha <amul.saha@samsung.com>:
> Hi,
>
>
>>Ok,
>>but at the remount how do you verify if the TOKEN_BLOCK
>>is updated or not (in case of power loss) ? I suggest using
>>a commit mark (one page wasted) at the end of the update
>>procedure.
>>So you'll have in the TOKEN_BLOCK the following layout:
>>
>>MAGIC_NUMBER of the TOKEN_BLOCK
>>ready_to_use info
>>commit mark
>>
>>What do you think ?
>>
>
> Yes.
> However, the TOKEN_BLOCK update would then
> only be limited to mount and unmount.
> As writing a commit mark while the system is still running,
> would validate an invalid block on abrupt power-off.
>
> Regards,
> Amul
>
>
>

Re: MLC Support in JFFS2

[massimo cirillo]

yes,
the update only at the unmount and the usage of the commit
mark are ok for me.

2010/4/15 Raghav Gupta <raghav.gupta@partner.samsung.com>:
> Resending it due to formatting issues. My bad.
>
> Hi,
>
>> I agree with you, and the following is my thinking.
>> Let's suppose we have a flash with 2KB page, 128KB block,
>> 4096 blocks (512MB as size of the flash).
>> The cleanmarker would take one page for every block, so the
>> wasted space is 2KB*4096=8MB. Instead if we put the erased
>> info in a single block (TOKEN_BLOCK) we waste only 128KB.
>> But we need to consider the overhead in the management of
>> such a block. We need to store information for 4096 blocks,
>> for example an array of 4096 bit (1=erased, 0=not erased),
>> protected by ECC (against read error) and CRC (against
>> power loss in writing the array on the block). We need 2 pages
>> for each instance of the array. The question is: how often do we
>> need to update this array? If we choose to update each time it is
>> needed (that is when a block is  just erased and just before a block
>> is taken from the free list), at the 32nd update we should erase the
>> TOKEN_BLOCK and allocate a new one. IMO this overhead is
>> unacceptable in terms of time performance. So we update the info
>> only at the unmount and when the system is idle.
>> Moreover, there is the problem of power loss between two updates:
>> how to recognize at the mount that the array info is updated? In my
>> opinion the solution is to write an invalidation mark (one page with
>> all zeros) just after the instance of the array when the instance
>> becomes invalid.
>
>
> Yes, I agree with you. The overhead involved in updating the token block
> on every change in the "ready_to_use" list is unacceptable.
> So, we can update the TOKEN_BLOCK only at unmount.
>
> Also we can authenticate that the TOKEN_BLOCK as valid
> by using a "valid/invalid flag"(commit mark)
>
> And to guard against reading of an older version of
> a TOKEN_BLOCK (on power failure) as valid by immediately
> deleting the block on retrieval of the 'ready_to_use' list.
>
> Regards,
> Raghav Gupta
>
>
>

Re: MLC Support in JFFS2

[massimo cirillo]

Resending the message due to a Mail Delivery
error returned by raghav.gupta@partner.samsung.com.

2010/4/15 massimo cirillo <maxcir@gmail.com>:
> yes,
> the update only at the unmount and the usage of the commit
> mark are ok for me.
>
> 2010/4/15 Raghav Gupta <raghav.gupta@partner.samsung.com>:
>> Resending it due to formatting issues. My bad.
>>
>> Hi,
>>
>>> I agree with you, and the following is my thinking.
>>> Let's suppose we have a flash with 2KB page, 128KB block,
>>> 4096 blocks (512MB as size of the flash).
>>> The cleanmarker would take one page for every block, so the
>>> wasted space is 2KB*4096=8MB. Instead if we put the erased
>>> info in a single block (TOKEN_BLOCK) we waste only 128KB.
>>> But we need to consider the overhead in the management of
>>> such a block. We need to store information for 4096 blocks,
>>> for example an array of 4096 bit (1=erased, 0=not erased),
>>> protected by ECC (against read error) and CRC (against
>>> power loss in writing the array on the block). We need 2 pages
>>> for each instance of the array. The question is: how often do we
>>> need to update this array? If we choose to update each time it is
>>> needed (that is when a block is  just erased and just before a block
>>> is taken from the free list), at the 32nd update we should erase the
>>> TOKEN_BLOCK and allocate a new one. IMO this overhead is
>>> unacceptable in terms of time performance. So we update the info
>>> only at the unmount and when the system is idle.
>>> Moreover, there is the problem of power loss between two updates:
>>> how to recognize at the mount that the array info is updated? In my
>>> opinion the solution is to write an invalidation mark (one page with
>>> all zeros) just after the instance of the array when the instance
>>> becomes invalid.
>>
>>
>> Yes, I agree with you. The overhead involved in updating the token block
>> on every change in the "ready_to_use" list is unacceptable.
>> So, we can update the TOKEN_BLOCK only at unmount.
>>
>> Also we can authenticate that the TOKEN_BLOCK as valid
>> by using a "valid/invalid flag"(commit mark)
>>
>> And to guard against reading of an older version of
>> a TOKEN_BLOCK (on power failure) as valid by immediately
>> deleting the block on retrieval of the 'ready_to_use' list.
>>
>> Regards,
>> Raghav Gupta
>>
>>
>>
>

RE: MLC Support in JFFS2

[Artem Bityutskiy]

On Mon, 2009-06-22 at 19:03 +0530, apgmoorthy wrote:
> Yes it do that .
> There are two things in Handling MLCs
> 1.Paired Page Handling
> This is done in Flex-OneNAND Driver.
> flexonenand_lsb_page_recovery will handle that in MLC area. 

Ah, ok you do this at the driver level - good.

> 2.NOP==1
> This scenario has to be dealt in synchronization with JFFS2.
> This is what i asked about. 
> Pathces , in the links(above)are addressing this.

OK, I see.

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

RE: MLC Support in JFFS2

[apgmoorthy]

Hi Artem ,  

On Monday, June 22, 2009 1:18 PM Artem Bityutskiy wrote :
 
>> Currenlty , Is there a way to use JFFS2 with MLC Memories ?
>> 
>> If not , Felt that any one of the following patch can be of use
>> 
>> 1. http://lists.infradead.org/pipermail/linux-mtd/2007-December/020047.html
>>           - Which removes the Clean Marker.
>> 
>> 2. http://lists.infradead.org/pipermail/linux-mtd/2008-September/023101.html
>>    http://lists.infradead.org/pipermail/linux-mtd/2008-September/023044.html 
>>           - Leaving 1page/Block , for Clean Marker.
>> 
>> or some other solution, which solves the standoff.   
>> 
>> Without MLC Support , Flex-OneNAND is unsuable with JFFS2.

>Is JFFS2 usable with Flex-OneNAND by design? I thought it would have to be able to handle paired pages to become MLC-compatible. Does it do that?

Yes it do that .
There are two things in Handling MLCs
1.Paired Page Handling
This is done in Flex-OneNAND Driver.
flexonenand_lsb_page_recovery will handle that in MLC area. 

2.NOP==1
This scenario has to be dealt in synchronization with JFFS2.
This is what i asked about. 
Pathces , in the links(above)are addressing this.

With Regards
 Moorthy

Re: MLC Support in JFFS2

[Artem Bityutskiy]

apgmoorthy wrote:
> Hi David,
> 
> Currenlty , Is there a way to use JFFS2 with MLC Memories ?
> 
> If not , Felt that any one of the following patch can be of use
> 
> 1. http://lists.infradead.org/pipermail/linux-mtd/2007-December/020047.html
>           - Which removes the Clean Marker.
> 
> 2. http://lists.infradead.org/pipermail/linux-mtd/2008-September/023101.html
>    http://lists.infradead.org/pipermail/linux-mtd/2008-September/023044.html 
>           - Leaving 1page/Block , for Clean Marker.
> 
> or some other solution, which solves the standoff.   
> 
> Without MLC Support , Flex-OneNAND is unsuable with JFFS2.

Is JFFS2 usable with Flex-OneNAND by design? I thought it would have
to be able to handle paired pages to become MLC-compatible. Does it
do that?

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

MLC Support in JFFS2

[apgmoorthy]

Hi David,

Currenlty , Is there a way to use JFFS2 with MLC Memories ?

If not , Felt that any one of the following patch can be of use

1. http://lists.infradead.org/pipermail/linux-mtd/2007-December/020047.html
          - Which removes the Clean Marker.

2. http://lists.infradead.org/pipermail/linux-mtd/2008-September/023101.html
   http://lists.infradead.org/pipermail/linux-mtd/2008-September/023044.html 
          - Leaving 1page/Block , for Clean Marker.

or some other solution, which solves the standoff.   

Without MLC Support , Flex-OneNAND is unsuable with JFFS2.

With Regards
  Moorthy

MLC Support in JFFS2

[payagond]

Hello,



Which is the better way for MLC support in JFFS2?

Is it removing the CLEANMARKER or

reserving the first page of every block for it



If I am using the 2nd option ie., reserving a page in every block -

What else I have to take care of, other than jeb->freesize and 
jeb->used_size?



regards

payagond


________________________________________________________________________
Email and AIM finally together. You've gotta check out free AOL Mail! - 
http://mail.aol.com