Re: [PATCH 2/2] mtd: get rid of the mtd_add_device_partitions function

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Fri, 12 Jan 2018 15:40:34 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This simplifies code a bit by:
> 1) Avoiding an extra (tiny) function
> 2) Checking for amount of parsed (found) partitions just once
> 3) Avoiding clearing/filling struct mtd_partitions manually
> 
> With this commit a proper functions are called directly from the
> mtd_device_parse_register. It doesn't need to use minor tricks like
> memsetting struct to 0 to trigger an expected mtd_add_device_partitions
> behavior.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/mtd/mtdcore.c | 29 +++++++----------------------
>  1 file changed, 7 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index f6460862e2ad..0a414750bc8b 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -636,21 +636,6 @@ int del_mtd_device(struct mtd_info *mtd)
>  	return ret;
>  }
>  
> -static int mtd_add_device_partitions(struct mtd_info *mtd,
> -				     struct mtd_partitions *parts)
> -{
> -	const struct mtd_partition *real_parts = parts->parts;
> -	int nbparts = parts->nr_parts;
> -
> -	if (nbparts == 0 && !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
> -		return add_mtd_device(mtd);
> -
> -	if (nbparts > 0)
> -		return add_mtd_partitions(mtd, real_parts, nbparts);
> -
> -	return 0;
> -}
> -
>  /*
>   * Set a few defaults based on the parent devices, if not provided by the
>   * driver
> @@ -717,19 +702,19 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
>  	ret = parse_mtd_partitions(mtd, types, &parsed, parser_data);
>  	if ((ret < 0 || parsed.nr_parts == 0) && parts && nr_parts) {
>  		/* Fall back to driver-provided partitions */
> -		parsed = (struct mtd_partitions){
> -			.parts		= parts,
> -			.nr_parts	= nr_parts,
> -		};
> +		ret = add_mtd_partitions(mtd, parts, nr_parts);
>  	} else if (ret < 0) {
>  		/* Didn't come up with parsed OR fallback partitions */
>  		pr_info("mtd: failed to find partitions; one or more parsers reports errors (%d)\n",
>  			ret);
>  		/* Don't abort on errors; we can still use unpartitioned MTD */
> -		memset(&parsed, 0, sizeof(parsed));
> +		if (!IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
> +			ret = add_mtd_device(mtd);
> +		else
> +			ret = 0;
> +	} else {
> +		ret = add_mtd_partitions(mtd, parsed.parts, parsed.nr_parts);
>  	}

How about:

	ret = parse_mtd_partitions(mtd, types, &parsed, parser_data);
	if (!ret && parsed.nr_parts) {
		parts = parsed.parts;
		nr_parts = parsed.nr_parts;
	}

	if (nr_parts)
		ret = add_mtd_partitions(mtd, parts, nr_parts);
	else if (!device_is_registered(&mtd->dev))
		ret = add_mtd_device(mtd);
	else
		ret = 0;

	...
> -
> -	ret = mtd_add_device_partitions(mtd, &parsed);
>  	if (ret)
>  		goto out;
>