* [PATCH 01/10] nandwrite: trivial variable move
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-09-11 13:00 ` Artem Bityutskiy
2011-08-31 20:00 ` [PATCH 02/10] mtd-utils: update mtd-abi.h Brian Norris
` (10 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
nandwrite.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/nandwrite.c b/nandwrite.c
index 95baa38..17a717c 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -496,7 +496,7 @@ int main(int argc, char * const argv[])
}
if (!noecc) {
- int i, start, len;
+ int start, len;
struct nand_oobinfo old_oobinfo;
/* Read the current oob info */
@@ -514,7 +514,7 @@ int main(int argc, char * const argv[])
* such as the layout used by diskonchip.c
*/
if (old_oobinfo.useecc == MTD_NANDECC_AUTOPLACE) {
- int tags_pos = 0, tmp_ofs;
+ int i, tags_pos = 0, tmp_ofs;
for (i = 0; old_oobinfo.oobfree[i][1]; i++) {
/* Set the reserved bytes to 0xff */
start = old_oobinfo.oobfree[i][0];
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 02/10] mtd-utils: update mtd-abi.h
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
2011-08-31 20:00 ` [PATCH 01/10] nandwrite: trivial variable move Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-09-09 17:11 ` [PATCH v2 " Brian Norris
2011-08-31 20:00 ` [PATCH 03/10] nandwrite: consolidate buffer usage Brian Norris
` (9 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
Kernel ABI header added a new ioctl, killed an old one, and exposed OOB
modes to user-space. Plus, it added a lot of documentation.
We have some trivial name changes for some MTD mode constants as well.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
include/mtd/mtd-abi.h | 122 ++++++++++++++++++++++++++++++++++++++++++-------
nanddump.c | 2 +-
nandwrite.c | 2 +-
3 files changed, 107 insertions(+), 19 deletions(-)
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index a86364a..66be707 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -45,6 +45,51 @@ struct mtd_oob_buf64 {
__u64 usr_ptr;
};
+/**
+ * MTD operation modes
+ *
+ * @MTD_OPS_PLACE_OOB: OOB data are placed at the given offset (default)
+ * @MTD_OPS_AUTO_OOB: OOB data are automatically placed at the free areas
+ * which are defined by the internal ecclayout
+ * @MTD_OPS_RAW: data are transferred as-is, with no error correction;
+ * this mode implies %MTD_OPS_PLACE_OOB
+ *
+ * These modes can be passed to ioctl(MEMWRITE) and are also used internally.
+ * See notes on "MTD file modes" for discussion on %MTD_OPS_RAW vs.
+ * %MTD_FILE_MODE_RAW.
+ */
+enum {
+ MTD_OPS_PLACE_OOB = 0,
+ MTD_OPS_AUTO_OOB = 1,
+ MTD_OPS_RAW = 2,
+};
+
+/**
+ * struct mtd_write_req - data structure for requesting a write operation
+ *
+ * @start: start address
+ * @len: length of data buffer
+ * @ooblen: length of OOB buffer
+ * @usr_data: user-provided data buffer
+ * @usr_oob: user-provided OOB buffer
+ * @mode: MTD mode (see "MTD operation modes")
+ * @padding: reserved, must be set to 0
+ *
+ * This structure supports ioctl(MEMWRITE) operations, allowing data and/or OOB
+ * writes in various modes. To write to OOB-only, set @usr_data == NULL, and to
+ * write data-only, set @usr_oob == NULL. However, setting both @usr_data and
+ * @usr_oob to NULL is not allowed.
+ */
+struct mtd_write_req {
+ __u64 start;
+ __u32 len;
+ __u32 ooblen;
+ __u64 usr_data;
+ __u64 usr_oob;
+ __u8 mode;
+ __u8 padding[7];
+};
+
#define MTD_ABSENT 0
#define MTD_RAM 1
#define MTD_ROM 2
@@ -59,13 +104,13 @@ struct mtd_oob_buf64 {
#define MTD_NO_ERASE 0x1000 /* No erase necessary */
#define MTD_POWERUP_LOCK 0x2000 /* Always locked after reset */
-// Some common devices / combinations of capabilities
+/* 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)
-/* ECC byte placement */
+/* Obsolete ECC byte placement modes (used with obsolete MEMGETOOBSEL) */
#define MTD_NANDECC_OFF 0 // Switch off ECC (Not recommended)
#define MTD_NANDECC_PLACE 1 // Use the given placement in the structure (YAFFS1 legacy mode)
#define MTD_NANDECC_AUTOPLACE 2 // Use the default placement scheme
@@ -80,21 +125,18 @@ struct mtd_oob_buf64 {
struct mtd_info_user {
__u8 type;
__u32 flags;
- __u32 size; // Total size of the MTD
+ __u32 size; /* Total size of the MTD */
__u32 erasesize;
__u32 writesize;
- __u32 oobsize; // Amount of OOB data per block (e.g. 16)
- /* The below two fields are obsolete and broken, do not use them
- * (TODO: remove at some point) */
- __u32 ecctype;
- __u32 eccsize;
+ __u32 oobsize; /* Amount of OOB data per block (e.g. 16) */
+ __u64 padding; /* Old obsolete field; do not use */
};
struct region_info_user {
__u32 offset; /* At which this region starts,
- * from the beginning of the MTD */
- __u32 erasesize; /* For this region */
- __u32 numblocks; /* Number of blocks in this region */
+ * from the beginning of the MTD */
+ __u32 erasesize; /* For this region */
+ __u32 numblocks; /* Number of blocks in this region */
__u32 regionindex;
};
@@ -104,29 +146,61 @@ struct otp_info {
__u32 locked;
};
+/*
+ * Note, the following ioctl existed in the past and was removed:
+ * #define MEMSETOOBSEL _IOW('M', 9, struct nand_oobinfo)
+ * Try to avoid adding a new ioctl with the same ioctl number.
+ */
+
+/* Get basic MTD characteristics info (better to use sysfs) */
#define MEMGETINFO _IOR('M', 1, struct mtd_info_user)
+/* Erase segment of MTD */
#define MEMERASE _IOW('M', 2, struct erase_info_user)
+/* Write out-of-band data from MTD */
#define MEMWRITEOOB _IOWR('M', 3, struct mtd_oob_buf)
+/* Read out-of-band data from MTD */
#define MEMREADOOB _IOWR('M', 4, struct mtd_oob_buf)
+/* Lock a chip (for MTD that supports it) */
#define MEMLOCK _IOW('M', 5, struct erase_info_user)
+/* Unlock a chip (for MTD that supports it) */
#define MEMUNLOCK _IOW('M', 6, struct erase_info_user)
+/* Get the number of different erase regions */
#define MEMGETREGIONCOUNT _IOR('M', 7, int)
+/* Get information about the erase region for a specific index */
#define MEMGETREGIONINFO _IOWR('M', 8, struct region_info_user)
-#define MEMSETOOBSEL _IOW('M', 9, struct nand_oobinfo)
+/* Get info about OOB modes (e.g., RAW, PLACE, AUTO) - legacy interface */
#define MEMGETOOBSEL _IOR('M', 10, struct nand_oobinfo)
+/* Check if an eraseblock is bad */
#define MEMGETBADBLOCK _IOW('M', 11, __kernel_loff_t)
+/* Mark an eraseblock as bad */
#define MEMSETBADBLOCK _IOW('M', 12, __kernel_loff_t)
+/* Set OTP (One-Time Programmable) mode (factory vs. user) */
#define OTPSELECT _IOR('M', 13, int)
+/* Get number of OTP (One-Time Programmable) regions */
#define OTPGETREGIONCOUNT _IOW('M', 14, int)
+/* Get all OTP (One-Time Programmable) info about MTD */
#define OTPGETREGIONINFO _IOW('M', 15, struct otp_info)
+/* Lock a given range of user data (must be in mode %MTD_FILE_MODE_OTP_USER) */
#define OTPLOCK _IOR('M', 16, struct otp_info)
+/* Get ECC layout (deprecated) */
#define ECCGETLAYOUT _IOR('M', 17, struct nand_ecclayout_user)
+/* Get statistics about corrected/uncorrected errors */
#define ECCGETSTATS _IOR('M', 18, struct mtd_ecc_stats)
+/* Set MTD mode on a per-file-descriptor basis (see "MTD file modes") */
#define MTDFILEMODE _IO('M', 19)
+/* Erase segment of MTD (supports 64-bit address) */
#define MEMERASE64 _IOW('M', 20, struct erase_info_user64)
+/* Write data to OOB (64-bit version) */
#define MEMWRITEOOB64 _IOWR('M', 21, struct mtd_oob_buf64)
+/* Read data from OOB (64-bit version) */
#define MEMREADOOB64 _IOWR('M', 22, struct mtd_oob_buf64)
+/* Check if chip is locked (for MTD that supports it) */
#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")
+ */
+#define MEMWRITE _IOWR('M', 24, struct mtd_write_req)
/*
* Obsolete legacy interface. Keep it in order not to break userspace
@@ -177,13 +251,27 @@ struct mtd_ecc_stats {
};
/*
- * Read/write file modes for access to MTD
+ * MTD file modes - for read/write access to MTD
+ *
+ * @MTD_FILE_MODE_NORMAL: OTP disabled, ECC enabled
+ * @MTD_FILE_MODE_OTP_FACTORY: OTP enabled in factory mode
+ * @MTD_FILE_MODE_OTP_USER: OTP enabled in user mode
+ * @MTD_FILE_MODE_RAW: OTP disabled, ECC disabled
+ *
+ * These modes can be set via ioctl(MTDFILEMODE). The mode mode will be retained
+ * separately for each open file descriptor.
+ *
+ * Note: %MTD_FILE_MODE_RAW provides the same functionality as %MTD_OPS_RAW -
+ * raw access to the flash, without error correction or autoplacement schemes.
+ * Wherever possible, the MTD_OPS_* mode will override the MTD_FILE_MODE_* mode
+ * (e.g., when using ioctl(MEMWRITE)), but in some cases, the MTD_FILE_MODE is
+ * used out of necessity (e.g., `write()', ioctl(MEMWRITEOOB64)).
*/
enum mtd_file_modes {
- MTD_MODE_NORMAL = MTD_OTP_OFF,
- MTD_MODE_OTP_FACTORY = MTD_OTP_FACTORY,
- MTD_MODE_OTP_USER = MTD_OTP_USER,
- MTD_MODE_RAW,
+ MTD_FILE_MODE_NORMAL = MTD_OTP_OFF,
+ MTD_FILE_MODE_OTP_FACTORY = MTD_OTP_FACTORY,
+ MTD_FILE_MODE_OTP_USER = MTD_OTP_USER,
+ MTD_FILE_MODE_RAW,
};
#endif /* __MTD_ABI_H__ */
diff --git a/nanddump.c b/nanddump.c
index 7a24c0d..be458c6 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -336,7 +336,7 @@ int main(int argc, char * const argv[])
readbuf = xmalloc(sizeof(readbuf) * mtd.min_io_size);
if (noecc) {
- if (ioctl(fd, MTDFILEMODE, MTD_MODE_RAW) != 0) {
+ if (ioctl(fd, MTDFILEMODE, MTD_FILE_MODE_RAW) != 0) {
perror("MTDFILEMODE");
goto closeall;
}
diff --git a/nandwrite.c b/nandwrite.c
index 17a717c..3f70cb3 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -274,7 +274,7 @@ int main(int argc, char * const argv[])
}
if (noecc) {
- ret = ioctl(fd, MTDFILEMODE, MTD_MODE_RAW);
+ ret = ioctl(fd, MTDFILEMODE, MTD_FILE_MODE_RAW);
if (ret) {
switch (errno) {
case ENOTTY:
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 02/10] mtd-utils: update mtd-abi.h
2011-08-31 20:00 ` [PATCH 02/10] mtd-utils: update mtd-abi.h Brian Norris
@ 2011-09-09 17:11 ` Brian Norris
0 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2011-09-09 17:11 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
Kernel ABI header added a new ioctl, killed an old one, and exposed OOB
modes to user-space. Plus, it added a lot of documentation.
We have some trivial name changes for some MTD mode constants as well.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
v2: change __u32 to __u64 in mtd_write_req for future-proofing; this
depends on the v2 patch for ioctl(MEMWRITE); see
http://lists.infradead.org/pipermail/linux-mtd/2011-September/037838.html
include/mtd/mtd-abi.h | 122 ++++++++++++++++++++++++++++++++++++++++++-------
nanddump.c | 2 +-
nandwrite.c | 2 +-
3 files changed, 107 insertions(+), 19 deletions(-)
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index a86364a..4de167b 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -45,6 +45,51 @@ struct mtd_oob_buf64 {
__u64 usr_ptr;
};
+/**
+ * MTD operation modes
+ *
+ * @MTD_OPS_PLACE_OOB: OOB data are placed at the given offset (default)
+ * @MTD_OPS_AUTO_OOB: OOB data are automatically placed at the free areas
+ * which are defined by the internal ecclayout
+ * @MTD_OPS_RAW: data are transferred as-is, with no error correction;
+ * this mode implies %MTD_OPS_PLACE_OOB
+ *
+ * These modes can be passed to ioctl(MEMWRITE) and are also used internally.
+ * See notes on "MTD file modes" for discussion on %MTD_OPS_RAW vs.
+ * %MTD_FILE_MODE_RAW.
+ */
+enum {
+ MTD_OPS_PLACE_OOB = 0,
+ MTD_OPS_AUTO_OOB = 1,
+ MTD_OPS_RAW = 2,
+};
+
+/**
+ * struct mtd_write_req - data structure for requesting a write operation
+ *
+ * @start: start address
+ * @len: length of data buffer
+ * @ooblen: length of OOB buffer
+ * @usr_data: user-provided data buffer
+ * @usr_oob: user-provided OOB buffer
+ * @mode: MTD mode (see "MTD operation modes")
+ * @padding: reserved, must be set to 0
+ *
+ * This structure supports ioctl(MEMWRITE) operations, allowing data and/or OOB
+ * writes in various modes. To write to OOB-only, set @usr_data == NULL, and to
+ * write data-only, set @usr_oob == NULL. However, setting both @usr_data and
+ * @usr_oob to NULL is not allowed.
+ */
+struct mtd_write_req {
+ __u64 start;
+ __u64 len;
+ __u64 ooblen;
+ __u64 usr_data;
+ __u64 usr_oob;
+ __u8 mode;
+ __u8 padding[7];
+};
+
#define MTD_ABSENT 0
#define MTD_RAM 1
#define MTD_ROM 2
@@ -59,13 +104,13 @@ struct mtd_oob_buf64 {
#define MTD_NO_ERASE 0x1000 /* No erase necessary */
#define MTD_POWERUP_LOCK 0x2000 /* Always locked after reset */
-// Some common devices / combinations of capabilities
+/* 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)
-/* ECC byte placement */
+/* Obsolete ECC byte placement modes (used with obsolete MEMGETOOBSEL) */
#define MTD_NANDECC_OFF 0 // Switch off ECC (Not recommended)
#define MTD_NANDECC_PLACE 1 // Use the given placement in the structure (YAFFS1 legacy mode)
#define MTD_NANDECC_AUTOPLACE 2 // Use the default placement scheme
@@ -80,21 +125,18 @@ struct mtd_oob_buf64 {
struct mtd_info_user {
__u8 type;
__u32 flags;
- __u32 size; // Total size of the MTD
+ __u32 size; /* Total size of the MTD */
__u32 erasesize;
__u32 writesize;
- __u32 oobsize; // Amount of OOB data per block (e.g. 16)
- /* The below two fields are obsolete and broken, do not use them
- * (TODO: remove at some point) */
- __u32 ecctype;
- __u32 eccsize;
+ __u32 oobsize; /* Amount of OOB data per block (e.g. 16) */
+ __u64 padding; /* Old obsolete field; do not use */
};
struct region_info_user {
__u32 offset; /* At which this region starts,
- * from the beginning of the MTD */
- __u32 erasesize; /* For this region */
- __u32 numblocks; /* Number of blocks in this region */
+ * from the beginning of the MTD */
+ __u32 erasesize; /* For this region */
+ __u32 numblocks; /* Number of blocks in this region */
__u32 regionindex;
};
@@ -104,29 +146,61 @@ struct otp_info {
__u32 locked;
};
+/*
+ * Note, the following ioctl existed in the past and was removed:
+ * #define MEMSETOOBSEL _IOW('M', 9, struct nand_oobinfo)
+ * Try to avoid adding a new ioctl with the same ioctl number.
+ */
+
+/* Get basic MTD characteristics info (better to use sysfs) */
#define MEMGETINFO _IOR('M', 1, struct mtd_info_user)
+/* Erase segment of MTD */
#define MEMERASE _IOW('M', 2, struct erase_info_user)
+/* Write out-of-band data from MTD */
#define MEMWRITEOOB _IOWR('M', 3, struct mtd_oob_buf)
+/* Read out-of-band data from MTD */
#define MEMREADOOB _IOWR('M', 4, struct mtd_oob_buf)
+/* Lock a chip (for MTD that supports it) */
#define MEMLOCK _IOW('M', 5, struct erase_info_user)
+/* Unlock a chip (for MTD that supports it) */
#define MEMUNLOCK _IOW('M', 6, struct erase_info_user)
+/* Get the number of different erase regions */
#define MEMGETREGIONCOUNT _IOR('M', 7, int)
+/* Get information about the erase region for a specific index */
#define MEMGETREGIONINFO _IOWR('M', 8, struct region_info_user)
-#define MEMSETOOBSEL _IOW('M', 9, struct nand_oobinfo)
+/* Get info about OOB modes (e.g., RAW, PLACE, AUTO) - legacy interface */
#define MEMGETOOBSEL _IOR('M', 10, struct nand_oobinfo)
+/* Check if an eraseblock is bad */
#define MEMGETBADBLOCK _IOW('M', 11, __kernel_loff_t)
+/* Mark an eraseblock as bad */
#define MEMSETBADBLOCK _IOW('M', 12, __kernel_loff_t)
+/* Set OTP (One-Time Programmable) mode (factory vs. user) */
#define OTPSELECT _IOR('M', 13, int)
+/* Get number of OTP (One-Time Programmable) regions */
#define OTPGETREGIONCOUNT _IOW('M', 14, int)
+/* Get all OTP (One-Time Programmable) info about MTD */
#define OTPGETREGIONINFO _IOW('M', 15, struct otp_info)
+/* Lock a given range of user data (must be in mode %MTD_FILE_MODE_OTP_USER) */
#define OTPLOCK _IOR('M', 16, struct otp_info)
+/* Get ECC layout (deprecated) */
#define ECCGETLAYOUT _IOR('M', 17, struct nand_ecclayout_user)
+/* Get statistics about corrected/uncorrected errors */
#define ECCGETSTATS _IOR('M', 18, struct mtd_ecc_stats)
+/* Set MTD mode on a per-file-descriptor basis (see "MTD file modes") */
#define MTDFILEMODE _IO('M', 19)
+/* Erase segment of MTD (supports 64-bit address) */
#define MEMERASE64 _IOW('M', 20, struct erase_info_user64)
+/* Write data to OOB (64-bit version) */
#define MEMWRITEOOB64 _IOWR('M', 21, struct mtd_oob_buf64)
+/* Read data from OOB (64-bit version) */
#define MEMREADOOB64 _IOWR('M', 22, struct mtd_oob_buf64)
+/* Check if chip is locked (for MTD that supports it) */
#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")
+ */
+#define MEMWRITE _IOWR('M', 24, struct mtd_write_req)
/*
* Obsolete legacy interface. Keep it in order not to break userspace
@@ -177,13 +251,27 @@ struct mtd_ecc_stats {
};
/*
- * Read/write file modes for access to MTD
+ * MTD file modes - for read/write access to MTD
+ *
+ * @MTD_FILE_MODE_NORMAL: OTP disabled, ECC enabled
+ * @MTD_FILE_MODE_OTP_FACTORY: OTP enabled in factory mode
+ * @MTD_FILE_MODE_OTP_USER: OTP enabled in user mode
+ * @MTD_FILE_MODE_RAW: OTP disabled, ECC disabled
+ *
+ * These modes can be set via ioctl(MTDFILEMODE). The mode mode will be retained
+ * separately for each open file descriptor.
+ *
+ * Note: %MTD_FILE_MODE_RAW provides the same functionality as %MTD_OPS_RAW -
+ * raw access to the flash, without error correction or autoplacement schemes.
+ * Wherever possible, the MTD_OPS_* mode will override the MTD_FILE_MODE_* mode
+ * (e.g., when using ioctl(MEMWRITE)), but in some cases, the MTD_FILE_MODE is
+ * used out of necessity (e.g., `write()', ioctl(MEMWRITEOOB64)).
*/
enum mtd_file_modes {
- MTD_MODE_NORMAL = MTD_OTP_OFF,
- MTD_MODE_OTP_FACTORY = MTD_OTP_FACTORY,
- MTD_MODE_OTP_USER = MTD_OTP_USER,
- MTD_MODE_RAW,
+ MTD_FILE_MODE_NORMAL = MTD_OTP_OFF,
+ MTD_FILE_MODE_OTP_FACTORY = MTD_OTP_FACTORY,
+ MTD_FILE_MODE_OTP_USER = MTD_OTP_USER,
+ MTD_FILE_MODE_RAW,
};
#endif /* __MTD_ABI_H__ */
diff --git a/nanddump.c b/nanddump.c
index 7a24c0d..be458c6 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -336,7 +336,7 @@ int main(int argc, char * const argv[])
readbuf = xmalloc(sizeof(readbuf) * mtd.min_io_size);
if (noecc) {
- if (ioctl(fd, MTDFILEMODE, MTD_MODE_RAW) != 0) {
+ if (ioctl(fd, MTDFILEMODE, MTD_FILE_MODE_RAW) != 0) {
perror("MTDFILEMODE");
goto closeall;
}
diff --git a/nandwrite.c b/nandwrite.c
index 17a717c..3f70cb3 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -274,7 +274,7 @@ int main(int argc, char * const argv[])
}
if (noecc) {
- ret = ioctl(fd, MTDFILEMODE, MTD_MODE_RAW);
+ ret = ioctl(fd, MTDFILEMODE, MTD_FILE_MODE_RAW);
if (ret) {
switch (errno) {
case ENOTTY:
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 03/10] nandwrite: consolidate buffer usage
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
2011-08-31 20:00 ` [PATCH 01/10] nandwrite: trivial variable move Brian Norris
2011-08-31 20:00 ` [PATCH 02/10] mtd-utils: update mtd-abi.h Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-09-11 13:07 ` Artem Bityutskiy
2011-09-14 5:15 ` Mike Frysinger
2011-08-31 20:00 ` [PATCH 04/10] libmtd: modify `mtd_write' to cover OOB writes Brian Norris
` (8 subsequent siblings)
11 siblings, 2 replies; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
Instead of using two different output buffers for OOB data, let's just
use the same one for all output. This adds an extra memcpy, but it
simplifies some future work, so it's worth it.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
nandwrite.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/nandwrite.c b/nandwrite.c
index 3f70cb3..3eea6e2 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -531,11 +531,12 @@ int main(int argc, char * const argv[])
oobreadbuf + start,
len);
}
+ } else {
+ memcpy(oobbuf, oobreadbuf, mtd.oob_size);
}
/* Write OOB data first, as ecc will be placed in there */
if (mtd_write_oob(mtd_desc, &mtd, fd, mtdoffset,
- mtd.oob_size,
- noecc ? oobreadbuf : oobbuf)) {
+ mtd.oob_size, oobbuf)) {
sys_errmsg("%s: MTD writeoob failure", mtd_device);
goto closeall;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 03/10] nandwrite: consolidate buffer usage
2011-08-31 20:00 ` [PATCH 03/10] nandwrite: consolidate buffer usage Brian Norris
@ 2011-09-11 13:07 ` Artem Bityutskiy
2011-09-14 5:15 ` Mike Frysinger
1 sibling, 0 replies; 20+ messages in thread
From: Artem Bityutskiy @ 2011-09-11 13:07 UTC (permalink / raw)
To: Brian Norris; +Cc: b35362, linux-mtd, Mike Frysinger
On Wed, 2011-08-31 at 13:00 -0700, Brian Norris wrote:
> Instead of using two different output buffers for OOB data, let's just
> use the same one for all output. This adds an extra memcpy, but it
> simplifies some future work, so it's worth it.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Also pushed this one, thanks!
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 03/10] nandwrite: consolidate buffer usage
2011-08-31 20:00 ` [PATCH 03/10] nandwrite: consolidate buffer usage Brian Norris
2011-09-11 13:07 ` Artem Bityutskiy
@ 2011-09-14 5:15 ` Mike Frysinger
2011-09-14 18:22 ` Brian Norris
1 sibling, 1 reply; 20+ messages in thread
From: Mike Frysinger @ 2011-09-14 5:15 UTC (permalink / raw)
To: Brian Norris; +Cc: b35362, linux-mtd, Artem Bityutskiy
On Wed, Aug 31, 2011 at 16:00, Brian Norris wrote:
> Instead of using two different output buffers for OOB data, let's just
> use the same one for all output. This adds an extra memcpy, but it
> simplifies some future work, so it's worth it.
could it be done by pulling out the pointer ? make oobbuf a "char *",
rename existing oobbuf to like "char _oobbuf[]", and then assign
oobbuf to the relevant buffer and assume it's always set ...
-mike
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 03/10] nandwrite: consolidate buffer usage
2011-09-14 5:15 ` Mike Frysinger
@ 2011-09-14 18:22 ` Brian Norris
2011-09-18 2:53 ` Mike Frysinger
0 siblings, 1 reply; 20+ messages in thread
From: Brian Norris @ 2011-09-14 18:22 UTC (permalink / raw)
To: Mike Frysinger; +Cc: b35362, linux-mtd, Artem Bityutskiy
On Wed, Sep 14, 2011 at 1:15 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> On Wed, Aug 31, 2011 at 16:00, Brian Norris wrote:
>> Instead of using two different output buffers for OOB data, let's just
>> use the same one for all output. This adds an extra memcpy, but it
>> simplifies some future work, so it's worth it.
>
> could it be done by pulling out the pointer ? make oobbuf a "char *",
> rename existing oobbuf to like "char _oobbuf[]", and then assign
> oobbuf to the relevant buffer and assume it's always set ...
It could be done this way, but actually, this patch was not intended
to stand alone; it was a precursor to removing one of the buffers from
nandwrite.c, as done in patch 07/10:
mtdutils: move OOB auto-layout into libmtd's mtd_write
>From the comment in patch 07:
"This patch also cleans up a need for an extra OOB buffer in nandwrite."
So I'd actually recommend that Artem push this patch (patch 03) out of
'master' and into the 'brian' branch, then we can review the final
product there. In the end, I intended nandwrite to only have a single
buffer for OOB, instead of having a read buffer and an oob buffer. We
would simply pass our unmodified OOB data to mtd_write() and then to
the new MEMWRITE ioctl, where the kernel would do AUTO or PLACE
layouts for us depending on the mode we chose.
Of course, this is just the ideal approach; many kernels will not
support MEMWRITE yet, so they would fall back to the old code. That's
what patch 07 did; it cut out one buffer and moved the old layout code
into its own libmtd function, legacy_auto_oob_layout().
legacy_auto_oob_layout allocates its own buffer when needed; this part
is very inefficient (and actually has a memory leak now that I look at
it...) Perhaps the "legacy" handling should be reviewed a little more
to prevent too many trivial buffers and memcpy's.
If nothing else, I will at least need to send a v2 for patch 07 so we
free `tmp_buf' in `legacy_auto_oob_layout()'. But it'd be better just
to completely rework that function's buffers...
Brian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 03/10] nandwrite: consolidate buffer usage
2011-09-14 18:22 ` Brian Norris
@ 2011-09-18 2:53 ` Mike Frysinger
2011-09-19 18:50 ` Brian Norris
0 siblings, 1 reply; 20+ messages in thread
From: Mike Frysinger @ 2011-09-18 2:53 UTC (permalink / raw)
To: Brian Norris; +Cc: b35362, linux-mtd, Artem Bityutskiy
On Wed, Sep 14, 2011 at 14:22, Brian Norris wrote:
> On Wed, Sep 14, 2011 at 1:15 AM, Mike Frysinger wrote:
>> On Wed, Aug 31, 2011 at 16:00, Brian Norris wrote:
>>> Instead of using two different output buffers for OOB data, let's just
>>> use the same one for all output. This adds an extra memcpy, but it
>>> simplifies some future work, so it's worth it.
>>
>> could it be done by pulling out the pointer ? make oobbuf a "char *",
>> rename existing oobbuf to like "char _oobbuf[]", and then assign
>> oobbuf to the relevant buffer and assume it's always set ...
>
> It could be done this way, but actually, this patch was not intended
> to stand alone; it was a precursor to removing one of the buffers from
> nandwrite.c
if the useless memcpy is removed by way of future patches, then this is fine
-mike
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 03/10] nandwrite: consolidate buffer usage
2011-09-18 2:53 ` Mike Frysinger
@ 2011-09-19 18:50 ` Brian Norris
0 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2011-09-19 18:50 UTC (permalink / raw)
To: Mike Frysinger; +Cc: b35362, linux-mtd, Artem Bityutskiy
On Sat, Sep 17, 2011 at 7:53 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> if the useless memcpy is removed by way of future patches, then this is fine
It's not completely removed. Please see patch 07.
I'll summarize: the memcpy is moved out of the main code path so that
if ioctl(MEMWRITE) is supported in your kernel, you have no extra
memcpy's. However, I didn't optimize the original code, which uses the
standard write+ioctl(MEMWRITEOOB[64]); it still has the useless memcpy
that we are discussing.
I'm not sure if/how your solution can work embedded at that level of
libmtd. Perhaps some functions I moved around need to be restructured
a bit.
Brian
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 04/10] libmtd: modify `mtd_write' to cover OOB writes
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
` (2 preceding siblings ...)
2011-08-31 20:00 ` [PATCH 03/10] nandwrite: consolidate buffer usage Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-08-31 20:00 ` [PATCH 05/10] libmtd: support MEMWRITE ioctl Brian Norris
` (7 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
To support the MEMWRITE ioctl, we will need a different sort of libmtd
interface for writing to flash. We will expand mtd_write to include more
functionality; for now, we just change the function definition and
description as we begin to add the actual functionality.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
include/libmtd.h | 15 +++++++++++----
lib/libmtd.c | 10 ++++++----
nandwrite.c | 4 ++--
ubi-utils/ubiformat.c | 6 ++++--
4 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/include/libmtd.h b/include/libmtd.h
index 9efccbc..07c304a 100644
--- a/include/libmtd.h
+++ b/include/libmtd.h
@@ -255,19 +255,26 @@ int mtd_read(const struct mtd_dev_info *mtd, int fd, int eb, int offs,
/**
* mtd_write - write data to an MTD device.
+ * @desc: MTD library descriptor
* @mtd: MTD device description object
* @fd: MTD device node file descriptor
* @eb: eraseblock to write to
* @offs: offset withing the eraseblock to write to
- * @buf: buffer to write
- * @len: how many bytes to write
+ * @data: data buffer to write
+ * @len: how many data bytes to write
+ * @oob: OOB buffer to write
+ * @ooblen: how many OOB bytes to write
+ * @mode: write mode (e.g., %MTD_OOB_PLACE, %MTD_OOB_RAW)
*
* This function writes @len bytes of data to eraseblock @eb and offset @offs
* of the MTD device defined by @mtd. Returns %0 in case of success and %-1 in
* case of failure.
+ *
+ * Can only write to a single page at a time if writing to OOB.
*/
-int mtd_write(const struct mtd_dev_info *mtd, int fd, int eb, int offs,
- void *buf, int len);
+int mtd_write(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb,
+ int offs, void *data, int len, void *oob, int ooblen,
+ uint8_t mode);
/**
* mtd_read_oob - read out-of-band area.
diff --git a/lib/libmtd.c b/lib/libmtd.c
index c34874e..746ea69 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -970,7 +970,8 @@ int mtd_torture(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb)
/* Write a pattern and check it */
memset(buf, patterns[i], mtd->eb_size);
- err = mtd_write(mtd, fd, eb, 0, buf, mtd->eb_size);
+ err = mtd_write(desc, mtd, fd, eb, 0, buf, mtd->eb_size, NULL,
+ 0, 0);
if (err)
goto out;
@@ -1070,8 +1071,9 @@ int mtd_read(const struct mtd_dev_info *mtd, int fd, int eb, int offs,
return 0;
}
-int mtd_write(const struct mtd_dev_info *mtd, int fd, int eb, int offs,
- void *buf, int len)
+int mtd_write(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb,
+ int offs, void *data, int len, void *oob, int ooblen,
+ uint8_t mode)
{
int ret;
off_t seek;
@@ -1105,7 +1107,7 @@ int mtd_write(const struct mtd_dev_info *mtd, int fd, int eb, int offs,
return sys_errmsg("cannot seek mtd%d to offset %llu",
mtd->mtd_num, (unsigned long long)seek);
- ret = write(fd, buf, len);
+ ret = write(fd, data, len);
if (ret != len)
return sys_errmsg("cannot write %d bytes to mtd%d (eraseblock %d, offset %d)",
len, mtd->mtd_num, eb, offs);
diff --git a/nandwrite.c b/nandwrite.c
index 3eea6e2..33a3b8f 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -543,8 +543,8 @@ int main(int argc, char * const argv[])
}
/* Write out the Page data */
- if (!onlyoob && mtd_write(&mtd, fd, mtdoffset / mtd.eb_size, mtdoffset % mtd.eb_size,
- writebuf, mtd.min_io_size)) {
+ if (!onlyoob && mtd_write(mtd_desc, &mtd, fd, mtdoffset / mtd.eb_size, mtdoffset % mtd.eb_size,
+ writebuf, mtd.min_io_size, NULL, 0, 0)) {
int i;
if (errno != EIO) {
sys_errmsg("%s: MTD write failure", mtd_device);
diff --git a/ubi-utils/ubiformat.c b/ubi-utils/ubiformat.c
index bfa1730..ed2b8d0 100644
--- a/ubi-utils/ubiformat.c
+++ b/ubi-utils/ubiformat.c
@@ -534,7 +534,8 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd,
new_len = drop_ffs(mtd, buf, mtd->eb_size);
- err = mtd_write(mtd, args.node_fd, eb, 0, buf, new_len);
+ err = mtd_write(libmtd, mtd, args.node_fd, eb, 0, buf, new_len,
+ NULL, 0, 0);
if (err) {
sys_errmsg("cannot write eraseblock %d", eb);
@@ -637,7 +638,8 @@ static int format(libmtd_t libmtd, const struct mtd_dev_info *mtd,
fflush(stdout);
}
- err = mtd_write(mtd, args.node_fd, eb, 0, hdr, write_size);
+ err = mtd_write(libmtd, mtd, args.node_fd, eb, 0, hdr,
+ write_size, NULL, 0, 0);
if (err) {
if (!args.quiet && !args.verbose)
printf("\n");
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 05/10] libmtd: support MEMWRITE ioctl
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
` (3 preceding siblings ...)
2011-08-31 20:00 ` [PATCH 04/10] libmtd: modify `mtd_write' to cover OOB writes Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-08-31 20:00 ` [PATCH 06/10] nandwrite: merge `mtd_write_oob' and `mtd_write' calls Brian Norris
` (6 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
`mtd_write()' now will first attempt to use MEMWRITE. Then, if that
doesn't exist, it will attempt to fall back to old methods for writing
OOB and/or page data.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
lib/libmtd.c | 39 +++++++++++++++++++++++++++++++--------
1 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/lib/libmtd.c b/lib/libmtd.c
index 746ea69..d47b307 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -1077,6 +1077,7 @@ int mtd_write(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb,
{
int ret;
off_t seek;
+ struct mtd_write_req ops;
ret = mtd_valid_erase_block(mtd, eb);
if (ret)
@@ -1101,16 +1102,38 @@ int mtd_write(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb,
return -1;
}
- /* Seek to the beginning of the eraseblock */
+ /* Calculate seek address */
seek = (off_t)eb * mtd->eb_size + offs;
- if (lseek(fd, seek, SEEK_SET) != seek)
- return sys_errmsg("cannot seek mtd%d to offset %llu",
- mtd->mtd_num, (unsigned long long)seek);
- ret = write(fd, data, len);
- if (ret != len)
- return sys_errmsg("cannot write %d bytes to mtd%d (eraseblock %d, offset %d)",
- len, mtd->mtd_num, eb, offs);
+ ops.start = seek;
+ ops.len = len;
+ ops.ooblen = ooblen;
+ ops.usr_data = (uint64_t)(unsigned long)data;
+ ops.usr_oob = (uint64_t)(unsigned long)oob;
+ ops.mode = mode;
+
+ ret = ioctl(fd, MEMWRITE, &ops);
+ if (ret == 0)
+ return 0;
+ else if (errno != ENOTTY)
+ return mtd_ioctl_error(mtd, eb, "MEMWRITE");
+
+ /* Fall back to old methods if necessary */
+ if (oob) {
+ if (mtd_write_oob(desc, mtd, fd, seek, ooblen, oob) < 0)
+ return sys_errmsg("cannot write to OOB");
+ }
+ if (data) {
+ /* Seek to the beginning of the eraseblock */
+ if (lseek(fd, seek, SEEK_SET) != seek)
+ return sys_errmsg("cannot seek mtd%d to offset %llu",
+ mtd->mtd_num, (unsigned long long)seek);
+ ret = write(fd, data, len);
+ if (ret != len)
+ return sys_errmsg("cannot write %d bytes to mtd%d "
+ "(eraseblock %d, offset %d)",
+ len, mtd->mtd_num, eb, offs);
+ }
return 0;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 06/10] nandwrite: merge `mtd_write_oob' and `mtd_write' calls
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
` (4 preceding siblings ...)
2011-08-31 20:00 ` [PATCH 05/10] libmtd: support MEMWRITE ioctl Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-08-31 20:00 ` [PATCH 07/10] mtdutils: move OOB auto-layout into libmtd's mtd_write Brian Norris
` (5 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
Now that `mtd_write' can write to both OOB and data regions, we need to
perform our `mtd_write' call only once.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
nandwrite.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/nandwrite.c b/nandwrite.c
index 33a3b8f..a78b0b6 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -534,17 +534,17 @@ int main(int argc, char * const argv[])
} else {
memcpy(oobbuf, oobreadbuf, mtd.oob_size);
}
- /* Write OOB data first, as ecc will be placed in there */
- if (mtd_write_oob(mtd_desc, &mtd, fd, mtdoffset,
- mtd.oob_size, oobbuf)) {
- sys_errmsg("%s: MTD writeoob failure", mtd_device);
- goto closeall;
- }
}
- /* Write out the Page data */
- if (!onlyoob && mtd_write(mtd_desc, &mtd, fd, mtdoffset / mtd.eb_size, mtdoffset % mtd.eb_size,
- writebuf, mtd.min_io_size, NULL, 0, 0)) {
+ /* Write out data */
+ ret = mtd_write(mtd_desc, &mtd, fd, mtdoffset / mtd.eb_size,
+ mtdoffset % mtd.eb_size,
+ onlyoob ? NULL : writebuf,
+ onlyoob ? 0 : mtd.min_io_size,
+ writeoob ? oobbuf : NULL,
+ writeoob ? mtd.oob_size : 0,
+ noecc ? MTD_OPS_RAW : MTD_OPS_PLACE_OOB);
+ if (ret) {
int i;
if (errno != EIO) {
sys_errmsg("%s: MTD write failure", mtd_device);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 07/10] mtdutils: move OOB auto-layout into libmtd's mtd_write
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
` (5 preceding siblings ...)
2011-08-31 20:00 ` [PATCH 06/10] nandwrite: merge `mtd_write_oob' and `mtd_write' calls Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-08-31 20:00 ` [PATCH 08/10] nandwrite: kill `--raw' option Brian Norris
` (4 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
With the addition of the the new ioctl(MEMWRITE), we can use the
kernel's internal OOB autoplacement option. It's a cleaner interface and
avoids too much duplication of coding effort.
This patch moves any legacy code (using MEMGETOOBSEL) into a legacy
function in libmtd.c. It's not exactly a "pre-2.6.30" feature, so I'm not
moving it to libmtd_legacy.c.
Now, autoplacement features are only activated if we call mtd_write with
mode == MTD_OPS_AUTO_OOB. This should fix some discrepancies for
nandwrite, where we weren't handling OOB consistently (i.e., we had
different functionality when the kernel did/didn't support MEMWRITE).
But that also means that we now default to using MTD_OPS_PLACE_OOB
instead of AUTO layout. To re-enable autoplacement, we can re-implement
the `--autoplace' option that had previously rotted.
This patch also cleans up a need for an extra OOB buffer in nandwrite.
This has been tested a little in nandsim as well as on SLC NAND flash.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
lib/libmtd.c | 39 +++++++++++++++++++++++++++++++++++++++
nandwrite.c | 53 ++++-------------------------------------------------
2 files changed, 43 insertions(+), 49 deletions(-)
diff --git a/lib/libmtd.c b/lib/libmtd.c
index d47b307..1b16de5 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -1071,6 +1071,42 @@ int mtd_read(const struct mtd_dev_info *mtd, int fd, int eb, int offs,
return 0;
}
+static int legacy_auto_oob_layout(const struct mtd_dev_info *mtd, int fd,
+ int ooblen, void *oob) {
+ struct nand_oobinfo old_oobinfo;
+ int start, len;
+ uint8_t *tmp_buf;
+
+ /* Read the current oob info */
+ if (ioctl(fd, MEMGETOOBSEL, &old_oobinfo))
+ return sys_errmsg("MEMGETOOBSEL failed");
+
+ tmp_buf = malloc(ooblen);
+ memcpy(tmp_buf, oob, ooblen);
+
+ /*
+ * We use autoplacement and have the oobinfo with the autoplacement
+ * information from the kernel available
+ */
+ if (old_oobinfo.useecc == MTD_NANDECC_AUTOPLACE) {
+ int i, tags_pos = 0;
+ for (i = 0; old_oobinfo.oobfree[i][1]; i++) {
+ /* Set the reserved bytes to 0xff */
+ start = old_oobinfo.oobfree[i][0];
+ len = old_oobinfo.oobfree[i][1];
+ memcpy(oob + start, tmp_buf + tags_pos, len);
+ tags_pos += len;
+ }
+ } else {
+ /* Set at least the ecc byte positions to 0xff */
+ start = old_oobinfo.eccbytes;
+ len = mtd->oob_size - start;
+ memcpy(oob + start, tmp_buf + start, len);
+ }
+
+ return 0;
+}
+
int mtd_write(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb,
int offs, void *data, int len, void *oob, int ooblen,
uint8_t mode)
@@ -1120,6 +1156,9 @@ int mtd_write(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb,
/* Fall back to old methods if necessary */
if (oob) {
+ if (mode == MTD_OPS_AUTO_OOB)
+ if (legacy_auto_oob_layout(mtd, fd, ooblen, oob))
+ return -1;
if (mtd_write_oob(desc, mtd, fd, seek, ooblen, oob) < 0)
return sys_errmsg("cannot write to OOB");
}
diff --git a/nandwrite.c b/nandwrite.c
index a78b0b6..920863f 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -233,7 +233,6 @@ int main(int argc, char * const argv[])
/* points to the current page inside filebuf */
unsigned char *writebuf = NULL;
/* points to the OOB for the current page in filebuf */
- unsigned char *oobreadbuf = NULL;
unsigned char *oobbuf = NULL;
libmtd_t mtd_desc;
int ebsize_aligned;
@@ -345,9 +344,6 @@ int main(int argc, char * const argv[])
filebuf = xmalloc(filebuf_max);
erase_buffer(filebuf, filebuf_max);
- oobbuf = xmalloc(mtd.oob_size);
- erase_buffer(oobbuf, mtd.oob_size);
-
/*
* Get data from input and write to the device while there is
* still input to read and we are still within the device
@@ -460,16 +456,16 @@ int main(int argc, char * const argv[])
}
if (writeoob) {
- oobreadbuf = writebuf + mtd.min_io_size;
+ oobbuf = writebuf + mtd.min_io_size;
/* Read more data for the OOB from the input if there isn't enough in the buffer */
- if ((oobreadbuf + mtd.oob_size) > (filebuf + filebuf_len)) {
+ if ((oobbuf + mtd.oob_size) > (filebuf + filebuf_len)) {
int readlen = mtd.oob_size;
- int alreadyread = (filebuf + filebuf_len) - oobreadbuf;
+ int alreadyread = (filebuf + filebuf_len) - oobbuf;
int tinycnt = alreadyread;
while (tinycnt < readlen) {
- cnt = read(ifd, oobreadbuf + tinycnt, readlen - tinycnt);
+ cnt = read(ifd, oobbuf + tinycnt, readlen - tinycnt);
if (cnt == 0) { /* EOF */
break;
} else if (cnt < 0) {
@@ -494,46 +490,6 @@ int main(int argc, char * const argv[])
imglen = 0;
}
}
-
- if (!noecc) {
- int start, len;
- struct nand_oobinfo old_oobinfo;
-
- /* Read the current oob info */
- if (ioctl(fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
- perror("MEMGETOOBSEL");
- close(fd);
- exit(EXIT_FAILURE);
- }
-
- /*
- * We use autoplacement and have the oobinfo with the autoplacement
- * information from the kernel available
- *
- * Modified to support out of order oobfree segments,
- * such as the layout used by diskonchip.c
- */
- if (old_oobinfo.useecc == MTD_NANDECC_AUTOPLACE) {
- int i, tags_pos = 0, tmp_ofs;
- for (i = 0; old_oobinfo.oobfree[i][1]; i++) {
- /* Set the reserved bytes to 0xff */
- start = old_oobinfo.oobfree[i][0];
- len = old_oobinfo.oobfree[i][1];
- tmp_ofs = rawoob ? start : tags_pos;
- memcpy(oobbuf + start, oobreadbuf + tmp_ofs, len);
- tags_pos += len;
- }
- } else {
- /* Set at least the ecc byte positions to 0xff */
- start = old_oobinfo.eccbytes;
- len = mtd.oob_size - start;
- memcpy(oobbuf + start,
- oobreadbuf + start,
- len);
- }
- } else {
- memcpy(oobbuf, oobreadbuf, mtd.oob_size);
- }
}
/* Write out data */
@@ -588,7 +544,6 @@ closeall:
close(ifd);
libmtd_close(mtd_desc);
free(filebuf);
- free(oobbuf);
close(fd);
if (failed
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 08/10] nandwrite: kill `--raw' option
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
` (6 preceding siblings ...)
2011-08-31 20:00 ` [PATCH 07/10] mtdutils: move OOB auto-layout into libmtd's mtd_write Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-08-31 20:00 ` [PATCH 09/10] nandwrite: re-implement `--autoplace' option Brian Norris
` (3 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
The `--raw' option has lost all usefulness as it overlapped with several
other OOB modes. I cannot even figure out what it was actually intended
to do, but I'm sure its functionality fits somewhere in the
MTD_OPS_{AUTO_OOB,PLACE_OOB,RAW} options, which are mostly implemented
in libmtd's mtd_write().
I don't think users need a warning for this one, unless someone can tell
me what it actually was supposed to have done in the first place.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
nandwrite.c | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/nandwrite.c b/nandwrite.c
index 920863f..45782c7 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -54,7 +54,6 @@ static void display_help(void)
" -N, --noskipbad Write without bad block skipping\n"
" -o, --oob Image contains oob data\n"
" -O, --onlyoob Image contains oob data and only write the oob part\n"
-" -r, --raw Image contains the raw oob data dumped by nanddump\n"
" -s addr, --start=addr Set start address (default is 0)\n"
" -p, --pad Pad to page size\n"
" -b, --blockalign=1|2|4 Set multiple of eraseblocks to align to\n"
@@ -86,7 +85,6 @@ static const char *mtd_device, *img;
static long long mtdoffset = 0;
static bool quiet = false;
static bool writeoob = false;
-static bool rawoob = false;
static bool onlyoob = false;
static bool markbad = false;
static bool noecc = false;
@@ -100,7 +98,7 @@ static void process_options(int argc, char * const argv[])
for (;;) {
int option_index = 0;
- static const char *short_options = "b:mnNoOpqrs:";
+ static const char *short_options = "b:mnNoOpqs:";
static const struct option long_options[] = {
{"help", no_argument, 0, 0},
{"version", no_argument, 0, 0},
@@ -112,7 +110,6 @@ static void process_options(int argc, char * const argv[])
{"onlyoob", no_argument, 0, 'O'},
{"pad", no_argument, 0, 'p'},
{"quiet", no_argument, 0, 'q'},
- {"raw", no_argument, 0, 'r'},
{"start", required_argument, 0, 's'},
{0, 0, 0, 0},
};
@@ -156,10 +153,6 @@ static void process_options(int argc, char * const argv[])
case 'p':
pad = true;
break;
- case 'r':
- rawoob = true;
- writeoob = true;
- break;
case 's':
mtdoffset = simple_strtoll(optarg, &error);
break;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 09/10] nandwrite: re-implement `--autoplace' option
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
` (7 preceding siblings ...)
2011-08-31 20:00 ` [PATCH 08/10] nandwrite: kill `--raw' option Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-08-31 20:00 ` [PATCH 10/10] nandwrite: use common.h "errmsg" functions Brian Norris
` (2 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
The restructuring of mtd_write() has allowed us to use `--autoplace'
somewhat successfully; it is supported by the new ioctl(MEMWRITE) as
well as some legacy code utilizing the deprecated ioctl(MEMGETOOBSEL).
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
nandwrite.c | 22 ++++++++++++++++++++--
1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/nandwrite.c b/nandwrite.c
index 45782c7..ca72f16 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -49,6 +49,7 @@ static void display_help(void)
"Usage: nandwrite [OPTION] MTD_DEVICE [INPUTFILE|-]\n"
"Writes to the specified MTD device.\n"
"\n"
+" -a, --autoplace Use auto OOB layout\n"
" -m, --markbad Mark blocks bad if write fails\n"
" -n, --noecc Write without ecc\n"
" -N, --noskipbad Write without bad block skipping\n"
@@ -88,6 +89,7 @@ static bool writeoob = false;
static bool onlyoob = false;
static bool markbad = false;
static bool noecc = false;
+static bool autoplace = false;
static bool noskipbad = false;
static bool pad = false;
static int blockalign = 1; /* default to using actual block size */
@@ -98,7 +100,7 @@ static void process_options(int argc, char * const argv[])
for (;;) {
int option_index = 0;
- static const char *short_options = "b:mnNoOpqs:";
+ static const char *short_options = "b:mnNoOpqs:a";
static const struct option long_options[] = {
{"help", no_argument, 0, 0},
{"version", no_argument, 0, 0},
@@ -111,6 +113,7 @@ static void process_options(int argc, char * const argv[])
{"pad", no_argument, 0, 'p'},
{"quiet", no_argument, 0, 'q'},
{"start", required_argument, 0, 's'},
+ {"autoplace", no_argument, 0, 'a'},
{0, 0, 0, 0},
};
@@ -159,6 +162,9 @@ static void process_options(int argc, char * const argv[])
case 'b':
blockalign = atoi(optarg);
break;
+ case 'a':
+ autoplace = true;
+ break;
case '?':
error++;
break;
@@ -173,6 +179,9 @@ static void process_options(int argc, char * const argv[])
errmsg_die("Can't specify negative blockalign with option -b:"
" %d", blockalign);
+ if (autoplace && noecc)
+ errmsg_die("Autoplacement and no-ECC are mutually exclusive");
+
argc -= optind;
argv += optind;
@@ -229,6 +238,7 @@ int main(int argc, char * const argv[])
unsigned char *oobbuf = NULL;
libmtd_t mtd_desc;
int ebsize_aligned;
+ uint8_t write_mode;
process_options(argc, argv);
@@ -265,6 +275,14 @@ int main(int argc, char * const argv[])
exit(EXIT_FAILURE);
}
+ /* Select OOB write mode */
+ if (noecc)
+ write_mode = MTD_OPS_RAW;
+ else if (autoplace)
+ write_mode = MTD_OPS_AUTO_OOB;
+ else
+ write_mode = MTD_OPS_PLACE_OOB;
+
if (noecc) {
ret = ioctl(fd, MTDFILEMODE, MTD_FILE_MODE_RAW);
if (ret) {
@@ -492,7 +510,7 @@ int main(int argc, char * const argv[])
onlyoob ? 0 : mtd.min_io_size,
writeoob ? oobbuf : NULL,
writeoob ? mtd.oob_size : 0,
- noecc ? MTD_OPS_RAW : MTD_OPS_PLACE_OOB);
+ write_mode);
if (ret) {
int i;
if (errno != EIO) {
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 10/10] nandwrite: use common.h "errmsg" functions
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
` (8 preceding siblings ...)
2011-08-31 20:00 ` [PATCH 09/10] nandwrite: re-implement `--autoplace' option Brian Norris
@ 2011-08-31 20:00 ` Brian Norris
2011-09-11 13:06 ` [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Artem Bityutskiy
2011-09-14 5:17 ` Mike Frysinger
11 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2011-08-31 20:00 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: b35362, Brian Norris, linux-mtd, Mike Frysinger
Convert several forms of exit(), perror(), etc. to use common.h helper
functions.
Also, move one of our parameter checks inside the process_options()
function for consistency.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
nandwrite.c | 44 ++++++++++++++++----------------------------
1 files changed, 16 insertions(+), 28 deletions(-)
diff --git a/nandwrite.c b/nandwrite.c
index ca72f16..a42f7c9 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -182,6 +182,9 @@ static void process_options(int argc, char * const argv[])
if (autoplace && noecc)
errmsg_die("Autoplacement and no-ECC are mutually exclusive");
+ if (!onlyoob && (pad && writeoob))
+ errmsg_die("Can't pad when oob data is present");
+
argc -= optind;
argv += optind;
@@ -242,23 +245,16 @@ int main(int argc, char * const argv[])
process_options(argc, argv);
- if (!onlyoob && (pad && writeoob)) {
- fprintf(stderr, "Can't pad when oob data is present.\n");
- exit(EXIT_FAILURE);
- }
-
/* Open the device */
- if ((fd = open(mtd_device, O_RDWR)) == -1) {
- perror(mtd_device);
- exit(EXIT_FAILURE);
- }
+ if ((fd = open(mtd_device, O_RDWR)) == -1)
+ sys_errmsg_die("%s", mtd_device);
mtd_desc = libmtd_open();
if (!mtd_desc)
- return errmsg("can't initialize libmtd");
+ errmsg_die("can't initialize libmtd");
/* Fill in MTD device capability structure */
if (mtd_get_dev_info(mtd_desc, mtd_device, &mtd) < 0)
- return errmsg("mtd_get_dev_info failed");
+ errmsg_die("mtd_get_dev_info failed");
/*
* Pretend erasesize is specified number of blocks - to match jffs2
@@ -267,13 +263,10 @@ int main(int argc, char * const argv[])
*/
ebsize_aligned = mtd.eb_size * blockalign;
- if (mtdoffset & (mtd.min_io_size - 1)) {
- fprintf(stderr, "The start address is not page-aligned !\n"
- "The pagesize of this NAND Flash is 0x%x.\n",
- mtd.min_io_size);
- close(fd);
- exit(EXIT_FAILURE);
- }
+ if (mtdoffset & (mtd.min_io_size - 1))
+ errmsg_die("The start address is not page-aligned !\n"
+ "The pagesize of this NAND Flash is 0x%x.\n",
+ mtd.min_io_size);
/* Select OOB write mode */
if (noecc)
@@ -290,9 +283,7 @@ int main(int argc, char * const argv[])
case ENOTTY:
errmsg_die("ioctl MTDFILEMODE is missing");
default:
- perror("MTDFILEMODE");
- close(fd);
- exit(EXIT_FAILURE);
+ sys_errmsg_die("MTDFILEMODE");
}
}
}
@@ -341,7 +332,7 @@ int main(int argc, char * const argv[])
fprintf(stderr, "Image %d bytes, NAND page %d bytes, OOB area %d"
" bytes, device size %lld bytes\n",
imglen, pagelen, mtd.oob_size, mtd.size);
- perror("Input file does not fit into device");
+ sys_errmsg("Input file does not fit into device");
goto closeall;
}
@@ -557,12 +548,9 @@ closeall:
free(filebuf);
close(fd);
- if (failed
- || ((ifd != STDIN_FILENO) && (imglen > 0))
- || (writebuf < (filebuf + filebuf_len))) {
- perror("Data was only partially written due to error\n");
- exit(EXIT_FAILURE);
- }
+ if (failed || ((ifd != STDIN_FILENO) && (imglen > 0))
+ || (writebuf < (filebuf + filebuf_len)))
+ sys_errmsg_die("Data was only partially written due to error");
/* Return happy */
return EXIT_SUCCESS;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE)
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
` (9 preceding siblings ...)
2011-08-31 20:00 ` [PATCH 10/10] nandwrite: use common.h "errmsg" functions Brian Norris
@ 2011-09-11 13:06 ` Artem Bityutskiy
2011-09-14 5:17 ` Mike Frysinger
11 siblings, 0 replies; 20+ messages in thread
From: Artem Bityutskiy @ 2011-09-11 13:06 UTC (permalink / raw)
To: Brian Norris; +Cc: b35362, linux-mtd, Mike Frysinger
On Wed, 2011-08-31 at 13:00 -0700, Brian Norris wrote:
> This patch series cleans up old features in nandwrite and supports new
> features with ioctl(MEMWRITE).
>
> Byproducts of this include:
> (1) kill --raw
> (2) re-implement --autoplace
> (3) support OOB writes for MLC NAND (only write once per page)
I do not have time to thoroughly review the patches, but they look good,
and you make a great job splitting making then easy to follow by
splitting nicely.
I am not very comfortable with "no warning" --raw removal, but we can
try it this way, I guess, it is unlikely someone will be hurt, but there
is some probability.
And last but not least, I think we should not push your changes to the
master branch of mtd-utils _befor_ the corresponding kernel-space
changes have reached Linuses tree (e.g., dwmw2 might dislike something).
So I pushed 2 patches which look independent to the master branch, and
the rest are in the 'brian' branch so far.
Thanks!
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE)
2011-08-31 20:00 [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Brian Norris
` (10 preceding siblings ...)
2011-09-11 13:06 ` [PATCH 00/10] nandwrite: cleanup, support ioctl(MEMWRITE) Artem Bityutskiy
@ 2011-09-14 5:17 ` Mike Frysinger
11 siblings, 0 replies; 20+ messages in thread
From: Mike Frysinger @ 2011-09-14 5:17 UTC (permalink / raw)
To: Brian Norris; +Cc: b35362, linux-mtd, Artem Bityutskiy
On Wed, Aug 31, 2011 at 16:00, Brian Norris wrote:
> This patch series cleans up old features in nandwrite and supports new
> features with ioctl(MEMWRITE).
in general things look OK to me
(sorry for the delay, but i'm working on MTD stuff only in my random
spare time now unlike before)
-mike
^ permalink raw reply [flat|nested] 20+ messages in thread