Re: [PATCH V2] mtd: call of_platform_populate() for MTD partitions

Miquel Raynal
2022-05-09T14:17:49Z

Hi Rafał,

zajec5@gmail.com wrote on Wed, 4 May 2022 21:44:48 +0200:

> From: Rafał Miłecki <rafal@milecki.pl>
>
> Until this change MTD subsystem supported handling partitions only with
> MTD partitions parsers. That's a specific / limited API designed around
> partitions.
>
> Some MTD partitions may however require different handling. They may
> contain specific data that needs to be parsed and somehow extracted. For
> that purpose MTD subsystem should allow binding of standard platform
> drivers.
>
> An example can be U-Boot (sub)partition with environment variables.
> There exist a "u-boot,env" DT binding for MTD (sub)partition that
> requires an NVMEM driver.
>
> Ref: 5db1c2dbc04c ("dt-bindings: nvmem: add U-Boot environment variables binding")
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Fix case for # CONFIG_MTD_PARTITIONED_MASTER is not set
> master->dev can't be used blindly as it may point to unregistered
> device and cause WARNINGs
> ---
> drivers/mtd/mtdpart.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 357661b62c94..4971fa69d076 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -17,6 +17,7 @@
> #include <linux/mtd/partitions.h>
> #include <linux/err.h>
> #include <linux/of.h>
> +#include <linux/of_platform.h>
>
> #include "mtdcore.h"
>
> @@ -577,10 +578,16 @@ static int mtd_part_of_parse(struct mtd_info *master,
> struct mtd_part_parser *parser;
> struct device_node *np;
> struct property *prop;
> + struct device *dev;
> const char *compat;
> const char *fixed = "fixed-partitions";
> int ret, err = 0;
>
> + if (mtd_is_partition(master) || IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))

Are you sure about this condition? Isn't accessing master->dev.parent
going to fail if !IS_ENABLED(PARTITIONED_MASTER) ?

I'm not 100% sure my remark is correct but I fail to get the logic
here.

> + dev = &master->dev;
> + else
> + dev = master->dev.parent;
> +
> np = mtd_get_of_node(master);
> if (mtd_is_partition(master))
> of_node_get(np);
> @@ -593,6 +600,7 @@ static int mtd_part_of_parse(struct mtd_info *master,
> continue;
> ret = mtd_part_do_parse(parser, master, pparts, NULL);
> if (ret > 0) {
> + of_platform_populate(np, NULL, NULL, dev);
> of_node_put(np);
> return ret;
> }
> @@ -600,6 +608,7 @@ static int mtd_part_of_parse(struct mtd_info *master,
> if (ret < 0 && !err)
> err = ret;
> }
> + of_platform_populate(np, NULL, NULL, dev);
> of_node_put(np);
>
> /*

Thanks,
Miquèl