linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

[Mike Frysinger]

ive come across a situation where it would have been invaluable to
have the ability to "scrub" the nand flash while running linux.
unfortunately, while i can see options to mark blocks as bad, there
doesnt appear to be any ioctls to mark blocks as good.  this applies
both to the userspace api as well as the kernel api.

further, it seems like if (for whatever reason)  the bad block marker
in the oob region got screwed, there is absolutely no way to recover
(when not using a dedicated bbt).  in
drivers/mtd/nand/nand_base.c:nand_erase_nand(), the very first thing
it does is call nand_block_checkbad().  without a bbt, it'll tail into
chip->block_bad and the default nand_block_bad() reads the bad block
marker out of the oob.  but if the oob region has been screwed, the
block will be thought of as bad, and so you're stuck in a loop.  you
cant reset the oob to 0xff because the oob isnt 0xff ...

the logical thing in my mind would be to extend the userspace mtd abi
to allow a "do the erase even if people think it's bad" option.
perhaps MEMSCRUB ?
-mike

Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

[Artem Bityutskiy]

On Fri, 2010-09-10 at 19:53 -0400, Mike Frysinger wrote:
> the logical thing in my mind would be to extend the userspace mtd abi
> to allow a "do the erase even if people think it's bad" option.
> perhaps MEMSCRUB ?

If you do this, please do not use this name. In UBI we already use term
'scrubbing' for the process when we move contents of eraseblock because
we have bit-flips.

It will be confusing if the same word is used in MTD for "unmarking"
eraseblocks. How about: 'force erase' or 'bad erase' ?

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

Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

[Mike Frysinger]

On Sat, Sep 11, 2010 at 02:32, Artem Bityutskiy wrote:
> On Fri, 2010-09-10 at 19:53 -0400, Mike Frysinger wrote:
>> the logical thing in my mind would be to extend the userspace mtd abi
>> to allow a "do the erase even if people think it's bad" option.
>> perhaps MEMSCRUB ?
>
> If you do this, please do not use this name. In UBI we already use term
> 'scrubbing' for the process when we move contents of eraseblock because
> we have bit-flips.

that doesnt sound like scrubbing at all, but too late now i guess to fix

> It will be confusing if the same word is used in MTD for "unmarking"
> eraseblocks. How about: 'force erase' or 'bad erase' ?

that makes it sound like an option to the existing MEMERASE operation.
 so i guess what if we just do that -- extend the erase_info_user
structure to contain a flags field and add a MEMERASE2 that works with
the larger structure ?  for now we'd only have one option (FORCE), but
it makes it easy to extend in the future.
-mike

Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

[Artem Bityutskiy]

On Sun, 2010-09-12 at 00:03 -0400, Mike Frysinger wrote:
> On Sat, Sep 11, 2010 at 02:32, Artem Bityutskiy wrote:
> > On Fri, 2010-09-10 at 19:53 -0400, Mike Frysinger wrote:
> >> the logical thing in my mind would be to extend the userspace mtd abi
> >> to allow a "do the erase even if people think it's bad" option.
> >> perhaps MEMSCRUB ?
> >
> > If you do this, please do not use this name. In UBI we already use term
> > 'scrubbing' for the process when we move contents of eraseblock because
> > we have bit-flips.
> 
> that doesnt sound like scrubbing at all, but too late now i guess to fix

Not sure, I'm not native English speaker, and actually that was not me
who called the process this way.

> > It will be confusing if the same word is used in MTD for "unmarking"
> > eraseblocks. How about: 'force erase' or 'bad erase' ?
> 
> that makes it sound like an option to the existing MEMERASE operation.
>  so i guess what if we just do that -- extend the erase_info_user
> structure to contain a flags field and add a MEMERASE2 that works with
> the larger structure ?  for now we'd only have one option (FORCE), but
> it makes it easy to extend in the future.

Ohh, this was so stupid of me to not ask people to add extra fields to
'struct erase_info_user64' which was introduced relatively recently... I
always add extra fields to ioctl data structures...

But yeah, what you say sounds ok to me.

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

Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

[Mike Frysinger]

On Sunday, September 12, 2010 03:54:03 Artem Bityutskiy wrote:
> On Sun, 2010-09-12 at 00:03 -0400, Mike Frysinger wrote:
> > On Sat, Sep 11, 2010 at 02:32, Artem Bityutskiy wrote:
> > > It will be confusing if the same word is used in MTD for "unmarking"
> > > eraseblocks. How about: 'force erase' or 'bad erase' ?
> > 
> > that makes it sound like an option to the existing MEMERASE operation.
> > 
> >  so i guess what if we just do that -- extend the erase_info_user
> > 
> > structure to contain a flags field and add a MEMERASE2 that works with
> > the larger structure ?  for now we'd only have one option (FORCE), but
> > it makes it easy to extend in the future.
> 
> Ohh, this was so stupid of me to not ask people to add extra fields to
> 'struct erase_info_user64' which was introduced relatively recently... I
> always add extra fields to ioctl data structures...
> 
> But yeah, what you say sounds ok to me.

here's a POC that works for me.  with a simple tweak to `flash_eraseall`, i
can now recover my mtd devices with funky OOB layouts.

ive only extended MEMERASE64 as i believe the non-64 variants are EOL ?  or
should i also extend the 32bit interface as well ?
-mike

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 5b081cb..68c2864 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -513,6 +513,7 @@ static int mtd_ioctl(struct inode *inode, struct file 
*file,
 
 	case MEMERASE:
 	case MEMERASE64:
+	case MEMERASE64_FLAGS:
 	{
 		struct erase_info *erase;
 
@@ -538,6 +539,17 @@ static int mtd_ioctl(struct inode *inode, struct file 
*file,
 				}
 				erase->addr = einfo64.start;
 				erase->len = einfo64.length;
+			} else if (cmd == MEMERASE64_FLAGS) {
+				struct erase_info_user64_flags einfo64;
+
+				if (copy_from_user(&einfo64, argp,
+					    sizeof(struct erase_info_user64_flags))) {
+					kfree(erase);
+					return -EFAULT;
+				}
+				erase->addr = einfo64.start;
+				erase->len = einfo64.length;
+				erase->flags = einfo64.flags;
 			} else {
 				struct erase_info_user einfo32;
 
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 8f2958f..e440d84 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2354,7 +2354,8 @@ int nand_erase_nand(struct mtd_info *mtd, struct 
erase_info *instr,
 		/*
 		 * heck if we have a bad block, we do not erase bad blocks !
 		 */
-		if (nand_block_checkbad(mtd, ((loff_t) page) <<
+		if (!(instr->flags & MTD_ERASE_BADBLOCKS) &&
+			nand_block_checkbad(mtd, ((loff_t) page) <<
 					chip->page_shift, 0, allowbbt)) {
 			printk(KERN_WARNING "%s: attempt to erase a bad block "
 					"at page 0x%08x\n", __func__, page);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 0f32a9b..f1cda73 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -37,6 +37,7 @@ struct erase_info {
 	struct mtd_info *mtd;
 	uint64_t addr;
 	uint64_t len;
+	uint32_t flags;
 	uint64_t fail_addr;
 	u_long time;
 	u_long retries;
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index be51ae2..fef14ba 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -17,6 +17,12 @@ struct erase_info_user64 {
 	__u64 length;
 };
 
+struct erase_info_user64_flags {
+	__u64 start;
+	__u64 length;
+	__u32 flags;
+};
+
 struct mtd_oob_buf {
 	__u32 start;
 	__u32 length;
@@ -61,6 +67,9 @@ struct mtd_oob_buf64 {
 #define MTD_OTP_FACTORY		1
 #define MTD_OTP_USER		2
 
+/* Erase flags */
+#define MTD_ERASE_BADBLOCKS	0x1
+
 struct mtd_info_user {
 	__u8 type;
 	__u32 flags;
@@ -110,6 +119,7 @@ struct otp_info {
 #define MEMERASE64		_IOW('M', 20, struct erase_info_user64)
 #define MEMWRITEOOB64		_IOWR('M', 21, struct mtd_oob_buf64)
 #define MEMREADOOB64		_IOWR('M', 22, struct mtd_oob_buf64)
+#define MEMERASE64_FLAGS	_IOW('M', 23, struct erase_info_user64_flags)
 
 /*
  * Obsolete legacy interface. Keep it in order not to break userspace

Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

[Artem Bityutskiy]

On Wed, 2010-09-22 at 03:43 -0400, Mike Frysinger wrote:
> On Sunday, September 12, 2010 03:54:03 Artem Bityutskiy wrote:
> > On Sun, 2010-09-12 at 00:03 -0400, Mike Frysinger wrote:
> > > On Sat, Sep 11, 2010 at 02:32, Artem Bityutskiy wrote:
> > > > It will be confusing if the same word is used in MTD for "unmarking"
> > > > eraseblocks. How about: 'force erase' or 'bad erase' ?
> > > 
> > > that makes it sound like an option to the existing MEMERASE operation.
> > > 
> > >  so i guess what if we just do that -- extend the erase_info_user
> > > 
> > > structure to contain a flags field and add a MEMERASE2 that works with
> > > the larger structure ?  for now we'd only have one option (FORCE), but
> > > it makes it easy to extend in the future.
> > 
> > Ohh, this was so stupid of me to not ask people to add extra fields to
> > 'struct erase_info_user64' which was introduced relatively recently... I
> > always add extra fields to ioctl data structures...
> > 
> > But yeah, what you say sounds ok to me.
> 
> here's a POC that works for me.  with a simple tweak to `flash_eraseall`, i
> can now recover my mtd devices with funky OOB layouts.
> 
> ive only extended MEMERASE64 as i believe the non-64 variants are EOL ?  or
> should i also extend the 32bit interface as well ?

We need something consistent. This patch will just erase the bad
eraseblock. This will not mark it as good in the BBT (neither in-ram nor
on-flash). If the erasure succeeds, the block will still be marked as
bad in BBT, but after reboot, if the BBT is not on-flash, it will be
treated as good eraseblock, because scanning will not find the bad block
marker anymore. If the BBT is on-flash, it'll stay bad. This is
inconsistent.

> +struct erase_info_user64_flags {
> +	__u64 start;
> +	__u64 length;
> +	__u32 flags;
> +};

I think it needs to have som more room for possible future extentions.
Also, good tone for ioctls is to make them to be multiple of 64-bit -
less pain in mixed 32/64 bit setups.

Please, add some

u8 padding[12]

field and add a comment that this has to be zero, and may be used in
future.

Then in future we may extend ioctls and add more fields.

> +#define MEMERASE64_FLAGS	_IOW('M', 23, struct erase_info_user64_flags)

I do not like the name. We may add something else, not just flags later.
May be MEMERASE64_EXTENDED ?

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

Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

[Mike Frysinger]

On Thu, Sep 23, 2010 at 08:28, Artem Bityutskiy wrote:
> On Wed, 2010-09-22 at 03:43 -0400, Mike Frysinger wrote:
>> here's a POC that works for me.  with a simple tweak to `flash_eraseall`, i
>> can now recover my mtd devices with funky OOB layouts.
>>
>> ive only extended MEMERASE64 as i believe the non-64 variants are EOL ?  or
>> should i also extend the 32bit interface as well ?
>
> We need something consistent. This patch will just erase the bad
> eraseblock. This will not mark it as good in the BBT (neither in-ram nor
> on-flash). If the erasure succeeds, the block will still be marked as
> bad in BBT, but after reboot, if the BBT is not on-flash, it will be
> treated as good eraseblock, because scanning will not find the bad block
> marker anymore. If the BBT is on-flash, it'll stay bad. This is
> inconsistent.

i dont think the current BBT API supports marking bad blocks as good ?
 if it does, could you highlight it for me ?

>> +struct erase_info_user64_flags {
>> +     __u64 start;
>> +     __u64 length;
>> +     __u32 flags;
>> +};
>
> I think it needs to have som more room for possible future extentions.
> Also, good tone for ioctls is to make them to be multiple of 64-bit -
> less pain in mixed 32/64 bit setups.
>
> Please, add some
>
> u8 padding[12]
>
> field and add a comment that this has to be zero, and may be used in
> future.
>
> Then in future we may extend ioctls and add more fields.

well, the idea i have with flags is that we carve one bit out to mean
"extend".  so if in the future we wanted to extend the struct, we set
that flag and the kernel knows that userspace wants the larger
version.  that gives you a way of extending things indefinitely at the
cost of 1 bit.

which also means i should probably carve that bit out now and have the
kernel return an error when it is set today.

>> +#define MEMERASE64_FLAGS     _IOW('M', 23, struct erase_info_user64_flags)
>
> I do not like the name. We may add something else, not just flags later.

i was going to use "MEMERASE2", but that didnt scale well with "MEMERASE264".

> May be MEMERASE64_EXTENDED ?

OK
-mike

Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

[Artem Bityutskiy]

On Thu, 2010-09-23 at 15:55 -0400, Mike Frysinger wrote:
> On Thu, Sep 23, 2010 at 08:28, Artem Bityutskiy wrote:
> > On Wed, 2010-09-22 at 03:43 -0400, Mike Frysinger wrote:
> >> here's a POC that works for me.  with a simple tweak to `flash_eraseall`, i
> >> can now recover my mtd devices with funky OOB layouts.
> >>
> >> ive only extended MEMERASE64 as i believe the non-64 variants are EOL ?  or
> >> should i also extend the 32bit interface as well ?
> >
> > We need something consistent. This patch will just erase the bad
> > eraseblock. This will not mark it as good in the BBT (neither in-ram nor
> > on-flash). If the erasure succeeds, the block will still be marked as
> > bad in BBT, but after reboot, if the BBT is not on-flash, it will be
> > treated as good eraseblock, because scanning will not find the bad block
> > marker anymore. If the BBT is on-flash, it'll stay bad. This is
> > inconsistent.
> 
> i dont think the current BBT API supports marking bad blocks as good ?
>  if it does, could you highlight it for me ?

No, unfortunately, it'd require some work with ancient code - not very
easy task, IMO.

> >> +struct erase_info_user64_flags {
> >> +     __u64 start;
> >> +     __u64 length;
> >> +     __u32 flags;
> >> +};
> >
> > I think it needs to have som more room for possible future extentions.
> > Also, good tone for ioctls is to make them to be multiple of 64-bit -
> > less pain in mixed 32/64 bit setups.
> >
> > Please, add some
> >
> > u8 padding[12]
> >
> > field and add a comment that this has to be zero, and may be used in
> > future.
> >
> > Then in future we may extend ioctls and add more fields.
> 
> well, the idea i have with flags is that we carve one bit out to mean
> "extend".  so if in the future we wanted to extend the struct, we set
> that flag and the kernel knows that userspace wants the larger
> version.  that gives you a way of extending things indefinitely at the
> cost of 1 bit.

I think this is not fast-path and it does not hurt to have extra fields
just in case someone else needs them in future.

Also, make sure size of this structure is multiple of 8, not 4.

> which also means i should probably carve that bit out now and have the
> kernel return an error when it is set today.

I think no, you should ignore them.  But you should write a comment that
userspace has to zero them, this is kind of part of API. Then when new
flags are added, old kernels will not fail, but ignore them.

But yes, this will work only if userspace follows the API.

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

RE: linux equivalent of u-boot's "nand scrub" (erasing blocks even whenOOB says "bad")

[Jon Povey]

Mike Frysinger wrote:
> ive come across a situation where it would have been invaluable to
> have the ability to "scrub" the nand flash while running linux.

I would find this useful too, or more specifically I recently wanted to rewrite the BBT in software from linux (to change OOB layout), but those blocks are marked off-limits. Some way to force it would be nice.

I am just rambling though, no real useful suggestion :) Sorry..

As for clearing blocks marked bad, how about MEMSETGOODBLOCK as the inverse of the existing MEMSETBADBLOCK?

--
Jon Povey
jon.povey@racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

RE: linux equivalent of u-boot's "nand scrub" (erasing blocks even whenOOB says "bad")

[Artem Bityutskiy]

On Mon, 2010-09-13 at 06:54 +0100, Jon Povey wrote:
> Mike Frysinger wrote:
> > ive come across a situation where it would have been invaluable to
> > have the ability to "scrub" the nand flash while running linux.
> 
> I would find this useful too, or more specifically I recently wanted to rewrite the BBT in software from linux (to change OOB layout), but those blocks are marked off-limits. Some way to force it would be nice.
> 
> I am just rambling though, no real useful suggestion :) Sorry..
> 
> As for clearing blocks marked bad, how about MEMSETGOODBLOCK as the inverse of the existing MEMSETBADBLOCK?

So may be we need a separate ioctl for dealing with bad eraseblocks
which will also include abilities to re-write BBT?

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

Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even whenOOB says "bad")

[Mike Frysinger]

On Mon, Sep 13, 2010 at 02:25, Artem Bityutskiy wrote:
> On Mon, 2010-09-13 at 06:54 +0100, Jon Povey wrote:
>> Mike Frysinger wrote:
>> > ive come across a situation where it would have been invaluable to
>> > have the ability to "scrub" the nand flash while running linux.
>>
>> I would find this useful too, or more specifically I recently wanted to rewrite the BBT in software from linux (to change OOB layout), but those blocks are marked off-limits. Some way to force it would be nice.
>>
>> I am just rambling though, no real useful suggestion :) Sorry..
>>
>> As for clearing blocks marked bad, how about MEMSETGOODBLOCK as the inverse of the existing MEMSETBADBLOCK?
>
> So may be we need a separate ioctl for dealing with bad eraseblocks
> which will also include abilities to re-write BBT?

i dont have any devices that have dedicated software BBTs.  i'm
concerned with just the forced erasing of blocks since the rest works
for me already (forced reading and writing).  the ability to change
the definition of the OOB is apparently "deprecated" now, so i cant
fix things on the fly there.
-mike

RE: linux equivalent of u-boot's "nand scrub" (erasing blocks even whenOOB says "bad")

[Jon Povey]

Mike Frysinger wrote:
> i dont have any devices that have dedicated software BBTs.  i'm
> concerned with just the forced erasing of blocks since the rest works
> for me already (forced reading and writing).  the ability to change
> the definition of the OOB is apparently "deprecated" now, so i cant
> fix things on the fly there.

Not quite sure what you mean by the last part. I have used raw writes with OOB, software-generated ECC and altered layout of flash pages, to good effect.

I'm don't know what's deprecated, although the kind of hackery I am doing I wouldn't expect to be encouraged by the API. I do, however, appreciate when such things are possible. Especially on embedded, I don't want a kernel API thinking it knows best about, f.ex., allowing me to erase and rewrite the BBT. Stopping me doing it by accident is fine. Making it impossible is a pain.

--
Jon Povey
jon.povey@racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even whenOOB says "bad")

[Mike Frysinger]

On Mon, Sep 13, 2010 at 21:53, Jon Povey wrote:
> Mike Frysinger wrote:
>> i dont have any devices that have dedicated software BBTs.  i'm
>> concerned with just the forced erasing of blocks since the rest works
>> for me already (forced reading and writing).  the ability to change
>> the definition of the OOB is apparently "deprecated" now, so i cant
>> fix things on the fly there.
>
> Not quite sure what you mean by the last part. I have used raw writes with OOB, software-generated ECC and altered layout of flash pages, to good effect.
>
> I'm don't know what's deprecated, although the kind of hackery I am doing I wouldn't expect to be encouraged by the API. I do, however, appreciate when such things are possible. Especially on embedded, I don't want a kernel API thinking it knows best about, f.ex., allowing me to erase and rewrite the BBT. Stopping me doing it by accident is fine. Making it impossible is a pain.

please fix your e-mailer to not generate long  unwrapped lines

i'm talking about MEM{G,S}ETOOBSEL
-mike