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

["Rafał Miłecki" <zajec5@gmail.com>]

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.

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