* Re: [PATCH v3 2/2] mtd: rework partitions handling
[not found] ` <20190117152929.28256-2-miquel.raynal@bootlin.com>
@ 2019-01-17 20:29 ` Boris Brezillon
2019-01-22 11:12 ` Miquel Raynal
0 siblings, 1 reply; 4+ messages in thread
From: Boris Brezillon @ 2019-01-17 20:29 UTC (permalink / raw)
To: Miquel Raynal
Cc: Tudor Ambarus, Richard Weinberger, Marek Vasut, linux-mtd,
Brian Norris, David Woodhouse
On Thu, 17 Jan 2019 16:29:29 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> @@ -600,6 +601,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;
> @@ -611,9 +613,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;
> -
Keep this blank line.
> ops.mode = req.mode;
> ops.len = (size_t)req.len;
> ops.ooblen = (size_t)req.ooblen;
> @@ -936,20 +942,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);
^ master
I think I mentioned that one in my previous review ;-).
>
> 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);
...
> 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_offset(mtd, ofs));
> + if (ret)
> + return ret;
> +
> + while (mtd->parent) {
> + mtd->ecc_stats.badblocks++;
> + mtd = mtd->parent;
> + }
Not exactly related to this patch, but if we mark a block bad on the
master device this information is not propagated to parts exposing this
block.
> +
> + return 0;
> }
> EXPORT_SYMBOL_GPL(mtd_block_markbad);
...
>
> /*
> * 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_locked(struct mtd_info *mtd)
Should be static as the only users are in mtdpart.c.
> +{
> + struct mtd_info *child, *next;
> + int ret, err = 0;
> +
> + list_for_each_entry_safe(child, next, &mtd->partitions,
> + props.part.node) {
> + if (mtd_has_partitions(child))
> + del_mtd_partitions_locked(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->props.part.node);
> + free_partition(child);
> + }
> +
> + return err;
> +}
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 1/2] mtd: partitions: clarify __mtd_del_partition() name
[not found] <20190117152929.28256-1-miquel.raynal@bootlin.com>
[not found] ` <20190117152929.28256-2-miquel.raynal@bootlin.com>
@ 2019-01-21 10:03 ` Boris Brezillon
2019-01-22 11:01 ` Miquel Raynal
1 sibling, 1 reply; 4+ messages in thread
From: Boris Brezillon @ 2019-01-21 10:03 UTC (permalink / raw)
To: Miquel Raynal
Cc: Tudor Ambarus, Richard Weinberger, Marek Vasut, linux-mtd,
Brian Norris, David Woodhouse
On Thu, 17 Jan 2019 16:29:28 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> __mtd_del_partition() is a helper that must be called after the
> partition lock has ben taken. As there are already a few similar names
^been
> (mtd_del_partitions(), del_mtd_partitions(), __mtd_del_partitions()),
> make this clear by renaming the helper mtd_del_partition_locked().
That's indeed a good step forward, but we still have all those variants
using sightly different names (mtd_<action>() and <action>_mtd()).
Would be great if we could make the names more explicit for those
functions too (or remove those that are unneeded).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/mtdpart.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index b6af41b04622..32beb9c2641f 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -627,20 +627,20 @@ int mtd_add_partition(struct mtd_info *parent, const char *name,
> EXPORT_SYMBOL_GPL(mtd_add_partition);
>
> /**
> - * __mtd_del_partition - delete MTD partition
> + * mtd_del_partition_locked - delete MTD partition
> *
> * @priv: internal MTD struct for partition 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_locked(struct mtd_part *priv)
> {
> struct mtd_part *child, *next;
> int err;
>
> list_for_each_entry_safe(child, next, &mtd_partitions, list) {
> if (child->parent == &priv->mtd) {
> - err = __mtd_del_partition(child);
> + err = mtd_del_partition_locked(child);
> if (err)
> return err;
> }
> @@ -670,7 +670,7 @@ int del_mtd_partitions(struct mtd_info *mtd)
> mutex_lock(&mtd_partitions_mutex);
> list_for_each_entry_safe(slave, next, &mtd_partitions, list)
> if (slave->parent == mtd) {
> - ret = __mtd_del_partition(slave);
> + ret = mtd_del_partition_locked(slave);
> if (ret < 0)
> err = ret;
> }
> @@ -688,7 +688,7 @@ int mtd_del_partition(struct mtd_info *mtd, int partno)
> list_for_each_entry_safe(slave, next, &mtd_partitions, list)
> if ((slave->parent == mtd) &&
> (slave->mtd.index == partno)) {
> - ret = __mtd_del_partition(slave);
> + ret = mtd_del_partition_locked(slave);
> break;
> }
> mutex_unlock(&mtd_partitions_mutex);
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 1/2] mtd: partitions: clarify __mtd_del_partition() name
2019-01-21 10:03 ` [PATCH v3 1/2] mtd: partitions: clarify __mtd_del_partition() name Boris Brezillon
@ 2019-01-22 11:01 ` Miquel Raynal
0 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2019-01-22 11:01 UTC (permalink / raw)
To: Boris Brezillon
Cc: Tudor Ambarus, Richard Weinberger, Marek Vasut, linux-mtd,
Brian Norris, David Woodhouse
Hi Boris,
Boris Brezillon <bbrezillon@kernel.org> wrote on Mon, 21 Jan 2019
11:03:24 +0100:
> On Thu, 17 Jan 2019 16:29:28 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > __mtd_del_partition() is a helper that must be called after the
> > partition lock has ben taken. As there are already a few similar names
>
> ^been
Will correct it.
>
> > (mtd_del_partitions(), del_mtd_partitions(), __mtd_del_partitions()),
> > make this clear by renaming the helper mtd_del_partition_locked().
>
> That's indeed a good step forward, but we still have all those variants
> using sightly different names (mtd_<action>() and <action>_mtd()).
> Would be great if we could make the names more explicit for those
> functions too (or remove those that are unneeded).
I truly agree. I actually tried but this is much more time consuming
than expected so I will let this change aside for now but I would
really like these helpers to be merged.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 2/2] mtd: rework partitions handling
2019-01-17 20:29 ` [PATCH v3 2/2] mtd: rework partitions handling Boris Brezillon
@ 2019-01-22 11:12 ` Miquel Raynal
0 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2019-01-22 11:12 UTC (permalink / raw)
To: Boris Brezillon
Cc: Tudor Ambarus, Richard Weinberger, Marek Vasut, linux-mtd,
Brian Norris, David Woodhouse
Hi Boris,
> > 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_offset(mtd, ofs));
> > + if (ret)
> > + return ret;
> > +
> > + while (mtd->parent) {
> > + mtd->ecc_stats.badblocks++;
> > + mtd = mtd->parent;
> > + }
>
> Not exactly related to this patch, but if we mark a block bad on the
> master device this information is not propagated to parts exposing this
> block.
No but is this important? It is very quick to get up to the master
device so the penalty for doing that is almost non-existent.
Other comments addressed, thanks for the review.
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-22 11:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190117152929.28256-1-miquel.raynal@bootlin.com>
[not found] ` <20190117152929.28256-2-miquel.raynal@bootlin.com>
2019-01-17 20:29 ` [PATCH v3 2/2] mtd: rework partitions handling Boris Brezillon
2019-01-22 11:12 ` Miquel Raynal
2019-01-21 10:03 ` [PATCH v3 1/2] mtd: partitions: clarify __mtd_del_partition() name Boris Brezillon
2019-01-22 11:01 ` 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).