| Message ID | 20220504194448.17427-1-zajec5@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2] mtd: call of_platform_populate() for MTD partitions | expand |
On 9.05.2022 16:17, Miquel Raynal wrote: > 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. It seems to work as expected, I tested it using device with following layout: 2 fixed-partitions partitions found on MTD device brcmnand.0 Creating 2 MTD partitions on "brcmnand.0": 0x000000000000-0x000000100000 : "loader" 1 fixed-partitions partitions found on MTD device loader Creating 1 MTD partitions on "loader": 0x000000040000-0x000000044008 : "u-boot-env" 0x000000100000-0x00001ff00000 : "image" brcmnand.0 ├── loader │ └── u-boot-env └── image # CONFIG_MTD_PARTITIONED_MASTER is not set ┌────────┬────────────┬────────────────────────────────────────┐ │ Device │ Name │ struct device *dev │ ├────────┼────────────┼────────────────────────────────────────┤ │ - │ brcmnand.0 │ bcm63138_nand ff801800.nand-controller │ │ mtd0 │ loader │ mtd mtd0 │ │ mtd1 │ u-boot-env │ mtd mtd1 │ │ mtd2 │ image │ mtd mtd2 │ └────────┴────────────┴────────────────────────────────────────┘ CONFIG_MTD_PARTITIONED_MASTER=y ┌────────┬────────────┬────────────────────┐ │ Device │ Name │ struct device *dev │ ├────────┼────────────┼────────────────────┤ │ mtd0 │ brcmnand.0 │ mtd0 │ │ mtd1 │ loader │ mtd mtd1 │ │ mtd2 │ u-boot-env │ mtd mtd2 │ │ mtd3 │ image │ mtd mtd3 │ └────────┴────────────┴────────────────────┘ As you can see the only tricky case is *master* mtd *without* CONFIG_MTD_PARTITIONED_MASTER. In that case we don't register its device and so a parent (NAND controller) has to be used instead. My condition seems to handle that correctly.
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)) + 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); /*