Re: [PATCH v3 2/2] mtd: rework partitions handling

[Boris Brezillon <bbrezillon@xxxxxxxxxx>]

On Thu, 17 Jan 2019 16:29:29 +0100
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> 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/