* [PATCH v5 1/3] mtd: mtdpart: Make ECC stat handling consistent
@ 2018-01-09 8:50 Boris Brezillon
2018-01-09 8:50 ` [PATCH v5 2/3] mtd: Fallback to ->_read/write_oob() when ->_read/write() is missing Boris Brezillon
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Boris Brezillon @ 2018-01-09 8:50 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, linux-mtd
Cc: Robert Jarzmik, Kyungmin Park, Peter Pan, Frieder Schrempf,
Ladislav Michl
part_read() and part_read_oob() where counting ECC failures and
bitflips differently. Adjust part_read_oob() to mimic what is done in
part_read(). This is in needed to use ->_read_oob() as a fallback when
when ->_read() is not implemented.
Note that bitflips and ECC failure accounting on MTD partitions is
broken by design, because nothing prevents concurrent accesses to the
underlying master MTD device between the moment we save the stats in a
local variable and the moment master->_read[_oob]() returns. It's not
something that can easily be fixed, so leave it like that for now.
Suggested-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Tested-by: Ladislav Michl <ladis@linux-mips.org>
---
Changes in v5:
- save master's ECC stats before calling ->_read_oob()
Changes in v4:
- new patch
---
drivers/mtd/mtdpart.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index be088bccd593..79bf1f61c7a0 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -105,6 +105,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_oob_ops *ops)
{
struct mtd_part *part = mtd_to_part(mtd);
+ struct mtd_ecc_stats stats;
int res;
if (from >= mtd->size)
@@ -126,13 +127,14 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
return -EINVAL;
}
+ stats = part->parent->ecc_stats;
res = part->parent->_read_oob(part->parent, from + part->offset, ops);
- if (unlikely(res)) {
- if (mtd_is_bitflip(res))
- mtd->ecc_stats.corrected++;
- if (mtd_is_eccerr(res))
- mtd->ecc_stats.failed++;
- }
+ if (unlikely(mtd_is_eccerr(res)))
+ mtd->ecc_stats.failed +=
+ part->parent->ecc_stats.failed - stats.failed;
+ else
+ mtd->ecc_stats.corrected +=
+ part->parent->ecc_stats.corrected - stats.corrected;
return res;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v5 2/3] mtd: Fallback to ->_read/write_oob() when ->_read/write() is missing
2018-01-09 8:50 [PATCH v5 1/3] mtd: mtdpart: Make ECC stat handling consistent Boris Brezillon
@ 2018-01-09 8:50 ` Boris Brezillon
2018-01-09 8:50 ` [PATCH v5 3/3] mtd: Remove duplicate checks on mtd_oob_ops parameter Boris Brezillon
2018-01-11 9:06 ` [PATCH v5 1/3] mtd: mtdpart: Make ECC stat handling consistent Boris Brezillon
2 siblings, 0 replies; 4+ messages in thread
From: Boris Brezillon @ 2018-01-09 8:50 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, linux-mtd
Cc: Robert Jarzmik, Kyungmin Park, Peter Pan, Frieder Schrempf,
Ladislav Michl
Some MTD sublayers/drivers are implementing ->_read/write_oob() and
provide dummy wrappers for their ->_read/write() implementations.
Let the core handle this case instead of duplicating the logic.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Acked-by: Brian Norris <computersforpeace@gmail.com>
Reviewed-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Tested-by: Ladislav Michl <ladis@linux-mips.org>
---
Changes in v5:
- none
Changes in v4:
- Update mtdpart.c to assign slave->_read/_write() only if the master
has ->_read/_write != NULL
Changes in v3:
- none
Changes in v2:
- none
---
drivers/mtd/devices/docg3.c | 65 --------------------------------------
drivers/mtd/mtdcore.c | 31 ++++++++++++++++--
drivers/mtd/mtdpart.c | 6 ++--
drivers/mtd/nand/nand_base.c | 56 --------------------------------
drivers/mtd/onenand/onenand_base.c | 63 ------------------------------------
5 files changed, 33 insertions(+), 188 deletions(-)
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 0806f72102c0..5fb5e93d1547 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -990,36 +990,6 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
goto out;
}
-/**
- * doc_read - Read bytes from flash
- * @mtd: the device
- * @from: the offset from first block and first page, in bytes, aligned on page
- * size
- * @len: the number of bytes to read (must be a multiple of 4)
- * @retlen: the number of bytes actually read
- * @buf: the filled in buffer
- *
- * Reads flash memory pages. This function does not read the OOB chunk, but only
- * the page data.
- *
- * Returns 0 if read successful, of -EIO, -EINVAL if an error occurred
- */
-static int doc_read(struct mtd_info *mtd, loff_t from, size_t len,
- size_t *retlen, u_char *buf)
-{
- struct mtd_oob_ops ops;
- size_t ret;
-
- memset(&ops, 0, sizeof(ops));
- ops.datbuf = buf;
- ops.len = len;
- ops.mode = MTD_OPS_AUTO_OOB;
-
- ret = doc_read_oob(mtd, from, &ops);
- *retlen = ops.retlen;
- return ret;
-}
-
static int doc_reload_bbt(struct docg3 *docg3)
{
int block = DOC_LAYOUT_BLOCK_BBT;
@@ -1513,39 +1483,6 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
return ret;
}
-/**
- * doc_write - Write a buffer to the chip
- * @mtd: the device
- * @to: the offset from first block and first page, in bytes, aligned on page
- * size
- * @len: the number of bytes to write (must be a full page size, ie. 512)
- * @retlen: the number of bytes actually written (0 or 512)
- * @buf: the buffer to get bytes from
- *
- * Writes data to the chip.
- *
- * Returns 0 if write successful, -EIO if write error
- */
-static int doc_write(struct mtd_info *mtd, loff_t to, size_t len,
- size_t *retlen, const u_char *buf)
-{
- struct docg3 *docg3 = mtd->priv;
- int ret;
- struct mtd_oob_ops ops;
-
- doc_dbg("doc_write(to=%lld, len=%zu)\n", to, len);
- ops.datbuf = (char *)buf;
- ops.len = len;
- ops.mode = MTD_OPS_PLACE_OOB;
- ops.oobbuf = NULL;
- ops.ooblen = 0;
- ops.ooboffs = 0;
-
- ret = doc_write_oob(mtd, to, &ops);
- *retlen = ops.retlen;
- return ret;
-}
-
static struct docg3 *sysfs_dev2docg3(struct device *dev,
struct device_attribute *attr)
{
@@ -1866,8 +1803,6 @@ static int __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
mtd->writebufsize = mtd->writesize = DOC_LAYOUT_PAGE_SIZE;
mtd->oobsize = DOC_LAYOUT_OOB_SIZE;
mtd->_erase = doc_erase;
- mtd->_read = doc_read;
- mtd->_write = doc_write;
mtd->_read_oob = doc_read_oob;
mtd->_write_oob = doc_write_oob;
mtd->_block_isbad = doc_block_isbad;
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 642c35dde686..d7ab091b36b2 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1058,7 +1058,20 @@ int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
* representing the maximum number of bitflips that were corrected on
* any one ecc region (if applicable; zero otherwise).
*/
- ret_code = mtd->_read(mtd, from, len, retlen, buf);
+ if (mtd->_read) {
+ ret_code = mtd->_read(mtd, from, len, retlen, buf);
+ } else if (mtd->_read_oob) {
+ struct mtd_oob_ops ops = {
+ .len = len,
+ .datbuf = buf,
+ };
+
+ ret_code = mtd->_read_oob(mtd, from, &ops);
+ *retlen = ops.retlen;
+ } else {
+ return -ENOTSUPP;
+ }
+
if (unlikely(ret_code < 0))
return ret_code;
if (mtd->ecc_strength == 0)
@@ -1073,11 +1086,25 @@ int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
*retlen = 0;
if (to < 0 || to >= mtd->size || len > mtd->size - to)
return -EINVAL;
- if (!mtd->_write || !(mtd->flags & MTD_WRITEABLE))
+ if ((!mtd->_write && !mtd->_write_oob) ||
+ !(mtd->flags & MTD_WRITEABLE))
return -EROFS;
if (!len)
return 0;
ledtrig_mtd_activity();
+
+ if (!mtd->_write) {
+ struct mtd_oob_ops ops = {
+ .len = len,
+ .datbuf = (u8 *)buf,
+ };
+ int ret;
+
+ ret = mtd->_write_oob(mtd, to, &ops);
+ *retlen = ops.retlen;
+ return ret;
+ }
+
return mtd->_write(mtd, to, len, retlen, buf);
}
EXPORT_SYMBOL_GPL(mtd_write);
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 79bf1f61c7a0..dd28cb0de2c8 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -437,8 +437,10 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent,
parent->dev.parent;
slave->mtd.dev.of_node = part->of_node;
- slave->mtd._read = part_read;
- slave->mtd._write = part_write;
+ if (parent->_read)
+ slave->mtd._read = part_read;
+ if (parent->_write)
+ slave->mtd._write = part_write;
if (parent->_panic_write)
slave->mtd._panic_write = part_panic_write;
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 6135d007a068..889ceadbf607 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2027,33 +2027,6 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
}
/**
- * nand_read - [MTD Interface] MTD compatibility function for nand_do_read_ecc
- * @mtd: MTD device structure
- * @from: offset to read from
- * @len: number of bytes to read
- * @retlen: pointer to variable to store the number of read bytes
- * @buf: the databuffer to put data
- *
- * Get hold of the chip and call nand_do_read.
- */
-static int nand_read(struct mtd_info *mtd, loff_t from, size_t len,
- size_t *retlen, uint8_t *buf)
-{
- struct mtd_oob_ops ops;
- int ret;
-
- nand_get_device(mtd, FL_READING);
- memset(&ops, 0, sizeof(ops));
- ops.len = len;
- ops.datbuf = buf;
- ops.mode = MTD_OPS_PLACE_OOB;
- ret = nand_do_read_ops(mtd, from, &ops);
- *retlen = ops.retlen;
- nand_release_device(mtd);
- return ret;
-}
-
-/**
* nand_read_oob_std - [REPLACEABLE] the most common OOB data read function
* @mtd: mtd info structure
* @chip: nand chip info structure
@@ -2822,33 +2795,6 @@ static int panic_nand_write(struct mtd_info *mtd, loff_t to, size_t len,
}
/**
- * nand_write - [MTD Interface] NAND write with ECC
- * @mtd: MTD device structure
- * @to: offset to write to
- * @len: number of bytes to write
- * @retlen: pointer to variable to store the number of written bytes
- * @buf: the data to write
- *
- * NAND write with ECC.
- */
-static int nand_write(struct mtd_info *mtd, loff_t to, size_t len,
- size_t *retlen, const uint8_t *buf)
-{
- struct mtd_oob_ops ops;
- int ret;
-
- nand_get_device(mtd, FL_WRITING);
- memset(&ops, 0, sizeof(ops));
- ops.len = len;
- ops.datbuf = (uint8_t *)buf;
- ops.mode = MTD_OPS_PLACE_OOB;
- ret = nand_do_write_ops(mtd, to, &ops);
- *retlen = ops.retlen;
- nand_release_device(mtd);
- return ret;
-}
-
-/**
* nand_do_write_oob - [MTD Interface] NAND write out-of-band
* @mtd: MTD device structure
* @to: offset to write to
@@ -4917,8 +4863,6 @@ int nand_scan_tail(struct mtd_info *mtd)
mtd->_erase = nand_erase;
mtd->_point = NULL;
mtd->_unpoint = NULL;
- mtd->_read = nand_read;
- mtd->_write = nand_write;
mtd->_panic_write = panic_nand_write;
mtd->_read_oob = nand_read_oob;
mtd->_write_oob = nand_write_oob;
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 1a6d0e367b89..050ba8a87543 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1448,38 +1448,6 @@ static int onenand_read_oob_nolock(struct mtd_info *mtd, loff_t from,
}
/**
- * onenand_read - [MTD Interface] Read data from flash
- * @param mtd MTD device structure
- * @param from offset to read from
- * @param len number of bytes to read
- * @param retlen pointer to variable to store the number of read bytes
- * @param buf the databuffer to put data
- *
- * Read with ecc
-*/
-static int onenand_read(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 = {
- .len = len,
- .ooblen = 0,
- .datbuf = buf,
- .oobbuf = NULL,
- };
- int ret;
-
- onenand_get_device(mtd, FL_READING);
- ret = ONENAND_IS_4KB_PAGE(this) ?
- onenand_mlc_read_ops_nolock(mtd, from, &ops) :
- onenand_read_ops_nolock(mtd, from, &ops);
- onenand_release_device(mtd);
-
- *retlen = ops.retlen;
- return ret;
-}
-
-/**
* onenand_read_oob - [MTD Interface] Read main and/or out-of-band
* @param mtd: MTD device structure
* @param from: offset to read from
@@ -2129,35 +2097,6 @@ static int onenand_write_oob_nolock(struct mtd_info *mtd, loff_t to,
}
/**
- * onenand_write - [MTD Interface] write buffer to FLASH
- * @param mtd MTD device structure
- * @param to offset to write to
- * @param len number of bytes to write
- * @param retlen pointer to variable to store the number of written bytes
- * @param buf the data to write
- *
- * Write with ECC
- */
-static int onenand_write(struct mtd_info *mtd, loff_t to, size_t len,
- size_t *retlen, const u_char *buf)
-{
- struct mtd_oob_ops ops = {
- .len = len,
- .ooblen = 0,
- .datbuf = (u_char *) buf,
- .oobbuf = NULL,
- };
- int ret;
-
- onenand_get_device(mtd, FL_WRITING);
- ret = onenand_write_ops_nolock(mtd, to, &ops);
- onenand_release_device(mtd);
-
- *retlen = ops.retlen;
- return ret;
-}
-
-/**
* onenand_write_oob - [MTD Interface] NAND write data and/or out-of-band
* @param mtd: MTD device structure
* @param to: offset to write
@@ -4038,8 +3977,6 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
mtd->_erase = onenand_erase;
mtd->_point = NULL;
mtd->_unpoint = NULL;
- mtd->_read = onenand_read;
- mtd->_write = onenand_write;
mtd->_read_oob = onenand_read_oob;
mtd->_write_oob = onenand_write_oob;
mtd->_panic_write = onenand_panic_write;
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v5 3/3] mtd: Remove duplicate checks on mtd_oob_ops parameter
2018-01-09 8:50 [PATCH v5 1/3] mtd: mtdpart: Make ECC stat handling consistent Boris Brezillon
2018-01-09 8:50 ` [PATCH v5 2/3] mtd: Fallback to ->_read/write_oob() when ->_read/write() is missing Boris Brezillon
@ 2018-01-09 8:50 ` Boris Brezillon
2018-01-11 9:06 ` [PATCH v5 1/3] mtd: mtdpart: Make ECC stat handling consistent Boris Brezillon
2 siblings, 0 replies; 4+ messages in thread
From: Boris Brezillon @ 2018-01-09 8:50 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, linux-mtd
Cc: Robert Jarzmik, Kyungmin Park, Peter Pan, Frieder Schrempf,
Ladislav Michl
Some of the check done in custom ->_read/write_oob() implementation are
already done by the core (in mtd_check_oob_ops()).
Suggested-by: Peter Pan <peterpansjtu@gmail.com>
[Remove redundant checks done in mtdpart.c]
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Tested-by: Ladislav Michl <ladis@linux-mips.org>
---
Changes in v5:
- Remove more checks in nand_base.c
Changes in v4:
- Integrated into the SPI NAND preparation patchset
Changes in v3:
- Rebased on mtd/next after "mtd: Stop directly calling master ->_xxx()
hooks from mtdpart code" had been dropped
Changes in v2:
- Merge "mtd: Remove unneeded checks in part_{read,write}_oob()" into
this commit
---
drivers/mtd/devices/docg3.c | 5 -----
drivers/mtd/mtdpart.c | 23 -------------------
drivers/mtd/nand/nand_base.c | 45 --------------------------------------
drivers/mtd/onenand/onenand_base.c | 18 ---------------
4 files changed, 91 deletions(-)
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 5fb5e93d1547..a85af236b44d 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -904,9 +904,6 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
if (ooblen % DOC_LAYOUT_OOB_SIZE)
return -EINVAL;
- if (from + len > mtd->size)
- return -EINVAL;
-
ops->oobretlen = 0;
ops->retlen = 0;
ret = 0;
@@ -1441,8 +1438,6 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
if (len && ooblen &&
(len / DOC_LAYOUT_PAGE_SIZE) != (ooblen / oobdelta))
return -EINVAL;
- if (ofs + len > mtd->size)
- return -EINVAL;
ops->oobretlen = 0;
ops->retlen = 0;
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index dd28cb0de2c8..76cd21d1171b 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -108,25 +108,6 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_ecc_stats stats;
int res;
- if (from >= mtd->size)
- return -EINVAL;
- if (ops->datbuf && from + ops->len > mtd->size)
- return -EINVAL;
-
- /*
- * If OOB is also requested, make sure that we do not read past the end
- * of this partition.
- */
- if (ops->oobbuf) {
- size_t len, pages;
-
- len = mtd_oobavail(mtd, ops);
- pages = mtd_div_by_ws(mtd->size, mtd);
- pages -= mtd_div_by_ws(from, mtd);
- if (ops->ooboffs + ops->ooblen > pages * len)
- return -EINVAL;
- }
-
stats = part->parent->ecc_stats;
res = part->parent->_read_oob(part->parent, from + part->offset, ops);
if (unlikely(mtd_is_eccerr(res)))
@@ -191,10 +172,6 @@ static int part_write_oob(struct mtd_info *mtd, loff_t to,
{
struct mtd_part *part = mtd_to_part(mtd);
- if (to >= mtd->size)
- return -EINVAL;
- if (ops->datbuf && to + ops->len > mtd->size)
- return -EINVAL;
return part->parent->_write_oob(part->parent, to + part->offset, ops);
}
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 889ceadbf607..e7ec55b1d368 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2187,21 +2187,6 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
len = mtd_oobavail(mtd, ops);
- if (unlikely(ops->ooboffs >= len)) {
- pr_debug("%s: attempt to start read outside oob\n",
- __func__);
- return -EINVAL;
- }
-
- /* Do not allow reads past end of device */
- if (unlikely(from >= mtd->size ||
- ops->ooboffs + readlen > ((mtd->size >> chip->page_shift) -
- (from >> chip->page_shift)) * len)) {
- pr_debug("%s: attempt to read beyond end of device\n",
- __func__);
- return -EINVAL;
- }
-
chipnr = (int)(from >> chip->chip_shift);
chip->select_chip(mtd, chipnr);
@@ -2272,13 +2257,6 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from,
ops->retlen = 0;
- /* Do not allow reads past end of device */
- if (ops->datbuf && (from + ops->len) > mtd->size) {
- pr_debug("%s: attempt to read beyond end of device\n",
- __func__);
- return -EINVAL;
- }
-
if (ops->mode != MTD_OPS_PLACE_OOB &&
ops->mode != MTD_OPS_AUTO_OOB &&
ops->mode != MTD_OPS_RAW)
@@ -2820,22 +2798,6 @@ static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
return -EINVAL;
}
- if (unlikely(ops->ooboffs >= len)) {
- pr_debug("%s: attempt to start write outside oob\n",
- __func__);
- return -EINVAL;
- }
-
- /* Do not allow write past end of device */
- if (unlikely(to >= mtd->size ||
- ops->ooboffs + ops->ooblen >
- ((mtd->size >> chip->page_shift) -
- (to >> chip->page_shift)) * len)) {
- pr_debug("%s: attempt to write beyond end of device\n",
- __func__);
- return -EINVAL;
- }
-
chipnr = (int)(to >> chip->chip_shift);
/*
@@ -2891,13 +2853,6 @@ static int nand_write_oob(struct mtd_info *mtd, loff_t to,
ops->retlen = 0;
- /* Do not allow writes past end of device */
- if (ops->datbuf && (to + ops->len) > mtd->size) {
- pr_debug("%s: attempt to write beyond end of device\n",
- __func__);
- return -EINVAL;
- }
-
nand_get_device(mtd, FL_WRITING);
switch (ops->mode) {
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 050ba8a87543..979f4031f23c 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1383,15 +1383,6 @@ static int onenand_read_oob_nolock(struct mtd_info *mtd, loff_t from,
return -EINVAL;
}
- /* Do not allow reads past end of device */
- if (unlikely(from >= mtd->size ||
- column + len > ((mtd->size >> this->page_shift) -
- (from >> this->page_shift)) * oobsize)) {
- printk(KERN_ERR "%s: Attempted to read beyond end of device\n",
- __func__);
- return -EINVAL;
- }
-
stats = mtd->ecc_stats;
readcmd = ONENAND_IS_4KB_PAGE(this) ? ONENAND_CMD_READ : ONENAND_CMD_READOOB;
@@ -2024,15 +2015,6 @@ static int onenand_write_oob_nolock(struct mtd_info *mtd, loff_t to,
return -EINVAL;
}
- /* Do not allow reads past end of device */
- if (unlikely(to >= mtd->size ||
- column + len > ((mtd->size >> this->page_shift) -
- (to >> this->page_shift)) * oobsize)) {
- printk(KERN_ERR "%s: Attempted to write past end of device\n",
- __func__);
- return -EINVAL;
- }
-
oobbuf = this->oob_buf;
oobcmd = ONENAND_IS_4KB_PAGE(this) ? ONENAND_CMD_PROG : ONENAND_CMD_PROGOOB;
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v5 1/3] mtd: mtdpart: Make ECC stat handling consistent
2018-01-09 8:50 [PATCH v5 1/3] mtd: mtdpart: Make ECC stat handling consistent Boris Brezillon
2018-01-09 8:50 ` [PATCH v5 2/3] mtd: Fallback to ->_read/write_oob() when ->_read/write() is missing Boris Brezillon
2018-01-09 8:50 ` [PATCH v5 3/3] mtd: Remove duplicate checks on mtd_oob_ops parameter Boris Brezillon
@ 2018-01-11 9:06 ` Boris Brezillon
2 siblings, 0 replies; 4+ messages in thread
From: Boris Brezillon @ 2018-01-11 9:06 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, linux-mtd
Cc: Peter Pan, Kyungmin Park, Ladislav Michl, Robert Jarzmik,
Frieder Schrempf
On Tue, 9 Jan 2018 09:50:33 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> part_read() and part_read_oob() where counting ECC failures and
> bitflips differently. Adjust part_read_oob() to mimic what is done in
> part_read(). This is in needed to use ->_read_oob() as a fallback when
> when ->_read() is not implemented.
>
> Note that bitflips and ECC failure accounting on MTD partitions is
> broken by design, because nothing prevents concurrent accesses to the
> underlying master MTD device between the moment we save the stats in a
> local variable and the moment master->_read[_oob]() returns. It's not
> something that can easily be fixed, so leave it like that for now.
Applied the series (also fixed the typo reported by Robert).
>
> Suggested-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Tested-by: Ladislav Michl <ladis@linux-mips.org>
> ---
> Changes in v5:
> - save master's ECC stats before calling ->_read_oob()
>
> Changes in v4:
> - new patch
> ---
> drivers/mtd/mtdpart.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index be088bccd593..79bf1f61c7a0 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -105,6 +105,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
> struct mtd_oob_ops *ops)
> {
> struct mtd_part *part = mtd_to_part(mtd);
> + struct mtd_ecc_stats stats;
> int res;
>
> if (from >= mtd->size)
> @@ -126,13 +127,14 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
> return -EINVAL;
> }
>
> + stats = part->parent->ecc_stats;
> res = part->parent->_read_oob(part->parent, from + part->offset, ops);
> - if (unlikely(res)) {
> - if (mtd_is_bitflip(res))
> - mtd->ecc_stats.corrected++;
> - if (mtd_is_eccerr(res))
> - mtd->ecc_stats.failed++;
> - }
> + if (unlikely(mtd_is_eccerr(res)))
> + mtd->ecc_stats.failed +=
> + part->parent->ecc_stats.failed - stats.failed;
> + else
> + mtd->ecc_stats.corrected +=
> + part->parent->ecc_stats.corrected - stats.corrected;
> return res;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-11 9:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-09 8:50 [PATCH v5 1/3] mtd: mtdpart: Make ECC stat handling consistent Boris Brezillon
2018-01-09 8:50 ` [PATCH v5 2/3] mtd: Fallback to ->_read/write_oob() when ->_read/write() is missing Boris Brezillon
2018-01-09 8:50 ` [PATCH v5 3/3] mtd: Remove duplicate checks on mtd_oob_ops parameter Boris Brezillon
2018-01-11 9:06 ` [PATCH v5 1/3] mtd: mtdpart: Make ECC stat handling consistent Boris Brezillon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox