Patchwork libmtd: allow write operations when MEMWRITE is not supported

login
register
mail settings
Submitter Artem Bityutskiy
Date Dec. 12, 2011, 9:26 p.m.
Message ID <1323725206.2297.24.camel@koala>
Download mbox | patch
Permalink /patch/130854/
State New
Headers show

Comments

Artem Bityutskiy - Dec. 12, 2011, 9:26 p.m.
On Fri, 2011-12-09 at 11:45 -0800, Brian Norris wrote:
> MEMWRITE is a recently introduced write interface for MTD; however, it
> is, for now, only supported on NAND flash. mtd-utils should fall back to
> old write methods when either ENOTTY or EOPNOTSUPP are returned.
> 
> This is a showstopper when, for instance, using ubiformat on NOR, which
> don't have a mtd->write_oob interface (and thus don't support MEMWRITE):
> 
>   ubiformat: formatting eraseblock 2 --  1 % complete  libmtd: error!: MEMWRITE ioctl failed for eraseblock 2 (mtd3)
>           error 122 (Operation not supported)
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

I guess this require an urgent mtd-utils release.

Also, why you say 'for now', does even make-sense to implement this for
NOR?

I've also just pushed this patch, does it look OK to you?

From a4699233f927738db633687a2276a9f3e127d799 Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <Artem.Bityutskiy@linux.intel.com>
Date: Mon, 12 Dec 2011 23:24:59 +0200
Subject: [PATCH] mtd: document that MEMWRITE ioctl is NAND-specific

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@linux.intel.com>
---
 include/mtd/mtd-abi.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Brian Norris - Dec. 12, 2011, 9:40 p.m.
On Mon, Dec 12, 2011 at 1:26 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Fri, 2011-12-09 at 11:45 -0800, Brian Norris wrote:
>> MEMWRITE is a recently introduced write interface for MTD; however, it
>> is, for now, only supported on NAND flash. mtd-utils should fall back to
>> old write methods when either ENOTTY or EOPNOTSUPP are returned.
>>
>> This is a showstopper when, for instance, using ubiformat on NOR, which
>> don't have a mtd->write_oob interface (and thus don't support MEMWRITE):
>>
>>   ubiformat: formatting eraseblock 2 --  1 % complete  libmtd: error!: MEMWRITE ioctl failed for eraseblock 2 (mtd3)
>>           error 122 (Operation not supported)
>>
>> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
>
> I guess this require an urgent mtd-utils release.
>
> Also, why you say 'for now', does even make-sense to implement this for
> NOR?

I think when (somewhat hurriedly) writing this patch, I was
considering issues with the libmtd interface (mtd_write) and with the
ioctl (MEMWRITE). I kinda mixed them up and wrote something that
wasn't really applicable to either one. Deleting "for now" is good.

Another issue: there are strictly non-NAND flash with OOB as well, so
my statement ("only supported on NAND flash") isn't quite as good as
yours ("not supported for flashes without OOB, e.g., NOR flash"). Not
hugely important on a mtd-utils patch though.

> I've also just pushed this patch, does it look OK to you?

Yes, it looks fine.

Brian

P.S. FYI, I don't see the mtd-utils "push" yet.
Artem Bityutskiy - Dec. 17, 2011, 2:44 p.m.
On Mon, 2011-12-12 at 13:40 -0800, Brian Norris wrote:
> On Mon, Dec 12, 2011 at 1:26 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > On Fri, 2011-12-09 at 11:45 -0800, Brian Norris wrote:
> >> MEMWRITE is a recently introduced write interface for MTD; however, it
> >> is, for now, only supported on NAND flash. mtd-utils should fall back to
> >> old write methods when either ENOTTY or EOPNOTSUPP are returned.
> >>
> >> This is a showstopper when, for instance, using ubiformat on NOR, which
> >> don't have a mtd->write_oob interface (and thus don't support MEMWRITE):
> >>
> >>   ubiformat: formatting eraseblock 2 --  1 % complete  libmtd: error!: MEMWRITE ioctl failed for eraseblock 2 (mtd3)
> >>           error 122 (Operation not supported)
> >>
> >> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> >
> > I guess this require an urgent mtd-utils release.

OK, I've cut mtd-utils-1.4.9 bug-fix release, this time with my
new script, so I hope nothing is forgotten.

Peter, please, update - this release fixes a bad bug which breaks NOR
erase operation in not-yet-released 3.2 kernel.
Peter Korsgaard - Dec. 17, 2011, 3:03 p.m.
>>>>> "Artem" == Artem Bityutskiy <dedekind1@gmail.com> writes:

Hi,

 >> > I guess this require an urgent mtd-utils release.

 Artem> OK, I've cut mtd-utils-1.4.9 bug-fix release, this time with my
 Artem> new script, so I hope nothing is forgotten.

 Artem> Peter, please, update - this release fixes a bad bug which breaks NOR
 Artem> erase operation in not-yet-released 3.2 kernel.

Thanks for the heads up, I was just about to rebase a NOR-using platform
to 3.2.

Updated buildroot. It builds fine here, so it seems like the script is
working.
Artem Bityutskiy - Dec. 17, 2011, 3:11 p.m.
On Sat, 2011-12-17 at 16:03 +0100, Peter Korsgaard wrote:
> >>>>> "Artem" == Artem Bityutskiy <dedekind1@gmail.com> writes:
> 
> Hi,
> 
>  >> > I guess this require an urgent mtd-utils release.
> 
>  Artem> OK, I've cut mtd-utils-1.4.9 bug-fix release, this time with my
>  Artem> new script, so I hope nothing is forgotten.
> 
>  Artem> Peter, please, update - this release fixes a bad bug which breaks NOR
>  Artem> erase operation in not-yet-released 3.2 kernel.
> 
> Thanks for the heads up, I was just about to rebase a NOR-using platform
> to 3.2.
> 
> Updated buildroot. It builds fine here, so it seems like the script is
> working.

Notice also than now we also provide the mtd-utils-1.4.9.tar.bz2.asc
tarball signature and we also sign the mtd-utils tags.
Brian Norris - Dec. 19, 2011, 6:45 p.m.
On Sat, Dec 17, 2011 at 6:44 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> OK, I've cut mtd-utils-1.4.9 bug-fix release, this time with my
> new script, so I hope nothing is forgotten.

Well, it seems like you didn't update the master branch; I had to `git
fetch --tags` to get to the v1.4.9 tag, which is ahead of master. I
don't know if that needs/requires an update to your release script to
ensure you don't miss it in the future?

Brian
Artem Bityutskiy - Dec. 19, 2011, 8:38 p.m.
On Mon, 2011-12-19 at 10:45 -0800, Brian Norris wrote:
> On Sat, Dec 17, 2011 at 6:44 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > OK, I've cut mtd-utils-1.4.9 bug-fix release, this time with my
> > new script, so I hope nothing is forgotten.
> 
> Well, it seems like you didn't update the master branch; I had to `git
> fetch --tags` to get to the v1.4.9 tag, which is ahead of master. I
> don't know if that needs/requires an update to your release script to
> ensure you don't miss it in the future?

Oh, good point, I've pushed the tag but not the branch :-) Fixed,
thanks!

Artem.

Patch

diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index 1a7e1d2..36eace0 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -198,7 +198,8 @@  struct otp_info {
 #define MEMISLOCKED		_IOR('M', 23, struct erase_info_user)
 /*
  * Most generic write interface; can write in-band and/or out-of-band in various
- * modes (see "struct mtd_write_req")
+ * modes (see "struct mtd_write_req"). This ioctl is not supported for flashes
+ * without OOB, e.g., NOR flash.
  */
 #define MEMWRITE		_IOWR('M', 24, struct mtd_write_req)