| Message ID | c65631c48fb5d2de8d25f65bffa5e22d@localhost |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2010-07-07 at 17:30 -0700, Kevin Cernekee wrote: > Change mtd_erase() so that it attempts to use MEMERASE64 first, then falls > back to the old <2.6.31 MEMERASE if MEMERASE64 is unsupported. > > Add mtd_read_oob(), mtd_write_oob() functions to wrap the OOB ioctls. > Similar ioctl fallback logic is used in these functions as well. > > Signed-off-by: Kevin Cernekee <cernekee@gmail.com> > --- > include/libmtd.h | 36 ++++++++++++++++++++++- > lib/libmtd.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 111 insertions(+), 7 deletions(-) > > diff --git a/include/libmtd.h b/include/libmtd.h > index 0aea966..292d8c3 100644 > --- a/include/libmtd.h > +++ b/include/libmtd.h > @@ -66,6 +66,7 @@ struct mtd_info > * @region_cnt: count of additional erase regions > * @writable: zero if the device is read-only > * @bb_allowed: non-zero if the MTD device may have bad eraseblocks > + * @legacy_ioctls: non-zero if the kernel lacks MEMERASE64, MEM*OOB64 > */ > struct mtd_dev_info > { > @@ -84,6 +85,7 @@ struct mtd_dev_info > int region_cnt; > unsigned int writable:1; > unsigned int bb_allowed:1; > + unsigned int legacy_ioctls:1; > }; Kevin, I'm sorry, but thinking about this some more, this is not a property of mtd device, this is a property of whole mtd subsystem. So this flag should live in 'struct mtd_info', just like the 'unsigned int sysfs_supported:1;' field. You should find out whether the new ioctl's are supported in 'libmtd_open()'. This also means that all libmtd calls which need this flag will need a 'libmtd_t desc' parameter. Also, is it possible to have 2 separate patches - 1st adds support for 64-bit offest, second adds OOB calls. Is this doable? I think this should not be too difficult to change.
On Wed, 2010-07-07 at 17:30 -0700, Kevin Cernekee wrote: > Change mtd_erase() so that it attempts to use MEMERASE64 first, then falls > back to the old <2.6.31 MEMERASE if MEMERASE64 is unsupported. > > Add mtd_read_oob(), mtd_write_oob() functions to wrap the OOB ioctls. > Similar ioctl fallback logic is used in these functions as well. > > Signed-off-by: Kevin Cernekee <cernekee@gmail.com> > --- > include/libmtd.h | 36 ++++++++++++++++++++++- > lib/libmtd.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 111 insertions(+), 7 deletions(-) > > diff --git a/include/libmtd.h b/include/libmtd.h > index 0aea966..292d8c3 100644 > --- a/include/libmtd.h > +++ b/include/libmtd.h > @@ -66,6 +66,7 @@ struct mtd_info > * @region_cnt: count of additional erase regions > * @writable: zero if the device is read-only > * @bb_allowed: non-zero if the MTD device may have bad eraseblocks > + * @legacy_ioctls: non-zero if the kernel lacks MEMERASE64, MEM*OOB64 > */ > struct mtd_dev_info > { > @@ -84,6 +85,7 @@ struct mtd_dev_info > int region_cnt; > unsigned int writable:1; > unsigned int bb_allowed:1; > + unsigned int legacy_ioctls:1; > }; > > /** > @@ -146,7 +148,37 @@ int mtd_get_dev_info1(libmtd_t desc, int mtd_num, struct mtd_dev_info *mtd); > * This function erases eraseblock @eb of MTD device described by @fd. Returns > * %0 in case of success and %-1 in case of failure. > */ > -int mtd_erase(const struct mtd_dev_info *mtd, int fd, int eb); > +int mtd_erase(struct mtd_dev_info *mtd, int fd, int eb); > + > +/** > + * mtd_read_oob - read OOB bytes > + * @mtd: MTD device description object > + * @fd: MTD device node file descriptor > + * @start: page-aligned start address > + * @length: number of OOB bytes to read > + * @data: read buffer > + * > + * This function reads @length OOB bytes starting from address @start on > + * MTD device described by @fd. Returns %0 in case of success and %-1 in > + * case of failure. > + */ > +int mtd_read_oob(struct mtd_dev_info *mtd, int fd, uint64_t start, > + uint64_t length, void *data); Minor, but for consistency, please, use the same indentation style as the rest of the code. Namely, aline the second line: int mtd_read_oob(struct mtd_dev_info *mtd, int fd, uint64_t start, uint64_t length, void *data); Use tabs, but then adjust by adding spaces.
On Tue, Jul 13, 2010 at 3:49 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote: > You should find out whether the new ioctl's are supported in > 'libmtd_open()'. This also means that all libmtd calls which need this > flag will need a 'libmtd_t desc' parameter. Typically I just try the new ioctl first, then fall back to the old one if it returned -ENOTTY. Could you recommend a way to detect the presence of the new ioctls from libmtd_open()? Thanks.
On Tue, 2010-07-13 at 17:09 -0700, Kevin Cernekee wrote: > On Tue, Jul 13, 2010 at 3:49 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote: > > You should find out whether the new ioctl's are supported in > > 'libmtd_open()'. This also means that all libmtd calls which need this > > flag will need a 'libmtd_t desc' parameter. > > Typically I just try the new ioctl first, then fall back to the old > one if it returned -ENOTTY. Could you recommend a way to detect the > presence of the new ioctls from libmtd_open()? Hmm, how about MEMERASE64 with start = length = -1? You can distinguish between -ENOTTY and other error codes.
On Tue, 2010-07-13 at 13:49 +0300, Artem Bityutskiy wrote: > On Wed, 2010-07-07 at 17:30 -0700, Kevin Cernekee wrote: > > Change mtd_erase() so that it attempts to use MEMERASE64 first, then falls > > back to the old <2.6.31 MEMERASE if MEMERASE64 is unsupported. > > > > Add mtd_read_oob(), mtd_write_oob() functions to wrap the OOB ioctls. > > Similar ioctl fallback logic is used in these functions as well. > > > > Signed-off-by: Kevin Cernekee <cernekee@gmail.com> > > --- > > include/libmtd.h | 36 ++++++++++++++++++++++- > > lib/libmtd.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++--- > > 2 files changed, 111 insertions(+), 7 deletions(-) > > > > diff --git a/include/libmtd.h b/include/libmtd.h > > index 0aea966..292d8c3 100644 > > --- a/include/libmtd.h > > +++ b/include/libmtd.h > > @@ -66,6 +66,7 @@ struct mtd_info > > * @region_cnt: count of additional erase regions > > * @writable: zero if the device is read-only > > * @bb_allowed: non-zero if the MTD device may have bad eraseblocks > > + * @legacy_ioctls: non-zero if the kernel lacks MEMERASE64, MEM*OOB64 > > */ > > struct mtd_dev_info > > { > > @@ -84,6 +85,7 @@ struct mtd_dev_info > > int region_cnt; > > unsigned int writable:1; > > unsigned int bb_allowed:1; > > + unsigned int legacy_ioctls:1; > > }; > > Kevin, I'm sorry, but thinking about this some more, this is not a > property of mtd device, this is a property of whole mtd subsystem. So > this flag should live in 'struct mtd_info', just like the 'unsigned int > sysfs_supported:1;' field. > > You should find out whether the new ioctl's are supported in > 'libmtd_open()'. This also means that all libmtd calls which need this > flag will need a 'libmtd_t desc' parameter. > > Also, is it possible to have 2 separate patches - 1st adds support for > 64-bit offest, second adds OOB calls. > > Is this doable? I think this should not be too difficult to change. I thought some more, and no, this is not doable because you need a device node, which you do not have when the library is being initialized. I'll take a look at these patches once again.
diff --git a/include/libmtd.h b/include/libmtd.h index 0aea966..292d8c3 100644 --- a/include/libmtd.h +++ b/include/libmtd.h @@ -66,6 +66,7 @@ struct mtd_info * @region_cnt: count of additional erase regions * @writable: zero if the device is read-only * @bb_allowed: non-zero if the MTD device may have bad eraseblocks + * @legacy_ioctls: non-zero if the kernel lacks MEMERASE64, MEM*OOB64 */ struct mtd_dev_info { @@ -84,6 +85,7 @@ struct mtd_dev_info int region_cnt; unsigned int writable:1; unsigned int bb_allowed:1; + unsigned int legacy_ioctls:1; }; /** @@ -146,7 +148,37 @@ int mtd_get_dev_info1(libmtd_t desc, int mtd_num, struct mtd_dev_info *mtd); * This function erases eraseblock @eb of MTD device described by @fd. Returns * %0 in case of success and %-1 in case of failure. */ -int mtd_erase(const struct mtd_dev_info *mtd, int fd, int eb); +int mtd_erase(struct mtd_dev_info *mtd, int fd, int eb); + +/** + * mtd_read_oob - read OOB bytes + * @mtd: MTD device description object + * @fd: MTD device node file descriptor + * @start: page-aligned start address + * @length: number of OOB bytes to read + * @data: read buffer + * + * This function reads @length OOB bytes starting from address @start on + * MTD device described by @fd. Returns %0 in case of success and %-1 in + * case of failure. + */ +int mtd_read_oob(struct mtd_dev_info *mtd, int fd, uint64_t start, + uint64_t length, void *data); + +/** + * mtd_write_oob - write OOB bytes + * @mtd: MTD device description object + * @fd: MTD device node file descriptor + * @start: page-aligned start address + * @length: number of OOB bytes to write + * @data: write buffer + * + * This function writes @length OOB bytes starting from address @start on + * MTD device described by @fd. Returns %0 in case of success and %-1 in + * case of failure. + */ +int mtd_write_oob(struct mtd_dev_info *mtd, int fd, uint64_t start, + uint64_t length, void *data); /** * mtd_torture - torture an eraseblock. @@ -157,7 +189,7 @@ int mtd_erase(const struct mtd_dev_info *mtd, int fd, int eb); * This function tortures eraseblock @eb. Returns %0 in case of success and %-1 * in case of failure. */ -int mtd_torture(const struct mtd_dev_info *mtd, int fd, int eb); +int mtd_torture(struct mtd_dev_info *mtd, int fd, int eb); /** * mtd_is_bad - check if eraseblock is bad. diff --git a/lib/libmtd.c b/lib/libmtd.c index 3ff031c..66e794a 100644 --- a/lib/libmtd.c +++ b/lib/libmtd.c @@ -32,6 +32,7 @@ #include <sys/stat.h> #include <sys/ioctl.h> #include <mtd/mtd-user.h> +#include <mtd/mtd-abi.h> #include <libmtd.h> #include "libmtd_int.h" @@ -789,13 +790,84 @@ int mtd_get_dev_info(libmtd_t desc, const char *node, struct mtd_dev_info *mtd) return mtd_get_dev_info1(desc, mtd_num, mtd); } -int mtd_erase(const struct mtd_dev_info *mtd, int fd, int eb) +int mtd_erase(struct mtd_dev_info *mtd, int fd, int eb) { - struct erase_info_user ei; + int ret; + struct erase_info_user64 ei; - ei.start = eb * mtd->eb_size;; + ei.start = (__u64)eb * mtd->eb_size; ei.length = mtd->eb_size; - return ioctl(fd, MEMERASE, &ei); + + if (!mtd->legacy_ioctls) { + ret = ioctl(fd, MEMERASE64, &ei); + if (ret < 0 && errno == ENOTTY) + mtd->legacy_ioctls = 1; + } + + if (mtd->legacy_ioctls) { + struct erase_info_user ei_old; + + ei_old.start = ei.start; + ei_old.length = ei.length; + + if (ei_old.start != ei.start) { + errno = EOVERFLOW; + return -1; + } + + ret = ioctl(fd, MEMERASE, &ei_old); + } + + return ret; +} + +static int mtd_oob_op(int cmd, int cmd_old, struct mtd_dev_info *mtd, int fd, + uint64_t start, uint64_t length, void *data) +{ + int ret; + struct mtd_oob_buf64 oob; + + oob.start = start; + oob.length = length; + oob.usr_ptr = (__u64)(unsigned long)data; + + if (!mtd->legacy_ioctls) { + ret = ioctl(fd, cmd, &oob); + if (ret < 0 && errno == ENOTTY) + mtd->legacy_ioctls = 1; + } + + if (mtd->legacy_ioctls) { + struct mtd_oob_buf oob_old; + + oob_old.start = oob.start; + oob_old.length = oob.length; + oob_old.ptr = data; + + if (oob_old.start != oob.start || + oob_old.length != oob.length) { + errno = EOVERFLOW; + return -1; + } + + ret = ioctl(fd, cmd_old, &oob_old); + } + + return ret; +} + +int mtd_read_oob(struct mtd_dev_info *mtd, int fd, uint64_t start, + uint64_t length, void *data) +{ + return mtd_oob_op(MEMREADOOB64, MEMREADOOB, mtd, fd, start, length, + data); +} + +int mtd_write_oob(struct mtd_dev_info *mtd, int fd, uint64_t start, + uint64_t length, void *data) +{ + return mtd_oob_op(MEMWRITEOOB64, MEMWRITEOOB, mtd, fd, start, length, + data); } /* Patterns to write to a physical eraseblock when torturing it */ @@ -820,7 +892,7 @@ static int check_pattern(const void *buf, uint8_t patt, int size) return 1; } -int mtd_torture(const struct mtd_dev_info *mtd, int fd, int eb) +int mtd_torture(struct mtd_dev_info *mtd, int fd, int eb) { int err, i, patt_count; void *buf;
Change mtd_erase() so that it attempts to use MEMERASE64 first, then falls back to the old <2.6.31 MEMERASE if MEMERASE64 is unsupported. Add mtd_read_oob(), mtd_write_oob() functions to wrap the OOB ioctls. Similar ioctl fallback logic is used in these functions as well. Signed-off-by: Kevin Cernekee <cernekee@gmail.com> --- include/libmtd.h | 36 ++++++++++++++++++++++- lib/libmtd.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 111 insertions(+), 7 deletions(-)