* [PATCH 0/2] mtd: mtdconcat: Fix callback functions check
@ 2021-07-31 2:32 Zhihao Cheng
2021-07-31 2:32 ` [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition Zhihao Cheng
2021-07-31 2:32 ` [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob Zhihao Cheng
0 siblings, 2 replies; 13+ messages in thread
From: Zhihao Cheng @ 2021-07-31 2:32 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr, bbrezillon
Cc: linux-mtd, linux-kernel, chengzhihao1, yukuai3
Patch 1: Check subdev's master mtd device's callback function while
making concatenated device.
Patch 2: Remove bad callback function assignments.
Zhihao Cheng (2):
mtd: mtdconcat: Judge callback function existence getting from master
for each partition
mtd: mtdconcat: Remove concat_{read|write}_oob
drivers/mtd/mtdconcat.c | 123 +++-------------------------------------
1 file changed, 8 insertions(+), 115 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition 2021-07-31 2:32 [PATCH 0/2] mtd: mtdconcat: Fix callback functions check Zhihao Cheng @ 2021-07-31 2:32 ` Zhihao Cheng 2021-08-06 19:28 ` Miquel Raynal 2021-08-16 14:27 ` Miquel Raynal 2021-07-31 2:32 ` [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob Zhihao Cheng 1 sibling, 2 replies; 13+ messages in thread From: Zhihao Cheng @ 2021-07-31 2:32 UTC (permalink / raw) To: miquel.raynal, richard, vigneshr, bbrezillon Cc: linux-mtd, linux-kernel, chengzhihao1, yukuai3 Since commit 46b5889cc2c5("mtd: implement proper partition handling") applied, mtd partition device won't hold some callback functions, such as _block_isbad, _block_markbad, etc. Besides, function mtd_block_isbad() will get mtd device's master mtd device, then invokes master mtd device's callback function. So, following process may result mtd_block_isbad() always return 0, even though mtd device has bad blocks: 1. Split a mtd device into 3 partitions: PA, PB, PC [ Each mtd partition device won't has callback function _block_isbad(). ] 2. Concatenate PA and PB as a new mtd device PN [ mtd_concat_create() finds out each subdev has no callback function _block_isbad(), so PN won't be assigned callback function concat_block_isbad(). ] Then, mtd_block_isbad() checks "!master->_block_isbad" is true, will always return 0. Reproducer: // reproduce.c static int __init init_diy_module(void) { struct mtd_info *mtd[2]; struct mtd_info *mtd_combine = NULL; mtd[0] = get_mtd_device_nm("NAND simulator partition 0"); if (!mtd[0]) { pr_err("cannot find mtd1\n"); return -EINVAL; } mtd[1] = get_mtd_device_nm("NAND simulator partition 1"); if (!mtd[1]) { pr_err("cannot find mtd2\n"); return -EINVAL; } put_mtd_device(mtd[0]); put_mtd_device(mtd[1]); mtd_combine = mtd_concat_create(mtd, 2, "Combine mtd"); if (mtd_combine == NULL) { pr_err("comnoine fail\n"); return -EINVAL; } mtd_device_register(mtd_combine, NULL, 0); pr_info("Combine success\n"); return 0; } 1. ID="0x20,0xac,0x00,0x15" 2. modprobe nandsim id_bytes=$ID parts=50,100 badblocks=100 3. insmod reproduce.ko 4. flash_erase /dev/mtd3 0 0 libmtd: error!: MEMERASE64 ioctl failed for eraseblock 100 (mtd3) error 5 (Input/output error) // Should be "flash_erase: Skipping bad block at 00c80000" Fixes: 46b5889cc2c54bac ("mtd: implement proper partition handling") Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> --- drivers/mtd/mtdconcat.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c index 6e4d0017c0bd..ea130eeb54d5 100644 --- a/drivers/mtd/mtdconcat.c +++ b/drivers/mtd/mtdconcat.c @@ -641,6 +641,7 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c int i; size_t size; struct mtd_concat *concat; + struct mtd_info *subdev_master = NULL; uint32_t max_erasesize, curr_erasesize; int num_erase_region; int max_writebufsize = 0; @@ -679,17 +680,19 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c concat->mtd.subpage_sft = subdev[0]->subpage_sft; concat->mtd.oobsize = subdev[0]->oobsize; concat->mtd.oobavail = subdev[0]->oobavail; - if (subdev[0]->_writev) + + subdev_master = mtd_get_master(subdev[0]); + if (subdev_master->_writev) concat->mtd._writev = concat_writev; - if (subdev[0]->_read_oob) + if (subdev_master->_read_oob) concat->mtd._read_oob = concat_read_oob; - if (subdev[0]->_write_oob) + if (subdev_master->_write_oob) concat->mtd._write_oob = concat_write_oob; - if (subdev[0]->_block_isbad) + if (subdev_master->_block_isbad) concat->mtd._block_isbad = concat_block_isbad; - if (subdev[0]->_block_markbad) + if (subdev_master->_block_markbad) concat->mtd._block_markbad = concat_block_markbad; - if (subdev[0]->_panic_write) + if (subdev_master->_panic_write) concat->mtd._panic_write = concat_panic_write; concat->mtd.ecc_stats.badblocks = subdev[0]->ecc_stats.badblocks; @@ -721,14 +724,15 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c subdev[i]->flags & MTD_WRITEABLE; } + subdev_master = mtd_get_master(subdev[i]); concat->mtd.size += subdev[i]->size; concat->mtd.ecc_stats.badblocks += subdev[i]->ecc_stats.badblocks; if (concat->mtd.writesize != subdev[i]->writesize || concat->mtd.subpage_sft != subdev[i]->subpage_sft || concat->mtd.oobsize != subdev[i]->oobsize || - !concat->mtd._read_oob != !subdev[i]->_read_oob || - !concat->mtd._write_oob != !subdev[i]->_write_oob) { + !concat->mtd._read_oob != !subdev_master->_read_oob || + !concat->mtd._write_oob != !subdev_master->_write_oob) { kfree(concat); printk("Incompatible OOB or ECC data on \"%s\"\n", subdev[i]->name); -- 2.31.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition 2021-07-31 2:32 ` [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition Zhihao Cheng @ 2021-08-06 19:28 ` Miquel Raynal 2021-08-07 2:15 ` Zhihao Cheng 2021-08-16 14:27 ` Miquel Raynal 1 sibling, 1 reply; 13+ messages in thread From: Miquel Raynal @ 2021-08-06 19:28 UTC (permalink / raw) To: Zhihao Cheng Cc: richard, vigneshr, bbrezillon, linux-mtd, linux-kernel, yukuai3 Hi Zhihao, Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 31 Jul 2021 10:32:42 +0800: > Since commit 46b5889cc2c5("mtd: implement proper partition handling") > applied, mtd partition device won't hold some callback functions, such > as _block_isbad, _block_markbad, etc. Besides, function mtd_block_isbad() > will get mtd device's master mtd device, then invokes master mtd device's > callback function. So, following process may result mtd_block_isbad() > always return 0, even though mtd device has bad blocks: > > 1. Split a mtd device into 3 partitions: PA, PB, PC > [ Each mtd partition device won't has callback function _block_isbad(). ] > 2. Concatenate PA and PB as a new mtd device PN > [ mtd_concat_create() finds out each subdev has no callback function > _block_isbad(), so PN won't be assigned callback function > concat_block_isbad(). ] > Then, mtd_block_isbad() checks "!master->_block_isbad" is true, will > always return 0. > > Reproducer: > // reproduce.c > static int __init init_diy_module(void) > { > struct mtd_info *mtd[2]; > struct mtd_info *mtd_combine = NULL; > > mtd[0] = get_mtd_device_nm("NAND simulator partition 0"); > if (!mtd[0]) { > pr_err("cannot find mtd1\n"); > return -EINVAL; > } > mtd[1] = get_mtd_device_nm("NAND simulator partition 1"); > if (!mtd[1]) { > pr_err("cannot find mtd2\n"); > return -EINVAL; > } > > put_mtd_device(mtd[0]); > put_mtd_device(mtd[1]); > > mtd_combine = mtd_concat_create(mtd, 2, "Combine mtd"); > if (mtd_combine == NULL) { > pr_err("comnoine fail\n"); > return -EINVAL; > } > > mtd_device_register(mtd_combine, NULL, 0); > pr_info("Combine success\n"); > > return 0; > } > > 1. ID="0x20,0xac,0x00,0x15" > 2. modprobe nandsim id_bytes=$ID parts=50,100 badblocks=100 > 3. insmod reproduce.ko > 4. flash_erase /dev/mtd3 0 0 > libmtd: error!: MEMERASE64 ioctl failed for eraseblock 100 (mtd3) > error 5 (Input/output error) > // Should be "flash_erase: Skipping bad block at 00c80000" > > Fixes: 46b5889cc2c54bac ("mtd: implement proper partition handling") > Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> > --- > drivers/mtd/mtdconcat.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c > index 6e4d0017c0bd..ea130eeb54d5 100644 > --- a/drivers/mtd/mtdconcat.c > +++ b/drivers/mtd/mtdconcat.c > @@ -641,6 +641,7 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c > int i; > size_t size; > struct mtd_concat *concat; > + struct mtd_info *subdev_master = NULL; > uint32_t max_erasesize, curr_erasesize; > int num_erase_region; > int max_writebufsize = 0; > @@ -679,17 +680,19 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c > concat->mtd.subpage_sft = subdev[0]->subpage_sft; > concat->mtd.oobsize = subdev[0]->oobsize; > concat->mtd.oobavail = subdev[0]->oobavail; > - if (subdev[0]->_writev) > + > + subdev_master = mtd_get_master(subdev[0]); > + if (subdev_master->_writev) > concat->mtd._writev = concat_writev; > - if (subdev[0]->_read_oob) > + if (subdev_master->_read_oob) > concat->mtd._read_oob = concat_read_oob; > - if (subdev[0]->_write_oob) > + if (subdev_master->_write_oob) > concat->mtd._write_oob = concat_write_oob; > - if (subdev[0]->_block_isbad) > + if (subdev_master->_block_isbad) > concat->mtd._block_isbad = concat_block_isbad; > - if (subdev[0]->_block_markbad) > + if (subdev_master->_block_markbad) > concat->mtd._block_markbad = concat_block_markbad; > - if (subdev[0]->_panic_write) > + if (subdev_master->_panic_write) > concat->mtd._panic_write = concat_panic_write; > > concat->mtd.ecc_stats.badblocks = subdev[0]->ecc_stats.badblocks; > @@ -721,14 +724,15 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c > subdev[i]->flags & MTD_WRITEABLE; > } > > + subdev_master = mtd_get_master(subdev[i]); > concat->mtd.size += subdev[i]->size; > concat->mtd.ecc_stats.badblocks += > subdev[i]->ecc_stats.badblocks; > if (concat->mtd.writesize != subdev[i]->writesize || > concat->mtd.subpage_sft != subdev[i]->subpage_sft || > concat->mtd.oobsize != subdev[i]->oobsize || > - !concat->mtd._read_oob != !subdev[i]->_read_oob || > - !concat->mtd._write_oob != !subdev[i]->_write_oob) { > + !concat->mtd._read_oob != !subdev_master->_read_oob || > + !concat->mtd._write_oob != !subdev_master->_write_oob) { Do you really need this change? > kfree(concat); > printk("Incompatible OOB or ECC data on \"%s\"\n", > subdev[i]->name); Thanks, Miquèl ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition 2021-08-06 19:28 ` Miquel Raynal @ 2021-08-07 2:15 ` Zhihao Cheng 2021-08-07 10:32 ` Miquel Raynal 0 siblings, 1 reply; 13+ messages in thread From: Zhihao Cheng @ 2021-08-07 2:15 UTC (permalink / raw) To: Miquel Raynal Cc: richard, vigneshr, bbrezillon, linux-mtd, linux-kernel, yukuai3 在 2021/8/7 3:28, Miquel Raynal 写道: Hi Miquel, > Hi Zhihao, > > Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 31 Jul 2021 > 10:32:42 +0800: > @@ -721,14 +724,15 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c > subdev[i]->flags & MTD_WRITEABLE; > } > > + subdev_master = mtd_get_master(subdev[i]); > concat->mtd.size += subdev[i]->size; > concat->mtd.ecc_stats.badblocks += > subdev[i]->ecc_stats.badblocks; > if (concat->mtd.writesize != subdev[i]->writesize || > concat->mtd.subpage_sft != subdev[i]->subpage_sft || > concat->mtd.oobsize != subdev[i]->oobsize || > - !concat->mtd._read_oob != !subdev[i]->_read_oob || > - !concat->mtd._write_oob != !subdev[i]->_write_oob) { > + !concat->mtd._read_oob != !subdev_master->_read_oob || > + !concat->mtd._write_oob != !subdev_master->_write_oob) { > Do you really need this change? I think both "!concat->mtd._read_oob != !subdev[i]->_read_oob" and "!concat->mtd._write_oob != !subdev[i]->_write_oob" need to be modified otherwise concatenating goes failure. I thought there exists two problems: 1. Wrong callback fetching in mtd partition device 2. Warning for existence of _read and _read_oob at the same time so I solved them in two steps to make history commit logs a bit clear. Though these two patches can be combined to one. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition 2021-08-07 2:15 ` Zhihao Cheng @ 2021-08-07 10:32 ` Miquel Raynal 2021-08-10 11:35 ` Zhihao Cheng 0 siblings, 1 reply; 13+ messages in thread From: Miquel Raynal @ 2021-08-07 10:32 UTC (permalink / raw) To: Zhihao Cheng Cc: richard, vigneshr, bbrezillon, linux-mtd, linux-kernel, yukuai3 Hi Zhihao, Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 7 Aug 2021 10:15:46 +0800: > 在 2021/8/7 3:28, Miquel Raynal 写道: > Hi Miquel, > > Hi Zhihao, > > > > Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 31 Jul 2021 > > 10:32:42 +0800: > > @@ -721,14 +724,15 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c > > subdev[i]->flags & MTD_WRITEABLE; > > } > > > + subdev_master = mtd_get_master(subdev[i]); > > concat->mtd.size += subdev[i]->size; > > concat->mtd.ecc_stats.badblocks += > > subdev[i]->ecc_stats.badblocks; > > if (concat->mtd.writesize != subdev[i]->writesize || > > concat->mtd.subpage_sft != subdev[i]->subpage_sft || > > concat->mtd.oobsize != subdev[i]->oobsize || > > - !concat->mtd._read_oob != !subdev[i]->_read_oob || > > - !concat->mtd._write_oob != !subdev[i]->_write_oob) { > > + !concat->mtd._read_oob != !subdev_master->_read_oob || > > + !concat->mtd._write_oob != !subdev_master->_write_oob) { > > Do you really need this change? > > I think both "!concat->mtd._read_oob != !subdev[i]->_read_oob" and "!concat->mtd._write_oob != !subdev[i]->_write_oob" need to be modified otherwise concatenating goes failure. > > I thought there exists two problems: > > 1. Wrong callback fetching in mtd partition device > > 2. Warning for existence of _read and _read_oob at the same time > > so I solved them in two steps to make history commit logs a bit clear. > > Though these two patches can be combined to one. No please keep the split. What I mean here is that I don't think your fix is valid. Maybe we should propagate these callbacks as well instead of trying to hack into this condition. I don't see why you should check against subdev[i] for half of the callbacks and check for subdev_master for the last two. Thanks, Miquèl ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition 2021-08-07 10:32 ` Miquel Raynal @ 2021-08-10 11:35 ` Zhihao Cheng 2021-08-16 13:43 ` Miquel Raynal 0 siblings, 1 reply; 13+ messages in thread From: Zhihao Cheng @ 2021-08-10 11:35 UTC (permalink / raw) To: Miquel Raynal Cc: richard, vigneshr, bbrezillon, linux-mtd, linux-kernel, yukuai3 在 2021/8/7 18:32, Miquel Raynal 写道: Hi Miquel, > Hi Zhihao, > > Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 7 Aug 2021 > 10:15:46 +0800: > >> 在 2021/8/7 3:28, Miquel Raynal 写道: >> Hi Miquel, >>> Hi Zhihao, >>> >>> Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 31 Jul 2021 >>> 10:32:42 +0800: >>> @@ -721,14 +724,15 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c >>> subdev[i]->flags & MTD_WRITEABLE; >>> } >>> > + subdev_master = mtd_get_master(subdev[i]); >>> concat->mtd.size += subdev[i]->size; >>> concat->mtd.ecc_stats.badblocks += >>> subdev[i]->ecc_stats.badblocks; >>> if (concat->mtd.writesize != subdev[i]->writesize || >>> concat->mtd.subpage_sft != subdev[i]->subpage_sft || >>> concat->mtd.oobsize != subdev[i]->oobsize || >>> - !concat->mtd._read_oob != !subdev[i]->_read_oob || >>> - !concat->mtd._write_oob != !subdev[i]->_write_oob) { >>> + !concat->mtd._read_oob != !subdev_master->_read_oob || >>> + !concat->mtd._write_oob != !subdev_master->_write_oob) { >>> Do you really need this change? >> I think both "!concat->mtd._read_oob != !subdev[i]->_read_oob" and "!concat->mtd._write_oob != !subdev[i]->_write_oob" need to be modified otherwise concatenating goes failure. >> >> I thought there exists two problems: >> >> 1. Wrong callback fetching in mtd partition device >> >> 2. Warning for existence of _read and _read_oob at the same time >> >> so I solved them in two steps to make history commit logs a bit clear. >> >> Though these two patches can be combined to one. > No please keep the split. > > What I mean here is that I don't think your fix is valid. Maybe we > should propagate these callbacks as well instead of trying to hack into > this condition. I don't see why you should check against subdev[i] for > half of the callbacks and check for subdev_master for the last two. I have the following understanding of mtd: 1. The existence of mtd partition device's callbacks(what can mtd do, _read, _write, _read_oob, etc.) is decided by mtd master device. All mtd common functions (mtd_read, mtd_read_oob) pass master mtd device rather than partition mtd device as parameter, because nand_chip(initialized as master mtd device) process requests. So there are two steps in mtd common function: fetch master mtd device; invoke master mtd devices's callback and pass in master mtd device. Propogating callbacks to partition mtd device may bring some imperfections: A. No adaptions to mtd common functions: partition mtd device's callbacks will never be invoked, they are only used to judge whether assigin concatenated device's callback while concatenating. Looks weird. @@ -86,6 +86,61 @@ static struct mtd_info *allocate_partition(struct mtd_info *parent, child->part.offset = part->offset; INIT_LIST_HEAD(&child->partitions); + if (parent->_read) + child->_read = parent->_read; + if (parent->_write) + child->_write = parent->_write; [...] + if (parent->_read_oob) + child->_read_oob = parent->_read_oob; + if (parent->_write_oob) B. Re-import removed partition mtd device's callbacks and adapt mtd common functions: Current implemention is simplier than the version before 46b5889cc2c54("mtd: implement proper partition handling"). Adapting mtd common functions is a risky thing, which could effect other modules, should we do that? +static int part_read(struct mtd_info *mtd, loff_t from, size_t len, + size_t *retlen, u_char *buf) +{ + struct mtd_part *part = mtd_to_part(mtd); + struct mtd_ecc_stats stats; + int res; + + stats = part->parent->ecc_stats; + res = part->parent->_read(part->parent, from + part->offset, len, + retlen, buf); + 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; +} static int mtd_read_oob_std(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops) { - struct mtd_info *master = mtd_get_master(mtd); int ret; - from = mtd_get_master_ofs(mtd, from); - if (master->_read_oob) - ret = master->_read_oob(master, from, ops); + if (mtd->_read_oob) + ret = mtd->_read_oob(mtd, from, ops); else - ret = master->_read(master, from, ops->len, &ops->retlen, + ret = mtd->_read(mtd, from, ops->len, &ops->retlen, ops->datbuf); 2. Checking against subdev[i] for data members and check against subdev_master for the callbacks looks weird. I modified it based on the assumption that partition mtd device' callbacks inherit from master mtd device but data members(name, size) may not. Maybe I should add some comment to explain why checking against subdev[i] for data members and checking against subdev_master for the callbacks. So, which method is better? Any other method? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition 2021-08-10 11:35 ` Zhihao Cheng @ 2021-08-16 13:43 ` Miquel Raynal 0 siblings, 0 replies; 13+ messages in thread From: Miquel Raynal @ 2021-08-16 13:43 UTC (permalink / raw) To: Zhihao Cheng Cc: richard, vigneshr, bbrezillon, linux-mtd, linux-kernel, yukuai3 Hi Zhihao, Zhihao Cheng <chengzhihao1@huawei.com> wrote on Tue, 10 Aug 2021 19:35:02 +0800: > 在 2021/8/7 18:32, Miquel Raynal 写道: > Hi Miquel, > > Hi Zhihao, > > > > Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 7 Aug 2021 > > 10:15:46 +0800: > > > >> 在 2021/8/7 3:28, Miquel Raynal 写道: > >> Hi Miquel, > >>> Hi Zhihao, > >>> > >>> Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 31 Jul 2021 > >>> 10:32:42 +0800: > >>> @@ -721,14 +724,15 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c > >>> subdev[i]->flags & MTD_WRITEABLE; > >>> } > >>> > + subdev_master = mtd_get_master(subdev[i]); > >>> concat->mtd.size += subdev[i]->size; > >>> concat->mtd.ecc_stats.badblocks += > >>> subdev[i]->ecc_stats.badblocks; > >>> if (concat->mtd.writesize != subdev[i]->writesize || > >>> concat->mtd.subpage_sft != subdev[i]->subpage_sft || > >>> concat->mtd.oobsize != subdev[i]->oobsize || > >>> - !concat->mtd._read_oob != !subdev[i]->_read_oob || > >>> - !concat->mtd._write_oob != !subdev[i]->_write_oob) { > >>> + !concat->mtd._read_oob != !subdev_master->_read_oob || > >>> + !concat->mtd._write_oob != !subdev_master->_write_oob) { > >>> Do you really need this change? > >> I think both "!concat->mtd._read_oob != !subdev[i]->_read_oob" and "!concat->mtd._write_oob != !subdev[i]->_write_oob" need to be modified otherwise concatenating goes failure. > >> > >> I thought there exists two problems: > >> > >> 1. Wrong callback fetching in mtd partition device > >> > >> 2. Warning for existence of _read and _read_oob at the same time > >> > >> so I solved them in two steps to make history commit logs a bit clear. > >> > >> Though these two patches can be combined to one. > > No please keep the split. > > > > What I mean here is that I don't think your fix is valid. Maybe we > > should propagate these callbacks as well instead of trying to hack into > > this condition. I don't see why you should check against subdev[i] for > > half of the callbacks and check for subdev_master for the last two. > > I have the following understanding of mtd: > > 1. The existence of mtd partition device's callbacks(what can mtd do, _read, _write, _read_oob, etc.) is decided by mtd master device. All mtd common functions (mtd_read, mtd_read_oob) pass master mtd device rather than partition mtd device as parameter, because nand_chip(initialized as master mtd device) process requests. So there are two steps in mtd common function: fetch master mtd device; invoke master mtd devices's callback and pass in master mtd device. > > Propogating callbacks to partition mtd device may bring some imperfections: > > A. No adaptions to mtd common functions: partition mtd device's callbacks will never be invoked, they are only used to judge whether assigin concatenated device's callback while concatenating. Looks weird. > > @@ -86,6 +86,61 @@ static struct mtd_info *allocate_partition(struct mtd_info *parent, > child->part.offset = part->offset; > INIT_LIST_HEAD(&child->partitions); > > + if (parent->_read) > + child->_read = parent->_read; > + if (parent->_write) > + child->_write = parent->_write; > [...] > + if (parent->_read_oob) > + child->_read_oob = parent->_read_oob; > + if (parent->_write_oob) > > > B. Re-import removed partition mtd device's callbacks and adapt mtd common functions: Current implemention is simplier than the version before 46b5889cc2c54("mtd: implement proper partition handling"). Adapting mtd common functions is a risky thing, which could effect other modules, should we do that? > > +static int part_read(struct mtd_info *mtd, loff_t from, size_t len, > + size_t *retlen, u_char *buf) > +{ > + struct mtd_part *part = mtd_to_part(mtd); > + struct mtd_ecc_stats stats; > + int res; > + > + stats = part->parent->ecc_stats; > + res = part->parent->_read(part->parent, from + part->offset, len, > + retlen, buf); > + 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; > +} > > static int mtd_read_oob_std(struct mtd_info *mtd, loff_t from, > struct mtd_oob_ops *ops) > { > - struct mtd_info *master = mtd_get_master(mtd); > int ret; > > - from = mtd_get_master_ofs(mtd, from); > - if (master->_read_oob) > - ret = master->_read_oob(master, from, ops); > + if (mtd->_read_oob) > + ret = mtd->_read_oob(mtd, from, ops); > else > - ret = master->_read(master, from, ops->len, &ops->retlen, > + ret = mtd->_read(mtd, from, ops->len, &ops->retlen, > ops->datbuf); > > > 2. Checking against subdev[i] for data members and check against subdev_master for the callbacks looks weird. I modified it based on the assumption that partition mtd device' callbacks inherit from master mtd device but data members(name, size) may not. Maybe I should add some comment to explain why checking against subdev[i] for data members and checking against subdev_master for the callbacks. > > > So, which method is better? Any other method? > I see your point, actually my concern was about checking half of the *callbacks* from a particular device, and the other half from another device (eg. the master) but as you stated it here, we check the callbacks of the master but the "data members" as you call them from the subdevice, which I think is fine. So I guess I'll take these changes as they currently are. Thanks, Miquèl ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition 2021-07-31 2:32 ` [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition Zhihao Cheng 2021-08-06 19:28 ` Miquel Raynal @ 2021-08-16 14:27 ` Miquel Raynal 1 sibling, 0 replies; 13+ messages in thread From: Miquel Raynal @ 2021-08-16 14:27 UTC (permalink / raw) To: Zhihao Cheng, miquel.raynal, richard, vigneshr, bbrezillon Cc: linux-mtd, linux-kernel, yukuai3 On Sat, 2021-07-31 at 02:32:42 UTC, Zhihao Cheng wrote: > Since commit 46b5889cc2c5("mtd: implement proper partition handling") > applied, mtd partition device won't hold some callback functions, such > as _block_isbad, _block_markbad, etc. Besides, function mtd_block_isbad() > will get mtd device's master mtd device, then invokes master mtd device's > callback function. So, following process may result mtd_block_isbad() > always return 0, even though mtd device has bad blocks: > > 1. Split a mtd device into 3 partitions: PA, PB, PC > [ Each mtd partition device won't has callback function _block_isbad(). ] > 2. Concatenate PA and PB as a new mtd device PN > [ mtd_concat_create() finds out each subdev has no callback function > _block_isbad(), so PN won't be assigned callback function > concat_block_isbad(). ] > Then, mtd_block_isbad() checks "!master->_block_isbad" is true, will > always return 0. > > Reproducer: > // reproduce.c > static int __init init_diy_module(void) > { > struct mtd_info *mtd[2]; > struct mtd_info *mtd_combine = NULL; > > mtd[0] = get_mtd_device_nm("NAND simulator partition 0"); > if (!mtd[0]) { > pr_err("cannot find mtd1\n"); > return -EINVAL; > } > mtd[1] = get_mtd_device_nm("NAND simulator partition 1"); > if (!mtd[1]) { > pr_err("cannot find mtd2\n"); > return -EINVAL; > } > > put_mtd_device(mtd[0]); > put_mtd_device(mtd[1]); > > mtd_combine = mtd_concat_create(mtd, 2, "Combine mtd"); > if (mtd_combine == NULL) { > pr_err("comnoine fail\n"); > return -EINVAL; > } > > mtd_device_register(mtd_combine, NULL, 0); > pr_info("Combine success\n"); > > return 0; > } > > 1. ID="0x20,0xac,0x00,0x15" > 2. modprobe nandsim id_bytes=$ID parts=50,100 badblocks=100 > 3. insmod reproduce.ko > 4. flash_erase /dev/mtd3 0 0 > libmtd: error!: MEMERASE64 ioctl failed for eraseblock 100 (mtd3) > error 5 (Input/output error) > // Should be "flash_erase: Skipping bad block at 00c80000" > > Fixes: 46b5889cc2c54bac ("mtd: implement proper partition handling") > Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks. Miquel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob 2021-07-31 2:32 [PATCH 0/2] mtd: mtdconcat: Fix callback functions check Zhihao Cheng 2021-07-31 2:32 ` [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition Zhihao Cheng @ 2021-07-31 2:32 ` Zhihao Cheng 2021-08-06 19:26 ` Miquel Raynal 2021-08-16 14:27 ` Miquel Raynal 1 sibling, 2 replies; 13+ messages in thread From: Zhihao Cheng @ 2021-07-31 2:32 UTC (permalink / raw) To: miquel.raynal, richard, vigneshr, bbrezillon Cc: linux-mtd, linux-kernel, chengzhihao1, yukuai3 Since 2431c4f5b46c3("mtd: Implement mtd_{read,write}() as wrappers around mtd_{read,write}_oob()") don't allow _write|_read and _write_oob|_read_oob existing at the same time. We should stop these two callback functions assigning, otherwise following warning occurs while making concatenated device: WARNING: CPU: 2 PID: 6728 at drivers/mtd/mtdcore.c:595 add_mtd_device+0x7f/0x7b0 Fixes: 2431c4f5b46c3("mtd: Implement mtd_{read,write}() around ...") Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> --- drivers/mtd/mtdconcat.c | 113 +--------------------------------------- 1 file changed, 1 insertion(+), 112 deletions(-) diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c index ea130eeb54d5..98d1c79cf51d 100644 --- a/drivers/mtd/mtdconcat.c +++ b/drivers/mtd/mtdconcat.c @@ -256,110 +256,6 @@ concat_writev(struct mtd_info *mtd, const struct kvec *vecs, return err; } -static int -concat_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops) -{ - struct mtd_concat *concat = CONCAT(mtd); - struct mtd_oob_ops devops = *ops; - int i, err, ret = 0; - - ops->retlen = ops->oobretlen = 0; - - for (i = 0; i < concat->num_subdev; i++) { - struct mtd_info *subdev = concat->subdev[i]; - - if (from >= subdev->size) { - from -= subdev->size; - continue; - } - - /* partial read ? */ - if (from + devops.len > subdev->size) - devops.len = subdev->size - from; - - err = mtd_read_oob(subdev, from, &devops); - ops->retlen += devops.retlen; - ops->oobretlen += devops.oobretlen; - - /* Save information about bitflips! */ - if (unlikely(err)) { - if (mtd_is_eccerr(err)) { - mtd->ecc_stats.failed++; - ret = err; - } else if (mtd_is_bitflip(err)) { - mtd->ecc_stats.corrected++; - /* Do not overwrite -EBADMSG !! */ - if (!ret) - ret = err; - } else - return err; - } - - if (devops.datbuf) { - devops.len = ops->len - ops->retlen; - if (!devops.len) - return ret; - devops.datbuf += devops.retlen; - } - if (devops.oobbuf) { - devops.ooblen = ops->ooblen - ops->oobretlen; - if (!devops.ooblen) - return ret; - devops.oobbuf += ops->oobretlen; - } - - from = 0; - } - return -EINVAL; -} - -static int -concat_write_oob(struct mtd_info *mtd, loff_t to, struct mtd_oob_ops *ops) -{ - struct mtd_concat *concat = CONCAT(mtd); - struct mtd_oob_ops devops = *ops; - int i, err; - - if (!(mtd->flags & MTD_WRITEABLE)) - return -EROFS; - - ops->retlen = ops->oobretlen = 0; - - for (i = 0; i < concat->num_subdev; i++) { - struct mtd_info *subdev = concat->subdev[i]; - - if (to >= subdev->size) { - to -= subdev->size; - continue; - } - - /* partial write ? */ - if (to + devops.len > subdev->size) - devops.len = subdev->size - to; - - err = mtd_write_oob(subdev, to, &devops); - ops->retlen += devops.retlen; - ops->oobretlen += devops.oobretlen; - if (err) - return err; - - if (devops.datbuf) { - devops.len = ops->len - ops->retlen; - if (!devops.len) - return 0; - devops.datbuf += devops.retlen; - } - if (devops.oobbuf) { - devops.ooblen = ops->ooblen - ops->oobretlen; - if (!devops.ooblen) - return 0; - devops.oobbuf += devops.oobretlen; - } - to = 0; - } - return -EINVAL; -} - static int concat_erase(struct mtd_info *mtd, struct erase_info *instr) { struct mtd_concat *concat = CONCAT(mtd); @@ -684,10 +580,6 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c subdev_master = mtd_get_master(subdev[0]); if (subdev_master->_writev) concat->mtd._writev = concat_writev; - if (subdev_master->_read_oob) - concat->mtd._read_oob = concat_read_oob; - if (subdev_master->_write_oob) - concat->mtd._write_oob = concat_write_oob; if (subdev_master->_block_isbad) concat->mtd._block_isbad = concat_block_isbad; if (subdev_master->_block_markbad) @@ -724,15 +616,12 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c subdev[i]->flags & MTD_WRITEABLE; } - subdev_master = mtd_get_master(subdev[i]); concat->mtd.size += subdev[i]->size; concat->mtd.ecc_stats.badblocks += subdev[i]->ecc_stats.badblocks; if (concat->mtd.writesize != subdev[i]->writesize || concat->mtd.subpage_sft != subdev[i]->subpage_sft || - concat->mtd.oobsize != subdev[i]->oobsize || - !concat->mtd._read_oob != !subdev_master->_read_oob || - !concat->mtd._write_oob != !subdev_master->_write_oob) { + concat->mtd.oobsize != subdev[i]->oobsize) { kfree(concat); printk("Incompatible OOB or ECC data on \"%s\"\n", subdev[i]->name); -- 2.31.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob 2021-07-31 2:32 ` [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob Zhihao Cheng @ 2021-08-06 19:26 ` Miquel Raynal 2021-08-07 2:59 ` Zhihao Cheng 2021-08-16 14:27 ` Miquel Raynal 1 sibling, 1 reply; 13+ messages in thread From: Miquel Raynal @ 2021-08-06 19:26 UTC (permalink / raw) To: Zhihao Cheng Cc: richard, vigneshr, bbrezillon, linux-mtd, linux-kernel, yukuai3 Hi Zhihao, Richard, a question for you below :) Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 31 Jul 2021 10:32:43 +0800: > Since 2431c4f5b46c3("mtd: Implement mtd_{read,write}() as wrappers > around mtd_{read,write}_oob()") don't allow _write|_read and > _write_oob|_read_oob existing at the same time. We should stop these > two callback functions assigning, otherwise following warning occurs > while making concatenated device: > > WARNING: CPU: 2 PID: 6728 at drivers/mtd/mtdcore.c:595 > add_mtd_device+0x7f/0x7b0 > > Fixes: 2431c4f5b46c3("mtd: Implement mtd_{read,write}() around ...") > Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> > --- > drivers/mtd/mtdconcat.c | 113 +--------------------------------------- > 1 file changed, 1 insertion(+), 112 deletions(-) > > diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c > index ea130eeb54d5..98d1c79cf51d 100644 > --- a/drivers/mtd/mtdconcat.c > +++ b/drivers/mtd/mtdconcat.c > @@ -256,110 +256,6 @@ concat_writev(struct mtd_info *mtd, const struct kvec *vecs, > return err; > } > > -static int > -concat_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops) > -{ > - struct mtd_concat *concat = CONCAT(mtd); > - struct mtd_oob_ops devops = *ops; > - int i, err, ret = 0; > - > - ops->retlen = ops->oobretlen = 0; > - > - for (i = 0; i < concat->num_subdev; i++) { > - struct mtd_info *subdev = concat->subdev[i]; > - > - if (from >= subdev->size) { > - from -= subdev->size; > - continue; > - } > - > - /* partial read ? */ > - if (from + devops.len > subdev->size) > - devops.len = subdev->size - from; > - > - err = mtd_read_oob(subdev, from, &devops); > - ops->retlen += devops.retlen; > - ops->oobretlen += devops.oobretlen; > - > - /* Save information about bitflips! */ > - if (unlikely(err)) { > - if (mtd_is_eccerr(err)) { > - mtd->ecc_stats.failed++; > - ret = err; > - } else if (mtd_is_bitflip(err)) { > - mtd->ecc_stats.corrected++; > - /* Do not overwrite -EBADMSG !! */ > - if (!ret) > - ret = err; > - } else > - return err; > - } > - > - if (devops.datbuf) { > - devops.len = ops->len - ops->retlen; > - if (!devops.len) > - return ret; > - devops.datbuf += devops.retlen; > - } > - if (devops.oobbuf) { > - devops.ooblen = ops->ooblen - ops->oobretlen; > - if (!devops.ooblen) > - return ret; > - devops.oobbuf += ops->oobretlen; > - } > - > - from = 0; > - } > - return -EINVAL; > -} > - > -static int > -concat_write_oob(struct mtd_info *mtd, loff_t to, struct mtd_oob_ops *ops) > -{ > - struct mtd_concat *concat = CONCAT(mtd); > - struct mtd_oob_ops devops = *ops; > - int i, err; > - > - if (!(mtd->flags & MTD_WRITEABLE)) > - return -EROFS; > - > - ops->retlen = ops->oobretlen = 0; > - > - for (i = 0; i < concat->num_subdev; i++) { > - struct mtd_info *subdev = concat->subdev[i]; > - > - if (to >= subdev->size) { > - to -= subdev->size; > - continue; > - } > - > - /* partial write ? */ > - if (to + devops.len > subdev->size) > - devops.len = subdev->size - to; > - > - err = mtd_write_oob(subdev, to, &devops); > - ops->retlen += devops.retlen; > - ops->oobretlen += devops.oobretlen; > - if (err) > - return err; > - > - if (devops.datbuf) { > - devops.len = ops->len - ops->retlen; > - if (!devops.len) > - return 0; > - devops.datbuf += devops.retlen; > - } > - if (devops.oobbuf) { > - devops.ooblen = ops->ooblen - ops->oobretlen; > - if (!devops.ooblen) > - return 0; > - devops.oobbuf += devops.oobretlen; > - } > - to = 0; > - } > - return -EINVAL; > -} > - > static int concat_erase(struct mtd_info *mtd, struct erase_info *instr) > { > struct mtd_concat *concat = CONCAT(mtd); > @@ -684,10 +580,6 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c > subdev_master = mtd_get_master(subdev[0]); > if (subdev_master->_writev) > concat->mtd._writev = concat_writev; > - if (subdev_master->_read_oob) > - concat->mtd._read_oob = concat_read_oob; > - if (subdev_master->_write_oob) > - concat->mtd._write_oob = concat_write_oob; Actually I am not sure _read|write_oob() is the right callback to remove. Richard, what is your input on this? Shall we remove _read|write() instead? I don't remember the exact rationale behind these two helpers. > if (subdev_master->_block_isbad) > concat->mtd._block_isbad = concat_block_isbad; > if (subdev_master->_block_markbad) > @@ -724,15 +616,12 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c > subdev[i]->flags & MTD_WRITEABLE; > } > > - subdev_master = mtd_get_master(subdev[i]); > concat->mtd.size += subdev[i]->size; > concat->mtd.ecc_stats.badblocks += > subdev[i]->ecc_stats.badblocks; > if (concat->mtd.writesize != subdev[i]->writesize || > concat->mtd.subpage_sft != subdev[i]->subpage_sft || > - concat->mtd.oobsize != subdev[i]->oobsize || > - !concat->mtd._read_oob != !subdev_master->_read_oob || > - !concat->mtd._write_oob != !subdev_master->_write_oob) { > + concat->mtd.oobsize != subdev[i]->oobsize) { > kfree(concat); > printk("Incompatible OOB or ECC data on \"%s\"\n", > subdev[i]->name); Thanks, Miquèl ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob 2021-08-06 19:26 ` Miquel Raynal @ 2021-08-07 2:59 ` Zhihao Cheng 2021-08-07 10:28 ` Miquel Raynal 0 siblings, 1 reply; 13+ messages in thread From: Zhihao Cheng @ 2021-08-07 2:59 UTC (permalink / raw) To: Miquel Raynal Cc: richard, vigneshr, bbrezillon, linux-mtd, linux-kernel, yukuai3 在 2021/8/7 3:26, Miquel Raynal 写道: Hi Miquel, >> static int concat_erase(struct mtd_info *mtd, struct erase_info *instr) >> { >> struct mtd_concat *concat = CONCAT(mtd); >> @@ -684,10 +580,6 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c >> subdev_master = mtd_get_master(subdev[0]); >> if (subdev_master->_writev) >> concat->mtd._writev = concat_writev; >> - if (subdev_master->_read_oob) >> - concat->mtd._read_oob = concat_read_oob; >> - if (subdev_master->_write_oob) >> - concat->mtd._write_oob = concat_write_oob; > Actually I am not sure _read|write_oob() is the right callback to > remove. > > Richard, what is your input on this? Shall we remove _read|write() > instead? I don't remember the exact rationale behind these two helpers. Oh, I guess I made a mistake. It looks like that reserving _{read|write}_oob is a better method in my limited knowledge to nand driver. For example, nand_do_read_oob() behaves different from nand_do_read_ops(), and calling which function is decided by mtd_oob_ops.databuf. Callback _read_oobs() can support both functions, but callback _read() don't support nand_do_read_oob(). So mtd_read_oobs() covers mtd_read()? Is my understand right? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob 2021-08-07 2:59 ` Zhihao Cheng @ 2021-08-07 10:28 ` Miquel Raynal 0 siblings, 0 replies; 13+ messages in thread From: Miquel Raynal @ 2021-08-07 10:28 UTC (permalink / raw) To: Zhihao Cheng Cc: richard, vigneshr, bbrezillon, linux-mtd, linux-kernel, yukuai3 Hi Zhihao, Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sat, 7 Aug 2021 10:59:32 +0800: > 在 2021/8/7 3:26, Miquel Raynal 写道: > Hi Miquel, > >> static int concat_erase(struct mtd_info *mtd, struct erase_info *instr) > >> { > >> struct mtd_concat *concat = CONCAT(mtd); > >> @@ -684,10 +580,6 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c > >> subdev_master = mtd_get_master(subdev[0]); > >> if (subdev_master->_writev) > >> concat->mtd._writev = concat_writev; > >> - if (subdev_master->_read_oob) > >> - concat->mtd._read_oob = concat_read_oob; > >> - if (subdev_master->_write_oob) > >> - concat->mtd._write_oob = concat_write_oob; > > Actually I am not sure _read|write_oob() is the right callback to > > remove. > > > > Richard, what is your input on this? Shall we remove _read|write() > > instead? I don't remember the exact rationale behind these two helpers. > > Oh, I guess I made a mistake. It looks like that reserving _{read|write}_oob is a better method in my limited knowledge to nand driver. For example, nand_do_read_oob() behaves different from nand_do_read_ops(), and calling which function is decided by mtd_oob_ops.databuf. > Callback _read_oobs() can support both functions, but callback _read() don't support nand_do_read_oob(). So mtd_read_oobs() covers mtd_read()? > Is my understand right? > Yes please let's drop _read|write() instead. Thanks, Miquèl ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob 2021-07-31 2:32 ` [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob Zhihao Cheng 2021-08-06 19:26 ` Miquel Raynal @ 2021-08-16 14:27 ` Miquel Raynal 1 sibling, 0 replies; 13+ messages in thread From: Miquel Raynal @ 2021-08-16 14:27 UTC (permalink / raw) To: Zhihao Cheng, miquel.raynal, richard, vigneshr, bbrezillon Cc: linux-mtd, linux-kernel, yukuai3 On Sat, 2021-07-31 at 02:32:43 UTC, Zhihao Cheng wrote: > Since 2431c4f5b46c3("mtd: Implement mtd_{read,write}() as wrappers > around mtd_{read,write}_oob()") don't allow _write|_read and > _write_oob|_read_oob existing at the same time. We should stop these > two callback functions assigning, otherwise following warning occurs > while making concatenated device: > > WARNING: CPU: 2 PID: 6728 at drivers/mtd/mtdcore.c:595 > add_mtd_device+0x7f/0x7b0 > > Fixes: 2431c4f5b46c3("mtd: Implement mtd_{read,write}() around ...") > Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks. Miquel ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-08-16 14:27 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-31 2:32 [PATCH 0/2] mtd: mtdconcat: Fix callback functions check Zhihao Cheng
2021-07-31 2:32 ` [PATCH 1/2] mtd: mtdconcat: Judge callback function existence getting from master for each partition Zhihao Cheng
2021-08-06 19:28 ` Miquel Raynal
2021-08-07 2:15 ` Zhihao Cheng
2021-08-07 10:32 ` Miquel Raynal
2021-08-10 11:35 ` Zhihao Cheng
2021-08-16 13:43 ` Miquel Raynal
2021-08-16 14:27 ` Miquel Raynal
2021-07-31 2:32 ` [PATCH 2/2] mtd: mtdconcat: Remove concat_{read|write}_oob Zhihao Cheng
2021-08-06 19:26 ` Miquel Raynal
2021-08-07 2:59 ` Zhihao Cheng
2021-08-07 10:28 ` Miquel Raynal
2021-08-16 14:27 ` Miquel Raynal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).