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

[Miquel Raynal <miquel.raynal@bootlin.com>]

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/