* [PATCH v2 1/5] mtdchar: add MEMREAD ioctl
2021-12-16 8:34 [PATCH v2 0/5] mtdchar: add MEMREAD ioctl Michał Kępień
@ 2021-12-16 8:34 ` Michał Kępień
2021-12-16 8:34 ` [PATCH v2 2/5] mtd: track maximum number of bitflips for each read request Michał Kępień
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Michał Kępień @ 2021-12-16 8:34 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
Cc: Boris Brezillon, linux-mtd, linux-kernel
User-space applications making use of MTD devices via /dev/mtd*
character devices currently have limited capabilities for reading data:
- only deprecated methods of accessing OOB layout information exist,
- there is no way to explicitly specify MTD operation mode to use; it
is auto-selected based on the MTD file mode (MTD_FILE_MODE_*) set
for the character device; in particular, this prevents using
MTD_OPS_AUTO_OOB for reads,
- all existing user-space interfaces which cause mtd_read() or
mtd_read_oob() to be called (via mtdchar_read() and
mtdchar_read_oob(), respectively) return success even when those
functions return -EUCLEAN or -EBADMSG; this renders user-space
applications using these interfaces unaware of any corrected
bitflips or uncorrectable ECC errors detected during reads.
Note that the existing MEMWRITE ioctl allows the MTD operation mode to
be explicitly set, allowing user-space applications to write page data
and OOB data without requiring them to know anything about the OOB
layout of the MTD device they are writing to (MTD_OPS_AUTO_OOB). Also,
the MEMWRITE ioctl does not mangle the return value of mtd_write_oob().
Add a new ioctl, MEMREAD, which addresses the above issues. It is
intended to be a read-side counterpart of the existing MEMWRITE ioctl.
Similarly to the latter, the read operation is performed in a loop which
processes at most mtd->erasesize bytes in each iteration. This is done
to prevent unbounded memory allocations caused by calling kmalloc() with
the 'size' argument taken directly from the struct mtd_read_req provided
by user space. However, the new ioctl is implemented so that the values
it returns match those that would have been returned if just a single
mtd_read_oob() call was issued to handle the entire read operation in
one go.
Update include/uapi/mtd/mtd-abi.h accordingly.
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
drivers/mtd/mtdchar.c | 110 +++++++++++++++++++++++++++++++++++++
include/uapi/mtd/mtd-abi.h | 43 +++++++++++++--
2 files changed, 148 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index d0f9c4b0285c..6ad59c9eed2f 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -685,6 +685,108 @@ static int mtdchar_write_ioctl(struct mtd_info *mtd,
return ret;
}
+static int mtdchar_read_ioctl(struct mtd_info *mtd,
+ struct mtd_read_req __user *argp)
+{
+ struct mtd_info *master = mtd_get_master(mtd);
+ struct mtd_read_req req;
+ void __user *usr_data, *usr_oob;
+ uint8_t *datbuf = NULL, *oobbuf = NULL;
+ size_t datbuf_len, oobbuf_len;
+ size_t orig_len, orig_ooblen;
+ int ret = 0;
+
+ if (copy_from_user(&req, argp, sizeof(req)))
+ return -EFAULT;
+
+ orig_len = req.len;
+ orig_ooblen = req.ooblen;
+
+ usr_data = (void __user *)(uintptr_t)req.usr_data;
+ usr_oob = (void __user *)(uintptr_t)req.usr_oob;
+
+ if (!master->_read_oob)
+ return -EOPNOTSUPP;
+
+ if (!usr_data)
+ req.len = 0;
+
+ if (!usr_oob)
+ req.ooblen = 0;
+
+ if (req.start + req.len > mtd->size) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ datbuf_len = min_t(size_t, req.len, mtd->erasesize);
+ if (datbuf_len > 0) {
+ datbuf = kmalloc(datbuf_len, GFP_KERNEL);
+ if (!datbuf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+
+ oobbuf_len = min_t(size_t, req.ooblen, mtd->erasesize);
+ if (oobbuf_len > 0) {
+ oobbuf = kmalloc(oobbuf_len, GFP_KERNEL);
+ if (!oobbuf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+
+ while (req.len > 0 || (!usr_data && req.ooblen > 0)) {
+ struct mtd_oob_ops ops = {
+ .mode = req.mode,
+ .len = min_t(size_t, req.len, datbuf_len),
+ .ooblen = min_t(size_t, req.ooblen, oobbuf_len),
+ .datbuf = datbuf,
+ .oobbuf = oobbuf,
+ };
+
+ /*
+ * Shorten non-page-aligned, eraseblock-sized reads so that the
+ * read ends on an eraseblock boundary. This is necessary in
+ * order to prevent OOB data for some pages from being
+ * duplicated in the output of non-page-aligned reads requiring
+ * multiple mtd_read_oob() calls to be completed.
+ */
+ if (ops.len == mtd->erasesize)
+ ops.len -= mtd_mod_by_ws(req.start + ops.len, mtd);
+
+ ret = mtd_read_oob(mtd, (loff_t)req.start, &ops);
+ if (ret && !mtd_is_bitflip_or_eccerr(ret))
+ break;
+
+ if (copy_to_user(usr_data, ops.datbuf, ops.retlen) ||
+ copy_to_user(usr_oob, ops.oobbuf, ops.oobretlen)) {
+ ret = -EFAULT;
+ break;
+ }
+
+ req.start += ops.retlen;
+ req.len -= ops.retlen;
+ usr_data += ops.retlen;
+
+ req.ooblen -= ops.oobretlen;
+ usr_oob += ops.oobretlen;
+ }
+
+out:
+ req.len = orig_len - req.len;
+ req.ooblen = orig_ooblen - req.ooblen;
+
+ if (copy_to_user(argp, &req, sizeof(req)))
+ ret = -EFAULT;
+
+ kfree(datbuf);
+ kfree(oobbuf);
+
+ return ret;
+}
+
static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
{
struct mtd_file_info *mfi = file->private_data;
@@ -707,6 +809,7 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
case MEMGETINFO:
case MEMREADOOB:
case MEMREADOOB64:
+ case MEMREAD:
case MEMISLOCKED:
case MEMGETOOBSEL:
case MEMGETBADBLOCK:
@@ -881,6 +984,13 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
break;
}
+ case MEMREAD:
+ {
+ ret = mtdchar_read_ioctl(mtd,
+ (struct mtd_read_req __user *)arg);
+ break;
+ }
+
case MEMLOCK:
{
struct erase_info_user einfo;
diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
index b869990c2db2..337e6e597fad 100644
--- a/include/uapi/mtd/mtd-abi.h
+++ b/include/uapi/mtd/mtd-abi.h
@@ -55,9 +55,9 @@ struct mtd_oob_buf64 {
* @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.
+ * These modes can be passed to ioctl(MEMWRITE) and ioctl(MEMREAD); they 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,
@@ -91,6 +91,32 @@ struct mtd_write_req {
__u8 padding[7];
};
+/**
+ * struct mtd_read_req - data structure for requesting a read 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(MEMREAD) operations, allowing data and/or OOB
+ * reads in various modes. To read from OOB-only, set @usr_data == NULL, and to
+ * read data-only, set @usr_oob == NULL. However, setting both @usr_data and
+ * @usr_oob to NULL is not allowed.
+ */
+struct mtd_read_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
@@ -207,6 +233,12 @@ struct otp_info {
#define MEMWRITE _IOWR('M', 24, struct mtd_write_req)
/* Erase a given range of user data (must be in mode %MTD_FILE_MODE_OTP_USER) */
#define OTPERASE _IOW('M', 25, struct otp_info)
+/*
+ * Most generic read interface; can read in-band and/or out-of-band in various
+ * modes (see "struct mtd_read_req"). This ioctl is not supported for flashes
+ * without OOB, e.g., NOR flash.
+ */
+#define MEMREAD _IOWR('M', 26, struct mtd_read_req)
/*
* Obsolete legacy interface. Keep it in order not to break userspace
@@ -270,8 +302,9 @@ struct mtd_ecc_stats {
* 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)).
+ * (e.g., when using ioctl(MEMWRITE) or ioctl(MEMREAD)), but in some cases, the
+ * MTD_FILE_MODE is used out of necessity (e.g., `write()',
+ * ioctl(MEMWRITEOOB64)).
*/
enum mtd_file_modes {
MTD_FILE_MODE_NORMAL = MTD_OTP_OFF,
--
2.34.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 2/5] mtd: track maximum number of bitflips for each read request
2021-12-16 8:34 [PATCH v2 0/5] mtdchar: add MEMREAD ioctl Michał Kępień
2021-12-16 8:34 ` [PATCH v2 1/5] " Michał Kępień
@ 2021-12-16 8:34 ` Michał Kępień
2021-12-16 8:34 ` [PATCH v2 3/5] mtd: always initialize 'stats' in struct mtd_oob_ops Michał Kępień
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Michał Kępień @ 2021-12-16 8:34 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
Cc: Boris Brezillon, linux-mtd, linux-kernel
mtd_read_oob() callers are currently oblivious to the details of ECC
errors detected during the read operation - they only learn (through the
return value) whether any corrected bitflips or uncorrectable errors
occurred. More detailed ECC information can be useful to user-space
applications for making better-informed choices about moving data
around.
Extend struct mtd_oob_ops with a pointer to a newly-introduced struct
mtd_req_stats and set its 'max_bitflips' field to the maximum number of
bitflips found in a single ECC step during the read operation performed
by mtd_read_oob(). This is a prerequisite for ultimately passing that
value back to user space.
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
drivers/mtd/mtdcore.c | 5 +++++
include/linux/mtd/mtd.h | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 70f492dce158..9423af6db385 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1575,6 +1575,9 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
if (!master->_read_oob && (!master->_read || ops->oobbuf))
return -EOPNOTSUPP;
+ if (ops->stats)
+ memset(ops->stats, 0, sizeof(*ops->stats));
+
if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
ret_code = mtd_io_emulated_slc(mtd, from, true, ops);
else
@@ -1592,6 +1595,8 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
return ret_code;
if (mtd->ecc_strength == 0)
return 0; /* device lacks ecc */
+ if (ops->stats)
+ ops->stats->max_bitflips = ret_code;
return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;
}
EXPORT_SYMBOL_GPL(mtd_read_oob);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 1ffa933121f6..f976aabcb378 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -40,6 +40,10 @@ struct mtd_erase_region_info {
unsigned long *lockmap; /* If keeping bitmap of locks */
};
+struct mtd_req_stats {
+ unsigned int max_bitflips;
+};
+
/**
* struct mtd_oob_ops - oob operation operands
* @mode: operation mode
@@ -70,6 +74,7 @@ struct mtd_oob_ops {
uint32_t ooboffs;
uint8_t *datbuf;
uint8_t *oobbuf;
+ struct mtd_req_stats *stats;
};
/**
--
2.34.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 3/5] mtd: always initialize 'stats' in struct mtd_oob_ops
2021-12-16 8:34 [PATCH v2 0/5] mtdchar: add MEMREAD ioctl Michał Kępień
2021-12-16 8:34 ` [PATCH v2 1/5] " Michał Kępień
2021-12-16 8:34 ` [PATCH v2 2/5] mtd: track maximum number of bitflips for each read request Michał Kępień
@ 2021-12-16 8:34 ` Michał Kępień
2021-12-16 8:34 ` [PATCH v2 4/5] mtd: add ECC error accounting for each read request Michał Kępień
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Michał Kępień @ 2021-12-16 8:34 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
Cc: Boris Brezillon, linux-mtd, linux-kernel
As the 'stats' field in struct mtd_oob_ops is used in conditional
expressions, ensure it is always zero-initialized in all such structures
to prevent random stack garbage from being interpreted as a pointer.
Strictly speaking, this problem currently only needs to be fixed for
struct mtd_oob_ops structures subsequently passed to mtd_read_oob().
However, this commit goes a step further and makes all instances of
struct mtd_oob_ops in the tree zero-initialized, in hope of preventing
future problems, e.g. if struct mtd_req_stats gets extended with write
statistics at some point.
Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
Obviously this objective can be achieved in various ways. I was aiming
for a minimal diff which does the job.
drivers/mtd/inftlcore.c | 6 +++---
drivers/mtd/mtdswap.c | 6 +++---
drivers/mtd/nand/onenand/onenand_base.c | 4 ++--
drivers/mtd/nand/onenand/onenand_bbt.c | 2 +-
drivers/mtd/nand/raw/nand_bbt.c | 8 ++++----
drivers/mtd/nand/raw/sm_common.c | 2 +-
drivers/mtd/nftlcore.c | 6 +++---
drivers/mtd/sm_ftl.c | 4 ++--
drivers/mtd/ssfdc.c | 2 +-
drivers/mtd/tests/nandbiterrs.c | 2 +-
drivers/mtd/tests/oobtest.c | 8 ++++----
drivers/mtd/tests/readtest.c | 2 +-
fs/jffs2/wbuf.c | 6 +++---
13 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/mtd/inftlcore.c b/drivers/mtd/inftlcore.c
index 6b48397c750c..58ca1c21ebe6 100644
--- a/drivers/mtd/inftlcore.c
+++ b/drivers/mtd/inftlcore.c
@@ -136,7 +136,7 @@ static void inftl_remove_dev(struct mtd_blktrans_dev *dev)
int inftl_read_oob(struct mtd_info *mtd, loff_t offs, size_t len,
size_t *retlen, uint8_t *buf)
{
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int res;
ops.mode = MTD_OPS_PLACE_OOB;
@@ -156,7 +156,7 @@ int inftl_read_oob(struct mtd_info *mtd, loff_t offs, size_t len,
int inftl_write_oob(struct mtd_info *mtd, loff_t offs, size_t len,
size_t *retlen, uint8_t *buf)
{
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int res;
ops.mode = MTD_OPS_PLACE_OOB;
@@ -176,7 +176,7 @@ int inftl_write_oob(struct mtd_info *mtd, loff_t offs, size_t len,
static int inftl_write(struct mtd_info *mtd, loff_t offs, size_t len,
size_t *retlen, uint8_t *buf, uint8_t *oob)
{
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int res;
ops.mode = MTD_OPS_PLACE_OOB;
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index e86b04bc1d6b..ce3796b929e7 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -323,7 +323,7 @@ static int mtdswap_read_markers(struct mtdswap_dev *d, struct swap_eb *eb)
struct mtdswap_oobdata *data, *data2;
int ret;
loff_t offset;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
offset = mtdswap_eb_offset(d, eb);
@@ -370,7 +370,7 @@ static int mtdswap_write_marker(struct mtdswap_dev *d, struct swap_eb *eb,
struct mtdswap_oobdata n;
int ret;
loff_t offset;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
ops.ooboffs = 0;
ops.oobbuf = (uint8_t *)&n;
@@ -878,7 +878,7 @@ static unsigned int mtdswap_eblk_passes(struct mtdswap_dev *d,
loff_t base, pos;
unsigned int *p1 = (unsigned int *)d->page_buf;
unsigned char *p2 = (unsigned char *)d->oob_buf;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int ret;
ops.mode = MTD_OPS_AUTO_OOB;
diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c
index 958bac54b190..5810104420a2 100644
--- a/drivers/mtd/nand/onenand/onenand_base.c
+++ b/drivers/mtd/nand/onenand/onenand_base.c
@@ -2935,7 +2935,7 @@ static int do_otp_write(struct mtd_info *mtd, loff_t to, size_t len,
struct onenand_chip *this = mtd->priv;
unsigned char *pbuf = buf;
int ret;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
/* Force buffer page aligned */
if (len < mtd->writesize) {
@@ -2977,7 +2977,7 @@ static int do_otp_lock(struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, u_char *buf)
{
struct onenand_chip *this = mtd->priv;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int ret;
if (FLEXONENAND(this)) {
diff --git a/drivers/mtd/nand/onenand/onenand_bbt.c b/drivers/mtd/nand/onenand/onenand_bbt.c
index def89f108007..c9d106d87ab3 100644
--- a/drivers/mtd/nand/onenand/onenand_bbt.c
+++ b/drivers/mtd/nand/onenand/onenand_bbt.c
@@ -61,7 +61,7 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_descr
int startblock;
loff_t from;
size_t readlen, ooblen;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int rgn;
printk(KERN_INFO "Scanning device for bad blocks\n");
diff --git a/drivers/mtd/nand/raw/nand_bbt.c b/drivers/mtd/nand/raw/nand_bbt.c
index ab630af3a309..817fff3584e3 100644
--- a/drivers/mtd/nand/raw/nand_bbt.c
+++ b/drivers/mtd/nand/raw/nand_bbt.c
@@ -313,7 +313,7 @@ static int scan_read_oob(struct nand_chip *this, uint8_t *buf, loff_t offs,
size_t len)
{
struct mtd_info *mtd = nand_to_mtd(this);
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int res, ret = 0;
ops.mode = MTD_OPS_PLACE_OOB;
@@ -354,7 +354,7 @@ static int scan_write_bbt(struct nand_chip *this, loff_t offs, size_t len,
uint8_t *buf, uint8_t *oob)
{
struct mtd_info *mtd = nand_to_mtd(this);
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
ops.mode = MTD_OPS_PLACE_OOB;
ops.ooboffs = 0;
@@ -416,7 +416,7 @@ static int scan_block_fast(struct nand_chip *this, struct nand_bbt_descr *bd,
{
struct mtd_info *mtd = nand_to_mtd(this);
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int ret, page_offset;
ops.ooblen = mtd->oobsize;
@@ -756,7 +756,7 @@ static int write_bbt(struct nand_chip *this, uint8_t *buf,
uint8_t rcode = td->reserved_block_code;
size_t retlen, len = 0;
loff_t to;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
ops.ooblen = mtd->oobsize;
ops.ooboffs = 0;
diff --git a/drivers/mtd/nand/raw/sm_common.c b/drivers/mtd/nand/raw/sm_common.c
index ba24cb36d0b9..6df33e8d77df 100644
--- a/drivers/mtd/nand/raw/sm_common.c
+++ b/drivers/mtd/nand/raw/sm_common.c
@@ -99,7 +99,7 @@ static const struct mtd_ooblayout_ops oob_sm_small_ops = {
static int sm_block_markbad(struct nand_chip *chip, loff_t ofs)
{
struct mtd_info *mtd = nand_to_mtd(chip);
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
struct sm_oob oob;
int ret;
diff --git a/drivers/mtd/nftlcore.c b/drivers/mtd/nftlcore.c
index 913db0dd6a8d..64d319e959b2 100644
--- a/drivers/mtd/nftlcore.c
+++ b/drivers/mtd/nftlcore.c
@@ -124,7 +124,7 @@ int nftl_read_oob(struct mtd_info *mtd, loff_t offs, size_t len,
size_t *retlen, uint8_t *buf)
{
loff_t mask = mtd->writesize - 1;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int res;
ops.mode = MTD_OPS_PLACE_OOB;
@@ -145,7 +145,7 @@ int nftl_write_oob(struct mtd_info *mtd, loff_t offs, size_t len,
size_t *retlen, uint8_t *buf)
{
loff_t mask = mtd->writesize - 1;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int res;
ops.mode = MTD_OPS_PLACE_OOB;
@@ -168,7 +168,7 @@ static int nftl_write(struct mtd_info *mtd, loff_t offs, size_t len,
size_t *retlen, uint8_t *buf, uint8_t *oob)
{
loff_t mask = mtd->writesize - 1;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int res;
ops.mode = MTD_OPS_PLACE_OOB;
diff --git a/drivers/mtd/sm_ftl.c b/drivers/mtd/sm_ftl.c
index 0cff2cda1b5a..cb182333d635 100644
--- a/drivers/mtd/sm_ftl.c
+++ b/drivers/mtd/sm_ftl.c
@@ -239,7 +239,7 @@ static int sm_read_sector(struct sm_ftl *ftl,
uint8_t *buffer, struct sm_oob *oob)
{
struct mtd_info *mtd = ftl->trans->mtd;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
struct sm_oob tmp_oob;
int ret = -EIO;
int try = 0;
@@ -323,7 +323,7 @@ static int sm_write_sector(struct sm_ftl *ftl,
int zone, int block, int boffset,
uint8_t *buffer, struct sm_oob *oob)
{
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
struct mtd_info *mtd = ftl->trans->mtd;
int ret;
diff --git a/drivers/mtd/ssfdc.c b/drivers/mtd/ssfdc.c
index 1d05c121904c..04da685c36be 100644
--- a/drivers/mtd/ssfdc.c
+++ b/drivers/mtd/ssfdc.c
@@ -163,7 +163,7 @@ static int read_physical_sector(struct mtd_info *mtd, uint8_t *sect_buf,
/* Read redundancy area (wrapper to MTD_READ_OOB */
static int read_raw_oob(struct mtd_info *mtd, loff_t offs, uint8_t *buf)
{
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int ret;
ops.mode = MTD_OPS_RAW;
diff --git a/drivers/mtd/tests/nandbiterrs.c b/drivers/mtd/tests/nandbiterrs.c
index 08084c018a59..98d7508f95b1 100644
--- a/drivers/mtd/tests/nandbiterrs.c
+++ b/drivers/mtd/tests/nandbiterrs.c
@@ -99,7 +99,7 @@ static int write_page(int log)
static int rewrite_page(int log)
{
int err = 0;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
if (log)
pr_info("rewrite page\n");
diff --git a/drivers/mtd/tests/oobtest.c b/drivers/mtd/tests/oobtest.c
index 532997e10e29..13fed398937e 100644
--- a/drivers/mtd/tests/oobtest.c
+++ b/drivers/mtd/tests/oobtest.c
@@ -56,7 +56,7 @@ static void do_vary_offset(void)
static int write_eraseblock(int ebnum)
{
int i;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int err = 0;
loff_t addr = (loff_t)ebnum * mtd->erasesize;
@@ -165,7 +165,7 @@ static size_t memffshow(loff_t addr, loff_t offset, const void *cs,
static int verify_eraseblock(int ebnum)
{
int i;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int err = 0;
loff_t addr = (loff_t)ebnum * mtd->erasesize;
size_t bitflips;
@@ -260,7 +260,7 @@ static int verify_eraseblock(int ebnum)
static int verify_eraseblock_in_one_go(int ebnum)
{
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int err = 0;
loff_t addr = (loff_t)ebnum * mtd->erasesize;
size_t len = mtd->oobavail * pgcnt;
@@ -338,7 +338,7 @@ static int __init mtd_oobtest_init(void)
int err = 0;
unsigned int i;
uint64_t tmp;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
loff_t addr = 0, addr0;
printk(KERN_INFO "\n");
diff --git a/drivers/mtd/tests/readtest.c b/drivers/mtd/tests/readtest.c
index e70d588083a3..99670ef91f2b 100644
--- a/drivers/mtd/tests/readtest.c
+++ b/drivers/mtd/tests/readtest.c
@@ -47,7 +47,7 @@ static int read_eraseblock_by_page(int ebnum)
err = ret;
}
if (mtd->oobsize) {
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
ops.mode = MTD_OPS_PLACE_OOB;
ops.len = 0;
diff --git a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c
index c6821a509481..4061e0ba7010 100644
--- a/fs/jffs2/wbuf.c
+++ b/fs/jffs2/wbuf.c
@@ -1035,7 +1035,7 @@ int jffs2_check_oob_empty(struct jffs2_sb_info *c,
{
int i, ret;
int cmlen = min_t(int, c->oobavail, OOB_CM_SIZE);
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
ops.mode = MTD_OPS_AUTO_OOB;
ops.ooblen = NR_OOB_SCAN_PAGES * c->oobavail;
@@ -1076,7 +1076,7 @@ int jffs2_check_oob_empty(struct jffs2_sb_info *c,
int jffs2_check_nand_cleanmarker(struct jffs2_sb_info *c,
struct jffs2_eraseblock *jeb)
{
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int ret, cmlen = min_t(int, c->oobavail, OOB_CM_SIZE);
ops.mode = MTD_OPS_AUTO_OOB;
@@ -1101,7 +1101,7 @@ int jffs2_write_nand_cleanmarker(struct jffs2_sb_info *c,
struct jffs2_eraseblock *jeb)
{
int ret;
- struct mtd_oob_ops ops;
+ struct mtd_oob_ops ops = { };
int cmlen = min_t(int, c->oobavail, OOB_CM_SIZE);
ops.mode = MTD_OPS_AUTO_OOB;
--
2.34.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 4/5] mtd: add ECC error accounting for each read request
2021-12-16 8:34 [PATCH v2 0/5] mtdchar: add MEMREAD ioctl Michał Kępień
` (2 preceding siblings ...)
2021-12-16 8:34 ` [PATCH v2 3/5] mtd: always initialize 'stats' in struct mtd_oob_ops Michał Kępień
@ 2021-12-16 8:34 ` Michał Kępień
2021-12-16 8:34 ` [PATCH v2 5/5] mtdchar: extend MEMREAD ioctl to return ECC statistics Michał Kępień
2021-12-16 10:24 ` [PATCH v2 0/5] mtdchar: add MEMREAD ioctl Boris Brezillon
5 siblings, 0 replies; 8+ messages in thread
From: Michał Kępień @ 2021-12-16 8:34 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
Cc: Boris Brezillon, linux-mtd, linux-kernel
Extend struct mtd_req_stats with two new fields holding the number of
corrected bitflips and uncorrectable errors detected during a read
operation. This is a prerequisite for ultimately passing those counters
to user space, where they can be useful to applications for making
better-informed choices about moving data around. Maintaining these
counters is also necessary for the value returned by mtdchar's MEMREAD
ioctl to be determined correctly.
Unlike 'max_bitflips' (which is set - in a common code path - to the
return value of a function called while the MTD device's mutex is held),
these counters have to be maintained in each MTD driver which defines
the '_read_oob' callback because the statistics need to be calculated
while the MTD device's mutex is held.
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
drivers/mtd/devices/docg3.c | 8 ++++++++
drivers/mtd/nand/onenand/onenand_base.c | 12 ++++++++++++
drivers/mtd/nand/raw/nand_base.c | 10 ++++++++++
drivers/mtd/nand/spi/core.c | 10 ++++++++++
include/linux/mtd/mtd.h | 2 ++
5 files changed, 42 insertions(+)
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 5b0ae5ddad74..3783ae5c6d23 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -871,6 +871,7 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
u8 *buf = ops->datbuf;
size_t len, ooblen, nbdata, nboob;
u8 hwecc[DOC_ECC_BCH_SIZE], eccconf1;
+ struct mtd_ecc_stats old_stats;
int max_bitflips = 0;
if (buf)
@@ -895,6 +896,7 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
ret = 0;
skip = from % DOC_LAYOUT_PAGE_SIZE;
mutex_lock(&docg3->cascade->lock);
+ old_stats = mtd->ecc_stats;
while (ret >= 0 && (len > 0 || ooblen > 0)) {
calc_block_sector(from - skip, &block0, &block1, &page, &ofs,
docg3->reliable);
@@ -966,6 +968,12 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
}
out:
+ if (ops->stats) {
+ ops->stats->uncorrectable_errors +=
+ mtd->ecc_stats.failed - old_stats.failed;
+ ops->stats->corrected_bitflips +=
+ mtd->ecc_stats.corrected - old_stats.corrected;
+ }
mutex_unlock(&docg3->cascade->lock);
return ret;
err_in_read:
diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c
index 5810104420a2..f66385faf631 100644
--- a/drivers/mtd/nand/onenand/onenand_base.c
+++ b/drivers/mtd/nand/onenand/onenand_base.c
@@ -1440,6 +1440,7 @@ static int onenand_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_oob_ops *ops)
{
struct onenand_chip *this = mtd->priv;
+ struct mtd_ecc_stats old_stats;
int ret;
switch (ops->mode) {
@@ -1453,12 +1454,23 @@ static int onenand_read_oob(struct mtd_info *mtd, loff_t from,
}
onenand_get_device(mtd, FL_READING);
+
+ old_stats = mtd->ecc_stats;
+
if (ops->datbuf)
ret = ONENAND_IS_4KB_PAGE(this) ?
onenand_mlc_read_ops_nolock(mtd, from, ops) :
onenand_read_ops_nolock(mtd, from, ops);
else
ret = onenand_read_oob_nolock(mtd, from, ops);
+
+ if (ops->stats) {
+ ops->stats->uncorrectable_errors +=
+ mtd->ecc_stats.failed - old_stats.failed;
+ ops->stats->corrected_bitflips +=
+ mtd->ecc_stats.corrected - old_stats.corrected;
+ }
+
onenand_release_device(mtd);
return ret;
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index b3a9bc08b4bb..902a88a4caaf 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3750,6 +3750,7 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_oob_ops *ops)
{
struct nand_chip *chip = mtd_to_nand(mtd);
+ struct mtd_ecc_stats old_stats;
int ret;
ops->retlen = 0;
@@ -3763,11 +3764,20 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from,
if (ret)
return ret;
+ old_stats = mtd->ecc_stats;
+
if (!ops->datbuf)
ret = nand_do_read_oob(chip, from, ops);
else
ret = nand_do_read_ops(chip, from, ops);
+ if (ops->stats) {
+ ops->stats->uncorrectable_errors +=
+ mtd->ecc_stats.failed - old_stats.failed;
+ ops->stats->corrected_bitflips +=
+ mtd->ecc_stats.corrected - old_stats.corrected;
+ }
+
nand_release_device(chip);
return ret;
}
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 2c8685f1f2fa..5c956c8cae9f 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -629,6 +629,7 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
{
struct spinand_device *spinand = mtd_to_spinand(mtd);
struct nand_device *nand = mtd_to_nanddev(mtd);
+ struct mtd_ecc_stats old_stats;
unsigned int max_bitflips = 0;
struct nand_io_iter iter;
bool disable_ecc = false;
@@ -640,6 +641,8 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
mutex_lock(&spinand->lock);
+ old_stats = mtd->ecc_stats;
+
nanddev_io_for_each_page(nand, NAND_PAGE_READ, from, ops, &iter) {
if (disable_ecc)
iter.req.mode = MTD_OPS_RAW;
@@ -662,6 +665,13 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
ops->oobretlen += iter.req.ooblen;
}
+ if (ops->stats) {
+ ops->stats->uncorrectable_errors +=
+ mtd->ecc_stats.failed - old_stats.failed;
+ ops->stats->corrected_bitflips +=
+ mtd->ecc_stats.corrected - old_stats.corrected;
+ }
+
mutex_unlock(&spinand->lock);
if (ecc_failed && !ret)
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index f976aabcb378..45a0c20305b0 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -41,6 +41,8 @@ struct mtd_erase_region_info {
};
struct mtd_req_stats {
+ unsigned int uncorrectable_errors;
+ unsigned int corrected_bitflips;
unsigned int max_bitflips;
};
--
2.34.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 5/5] mtdchar: extend MEMREAD ioctl to return ECC statistics
2021-12-16 8:34 [PATCH v2 0/5] mtdchar: add MEMREAD ioctl Michał Kępień
` (3 preceding siblings ...)
2021-12-16 8:34 ` [PATCH v2 4/5] mtd: add ECC error accounting for each read request Michał Kępień
@ 2021-12-16 8:34 ` Michał Kępień
2021-12-16 10:24 ` [PATCH v2 0/5] mtdchar: add MEMREAD ioctl Boris Brezillon
5 siblings, 0 replies; 8+ messages in thread
From: Michał Kępień @ 2021-12-16 8:34 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
Cc: Boris Brezillon, linux-mtd, linux-kernel
While returning -EUCLEAN or -EBADMSG is a valid signal for user space
that the ECC algorithm detected errors during a read operation, that
signal is not granular enough to cover all use cases. For example,
knowing the maximum number of bitflips detected in a single ECC step
during a read operation performed on a given page may be useful when
dealing with an MTD partition whose ECC layout varies across pages (e.g.
a partition consisting of a bootloader area using a "custom" ECC layout
followed by data pages using a "standard" ECC layout).
Extend struct mtd_read_req with a newly introduced struct
mtd_read_req_ecc_stats and set the fields of that structure in
mtdchar_read_ioctl(). Use ECC statistics to ensure the error code
returned by the MEMREAD ioctl accounts for all ECC errors detected
during the read operation.
Link: https://www.infradead.org/pipermail/linux-mtd/2016-April/067085.html
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
drivers/mtd/mtdchar.c | 26 ++++++++++++++++++++++++++
include/uapi/mtd/mtd-abi.h | 21 +++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 6ad59c9eed2f..68cc91d82a5d 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -714,6 +714,10 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
if (!usr_oob)
req.ooblen = 0;
+ req.ecc_stats.uncorrectable_errors = 0;
+ req.ecc_stats.corrected_bitflips = 0;
+ req.ecc_stats.max_bitflips = 0;
+
if (req.start + req.len > mtd->size) {
ret = -EINVAL;
goto out;
@@ -738,12 +742,14 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
}
while (req.len > 0 || (!usr_data && req.ooblen > 0)) {
+ struct mtd_req_stats stats;
struct mtd_oob_ops ops = {
.mode = req.mode,
.len = min_t(size_t, req.len, datbuf_len),
.ooblen = min_t(size_t, req.ooblen, oobbuf_len),
.datbuf = datbuf,
.oobbuf = oobbuf,
+ .stats = &stats,
};
/*
@@ -757,6 +763,13 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
ops.len -= mtd_mod_by_ws(req.start + ops.len, mtd);
ret = mtd_read_oob(mtd, (loff_t)req.start, &ops);
+
+ req.ecc_stats.uncorrectable_errors +=
+ stats.uncorrectable_errors;
+ req.ecc_stats.corrected_bitflips += stats.corrected_bitflips;
+ req.ecc_stats.max_bitflips =
+ max(req.ecc_stats.max_bitflips, stats.max_bitflips);
+
if (ret && !mtd_is_bitflip_or_eccerr(ret))
break;
@@ -774,6 +787,19 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
usr_oob += ops.oobretlen;
}
+ /*
+ * As multiple iterations of the above loop (and therefore multiple
+ * mtd_read_oob() calls) may be necessary to complete the read request,
+ * adjust the final return code to ensure it accounts for all detected
+ * ECC errors.
+ */
+ if (!ret || mtd_is_bitflip(ret)) {
+ if (req.ecc_stats.uncorrectable_errors > 0)
+ ret = -EBADMSG;
+ else if (req.ecc_stats.corrected_bitflips > 0)
+ ret = -EUCLEAN;
+ }
+
out:
req.len = orig_len - req.len;
req.ooblen = orig_ooblen - req.ooblen;
diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
index 337e6e597fad..bc68f266c174 100644
--- a/include/uapi/mtd/mtd-abi.h
+++ b/include/uapi/mtd/mtd-abi.h
@@ -91,6 +91,25 @@ struct mtd_write_req {
__u8 padding[7];
};
+/**
+ * struct mtd_read_req_ecc_stats - ECC statistics for a read operation
+ *
+ * @uncorrectable_errors: the number of uncorrectable errors that happened
+ * during the read operation
+ * @corrected_bitflips: the number of bitflips corrected during the read
+ * operation
+ * @max_bitflips: the maximum number of bitflips detected in any single ECC
+ * step for the data read during the operation; this information
+ * can be used to decide whether the data stored in a specific
+ * region of the MTD device should be moved somewhere else to
+ * avoid data loss.
+ */
+struct mtd_read_req_ecc_stats {
+ __u32 uncorrectable_errors;
+ __u32 corrected_bitflips;
+ __u32 max_bitflips;
+};
+
/**
* struct mtd_read_req - data structure for requesting a read operation
*
@@ -101,6 +120,7 @@ struct mtd_write_req {
* @usr_oob: user-provided OOB buffer
* @mode: MTD mode (see "MTD operation modes")
* @padding: reserved, must be set to 0
+ * @ecc_stats: ECC statistics for the read operation
*
* This structure supports ioctl(MEMREAD) operations, allowing data and/or OOB
* reads in various modes. To read from OOB-only, set @usr_data == NULL, and to
@@ -115,6 +135,7 @@ struct mtd_read_req {
__u64 usr_oob;
__u8 mode;
__u8 padding[7];
+ struct mtd_read_req_ecc_stats ecc_stats;
};
#define MTD_ABSENT 0
--
2.34.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 0/5] mtdchar: add MEMREAD ioctl
2021-12-16 8:34 [PATCH v2 0/5] mtdchar: add MEMREAD ioctl Michał Kępień
` (4 preceding siblings ...)
2021-12-16 8:34 ` [PATCH v2 5/5] mtdchar: extend MEMREAD ioctl to return ECC statistics Michał Kępień
@ 2021-12-16 10:24 ` Boris Brezillon
2021-12-16 13:12 ` Michał Kępień
5 siblings, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2021-12-16 10:24 UTC (permalink / raw)
To: Michał Kępień
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, linux-mtd,
linux-kernel
On Thu, 16 Dec 2021 09:34:13 +0100
Michał Kępień <kernel@kempniu.pl> wrote:
> This patch series adds a new mtdchar ioctl, MEMREAD. Its purpose is to
> serve as a read counterpart of the MEMWRITE ioctl, exposing a broader
> set of capabilities for read operations (e.g. use of MTD_OPS_AUTO_OOB,
> access to ECC statistics) to user-space applications making use of MTD
> devices via /dev/mtd* character devices.
>
> Changes from v1:
>
> - Added patches 2-5 which enable the new MEMREAD ioctl to report ECC
> statistics for the read operation back to user space. (There are
> obviously different ways these changes can be split up into separate
> commits; I was aiming for maximum ease of review.)
>
> - The 'retlen' and 'oobretlen' fields were not set in the struct
> mtd_read_req returned to userspace. This was done properly in
> Boris' original draft patch [1], but I missed it in my v1.
>
> - Invalid IS_ERR() checks were replaced with NULL checks. This was an
> artifact of copy-pasting mtdchar_write_ioctl() in v1: unlike
> memdup_user() used therein, kmalloc() always returns NULL on error.
>
> - Minor subject prefix adjustment for patch 1/5 ("mtd" -> "mtdchar").
>
> [1] https://www.infradead.org/pipermail/linux-mtd/2016-April/067187.html
>
> Michał Kępień (5):
> mtdchar: add MEMREAD ioctl
> mtd: track maximum number of bitflips for each read request
> mtd: always initialize 'stats' in struct mtd_oob_ops
> mtd: add ECC error accounting for each read request
> mtdchar: extend MEMREAD ioctl to return ECC statistics
Splitting patch 1 and 5 means you have an incompatible ABI change
between those 2 commits, thus breaking bisectability. I'd recommend
putting patches 2-4 first and squashing patch 1 and 5 in a single
commit placed at the end of the series. The other options would be to
add a way to extend ioctls in a backward compatible way (the DRM
subsystem does that by filling the unspecified part of the struct with
zeros, and relying on the fact that 0 values always implies 'default
behavior' when the struct is extended [1]).
[1]https://elixir.bootlin.com/linux/v5.16-rc5/source/drivers/gpu/drm/drm_ioctl.c#L882
>
> drivers/mtd/devices/docg3.c | 8 ++
> drivers/mtd/inftlcore.c | 6 +-
> drivers/mtd/mtdchar.c | 136 ++++++++++++++++++++++++
> drivers/mtd/mtdcore.c | 5 +
> drivers/mtd/mtdswap.c | 6 +-
> drivers/mtd/nand/onenand/onenand_base.c | 16 ++-
> drivers/mtd/nand/onenand/onenand_bbt.c | 2 +-
> drivers/mtd/nand/raw/nand_base.c | 10 ++
> drivers/mtd/nand/raw/nand_bbt.c | 8 +-
> drivers/mtd/nand/raw/sm_common.c | 2 +-
> drivers/mtd/nand/spi/core.c | 10 ++
> drivers/mtd/nftlcore.c | 6 +-
> drivers/mtd/sm_ftl.c | 4 +-
> drivers/mtd/ssfdc.c | 2 +-
> drivers/mtd/tests/nandbiterrs.c | 2 +-
> drivers/mtd/tests/oobtest.c | 8 +-
> drivers/mtd/tests/readtest.c | 2 +-
> fs/jffs2/wbuf.c | 6 +-
> include/linux/mtd/mtd.h | 7 ++
> include/uapi/mtd/mtd-abi.h | 64 ++++++++++-
> 20 files changed, 276 insertions(+), 34 deletions(-)
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 0/5] mtdchar: add MEMREAD ioctl
2021-12-16 10:24 ` [PATCH v2 0/5] mtdchar: add MEMREAD ioctl Boris Brezillon
@ 2021-12-16 13:12 ` Michał Kępień
0 siblings, 0 replies; 8+ messages in thread
From: Michał Kępień @ 2021-12-16 13:12 UTC (permalink / raw)
To: Boris Brezillon
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, linux-mtd,
linux-kernel
> Splitting patch 1 and 5 means you have an incompatible ABI change
> between those 2 commits, thus breaking bisectability. I'd recommend
> putting patches 2-4 first and squashing patch 1 and 5 in a single
> commit placed at the end of the series. The other options would be to
> add a way to extend ioctls in a backward compatible way (the DRM
> subsystem does that by filling the unspecified part of the struct with
> zeros, and relying on the fact that 0 values always implies 'default
> behavior' when the struct is extended [1]).
Ack, thanks. I will follow your recommendation in v3 and in the
meantime the existing split should make it easier to follow how v2
extends v1 with ECC statistics handling.
--
Best regards,
Michał Kępień
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 8+ messages in thread