linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mtd: implement proper partition handling
@ 2019-12-30 11:19 Miquel Raynal
  2020-01-08 23:34 ` Richard Weinberger
  0 siblings, 1 reply; 11+ messages in thread
From: Miquel Raynal @ 2019-12-30 11:19 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
  Cc: Boris Brezillon, Thomas Petazzoni, Miquel Raynal

Instead of collecting partitions in a flat list, create a hierarchy
within the mtd_info structure: use a partitions list to keep track of
the partitions of an MTD device (which might be itself a partition of
another MTD device), a pointer to the parent device (NULL when the MTD
device is the root one, not a partition).

By also saving directly in mtd_info the offset of the partition, we
can get rid of the mtd_part structure.

While at it, be consistent in the naming of the mtd_info structures to
ease the understanding of the new hierarchy: these structures are
usually called 'mtd', unless there are multiple instances of the same
structure. In this case, there is usually a parent/child bound so we
will call them 'parent' and 'child'.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---

Changes since v1:
* Rebased on a mainline kernel.
* Fixed the mtd_get_device_size() helper by adressing the case where
  there are multiple levels of partitions.
* Changed the global lock into a per-mtd-object lock.
* Typos/spaces.

 drivers/mtd/mtdchar.c          |  12 +-
 drivers/mtd/mtdcore.c          | 250 ++++++++----
 drivers/mtd/mtdpart.c          | 697 +++++++++------------------------
 include/linux/mtd/mtd.h        | 122 +++++-
 include/linux/mtd/partitions.h |   1 -
 5 files changed, 476 insertions(+), 606 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index b841008a9eb7..c5935b2f9cd1 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -349,6 +349,7 @@ static int mtdchar_writeoob(struct file *file, struct mtd_info *mtd,
 	uint64_t start, uint32_t length, void __user *ptr,
 	uint32_t __user *retp)
 {
+	struct mtd_info *master  = mtd_get_master(mtd);
 	struct mtd_file_info *mfi = file->private_data;
 	struct mtd_oob_ops ops = {};
 	uint32_t retlen;
@@ -360,7 +361,7 @@ static int mtdchar_writeoob(struct file *file, struct mtd_info *mtd,
 	if (length > 4096)
 		return -EINVAL;
 
-	if (!mtd->_write_oob)
+	if (!master->_write_oob)
 		return -EOPNOTSUPP;
 
 	ops.ooblen = length;
@@ -586,6 +587,7 @@ static int mtdchar_blkpg_ioctl(struct mtd_info *mtd,
 static int mtdchar_write_ioctl(struct mtd_info *mtd,
 		struct mtd_write_req __user *argp)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
 	struct mtd_write_req req;
 	struct mtd_oob_ops ops = {};
 	const void __user *usr_data, *usr_oob;
@@ -597,9 +599,8 @@ static int mtdchar_write_ioctl(struct mtd_info *mtd,
 	usr_data = (const void __user *)(uintptr_t)req.usr_data;
 	usr_oob = (const void __user *)(uintptr_t)req.usr_oob;
 
-	if (!mtd->_write_oob)
+	if (!master->_write_oob)
 		return -EOPNOTSUPP;
-
 	ops.mode = req.mode;
 	ops.len = (size_t)req.len;
 	ops.ooblen = (size_t)req.ooblen;
@@ -635,6 +636,7 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 {
 	struct mtd_file_info *mfi = file->private_data;
 	struct mtd_info *mtd = mfi->mtd;
+	struct mtd_info *master = mtd_get_master(mtd);
 	void __user *argp = (void __user *)arg;
 	int ret = 0;
 	struct mtd_info_user info;
@@ -824,7 +826,7 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 	{
 		struct nand_oobinfo oi;
 
-		if (!mtd->ooblayout)
+		if (!master->ooblayout)
 			return -EOPNOTSUPP;
 
 		ret = get_oobinfo(mtd, &oi);
@@ -918,7 +920,7 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 	{
 		struct nand_ecclayout_user *usrlay;
 
-		if (!mtd->ooblayout)
+		if (!master->ooblayout)
 			return -EOPNOTSUPP;
 
 		usrlay = kmalloc(sizeof(*usrlay), GFP_KERNEL);
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 5fac4355b9c2..2916674208b3 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -456,13 +456,14 @@ static int mtd_reboot_notifier(struct notifier_block *n, unsigned long state,
 int mtd_wunit_to_pairing_info(struct mtd_info *mtd, int wunit,
 			      struct mtd_pairing_info *info)
 {
-	int npairs = mtd_wunit_per_eb(mtd) / mtd_pairing_groups(mtd);
+	struct mtd_info *master = mtd_get_master(mtd);
+	int npairs = mtd_wunit_per_eb(master) / mtd_pairing_groups(master);
 
 	if (wunit < 0 || wunit >= npairs)
 		return -EINVAL;
 
-	if (mtd->pairing && mtd->pairing->get_info)
-		return mtd->pairing->get_info(mtd, wunit, info);
+	if (master->pairing && master->pairing->get_info)
+		return master->pairing->get_info(master, wunit, info);
 
 	info->group = 0;
 	info->pair = wunit;
@@ -498,15 +499,16 @@ EXPORT_SYMBOL_GPL(mtd_wunit_to_pairing_info);
 int mtd_pairing_info_to_wunit(struct mtd_info *mtd,
 			      const struct mtd_pairing_info *info)
 {
-	int ngroups = mtd_pairing_groups(mtd);
-	int npairs = mtd_wunit_per_eb(mtd) / ngroups;
+	struct mtd_info *master = mtd_get_master(mtd);
+	int ngroups = mtd_pairing_groups(master);
+	int npairs = mtd_wunit_per_eb(master) / ngroups;
 
 	if (!info || info->pair < 0 || info->pair >= npairs ||
 	    info->group < 0 || info->group >= ngroups)
 		return -EINVAL;
 
-	if (mtd->pairing && mtd->pairing->get_wunit)
-		return mtd->pairing->get_wunit(mtd, info);
+	if (master->pairing && master->pairing->get_wunit)
+		return mtd->pairing->get_wunit(master, info);
 
 	return info->pair;
 }
@@ -524,10 +526,12 @@ EXPORT_SYMBOL_GPL(mtd_pairing_info_to_wunit);
  */
 int mtd_pairing_groups(struct mtd_info *mtd)
 {
-	if (!mtd->pairing || !mtd->pairing->ngroups)
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->pairing || !master->pairing->ngroups)
 		return 1;
 
-	return mtd->pairing->ngroups;
+	return master->pairing->ngroups;
 }
 EXPORT_SYMBOL_GPL(mtd_pairing_groups);
 
@@ -587,6 +591,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
 
 int add_mtd_device(struct mtd_info *mtd)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
 	struct mtd_notifier *not;
 	int i, error;
 
@@ -608,7 +613,7 @@ int add_mtd_device(struct mtd_info *mtd)
 		    (mtd->_read && mtd->_read_oob)))
 		return -EINVAL;
 
-	if (WARN_ON((!mtd->erasesize || !mtd->_erase) &&
+	if (WARN_ON((!mtd->erasesize || !master->_erase) &&
 		    !(mtd->flags & MTD_NO_ERASE)))
 		return -EINVAL;
 
@@ -765,7 +770,8 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd)
 		pr_debug("mtd device won't show a device symlink in sysfs\n");
 	}
 
-	mtd->orig_flags = mtd->flags;
+	INIT_LIST_HEAD(&mtd->partitions);
+	mutex_init(&mtd->master.partitions_lock);
 }
 
 /**
@@ -971,20 +977,26 @@ EXPORT_SYMBOL_GPL(get_mtd_device);
 
 int __get_mtd_device(struct mtd_info *mtd)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
 	int err;
 
-	if (!try_module_get(mtd->owner))
+	if (!try_module_get(master->owner))
 		return -ENODEV;
 
-	if (mtd->_get_device) {
-		err = mtd->_get_device(mtd);
+	if (master->_get_device) {
+		err = master->_get_device(mtd);
 
 		if (err) {
-			module_put(mtd->owner);
+			module_put(master->owner);
 			return err;
 		}
 	}
-	mtd->usecount++;
+
+	while (mtd->parent) {
+		mtd->usecount++;
+		mtd = mtd->parent;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__get_mtd_device);
@@ -1038,13 +1050,18 @@ EXPORT_SYMBOL_GPL(put_mtd_device);
 
 void __put_mtd_device(struct mtd_info *mtd)
 {
-	--mtd->usecount;
-	BUG_ON(mtd->usecount < 0);
+	struct mtd_info *master = mtd_get_master(mtd);
 
-	if (mtd->_put_device)
-		mtd->_put_device(mtd);
+	while (mtd->parent) {
+		--mtd->usecount;
+		BUG_ON(mtd->usecount < 0);
+		mtd = mtd->parent;
+	}
 
-	module_put(mtd->owner);
+	if (master->_put_device)
+		master->_put_device(master);
+
+	module_put(master->owner);
 }
 EXPORT_SYMBOL_GPL(__put_mtd_device);
 
@@ -1055,9 +1072,13 @@ EXPORT_SYMBOL_GPL(__put_mtd_device);
  */
 int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+	u64 mst_ofs = mtd_get_master_ofs(mtd, 0);
+	int ret;
+
 	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
 
-	if (!mtd->erasesize || !mtd->_erase)
+	if (!mtd->erasesize || !master->_erase)
 		return -ENOTSUPP;
 
 	if (instr->addr >= mtd->size || instr->len > mtd->size - instr->addr)
@@ -1069,7 +1090,14 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 		return 0;
 
 	ledtrig_mtd_activity();
-	return mtd->_erase(mtd, instr);
+
+	instr->addr += mst_ofs;
+	ret = master->_erase(master, instr);
+	if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
+		instr->fail_addr -= mst_ofs;
+
+	instr->addr -= mst_ofs;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(mtd_erase);
 
@@ -1079,30 +1107,36 @@ EXPORT_SYMBOL_GPL(mtd_erase);
 int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
 	      void **virt, resource_size_t *phys)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+
 	*retlen = 0;
 	*virt = NULL;
 	if (phys)
 		*phys = 0;
-	if (!mtd->_point)
+	if (!master->_point)
 		return -EOPNOTSUPP;
 	if (from < 0 || from >= mtd->size || len > mtd->size - from)
 		return -EINVAL;
 	if (!len)
 		return 0;
-	return mtd->_point(mtd, from, len, retlen, virt, phys);
+
+	from = mtd_get_master_ofs(mtd, from);
+	return master->_point(master, from, len, retlen, virt, phys);
 }
 EXPORT_SYMBOL_GPL(mtd_point);
 
 /* We probably shouldn't allow XIP if the unpoint isn't a NULL */
 int mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
 {
-	if (!mtd->_unpoint)
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->_unpoint)
 		return -EOPNOTSUPP;
 	if (from < 0 || from >= mtd->size || len > mtd->size - from)
 		return -EINVAL;
 	if (!len)
 		return 0;
-	return mtd->_unpoint(mtd, from, len);
+	return master->_unpoint(master, mtd_get_master_ofs(mtd, from), len);
 }
 EXPORT_SYMBOL_GPL(mtd_unpoint);
 
@@ -1129,6 +1163,25 @@ unsigned long mtd_get_unmapped_area(struct mtd_info *mtd, unsigned long len,
 }
 EXPORT_SYMBOL_GPL(mtd_get_unmapped_area);
 
+static void mtd_update_ecc_stats(struct mtd_info *mtd, struct mtd_info *master,
+				 const struct mtd_ecc_stats *old_stats)
+{
+	struct mtd_ecc_stats diff;
+
+	if (master == mtd)
+		return;
+
+	diff = master->ecc_stats;
+	diff.failed -= old_stats->failed;
+	diff.corrected -= old_stats->corrected;
+
+	while (mtd->parent) {
+		mtd->ecc_stats.failed += diff.failed;
+		mtd->ecc_stats.corrected += diff.corrected;
+		mtd = mtd->parent;
+	}
+}
+
 int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
 	     u_char *buf)
 {
@@ -1171,8 +1224,10 @@ EXPORT_SYMBOL_GPL(mtd_write);
 int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
 		    const u_char *buf)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+
 	*retlen = 0;
-	if (!mtd->_panic_write)
+	if (!master->_panic_write)
 		return -EOPNOTSUPP;
 	if (to < 0 || to >= mtd->size || len > mtd->size - to)
 		return -EINVAL;
@@ -1183,7 +1238,8 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
 	if (!mtd->oops_panic_write)
 		mtd->oops_panic_write = true;
 
-	return mtd->_panic_write(mtd, to, len, retlen, buf);
+	return master->_panic_write(master, mtd_get_master_ofs(mtd, to), len,
+				    retlen, buf);
 }
 EXPORT_SYMBOL_GPL(mtd_panic_write);
 
@@ -1222,7 +1278,10 @@ static int mtd_check_oob_ops(struct mtd_info *mtd, loff_t offs,
 
 int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+	struct mtd_ecc_stats old_stats = master->ecc_stats;
 	int ret_code;
+
 	ops->retlen = ops->oobretlen = 0;
 
 	ret_code = mtd_check_oob_ops(mtd, from, ops);
@@ -1232,14 +1291,17 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
 	ledtrig_mtd_activity();
 
 	/* Check the validity of a potential fallback on mtd->_read */
-	if (!mtd->_read_oob && (!mtd->_read || ops->oobbuf))
+	if (!master->_read_oob && (!master->_read || ops->oobbuf))
 		return -EOPNOTSUPP;
 
-	if (mtd->_read_oob)
-		ret_code = mtd->_read_oob(mtd, from, ops);
+	from = mtd_get_master_ofs(mtd, from);
+	if (master->_read_oob)
+		ret_code = master->_read_oob(master, from, ops);
 	else
-		ret_code = mtd->_read(mtd, from, ops->len, &ops->retlen,
-				      ops->datbuf);
+		ret_code = master->_read(master, from, ops->len, &ops->retlen,
+					 ops->datbuf);
+
+	mtd_update_ecc_stats(mtd, master, &old_stats);
 
 	/*
 	 * In cases where ops->datbuf != NULL, mtd->_read_oob() has semantics
@@ -1258,6 +1320,7 @@ EXPORT_SYMBOL_GPL(mtd_read_oob);
 int mtd_write_oob(struct mtd_info *mtd, loff_t to,
 				struct mtd_oob_ops *ops)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
 	int ret;
 
 	ops->retlen = ops->oobretlen = 0;
@@ -1272,14 +1335,16 @@ int mtd_write_oob(struct mtd_info *mtd, loff_t to,
 	ledtrig_mtd_activity();
 
 	/* Check the validity of a potential fallback on mtd->_write */
-	if (!mtd->_write_oob && (!mtd->_write || ops->oobbuf))
+	if (!master->_write_oob && (!master->_write || ops->oobbuf))
 		return -EOPNOTSUPP;
 
-	if (mtd->_write_oob)
-		return mtd->_write_oob(mtd, to, ops);
+	to = mtd_get_master_ofs(mtd, to);
+
+	if (master->_write_oob)
+		return master->_write_oob(master, to, ops);
 	else
-		return mtd->_write(mtd, to, ops->len, &ops->retlen,
-				   ops->datbuf);
+		return master->_write(master, to, ops->len, &ops->retlen,
+				      ops->datbuf);
 }
 EXPORT_SYMBOL_GPL(mtd_write_oob);
 
@@ -1302,15 +1367,17 @@ EXPORT_SYMBOL_GPL(mtd_write_oob);
 int mtd_ooblayout_ecc(struct mtd_info *mtd, int section,
 		      struct mtd_oob_region *oobecc)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+
 	memset(oobecc, 0, sizeof(*oobecc));
 
-	if (!mtd || section < 0)
+	if (!master || section < 0)
 		return -EINVAL;
 
-	if (!mtd->ooblayout || !mtd->ooblayout->ecc)
+	if (!master->ooblayout || !master->ooblayout->ecc)
 		return -ENOTSUPP;
 
-	return mtd->ooblayout->ecc(mtd, section, oobecc);
+	return master->ooblayout->ecc(master, section, oobecc);
 }
 EXPORT_SYMBOL_GPL(mtd_ooblayout_ecc);
 
@@ -1334,15 +1401,17 @@ EXPORT_SYMBOL_GPL(mtd_ooblayout_ecc);
 int mtd_ooblayout_free(struct mtd_info *mtd, int section,
 		       struct mtd_oob_region *oobfree)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+
 	memset(oobfree, 0, sizeof(*oobfree));
 
-	if (!mtd || section < 0)
+	if (!master || section < 0)
 		return -EINVAL;
 
-	if (!mtd->ooblayout || !mtd->ooblayout->free)
+	if (!master->ooblayout || !master->ooblayout->free)
 		return -ENOTSUPP;
 
-	return mtd->ooblayout->free(mtd, section, oobfree);
+	return master->ooblayout->free(master, section, oobfree);
 }
 EXPORT_SYMBOL_GPL(mtd_ooblayout_free);
 
@@ -1651,60 +1720,69 @@ EXPORT_SYMBOL_GPL(mtd_ooblayout_count_eccbytes);
 int mtd_get_fact_prot_info(struct mtd_info *mtd, size_t len, size_t *retlen,
 			   struct otp_info *buf)
 {
-	if (!mtd->_get_fact_prot_info)
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->_get_fact_prot_info)
 		return -EOPNOTSUPP;
 	if (!len)
 		return 0;
-	return mtd->_get_fact_prot_info(mtd, len, retlen, buf);
+	return master->_get_fact_prot_info(master, len, retlen, buf);
 }
 EXPORT_SYMBOL_GPL(mtd_get_fact_prot_info);
 
 int mtd_read_fact_prot_reg(struct mtd_info *mtd, loff_t from, size_t len,
 			   size_t *retlen, u_char *buf)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+
 	*retlen = 0;
-	if (!mtd->_read_fact_prot_reg)
+	if (!master->_read_fact_prot_reg)
 		return -EOPNOTSUPP;
 	if (!len)
 		return 0;
-	return mtd->_read_fact_prot_reg(mtd, from, len, retlen, buf);
+	return master->_read_fact_prot_reg(master, from, len, retlen, buf);
 }
 EXPORT_SYMBOL_GPL(mtd_read_fact_prot_reg);
 
 int mtd_get_user_prot_info(struct mtd_info *mtd, size_t len, size_t *retlen,
 			   struct otp_info *buf)
 {
-	if (!mtd->_get_user_prot_info)
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->_get_user_prot_info)
 		return -EOPNOTSUPP;
 	if (!len)
 		return 0;
-	return mtd->_get_user_prot_info(mtd, len, retlen, buf);
+	return master->_get_user_prot_info(master, len, retlen, buf);
 }
 EXPORT_SYMBOL_GPL(mtd_get_user_prot_info);
 
 int mtd_read_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len,
 			   size_t *retlen, u_char *buf)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+
 	*retlen = 0;
-	if (!mtd->_read_user_prot_reg)
+	if (!master->_read_user_prot_reg)
 		return -EOPNOTSUPP;
 	if (!len)
 		return 0;
-	return mtd->_read_user_prot_reg(mtd, from, len, retlen, buf);
+	return master->_read_user_prot_reg(master, from, len, retlen, buf);
 }
 EXPORT_SYMBOL_GPL(mtd_read_user_prot_reg);
 
 int mtd_write_user_prot_reg(struct mtd_info *mtd, loff_t to, size_t len,
 			    size_t *retlen, u_char *buf)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
 	int ret;
 
 	*retlen = 0;
-	if (!mtd->_write_user_prot_reg)
+	if (!master->_write_user_prot_reg)
 		return -EOPNOTSUPP;
 	if (!len)
 		return 0;
-	ret = mtd->_write_user_prot_reg(mtd, to, len, retlen, buf);
+	ret = master->_write_user_prot_reg(master, to, len, retlen, buf);
 	if (ret)
 		return ret;
 
@@ -1718,80 +1796,105 @@ EXPORT_SYMBOL_GPL(mtd_write_user_prot_reg);
 
 int mtd_lock_user_prot_reg(struct mtd_info *mtd, loff_t from, size_t len)
 {
-	if (!mtd->_lock_user_prot_reg)
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->_lock_user_prot_reg)
 		return -EOPNOTSUPP;
 	if (!len)
 		return 0;
-	return mtd->_lock_user_prot_reg(mtd, from, len);
+	return master->_lock_user_prot_reg(master, from, len);
 }
 EXPORT_SYMBOL_GPL(mtd_lock_user_prot_reg);
 
 /* Chip-supported device locking */
 int mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 {
-	if (!mtd->_lock)
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->_lock)
 		return -EOPNOTSUPP;
 	if (ofs < 0 || ofs >= mtd->size || len > mtd->size - ofs)
 		return -EINVAL;
 	if (!len)
 		return 0;
-	return mtd->_lock(mtd, ofs, len);
+	return master->_lock(master, mtd_get_master_ofs(mtd, ofs), len);
 }
 EXPORT_SYMBOL_GPL(mtd_lock);
 
 int mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 {
-	if (!mtd->_unlock)
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->_unlock)
 		return -EOPNOTSUPP;
 	if (ofs < 0 || ofs >= mtd->size || len > mtd->size - ofs)
 		return -EINVAL;
 	if (!len)
 		return 0;
-	return mtd->_unlock(mtd, ofs, len);
+	return master->_unlock(master, mtd_get_master_ofs(mtd, ofs), len);
 }
 EXPORT_SYMBOL_GPL(mtd_unlock);
 
 int mtd_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 {
-	if (!mtd->_is_locked)
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->_is_locked)
 		return -EOPNOTSUPP;
 	if (ofs < 0 || ofs >= mtd->size || len > mtd->size - ofs)
 		return -EINVAL;
 	if (!len)
 		return 0;
-	return mtd->_is_locked(mtd, ofs, len);
+	return master->_is_locked(master, mtd_get_master_ofs(mtd, ofs), len);
 }
 EXPORT_SYMBOL_GPL(mtd_is_locked);
 
 int mtd_block_isreserved(struct mtd_info *mtd, loff_t ofs)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+
 	if (ofs < 0 || ofs >= mtd->size)
 		return -EINVAL;
-	if (!mtd->_block_isreserved)
+	if (!master->_block_isreserved)
 		return 0;
-	return mtd->_block_isreserved(mtd, ofs);
+	return master->_block_isreserved(master, mtd_get_master_ofs(mtd, ofs));
 }
 EXPORT_SYMBOL_GPL(mtd_block_isreserved);
 
 int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+
 	if (ofs < 0 || ofs >= mtd->size)
 		return -EINVAL;
-	if (!mtd->_block_isbad)
+	if (!master->_block_isbad)
 		return 0;
-	return mtd->_block_isbad(mtd, ofs);
+	return master->_block_isbad(master, mtd_get_master_ofs(mtd, ofs));
 }
 EXPORT_SYMBOL_GPL(mtd_block_isbad);
 
 int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
 {
-	if (!mtd->_block_markbad)
+	struct mtd_info *master = mtd_get_master(mtd);
+	int ret;
+
+	if (!master->_block_markbad)
 		return -EOPNOTSUPP;
 	if (ofs < 0 || ofs >= mtd->size)
 		return -EINVAL;
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
-	return mtd->_block_markbad(mtd, ofs);
+
+	ret = master->_block_markbad(master, mtd_get_master_ofs(mtd, ofs));
+	if (ret)
+		return ret;
+
+	while (mtd->parent) {
+		mtd->ecc_stats.badblocks++;
+		mtd = mtd->parent;
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(mtd_block_markbad);
 
@@ -1841,12 +1944,17 @@ static int default_mtd_writev(struct mtd_info *mtd, const struct kvec *vecs,
 int mtd_writev(struct mtd_info *mtd, const struct kvec *vecs,
 	       unsigned long count, loff_t to, size_t *retlen)
 {
+	struct mtd_info *master = mtd_get_master(mtd);
+
 	*retlen = 0;
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
-	if (!mtd->_writev)
+
+	if (!master->_writev)
 		return default_mtd_writev(mtd, vecs, count, to, retlen);
-	return mtd->_writev(mtd, vecs, count, to, retlen);
+
+	return master->_writev(master, vecs, count,
+			       mtd_get_master_ofs(mtd, to), retlen);
 }
 EXPORT_SYMBOL_GPL(mtd_writev);
 
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 7328c066c5ba..93ef8fdf1634 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -20,339 +20,52 @@
 
 #include "mtdcore.h"
 
-/* Our partition linked list */
-static LIST_HEAD(mtd_partitions);
-static DEFINE_MUTEX(mtd_partitions_mutex);
-
-/**
- * struct mtd_part - our partition node structure
- *
- * @mtd: struct holding partition details
- * @parent: parent mtd - flash device or another partition
- * @offset: partition offset relative to the *flash device*
- */
-struct mtd_part {
-	struct mtd_info mtd;
-	struct mtd_info *parent;
-	uint64_t offset;
-	struct list_head list;
-};
-
-/*
- * Given a pointer to the MTD object in the mtd_part structure, we can retrieve
- * the pointer to that structure.
- */
-static inline struct mtd_part *mtd_to_part(const struct mtd_info *mtd)
-{
-	return container_of(mtd, struct mtd_part, mtd);
-}
-
-static u64 part_absolute_offset(struct mtd_info *mtd)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-
-	if (!mtd_is_partition(mtd))
-		return 0;
-
-	return part_absolute_offset(part->parent) + part->offset;
-}
-
 /*
  * MTD methods which simply translate the effective address and pass through
  * to the _real_ device.
  */
 
-static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
-		size_t *retlen, u_char *buf)
+static inline void free_partition(struct mtd_info *mtd)
 {
-	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 part_point(struct mtd_info *mtd, loff_t from, size_t len,
-		size_t *retlen, void **virt, resource_size_t *phys)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-
-	return part->parent->_point(part->parent, from + part->offset, len,
-				    retlen, virt, phys);
-}
-
-static int part_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-
-	return part->parent->_unpoint(part->parent, from + part->offset, len);
-}
-
-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;
-
-	stats = part->parent->ecc_stats;
-	res = part->parent->_read_oob(part->parent, from + part->offset, ops);
-	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 part_read_user_prot_reg(struct mtd_info *mtd, loff_t from,
-		size_t len, size_t *retlen, u_char *buf)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_read_user_prot_reg(part->parent, from, len,
-						 retlen, buf);
-}
-
-static int part_get_user_prot_info(struct mtd_info *mtd, size_t len,
-				   size_t *retlen, struct otp_info *buf)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_get_user_prot_info(part->parent, len, retlen,
-						 buf);
-}
-
-static int part_read_fact_prot_reg(struct mtd_info *mtd, loff_t from,
-		size_t len, size_t *retlen, u_char *buf)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_read_fact_prot_reg(part->parent, from, len,
-						 retlen, buf);
-}
-
-static int part_get_fact_prot_info(struct mtd_info *mtd, size_t len,
-				   size_t *retlen, struct otp_info *buf)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_get_fact_prot_info(part->parent, len, retlen,
-						 buf);
-}
-
-static int part_write(struct mtd_info *mtd, loff_t to, size_t len,
-		size_t *retlen, const u_char *buf)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_write(part->parent, to + part->offset, len,
-				    retlen, buf);
-}
-
-static int part_panic_write(struct mtd_info *mtd, loff_t to, size_t len,
-		size_t *retlen, const u_char *buf)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_panic_write(part->parent, to + part->offset, len,
-					  retlen, buf);
-}
-
-static int part_write_oob(struct mtd_info *mtd, loff_t to,
-		struct mtd_oob_ops *ops)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-
-	return part->parent->_write_oob(part->parent, to + part->offset, ops);
-}
-
-static int part_write_user_prot_reg(struct mtd_info *mtd, loff_t from,
-		size_t len, size_t *retlen, u_char *buf)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_write_user_prot_reg(part->parent, from, len,
-						  retlen, buf);
-}
-
-static int part_lock_user_prot_reg(struct mtd_info *mtd, loff_t from,
-		size_t len)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_lock_user_prot_reg(part->parent, from, len);
-}
-
-static int part_writev(struct mtd_info *mtd, const struct kvec *vecs,
-		unsigned long count, loff_t to, size_t *retlen)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_writev(part->parent, vecs, count,
-				     to + part->offset, retlen);
-}
-
-static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	int ret;
-
-	instr->addr += part->offset;
-	ret = part->parent->_erase(part->parent, instr);
-	if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
-		instr->fail_addr -= part->offset;
-	instr->addr -= part->offset;
-
-	return ret;
-}
-
-static int part_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_lock(part->parent, ofs + part->offset, len);
-}
-
-static int part_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_unlock(part->parent, ofs + part->offset, len);
-}
-
-static int part_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_is_locked(part->parent, ofs + part->offset, len);
-}
-
-static void part_sync(struct mtd_info *mtd)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	part->parent->_sync(part->parent);
-}
-
-static int part_suspend(struct mtd_info *mtd)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_suspend(part->parent);
-}
-
-static void part_resume(struct mtd_info *mtd)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	part->parent->_resume(part->parent);
-}
-
-static int part_block_isreserved(struct mtd_info *mtd, loff_t ofs)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	ofs += part->offset;
-	return part->parent->_block_isreserved(part->parent, ofs);
-}
-
-static int part_block_isbad(struct mtd_info *mtd, loff_t ofs)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	ofs += part->offset;
-	return part->parent->_block_isbad(part->parent, ofs);
-}
-
-static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	int res;
-
-	ofs += part->offset;
-	res = part->parent->_block_markbad(part->parent, ofs);
-	if (!res)
-		mtd->ecc_stats.badblocks++;
-	return res;
-}
-
-static int part_get_device(struct mtd_info *mtd)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	return part->parent->_get_device(part->parent);
-}
-
-static void part_put_device(struct mtd_info *mtd)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-	part->parent->_put_device(part->parent);
-}
-
-static int part_ooblayout_ecc(struct mtd_info *mtd, int section,
-			      struct mtd_oob_region *oobregion)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-
-	return mtd_ooblayout_ecc(part->parent, section, oobregion);
-}
-
-static int part_ooblayout_free(struct mtd_info *mtd, int section,
-			       struct mtd_oob_region *oobregion)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-
-	return mtd_ooblayout_free(part->parent, section, oobregion);
-}
-
-static const struct mtd_ooblayout_ops part_ooblayout_ops = {
-	.ecc = part_ooblayout_ecc,
-	.free = part_ooblayout_free,
-};
-
-static int part_max_bad_blocks(struct mtd_info *mtd, loff_t ofs, size_t len)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-
-	return part->parent->_max_bad_blocks(part->parent,
-					     ofs + part->offset, len);
-}
-
-static inline void free_partition(struct mtd_part *p)
-{
-	kfree(p->mtd.name);
-	kfree(p);
+	kfree(mtd->name);
+	kfree(mtd);
 }
 
-static struct mtd_part *allocate_partition(struct mtd_info *parent,
-			const struct mtd_partition *part, int partno,
-			uint64_t cur_offset)
+static struct mtd_info *allocate_partition(struct mtd_info *parent,
+					   const struct mtd_partition *part,
+					   int partno, uint64_t cur_offset)
 {
 	int wr_alignment = (parent->flags & MTD_NO_ERASE) ? parent->writesize :
 							    parent->erasesize;
-	struct mtd_part *slave;
+	struct mtd_info *child, *master = mtd_get_master(parent);
 	u32 remainder;
 	char *name;
 	u64 tmp;
 
 	/* allocate the partition structure */
-	slave = kzalloc(sizeof(*slave), GFP_KERNEL);
+	child = kzalloc(sizeof(*child), GFP_KERNEL);
 	name = kstrdup(part->name, GFP_KERNEL);
-	if (!name || !slave) {
+	if (!name || !child) {
 		printk(KERN_ERR"memory allocation error while creating partitions for \"%s\"\n",
 		       parent->name);
 		kfree(name);
-		kfree(slave);
+		kfree(child);
 		return ERR_PTR(-ENOMEM);
 	}
 
 	/* set up the MTD object for this partition */
-	slave->mtd.type = parent->type;
-	slave->mtd.flags = parent->orig_flags & ~part->mask_flags;
-	slave->mtd.orig_flags = slave->mtd.flags;
-	slave->mtd.size = part->size;
-	slave->mtd.writesize = parent->writesize;
-	slave->mtd.writebufsize = parent->writebufsize;
-	slave->mtd.oobsize = parent->oobsize;
-	slave->mtd.oobavail = parent->oobavail;
-	slave->mtd.subpage_sft = parent->subpage_sft;
-	slave->mtd.pairing = parent->pairing;
+	child->type = parent->type;
+	child->part.flags = parent->flags & ~part->mask_flags;
+	child->flags = child->part.flags;
+	child->size = part->size;
+	child->writesize = parent->writesize;
+	child->writebufsize = parent->writebufsize;
+	child->oobsize = parent->oobsize;
+	child->oobavail = parent->oobavail;
+	child->subpage_sft = parent->subpage_sft;
 
-	slave->mtd.name = name;
-	slave->mtd.owner = parent->owner;
+	child->name = name;
+	child->owner = parent->owner;
 
 	/* NOTE: Historically, we didn't arrange MTDs as a tree out of
 	 * concern for showing the same data in multiple partitions.
@@ -360,134 +73,76 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent,
 	 * so the MTD_PARTITIONED_MASTER option allows that. The master
 	 * will have device nodes etc only if this is set, so make the
 	 * parent conditional on that option. Note, this is a way to
-	 * distinguish between the master and the partition in sysfs.
+	 * distinguish between the parent and its partitions in sysfs.
 	 */
-	slave->mtd.dev.parent = IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) || mtd_is_partition(parent) ?
-				&parent->dev :
-				parent->dev.parent;
-	slave->mtd.dev.of_node = part->of_node;
-
-	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;
-
-	if (parent->_point && parent->_unpoint) {
-		slave->mtd._point = part_point;
-		slave->mtd._unpoint = part_unpoint;
-	}
-
-	if (parent->_read_oob)
-		slave->mtd._read_oob = part_read_oob;
-	if (parent->_write_oob)
-		slave->mtd._write_oob = part_write_oob;
-	if (parent->_read_user_prot_reg)
-		slave->mtd._read_user_prot_reg = part_read_user_prot_reg;
-	if (parent->_read_fact_prot_reg)
-		slave->mtd._read_fact_prot_reg = part_read_fact_prot_reg;
-	if (parent->_write_user_prot_reg)
-		slave->mtd._write_user_prot_reg = part_write_user_prot_reg;
-	if (parent->_lock_user_prot_reg)
-		slave->mtd._lock_user_prot_reg = part_lock_user_prot_reg;
-	if (parent->_get_user_prot_info)
-		slave->mtd._get_user_prot_info = part_get_user_prot_info;
-	if (parent->_get_fact_prot_info)
-		slave->mtd._get_fact_prot_info = part_get_fact_prot_info;
-	if (parent->_sync)
-		slave->mtd._sync = part_sync;
-	if (!partno && !parent->dev.class && parent->_suspend &&
-	    parent->_resume) {
-		slave->mtd._suspend = part_suspend;
-		slave->mtd._resume = part_resume;
-	}
-	if (parent->_writev)
-		slave->mtd._writev = part_writev;
-	if (parent->_lock)
-		slave->mtd._lock = part_lock;
-	if (parent->_unlock)
-		slave->mtd._unlock = part_unlock;
-	if (parent->_is_locked)
-		slave->mtd._is_locked = part_is_locked;
-	if (parent->_block_isreserved)
-		slave->mtd._block_isreserved = part_block_isreserved;
-	if (parent->_block_isbad)
-		slave->mtd._block_isbad = part_block_isbad;
-	if (parent->_block_markbad)
-		slave->mtd._block_markbad = part_block_markbad;
-	if (parent->_max_bad_blocks)
-		slave->mtd._max_bad_blocks = part_max_bad_blocks;
-
-	if (parent->_get_device)
-		slave->mtd._get_device = part_get_device;
-	if (parent->_put_device)
-		slave->mtd._put_device = part_put_device;
-
-	slave->mtd._erase = part_erase;
-	slave->parent = parent;
-	slave->offset = part->offset;
-
-	if (slave->offset == MTDPART_OFS_APPEND)
-		slave->offset = cur_offset;
-	if (slave->offset == MTDPART_OFS_NXTBLK) {
+	child->dev.parent = IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) || mtd_is_partition(parent) ?
+			    &parent->dev : parent->dev.parent;
+	child->dev.of_node = part->of_node;
+	child->parent = parent;
+	child->part.offset = part->offset;
+	INIT_LIST_HEAD(&child->partitions);
+
+	if (child->part.offset == MTDPART_OFS_APPEND)
+		child->part.offset = cur_offset;
+	if (child->part.offset == MTDPART_OFS_NXTBLK) {
 		tmp = cur_offset;
-		slave->offset = cur_offset;
+		child->part.offset = cur_offset;
 		remainder = do_div(tmp, wr_alignment);
 		if (remainder) {
-			slave->offset += wr_alignment - remainder;
+			child->part.offset += wr_alignment - remainder;
 			printk(KERN_NOTICE "Moving partition %d: "
 			       "0x%012llx -> 0x%012llx\n", partno,
-			       (unsigned long long)cur_offset, (unsigned long long)slave->offset);
+			       (unsigned long long)cur_offset,
+			       child->part.offset);
 		}
 	}
-	if (slave->offset == MTDPART_OFS_RETAIN) {
-		slave->offset = cur_offset;
-		if (parent->size - slave->offset >= slave->mtd.size) {
-			slave->mtd.size = parent->size - slave->offset
-							- slave->mtd.size;
+	if (child->part.offset == MTDPART_OFS_RETAIN) {
+		child->part.offset = cur_offset;
+		if (parent->size - child->part.offset >= child->size) {
+			child->size = parent->size - child->part.offset -
+				      child->size;
 		} else {
 			printk(KERN_ERR "mtd partition \"%s\" doesn't have enough space: %#llx < %#llx, disabled\n",
-				part->name, parent->size - slave->offset,
-				slave->mtd.size);
+				part->name, parent->size - child->part.offset,
+				child->size);
 			/* register to preserve ordering */
 			goto out_register;
 		}
 	}
-	if (slave->mtd.size == MTDPART_SIZ_FULL)
-		slave->mtd.size = parent->size - slave->offset;
+	if (child->size == MTDPART_SIZ_FULL)
+		child->size = parent->size - child->part.offset;
 
-	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
-		(unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
+	printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n",
+	       child->part.offset, child->part.offset + child->size,
+	       child->name);
 
 	/* let's do some sanity checks */
-	if (slave->offset >= parent->size) {
+	if (child->part.offset >= parent->size) {
 		/* let's register it anyway to preserve ordering */
-		slave->offset = 0;
-		slave->mtd.size = 0;
+		child->part.offset = 0;
+		child->size = 0;
 
 		/* Initialize ->erasesize to make add_mtd_device() happy. */
-		slave->mtd.erasesize = parent->erasesize;
-
+		child->erasesize = parent->erasesize;
 		printk(KERN_ERR"mtd: partition \"%s\" is out of reach -- disabled\n",
 			part->name);
 		goto out_register;
 	}
-	if (slave->offset + slave->mtd.size > parent->size) {
-		slave->mtd.size = parent->size - slave->offset;
+	if (child->part.offset + child->size > parent->size) {
+		child->size = parent->size - child->part.offset;
 		printk(KERN_WARNING"mtd: partition \"%s\" extends beyond the end of device \"%s\" -- size truncated to %#llx\n",
-			part->name, parent->name, (unsigned long long)slave->mtd.size);
+			part->name, parent->name, child->size);
 	}
 	if (parent->numeraseregions > 1) {
 		/* Deal with variable erase size stuff */
 		int i, max = parent->numeraseregions;
-		u64 end = slave->offset + slave->mtd.size;
+		u64 end = child->part.offset + child->size;
 		struct mtd_erase_region_info *regions = parent->eraseregions;
 
 		/* Find the first erase regions which is part of this
 		 * partition. */
-		for (i = 0; i < max && regions[i].offset <= slave->offset; i++)
+		for (i = 0; i < max && regions[i].offset <= child->part.offset;
+		     i++)
 			;
 		/* The loop searched for the region _behind_ the first one */
 		if (i > 0)
@@ -495,70 +150,68 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent,
 
 		/* Pick biggest erasesize */
 		for (; i < max && regions[i].offset < end; i++) {
-			if (slave->mtd.erasesize < regions[i].erasesize) {
-				slave->mtd.erasesize = regions[i].erasesize;
-			}
+			if (child->erasesize < regions[i].erasesize)
+				child->erasesize = regions[i].erasesize;
 		}
-		BUG_ON(slave->mtd.erasesize == 0);
+		BUG_ON(child->erasesize == 0);
 	} else {
 		/* Single erase size */
-		slave->mtd.erasesize = parent->erasesize;
+		child->erasesize = parent->erasesize;
 	}
 
 	/*
-	 * Slave erasesize might differ from the master one if the master
+	 * Child erasesize might differ from the parent one if the parent
 	 * exposes several regions with different erasesize. Adjust
 	 * wr_alignment accordingly.
 	 */
-	if (!(slave->mtd.flags & MTD_NO_ERASE))
-		wr_alignment = slave->mtd.erasesize;
+	if (!(child->flags & MTD_NO_ERASE))
+		wr_alignment = child->erasesize;
 
-	tmp = part_absolute_offset(parent) + slave->offset;
+	tmp = mtd_get_master_ofs(child, 0);
 	remainder = do_div(tmp, wr_alignment);
-	if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
+	if ((child->flags & MTD_WRITEABLE) && remainder) {
 		/* Doesn't start on a boundary of major erase size */
 		/* FIXME: Let it be writable if it is on a boundary of
 		 * _minor_ erase size though */
-		slave->mtd.flags &= ~MTD_WRITEABLE;
+		child->flags &= ~MTD_WRITEABLE;
 		printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase/write block boundary -- force read-only\n",
 			part->name);
 	}
 
-	tmp = part_absolute_offset(parent) + slave->mtd.size;
+	tmp = mtd_get_master_ofs(child, 0) + child->size;
 	remainder = do_div(tmp, wr_alignment);
-	if ((slave->mtd.flags & MTD_WRITEABLE) && remainder) {
-		slave->mtd.flags &= ~MTD_WRITEABLE;
+	if ((child->flags & MTD_WRITEABLE) && remainder) {
+		child->flags &= ~MTD_WRITEABLE;
 		printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase/write block -- force read-only\n",
 			part->name);
 	}
 
-	mtd_set_ooblayout(&slave->mtd, &part_ooblayout_ops);
-	slave->mtd.ecc_step_size = parent->ecc_step_size;
-	slave->mtd.ecc_strength = parent->ecc_strength;
-	slave->mtd.bitflip_threshold = parent->bitflip_threshold;
+	child->ecc_step_size = parent->ecc_step_size;
+	child->ecc_strength = parent->ecc_strength;
+	child->bitflip_threshold = parent->bitflip_threshold;
 
-	if (parent->_block_isbad) {
+	if (master->_block_isbad) {
 		uint64_t offs = 0;
 
-		while (offs < slave->mtd.size) {
-			if (mtd_block_isreserved(parent, offs + slave->offset))
-				slave->mtd.ecc_stats.bbtblocks++;
-			else if (mtd_block_isbad(parent, offs + slave->offset))
-				slave->mtd.ecc_stats.badblocks++;
-			offs += slave->mtd.erasesize;
+		while (offs < child->size) {
+			if (mtd_block_isreserved(child, offs))
+				child->ecc_stats.bbtblocks++;
+			else if (mtd_block_isbad(child, offs))
+				child->ecc_stats.badblocks++;
+			offs += child->erasesize;
 		}
 	}
 
 out_register:
-	return slave;
+	return child;
 }
 
 static ssize_t mtd_partition_offset_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct mtd_info *mtd = dev_get_drvdata(dev);
-	struct mtd_part *part = mtd_to_part(mtd);
-	return snprintf(buf, PAGE_SIZE, "%llu\n", part->offset);
+
+	return snprintf(buf, PAGE_SIZE, "%lld\n", mtd->part.offset);
 }
 
 static DEVICE_ATTR(offset, S_IRUGO, mtd_partition_offset_show, NULL);
@@ -568,9 +221,9 @@ static const struct attribute *mtd_partition_attrs[] = {
 	NULL
 };
 
-static int mtd_add_partition_attrs(struct mtd_part *new)
+static int mtd_add_partition_attrs(struct mtd_info *new)
 {
-	int ret = sysfs_create_files(&new->mtd.dev.kobj, mtd_partition_attrs);
+	int ret = sysfs_create_files(&new->dev.kobj, mtd_partition_attrs);
 	if (ret)
 		printk(KERN_WARNING
 		       "mtd: failed to create partition attrs, err=%d\n", ret);
@@ -580,8 +233,9 @@ static int mtd_add_partition_attrs(struct mtd_part *new)
 int mtd_add_partition(struct mtd_info *parent, const char *name,
 		      long long offset, long long length)
 {
+	struct mtd_info *master = mtd_get_master(parent);
 	struct mtd_partition part;
-	struct mtd_part *new;
+	struct mtd_info *child;
 	int ret = 0;
 
 	/* the direct offset is expected */
@@ -600,28 +254,28 @@ int mtd_add_partition(struct mtd_info *parent, const char *name,
 	part.size = length;
 	part.offset = offset;
 
-	new = allocate_partition(parent, &part, -1, offset);
-	if (IS_ERR(new))
-		return PTR_ERR(new);
+	child = allocate_partition(parent, &part, -1, offset);
+	if (IS_ERR(child))
+		return PTR_ERR(child);
 
-	mutex_lock(&mtd_partitions_mutex);
-	list_add(&new->list, &mtd_partitions);
-	mutex_unlock(&mtd_partitions_mutex);
+	mutex_lock(&master->master.partitions_lock);
+	list_add_tail(&child->part.node, &parent->partitions);
+	mutex_unlock(&master->master.partitions_lock);
 
-	ret = add_mtd_device(&new->mtd);
+	ret = add_mtd_device(child);
 	if (ret)
 		goto err_remove_part;
 
-	mtd_add_partition_attrs(new);
+	mtd_add_partition_attrs(child);
 
 	return 0;
 
 err_remove_part:
-	mutex_lock(&mtd_partitions_mutex);
-	list_del(&new->list);
-	mutex_unlock(&mtd_partitions_mutex);
+	mutex_lock(&master->master.partitions_lock);
+	list_del(&child->part.node);
+	mutex_unlock(&master->master.partitions_lock);
 
-	free_partition(new);
+	free_partition(child);
 
 	return ret;
 }
@@ -630,119 +284,142 @@ EXPORT_SYMBOL_GPL(mtd_add_partition);
 /**
  * __mtd_del_partition - delete MTD partition
  *
- * @priv: internal MTD struct for partition to be deleted
+ * @priv: MTD structure to be deleted
  *
  * This function must be called with the partitions mutex locked.
  */
-static int __mtd_del_partition(struct mtd_part *priv)
+static int __mtd_del_partition(struct mtd_info *mtd)
 {
-	struct mtd_part *child, *next;
+	struct mtd_info *child, *next;
 	int err;
 
-	list_for_each_entry_safe(child, next, &mtd_partitions, list) {
-		if (child->parent == &priv->mtd) {
-			err = __mtd_del_partition(child);
-			if (err)
-				return err;
-		}
+	list_for_each_entry_safe(child, next, &mtd->partitions, part.node) {
+		err = __mtd_del_partition(child);
+		if (err)
+			return err;
 	}
 
-	sysfs_remove_files(&priv->mtd.dev.kobj, mtd_partition_attrs);
+	sysfs_remove_files(&mtd->dev.kobj, mtd_partition_attrs);
 
-	err = del_mtd_device(&priv->mtd);
+	err = del_mtd_device(mtd);
 	if (err)
 		return err;
 
-	list_del(&priv->list);
-	free_partition(priv);
+	list_del(&child->part.node);
+	free_partition(mtd);
 
 	return 0;
 }
 
 /*
  * This function unregisters and destroy all slave MTD objects which are
- * attached to the given MTD object.
+ * attached to the given MTD object, recursively.
  */
+int __del_mtd_partitions(struct mtd_info *mtd)
+{
+	struct mtd_info *child, *next;
+	LIST_HEAD(tmp_list);
+	int ret, err = 0;
+
+	list_for_each_entry_safe(child, next, &mtd->partitions, part.node) {
+		if (mtd_has_partitions(child))
+			del_mtd_partitions(child);
+
+		pr_info("Deleting %s MTD partition\n", child->name);
+		ret = del_mtd_device(child);
+		if (ret < 0) {
+			pr_err("Error when deleting partition \"%s\" (%d)\n",
+			       child->name, ret);
+			err = ret;
+			continue;
+		}
+
+		list_del(&child->part.node);
+		free_partition(child);
+	}
+
+	return err;
+}
+
 int del_mtd_partitions(struct mtd_info *mtd)
 {
-	struct mtd_part *slave, *next;
-	int ret, err = 0;
+	struct mtd_info *master = mtd_get_master(mtd);
+	int ret;
 
-	mutex_lock(&mtd_partitions_mutex);
-	list_for_each_entry_safe(slave, next, &mtd_partitions, list)
-		if (slave->parent == mtd) {
-			ret = __mtd_del_partition(slave);
-			if (ret < 0)
-				err = ret;
-		}
-	mutex_unlock(&mtd_partitions_mutex);
+	pr_info("Deleting MTD partitions on \"%s\":\n", mtd->name);
 
-	return err;
+	mutex_lock(&master->master.partitions_lock);
+	ret = __del_mtd_partitions(mtd);
+	mutex_unlock(&master->master.partitions_lock);
+
+	return ret;
 }
 
 int mtd_del_partition(struct mtd_info *mtd, int partno)
 {
-	struct mtd_part *slave, *next;
+	struct mtd_info *child, *master = mtd_get_master(mtd);
 	int ret = -EINVAL;
 
-	mutex_lock(&mtd_partitions_mutex);
-	list_for_each_entry_safe(slave, next, &mtd_partitions, list)
-		if ((slave->parent == mtd) &&
-		    (slave->mtd.index == partno)) {
-			ret = __mtd_del_partition(slave);
+	mutex_lock(&master->master.partitions_lock);
+	list_for_each_entry(child, &mtd->partitions, part.node) {
+		if (child->index == partno) {
+			ret = __mtd_del_partition(child);
 			break;
 		}
-	mutex_unlock(&mtd_partitions_mutex);
+	}
+	mutex_unlock(&master->master.partitions_lock);
 
 	return ret;
 }
 EXPORT_SYMBOL_GPL(mtd_del_partition);
 
 /*
- * This function, given a master MTD object and a partition table, creates
- * and registers slave MTD objects which are bound to the master according to
- * the partition definitions.
+ * This function, given a parent MTD object and a partition table, creates
+ * and registers the child MTD objects which are bound to the parent according
+ * to the partition definitions.
  *
- * For historical reasons, this function's caller only registers the master
+ * For historical reasons, this function's caller only registers the parent
  * if the MTD_PARTITIONED_MASTER config option is set.
  */
 
-int add_mtd_partitions(struct mtd_info *master,
+int add_mtd_partitions(struct mtd_info *parent,
 		       const struct mtd_partition *parts,
 		       int nbparts)
 {
-	struct mtd_part *slave;
+	struct mtd_info *child, *master = mtd_get_master(parent);
 	uint64_t cur_offset = 0;
 	int i, ret;
 
-	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
+	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n",
+	       nbparts, parent->name);
 
 	for (i = 0; i < nbparts; i++) {
-		slave = allocate_partition(master, parts + i, i, cur_offset);
-		if (IS_ERR(slave)) {
-			ret = PTR_ERR(slave);
+		child = allocate_partition(parent, parts + i, i, cur_offset);
+		if (IS_ERR(child)) {
+			ret = PTR_ERR(child);
 			goto err_del_partitions;
 		}
 
-		mutex_lock(&mtd_partitions_mutex);
-		list_add(&slave->list, &mtd_partitions);
-		mutex_unlock(&mtd_partitions_mutex);
+		mutex_lock(&master->master.partitions_lock);
+		list_add_tail(&child->part.node, &parent->partitions);
+		mutex_unlock(&master->master.partitions_lock);
 
-		ret = add_mtd_device(&slave->mtd);
+		ret = add_mtd_device(child);
 		if (ret) {
-			mutex_lock(&mtd_partitions_mutex);
-			list_del(&slave->list);
-			mutex_unlock(&mtd_partitions_mutex);
+			mutex_lock(&master->master.partitions_lock);
+			list_del(&child->part.node);
+			mutex_unlock(&master->master.partitions_lock);
 
-			free_partition(slave);
+			free_partition(child);
 			goto err_del_partitions;
 		}
 
-		mtd_add_partition_attrs(slave);
+		mtd_add_partition_attrs(child);
+
 		/* Look for subpartitions */
-		parse_mtd_partitions(&slave->mtd, parts[i].types, NULL);
+		parse_mtd_partitions(child, parts[i].types, NULL);
 
-		cur_offset = slave->offset + slave->mtd.size;
+		cur_offset = child->part.offset + child->size;
 	}
 
 	return 0;
@@ -1023,29 +700,11 @@ void mtd_part_parser_cleanup(struct mtd_partitions *parts)
 	}
 }
 
-int mtd_is_partition(const struct mtd_info *mtd)
-{
-	struct mtd_part *part;
-	int ispart = 0;
-
-	mutex_lock(&mtd_partitions_mutex);
-	list_for_each_entry(part, &mtd_partitions, list)
-		if (&part->mtd == mtd) {
-			ispart = 1;
-			break;
-		}
-	mutex_unlock(&mtd_partitions_mutex);
-
-	return ispart;
-}
-EXPORT_SYMBOL_GPL(mtd_is_partition);
-
 /* Returns the size of the entire flash chip */
 uint64_t mtd_get_device_size(const struct mtd_info *mtd)
 {
-	if (!mtd_is_partition(mtd))
-		return mtd->size;
+	struct mtd_info *master = mtd_get_master((struct mtd_info *)mtd);
 
-	return mtd_get_device_size(mtd_to_part(mtd)->parent);
+	return master->size;
 }
 EXPORT_SYMBOL_GPL(mtd_get_device_size);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 249e8d9bfbcd..508d14d45439 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -8,6 +8,7 @@
 
 #include <linux/types.h>
 #include <linux/uio.h>
+#include <linux/list.h>
 #include <linux/notifier.h>
 #include <linux/device.h>
 #include <linux/of.h>
@@ -194,10 +195,42 @@ struct mtd_debug_info {
 	const char *partid;
 };
 
+/**
+ * struct mtd_part - MTD partition specific fields
+ *
+ * @node: list node used to add an MTD partition to the parent partition list
+ * @offset: offset of the partition relatively to the parent offset
+ * @flags: original flags (before the mtdpart logic decided to tweak them based
+ *	   on flash constraints, like eraseblock/pagesize alignment)
+ *
+ * This struct is embedded in mtd_info and contains partition-specific
+ * properties/fields.
+ */
+struct mtd_part {
+	struct list_head node;
+	u64 offset;
+	u32 flags;
+};
+
+/**
+ * struct mtd_master - MTD master specific fields
+ *
+ * @partitions_lock: lock protecting accesses to the partition list. Protects
+ *		     not only the master partition list, but also all
+ *		     sub-partitions.
+ * @suspended: et to 1 when the device is suspended, 0 otherwise
+ *
+ * This struct is embedded in mtd_info and contains master-specific
+ * properties/fields.
+ */
+struct mtd_master {
+	struct mutex partitions_lock;
+	unsigned int suspended : 1;
+};
+
 struct mtd_info {
 	u_char type;
 	uint32_t flags;
-	uint32_t orig_flags; /* Flags as before running mtd checks */
 	uint64_t size;	 // Total size of the MTD
 
 	/* "Major" erase size for the device. Naïve users may take this
@@ -339,8 +372,50 @@ struct mtd_info {
 	int usecount;
 	struct mtd_debug_info dbg;
 	struct nvmem_device *nvmem;
+
+	/*
+	 * MTD masters do not have any parent, MTD partitions do. The parent
+	 * MTD device can itself be a partition.
+	 */
+	struct mtd_info *parent;
+
+	/* List of partitions attached to this MTD device */
+	struct list_head partitions;
+
+	union {
+		struct mtd_part part;
+		struct mtd_master master;
+	};
 };
 
+static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
+{
+	while (mtd->parent)
+		mtd = mtd->parent;
+
+	return mtd;
+}
+
+static inline u64 mtd_get_master_ofs(struct mtd_info *mtd, u64 ofs)
+{
+	while (mtd->parent) {
+		ofs += mtd->part.offset;
+		mtd = mtd->parent;
+	}
+
+	return ofs;
+}
+
+static inline bool mtd_is_partition(const struct mtd_info *mtd)
+{
+	return mtd->parent;
+}
+
+static inline bool mtd_has_partitions(const struct mtd_info *mtd)
+{
+	return !list_empty(&mtd->partitions);
+}
+
 int mtd_ooblayout_ecc(struct mtd_info *mtd, int section,
 		      struct mtd_oob_region *oobecc);
 int mtd_ooblayout_find_eccregion(struct mtd_info *mtd, int eccbyte,
@@ -392,13 +467,16 @@ static inline u32 mtd_oobavail(struct mtd_info *mtd, struct mtd_oob_ops *ops)
 static inline int mtd_max_bad_blocks(struct mtd_info *mtd,
 				     loff_t ofs, size_t len)
 {
-	if (!mtd->_max_bad_blocks)
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->_max_bad_blocks)
 		return -ENOTSUPP;
 
 	if (mtd->size < (len + ofs) || ofs < 0)
 		return -EINVAL;
 
-	return mtd->_max_bad_blocks(mtd, ofs, len);
+	return master->_max_bad_blocks(master, mtd_get_master_ofs(mtd, ofs),
+				       len);
 }
 
 int mtd_wunit_to_pairing_info(struct mtd_info *mtd, int wunit,
@@ -439,8 +517,10 @@ int mtd_writev(struct mtd_info *mtd, const struct kvec *vecs,
 
 static inline void mtd_sync(struct mtd_info *mtd)
 {
-	if (mtd->_sync)
-		mtd->_sync(mtd);
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (master->_sync)
+		master->_sync(master);
 }
 
 int mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
@@ -452,13 +532,31 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs);
 
 static inline int mtd_suspend(struct mtd_info *mtd)
 {
-	return mtd->_suspend ? mtd->_suspend(mtd) : 0;
+	struct mtd_info *master = mtd_get_master(mtd);
+	int ret;
+
+	if (master->master.suspended)
+		return 0;
+
+	ret = master->_suspend ? master->_suspend(master) : 0;
+	if (ret)
+		return ret;
+
+	master->master.suspended = 1;
+	return 0;
 }
 
 static inline void mtd_resume(struct mtd_info *mtd)
 {
-	if (mtd->_resume)
-		mtd->_resume(mtd);
+	struct mtd_info *master = mtd_get_master(mtd);
+
+	if (!master->master.suspended)
+		return;
+
+	if (master->_resume)
+		master->_resume(master);
+
+	master->master.suspended = 0;
 }
 
 static inline uint32_t mtd_div_by_eb(uint64_t sz, struct mtd_info *mtd)
@@ -538,7 +636,9 @@ static inline loff_t mtd_wunit_to_offset(struct mtd_info *mtd, loff_t base,
 
 static inline int mtd_has_oob(const struct mtd_info *mtd)
 {
-	return mtd->_read_oob && mtd->_write_oob;
+	struct mtd_info *master = mtd_get_master((struct mtd_info *)mtd);
+
+	return master->_read_oob && master->_write_oob;
 }
 
 static inline int mtd_type_is_nand(const struct mtd_info *mtd)
@@ -548,7 +648,9 @@ static inline int mtd_type_is_nand(const struct mtd_info *mtd)
 
 static inline int mtd_can_have_bb(const struct mtd_info *mtd)
 {
-	return !!mtd->_block_isbad;
+	struct mtd_info *master = mtd_get_master((struct mtd_info *)mtd);
+
+	return !!master->_block_isbad;
 }
 
 	/* Kernel-side ioctl definitions */
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 11cb0c50cd84..e545c050d3e8 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -105,7 +105,6 @@ extern void deregister_mtd_parser(struct mtd_part_parser *parser);
 	module_driver(__mtd_part_parser, register_mtd_parser, \
 		      deregister_mtd_parser)
 
-int mtd_is_partition(const struct mtd_info *mtd);
 int mtd_add_partition(struct mtd_info *master, const char *name,
 		      long long offset, long long length);
 int mtd_del_partition(struct mtd_info *master, int partno);
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2019-12-30 11:19 [PATCH v2] mtd: implement proper partition handling Miquel Raynal
@ 2020-01-08 23:34 ` Richard Weinberger
  2020-01-09 18:13   ` Miquel Raynal
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Weinberger @ 2020-01-08 23:34 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Boris Brezillon, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

----- Ursprüngliche Mail -----
> Von: "Miquel Raynal" <miquel.raynal@bootlin.com>
> An: "richard" <richard@nod.at>, "Vignesh Raghavendra" <vigneshr@ti.com>, "Tudor Ambarus" <Tudor.Ambarus@microchip.com>,
> "linux-mtd" <linux-mtd@lists.infradead.org>
> CC: "Boris Brezillon" <boris.brezillon@collabora.com>, "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>, "Miquel
> Raynal" <miquel.raynal@bootlin.com>
> Gesendet: Montag, 30. Dezember 2019 12:19:48
> Betreff: [PATCH v2] mtd: implement proper partition handling

> Instead of collecting partitions in a flat list, create a hierarchy
> within the mtd_info structure: use a partitions list to keep track of
> the partitions of an MTD device (which might be itself a partition of
> another MTD device), a pointer to the parent device (NULL when the MTD
> device is the root one, not a partition).

What problem does this solve?
...beside of a nice diffstat which removes more than it adds. :-)

> By also saving directly in mtd_info the offset of the partition, we
> can get rid of the mtd_part structure.
> 
> While at it, be consistent in the naming of the mtd_info structures to
> ease the understanding of the new hierarchy: these structures are
> usually called 'mtd', unless there are multiple instances of the same
> structure. In this case, there is usually a parent/child bound so we
> will call them 'parent' and 'child'.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

[...]

> +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> +{
> +	while (mtd->parent)
> +		mtd = mtd->parent;
> +
> +	return mtd;
> +}

So, parent == master?

When I create a MTD ontop of UBI using gluebi, who will be parent/master?

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2020-01-08 23:34 ` Richard Weinberger
@ 2020-01-09 18:13   ` Miquel Raynal
  2020-01-09 18:43     ` Richard Weinberger
  0 siblings, 1 reply; 11+ messages in thread
From: Miquel Raynal @ 2020-01-09 18:13 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Boris Brezillon, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

Hi Richard,

Richard Weinberger <richard@nod.at> wrote on Thu, 9 Jan 2020 00:34:18
+0100 (CET):

> ----- Ursprüngliche Mail -----
> > Von: "Miquel Raynal" <miquel.raynal@bootlin.com>
> > An: "richard" <richard@nod.at>, "Vignesh Raghavendra" <vigneshr@ti.com>, "Tudor Ambarus" <Tudor.Ambarus@microchip.com>,
> > "linux-mtd" <linux-mtd@lists.infradead.org>
> > CC: "Boris Brezillon" <boris.brezillon@collabora.com>, "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>, "Miquel
> > Raynal" <miquel.raynal@bootlin.com>
> > Gesendet: Montag, 30. Dezember 2019 12:19:48
> > Betreff: [PATCH v2] mtd: implement proper partition handling  
> 
> > Instead of collecting partitions in a flat list, create a hierarchy
> > within the mtd_info structure: use a partitions list to keep track of
> > the partitions of an MTD device (which might be itself a partition of
> > another MTD device), a pointer to the parent device (NULL when the MTD
> > device is the root one, not a partition).  
> 
> What problem does this solve?
> ...beside of a nice diffstat which removes more than it adds. :-)

It is much easier to escalade to the top most "master" device when
there are multiple levels of partitioning, which was not cleanly
described IMHO. Also it is already used in the MLC-in-pseudo-SLC-mode
series :)

> 
> > By also saving directly in mtd_info the offset of the partition, we
> > can get rid of the mtd_part structure.
> > 
> > While at it, be consistent in the naming of the mtd_info structures to
> > ease the understanding of the new hierarchy: these structures are
> > usually called 'mtd', unless there are multiple instances of the same
> > structure. In this case, there is usually a parent/child bound so we
> > will call them 'parent' and 'child'.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> 
> [...]
> 
> > +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> > +{
> > +	while (mtd->parent)
> > +		mtd = mtd->parent;
> > +
> > +	return mtd;
> > +}  
> 
> So, parent == master?

top most parent (the one without parent) == master !

> 
> When I create a MTD ontop of UBI using gluebi, who will be parent/master?

I don't really understand the issue here?

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2020-01-09 18:13   ` Miquel Raynal
@ 2020-01-09 18:43     ` Richard Weinberger
  2020-01-09 18:45       ` Miquel Raynal
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Weinberger @ 2020-01-09 18:43 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Boris Brezillon, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

Miquel,

----- Ursprüngliche Mail -----
>> What problem does this solve?
>> ...beside of a nice diffstat which removes more than it adds. :-)
> 
> It is much easier to escalade to the top most "master" device when
> there are multiple levels of partitioning, which was not cleanly
> described IMHO. Also it is already used in the MLC-in-pseudo-SLC-mode
> series :)

Ok. In fact I "found" this patch my looking at the SLC emulation patches.

>> > +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
>> > +{
>> > +	while (mtd->parent)
>> > +		mtd = mtd->parent;
>> > +
>> > +	return mtd;
>> > +}
>> 
>> So, parent == master?
> 
> top most parent (the one without parent) == master !
> 
>> 
>> When I create a MTD ontop of UBI using gluebi, who will be parent/master?
> 
> I don't really understand the issue here?

Let's say I have mtd0 with an ubi and a volume "xxx". After enabling
gluebi a new mtd1 will arrive on the system.
The stacking is mtd0 -> ubi (volume xxx) -> mtd1.
Is now a relationship between mtd1 and mtd0?

I'd expect mtd1's parent being mtd0.

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2020-01-09 18:43     ` Richard Weinberger
@ 2020-01-09 18:45       ` Miquel Raynal
  2020-01-09 19:13         ` Boris Brezillon
  0 siblings, 1 reply; 11+ messages in thread
From: Miquel Raynal @ 2020-01-09 18:45 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Boris Brezillon, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

Hi Richard,

Richard Weinberger <richard@nod.at> wrote on Thu, 9 Jan 2020 19:43:04
+0100 (CET):

> Miquel,
> 
> ----- Ursprüngliche Mail -----
> >> What problem does this solve?
> >> ...beside of a nice diffstat which removes more than it adds. :-)  
> > 
> > It is much easier to escalade to the top most "master" device when
> > there are multiple levels of partitioning, which was not cleanly
> > described IMHO. Also it is already used in the MLC-in-pseudo-SLC-mode
> > series :)  
> 
> Ok. In fact I "found" this patch my looking at the SLC emulation patches.
> 
> >> > +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> >> > +{
> >> > +	while (mtd->parent)
> >> > +		mtd = mtd->parent;
> >> > +
> >> > +	return mtd;
> >> > +}  
> >> 
> >> So, parent == master?  
> > 
> > top most parent (the one without parent) == master !
> >   
> >> 
> >> When I create a MTD ontop of UBI using gluebi, who will be parent/master?  
> > 
> > I don't really understand the issue here?  
> 
> Let's say I have mtd0 with an ubi and a volume "xxx". After enabling
> gluebi a new mtd1 will arrive on the system.
> The stacking is mtd0 -> ubi (volume xxx) -> mtd1.

This is much clearer, thanks!

> Is now a relationship between mtd1 and mtd0?

No there is none. 

> I'd expect mtd1's parent being mtd0.

This would be a new feature, right? I don't think it is the case today.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2020-01-09 18:45       ` Miquel Raynal
@ 2020-01-09 19:13         ` Boris Brezillon
  2020-01-09 19:23           ` Miquel Raynal
  0 siblings, 1 reply; 11+ messages in thread
From: Boris Brezillon @ 2020-01-09 19:13 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

On Thu, 9 Jan 2020 19:45:56 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Richard,
> 
> Richard Weinberger <richard@nod.at> wrote on Thu, 9 Jan 2020 19:43:04
> +0100 (CET):
> 
> > Miquel,
> > 
> > ----- Ursprüngliche Mail -----  
> > >> What problem does this solve?
> > >> ...beside of a nice diffstat which removes more than it adds. :-)    
> > > 
> > > It is much easier to escalade to the top most "master" device when
> > > there are multiple levels of partitioning, which was not cleanly
> > > described IMHO. Also it is already used in the MLC-in-pseudo-SLC-mode
> > > series :)    
> > 
> > Ok. In fact I "found" this patch my looking at the SLC emulation patches.
> >   
> > >> > +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> > >> > +{
> > >> > +	while (mtd->parent)
> > >> > +		mtd = mtd->parent;
> > >> > +
> > >> > +	return mtd;
> > >> > +}    
> > >> 
> > >> So, parent == master?    
> > > 
> > > top most parent (the one without parent) == master !
> > >     
> > >> 
> > >> When I create a MTD ontop of UBI using gluebi, who will be parent/master?    
> > > 
> > > I don't really understand the issue here?    
> > 
> > Let's say I have mtd0 with an ubi and a volume "xxx". After enabling
> > gluebi a new mtd1 will arrive on the system.
> > The stacking is mtd0 -> ubi (volume xxx) -> mtd1.  
> 
> This is much clearer, thanks!
> 
> > Is now a relationship between mtd1 and mtd0?  
> 
> No there is none. 
> 
> > I'd expect mtd1's parent being mtd0.  
> 
> This would be a new feature, right? I don't think it is the case today.

We definitely don't want mtd1 to appear as a partition of mtd0 in that
case (blocks in mtd1 can't be mapped to blocks in mtd0 without the UBI
layer being involved). Maybe it'd be clearer if we move the parent
field to mtd_part and add an MTD_IS_PARTITION flag. Or maybe we can
just choose a better name.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2020-01-09 19:13         ` Boris Brezillon
@ 2020-01-09 19:23           ` Miquel Raynal
  2020-01-09 19:37             ` Boris Brezillon
  0 siblings, 1 reply; 11+ messages in thread
From: Miquel Raynal @ 2020-01-09 19:23 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Richard Weinberger, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 9 Jan
2020 20:13:55 +0100:

> On Thu, 9 Jan 2020 19:45:56 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Richard,
> > 
> > Richard Weinberger <richard@nod.at> wrote on Thu, 9 Jan 2020 19:43:04
> > +0100 (CET):
> >   
> > > Miquel,
> > > 
> > > ----- Ursprüngliche Mail -----    
> > > >> What problem does this solve?
> > > >> ...beside of a nice diffstat which removes more than it adds. :-)      
> > > > 
> > > > It is much easier to escalade to the top most "master" device when
> > > > there are multiple levels of partitioning, which was not cleanly
> > > > described IMHO. Also it is already used in the MLC-in-pseudo-SLC-mode
> > > > series :)      
> > > 
> > > Ok. In fact I "found" this patch my looking at the SLC emulation patches.
> > >     
> > > >> > +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> > > >> > +{
> > > >> > +	while (mtd->parent)
> > > >> > +		mtd = mtd->parent;
> > > >> > +
> > > >> > +	return mtd;
> > > >> > +}      
> > > >> 
> > > >> So, parent == master?      
> > > > 
> > > > top most parent (the one without parent) == master !
> > > >       
> > > >> 
> > > >> When I create a MTD ontop of UBI using gluebi, who will be parent/master?      
> > > > 
> > > > I don't really understand the issue here?      
> > > 
> > > Let's say I have mtd0 with an ubi and a volume "xxx". After enabling
> > > gluebi a new mtd1 will arrive on the system.
> > > The stacking is mtd0 -> ubi (volume xxx) -> mtd1.    
> > 
> > This is much clearer, thanks!
> >   
> > > Is now a relationship between mtd1 and mtd0?    
> > 
> > No there is none. 
> >   
> > > I'd expect mtd1's parent being mtd0.    
> > 
> > This would be a new feature, right? I don't think it is the case today.  
> 
> We definitely don't want mtd1 to appear as a partition of mtd0 in that
> case (blocks in mtd1 can't be mapped to blocks in mtd0 without the UBI
> layer being involved). Maybe it'd be clearer if we move the parent
> field to mtd_part and add an MTD_IS_PARTITION flag. Or maybe we can
> just choose a better name.

I prefer the name change. I think the current struct organization
is right. What do you suggest?


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2020-01-09 19:23           ` Miquel Raynal
@ 2020-01-09 19:37             ` Boris Brezillon
  2020-01-10  8:28               ` Miquel Raynal
  0 siblings, 1 reply; 11+ messages in thread
From: Boris Brezillon @ 2020-01-09 19:37 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

On Thu, 9 Jan 2020 20:23:58 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 9 Jan
> 2020 20:13:55 +0100:
> 
> > On Thu, 9 Jan 2020 19:45:56 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Hi Richard,
> > > 
> > > Richard Weinberger <richard@nod.at> wrote on Thu, 9 Jan 2020 19:43:04
> > > +0100 (CET):
> > >     
> > > > Miquel,
> > > > 
> > > > ----- Ursprüngliche Mail -----      
> > > > >> What problem does this solve?
> > > > >> ...beside of a nice diffstat which removes more than it adds. :-)        
> > > > > 
> > > > > It is much easier to escalade to the top most "master" device when
> > > > > there are multiple levels of partitioning, which was not cleanly
> > > > > described IMHO. Also it is already used in the MLC-in-pseudo-SLC-mode
> > > > > series :)        
> > > > 
> > > > Ok. In fact I "found" this patch my looking at the SLC emulation patches.
> > > >       
> > > > >> > +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> > > > >> > +{
> > > > >> > +	while (mtd->parent)
> > > > >> > +		mtd = mtd->parent;
> > > > >> > +
> > > > >> > +	return mtd;
> > > > >> > +}        
> > > > >> 
> > > > >> So, parent == master?        
> > > > > 
> > > > > top most parent (the one without parent) == master !
> > > > >         
> > > > >> 
> > > > >> When I create a MTD ontop of UBI using gluebi, who will be parent/master?        
> > > > > 
> > > > > I don't really understand the issue here?        
> > > > 
> > > > Let's say I have mtd0 with an ubi and a volume "xxx". After enabling
> > > > gluebi a new mtd1 will arrive on the system.
> > > > The stacking is mtd0 -> ubi (volume xxx) -> mtd1.      
> > > 
> > > This is much clearer, thanks!
> > >     
> > > > Is now a relationship between mtd1 and mtd0?      
> > > 
> > > No there is none. 
> > >     
> > > > I'd expect mtd1's parent being mtd0.      
> > > 
> > > This would be a new feature, right? I don't think it is the case today.    
> > 
> > We definitely don't want mtd1 to appear as a partition of mtd0 in that
> > case (blocks in mtd1 can't be mapped to blocks in mtd0 without the UBI
> > layer being involved). Maybe it'd be clearer if we move the parent
> > field to mtd_part and add an MTD_IS_PARTITION flag. Or maybe we can
> > just choose a better name.  
> 
> I prefer the name change. I think the current struct organization
> is right. What do you suggest?

I don't have a better name, sorry.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2020-01-09 19:37             ` Boris Brezillon
@ 2020-01-10  8:28               ` Miquel Raynal
  2020-01-10  9:46                 ` Boris Brezillon
  0 siblings, 1 reply; 11+ messages in thread
From: Miquel Raynal @ 2020-01-10  8:28 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Richard Weinberger, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 9 Jan
2020 20:37:22 +0100:

> On Thu, 9 Jan 2020 20:23:58 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 9 Jan
> > 2020 20:13:55 +0100:
> >   
> > > On Thu, 9 Jan 2020 19:45:56 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > Hi Richard,
> > > > 
> > > > Richard Weinberger <richard@nod.at> wrote on Thu, 9 Jan 2020 19:43:04
> > > > +0100 (CET):
> > > >       
> > > > > Miquel,
> > > > > 
> > > > > ----- Ursprüngliche Mail -----        
> > > > > >> What problem does this solve?
> > > > > >> ...beside of a nice diffstat which removes more than it adds. :-)          
> > > > > > 
> > > > > > It is much easier to escalade to the top most "master" device when
> > > > > > there are multiple levels of partitioning, which was not cleanly
> > > > > > described IMHO. Also it is already used in the MLC-in-pseudo-SLC-mode
> > > > > > series :)          
> > > > > 
> > > > > Ok. In fact I "found" this patch my looking at the SLC emulation patches.
> > > > >         
> > > > > >> > +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> > > > > >> > +{
> > > > > >> > +	while (mtd->parent)
> > > > > >> > +		mtd = mtd->parent;
> > > > > >> > +
> > > > > >> > +	return mtd;
> > > > > >> > +}          
> > > > > >> 
> > > > > >> So, parent == master?          
> > > > > > 
> > > > > > top most parent (the one without parent) == master !
> > > > > >           
> > > > > >> 
> > > > > >> When I create a MTD ontop of UBI using gluebi, who will be parent/master?          
> > > > > > 
> > > > > > I don't really understand the issue here?          
> > > > > 
> > > > > Let's say I have mtd0 with an ubi and a volume "xxx". After enabling
> > > > > gluebi a new mtd1 will arrive on the system.
> > > > > The stacking is mtd0 -> ubi (volume xxx) -> mtd1.        
> > > > 
> > > > This is much clearer, thanks!
> > > >       
> > > > > Is now a relationship between mtd1 and mtd0?        
> > > > 
> > > > No there is none. 
> > > >       
> > > > > I'd expect mtd1's parent being mtd0.        
> > > > 
> > > > This would be a new feature, right? I don't think it is the case today.      
> > > 
> > > We definitely don't want mtd1 to appear as a partition of mtd0 in that
> > > case (blocks in mtd1 can't be mapped to blocks in mtd0 without the UBI
> > > layer being involved). Maybe it'd be clearer if we move the parent
> > > field to mtd_part and add an MTD_IS_PARTITION flag. Or maybe we can
> > > just choose a better name.    
> > 
> > I prefer the name change. I think the current struct organization
> > is right. What do you suggest?  
> 
> I don't have a better name, sorry.

Actually I find ->parent totally descriptive, and in Richard's example,
I would not call mtd1 as mtd0's parent, it's more like a "top virtual
device" but certainly not a "direct" parent.

mtd->direct_parent would work but I think it is a bit too long and most
of the people would not understand why we call it this way, instead of
just "parent".

I would like to take this patch into 5.6, so please tell me what you
prefer ("parent" being the most straightforward and simple solution to
me :) )

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2020-01-10  8:28               ` Miquel Raynal
@ 2020-01-10  9:46                 ` Boris Brezillon
  2020-01-10  9:55                   ` Miquel Raynal
  0 siblings, 1 reply; 11+ messages in thread
From: Boris Brezillon @ 2020-01-10  9:46 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

On Fri, 10 Jan 2020 09:28:55 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 9 Jan
> 2020 20:37:22 +0100:
> 
> > On Thu, 9 Jan 2020 20:23:58 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Hi Boris,
> > > 
> > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 9 Jan
> > > 2020 20:13:55 +0100:
> > >     
> > > > On Thu, 9 Jan 2020 19:45:56 +0100
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >       
> > > > > Hi Richard,
> > > > > 
> > > > > Richard Weinberger <richard@nod.at> wrote on Thu, 9 Jan 2020 19:43:04
> > > > > +0100 (CET):
> > > > >         
> > > > > > Miquel,
> > > > > > 
> > > > > > ----- Ursprüngliche Mail -----          
> > > > > > >> What problem does this solve?
> > > > > > >> ...beside of a nice diffstat which removes more than it adds. :-)            
> > > > > > > 
> > > > > > > It is much easier to escalade to the top most "master" device when
> > > > > > > there are multiple levels of partitioning, which was not cleanly
> > > > > > > described IMHO. Also it is already used in the MLC-in-pseudo-SLC-mode
> > > > > > > series :)            
> > > > > > 
> > > > > > Ok. In fact I "found" this patch my looking at the SLC emulation patches.
> > > > > >           
> > > > > > >> > +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> > > > > > >> > +{
> > > > > > >> > +	while (mtd->parent)
> > > > > > >> > +		mtd = mtd->parent;
> > > > > > >> > +
> > > > > > >> > +	return mtd;
> > > > > > >> > +}            
> > > > > > >> 
> > > > > > >> So, parent == master?            
> > > > > > > 
> > > > > > > top most parent (the one without parent) == master !
> > > > > > >             
> > > > > > >> 
> > > > > > >> When I create a MTD ontop of UBI using gluebi, who will be parent/master?            
> > > > > > > 
> > > > > > > I don't really understand the issue here?            
> > > > > > 
> > > > > > Let's say I have mtd0 with an ubi and a volume "xxx". After enabling
> > > > > > gluebi a new mtd1 will arrive on the system.
> > > > > > The stacking is mtd0 -> ubi (volume xxx) -> mtd1.          
> > > > > 
> > > > > This is much clearer, thanks!
> > > > >         
> > > > > > Is now a relationship between mtd1 and mtd0?          
> > > > > 
> > > > > No there is none. 
> > > > >         
> > > > > > I'd expect mtd1's parent being mtd0.          
> > > > > 
> > > > > This would be a new feature, right? I don't think it is the case today.        
> > > > 
> > > > We definitely don't want mtd1 to appear as a partition of mtd0 in that
> > > > case (blocks in mtd1 can't be mapped to blocks in mtd0 without the UBI
> > > > layer being involved). Maybe it'd be clearer if we move the parent
> > > > field to mtd_part and add an MTD_IS_PARTITION flag. Or maybe we can
> > > > just choose a better name.      
> > > 
> > > I prefer the name change. I think the current struct organization
> > > is right. What do you suggest?    
> > 
> > I don't have a better name, sorry.  
> 
> Actually I find ->parent totally descriptive, and in Richard's example,
> I would not call mtd1 as mtd0's parent, it's more like a "top virtual
> device" but certainly not a "direct" parent.

It depends what kind of parenting we're talking about :P. From the
device model perspective, mtd1's parent is ubi0. From the MTD partition
perspective, mtd1 has no parent (it's a master MTD device). The problem
here is that 'parent' is vague enough that it lets people think it's
representing more than a partition -> partition_parent relationship,
hence my suggestion to move the field to mtd_part so it can be
namespaced in mtd->part.parent.

> 
> mtd->direct_parent would work but I think it is a bit too long and most
> of the people would not understand why we call it this way, instead of
> just "parent".

->partition_parent would be more accurate, but I agree with you, I'm
not a big fan of those long names. Maybe ->container?

> 
> I would like to take this patch into 5.6, so please tell me what you
> prefer ("parent" being the most straightforward and simple solution to
> me :) )

I think the most important thing here is documenting what this field
is representing so people can refer to the doc if they get confused.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mtd: implement proper partition handling
  2020-01-10  9:46                 ` Boris Brezillon
@ 2020-01-10  9:55                   ` Miquel Raynal
  0 siblings, 0 replies; 11+ messages in thread
From: Miquel Raynal @ 2020-01-10  9:55 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Richard Weinberger, linux-mtd, Vignesh Raghavendra,
	Thomas Petazzoni, Tudor Ambarus

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Fri, 10 Jan
2020 10:46:26 +0100:

> On Fri, 10 Jan 2020 09:28:55 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 9 Jan
> > 2020 20:37:22 +0100:
> >   
> > > On Thu, 9 Jan 2020 20:23:58 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > Hi Boris,
> > > > 
> > > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 9 Jan
> > > > 2020 20:13:55 +0100:
> > > >       
> > > > > On Thu, 9 Jan 2020 19:45:56 +0100
> > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > >         
> > > > > > Hi Richard,
> > > > > > 
> > > > > > Richard Weinberger <richard@nod.at> wrote on Thu, 9 Jan 2020 19:43:04
> > > > > > +0100 (CET):
> > > > > >           
> > > > > > > Miquel,
> > > > > > > 
> > > > > > > ----- Ursprüngliche Mail -----            
> > > > > > > >> What problem does this solve?
> > > > > > > >> ...beside of a nice diffstat which removes more than it adds. :-)              
> > > > > > > > 
> > > > > > > > It is much easier to escalade to the top most "master" device when
> > > > > > > > there are multiple levels of partitioning, which was not cleanly
> > > > > > > > described IMHO. Also it is already used in the MLC-in-pseudo-SLC-mode
> > > > > > > > series :)              
> > > > > > > 
> > > > > > > Ok. In fact I "found" this patch my looking at the SLC emulation patches.
> > > > > > >             
> > > > > > > >> > +static inline struct mtd_info *mtd_get_master(struct mtd_info *mtd)
> > > > > > > >> > +{
> > > > > > > >> > +	while (mtd->parent)
> > > > > > > >> > +		mtd = mtd->parent;
> > > > > > > >> > +
> > > > > > > >> > +	return mtd;
> > > > > > > >> > +}              
> > > > > > > >> 
> > > > > > > >> So, parent == master?              
> > > > > > > > 
> > > > > > > > top most parent (the one without parent) == master !
> > > > > > > >               
> > > > > > > >> 
> > > > > > > >> When I create a MTD ontop of UBI using gluebi, who will be parent/master?              
> > > > > > > > 
> > > > > > > > I don't really understand the issue here?              
> > > > > > > 
> > > > > > > Let's say I have mtd0 with an ubi and a volume "xxx". After enabling
> > > > > > > gluebi a new mtd1 will arrive on the system.
> > > > > > > The stacking is mtd0 -> ubi (volume xxx) -> mtd1.            
> > > > > > 
> > > > > > This is much clearer, thanks!
> > > > > >           
> > > > > > > Is now a relationship between mtd1 and mtd0?            
> > > > > > 
> > > > > > No there is none. 
> > > > > >           
> > > > > > > I'd expect mtd1's parent being mtd0.            
> > > > > > 
> > > > > > This would be a new feature, right? I don't think it is the case today.          
> > > > > 
> > > > > We definitely don't want mtd1 to appear as a partition of mtd0 in that
> > > > > case (blocks in mtd1 can't be mapped to blocks in mtd0 without the UBI
> > > > > layer being involved). Maybe it'd be clearer if we move the parent
> > > > > field to mtd_part and add an MTD_IS_PARTITION flag. Or maybe we can
> > > > > just choose a better name.        
> > > > 
> > > > I prefer the name change. I think the current struct organization
> > > > is right. What do you suggest?      
> > > 
> > > I don't have a better name, sorry.    
> > 
> > Actually I find ->parent totally descriptive, and in Richard's example,
> > I would not call mtd1 as mtd0's parent, it's more like a "top virtual
> > device" but certainly not a "direct" parent.  
> 
> It depends what kind of parenting we're talking about :P. From the
> device model perspective, mtd1's parent is ubi0. From the MTD partition
> perspective, mtd1 has no parent (it's a master MTD device). The problem
> here is that 'parent' is vague enough that it lets people think it's
> representing more than a partition -> partition_parent relationship,
> hence my suggestion to move the field to mtd_part so it can be
> namespaced in mtd->part.parent.

I think only Richard got confused :p

> 
> > 
> > mtd->direct_parent would work but I think it is a bit too long and most
> > of the people would not understand why we call it this way, instead of
> > just "parent".  
> 
> ->partition_parent would be more accurate, but I agree with you, I'm  
> not a big fan of those long names. Maybe ->container?

I don't like ->container so much as for me it would refer to this
"virtual parent" I was talking about, ie. UBI! I think you are
right saying that the most important thing is to be clear in the
documentation (I'll try to explain better) because any word can
refer to almost any concept depending on the person reading it.

> > 
> > I would like to take this patch into 5.6, so please tell me what you
> > prefer ("parent" being the most straightforward and simple solution to
> > me :) )  
> 
> I think the most important thing here is documenting what this field
> is representing so people can refer to the doc if they get confused.

Yes, I will stick to ->parent. As we are in the MTD structure, pointing
the direct MTD parent seems logic to me, but I will explain in the kdoc
that it is the parent from the MTD partitioning scheme point of view,
and this is not related to the device-model hierarchy at all.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-01-10  9:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-30 11:19 [PATCH v2] mtd: implement proper partition handling Miquel Raynal
2020-01-08 23:34 ` Richard Weinberger
2020-01-09 18:13   ` Miquel Raynal
2020-01-09 18:43     ` Richard Weinberger
2020-01-09 18:45       ` Miquel Raynal
2020-01-09 19:13         ` Boris Brezillon
2020-01-09 19:23           ` Miquel Raynal
2020-01-09 19:37             ` Boris Brezillon
2020-01-10  8:28               ` Miquel Raynal
2020-01-10  9:46                 ` Boris Brezillon
2020-01-10  9:55                   ` 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).