Re: [PATCH V4 2/3] mtd: add core code reading DT specified part probes

Boris Brezillon
2017-04-17T19:32:36Z

On Mon, 17 Apr 2017 20:28:52 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
>
> Handling (creating) partitions for flash devices requires using a proper
> driver that will read partition table (out of somewhere). We can't
> simply try all existing drivers one by one:
> 1) It would increase boot time
> 2) The order could be a problem
> 3) In some corner cases parsers could misinterpret some data as a table
> Due to this MTD subsystem allows drivers to specify a list of applicable
> part probes.
>
> So far physmap_of was the only driver with support for linux,part-probe
> DT property. Other ones had list or probes hardcoded which wasn't making
> them really flexible. It prevented using common flash drivers on
> platforms that required some specific partition table access.
>
> This commit adds support for mentioned DT property directly to the MTD
> core. It's a rewritten implementation of physmap_of's code and it makes
> original code obsolete. Thanks to calling it on device parse
> registration (as suggested by Boris) all drivers gain support for it for
> free.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V4: Rename of functions
> Add comment to the of_get_mtd_part_probes
> Simplify mtd_device_parse_register changes
> Thanks Boris for help on all above!
> ---
> drivers/mtd/mtdcore.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 66a9dedd1062..07b07de7e080 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -664,6 +664,33 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd)
> }
> }
>
> +static const char * const *of_get_mtd_part_probes(struct device_node *np)
> +{
> + const char **res;
> + int count;
> +
> + count = of_property_count_strings(np, "linux,part-probe");
> + if (count < 0)
> + return NULL;
> +
> + /* This looks common: consider helper function if copying */
> + res = kzalloc((count + 1) * sizeof(*res), GFP_KERNEL);
> + if (!res)
> + return NULL;
> +
> + count = of_property_read_string_array(np, "linux,part-probe", res,
> + count);
> + if (count < 0)
> + return NULL;
> +
> + return res;
> +}
> +
> +static void of_free_mtd_part_probes(const char * const *probes)
> +{
> + kfree(probes);
> +}
> +
> /**
> * mtd_device_parse_register - parse partitions and register an MTD device.
> *
> @@ -698,11 +725,16 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
> const struct mtd_partition *parts,
> int nr_parts)
> {
> + const char * const *part_probe_types;

One last thing: can we rename this variable of_part_probe_types?

With this addressed,

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Thanks,

Boris

> struct mtd_partitions parsed;
> int ret;
>
> mtd_set_dev_defaults(mtd);
>
> + part_probe_types = of_get_mtd_part_probes(mtd_get_of_node(mtd));
> + if (part_probe_types)
> + types = part_probe_types;
> +
> memset(&parsed, 0, sizeof(parsed));
>
> ret = parse_mtd_partitions(mtd, types, &parsed, parser_data);
> @@ -720,6 +752,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
> memset(&parsed, 0, sizeof(parsed));
> }
>
> + of_free_mtd_part_probes(part_probe_types);
> +
> ret = mtd_add_device_partitions(mtd, &parsed);
> if (ret)
> goto out;