| Message ID | 20210302190012.1255-1-zajec5@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mtd: parsers: ofpart: limit parsing of deprecated DT syntax | expand |
> From: Rafał Miłecki <rafal@milecki.pl> > > For backward compatibility ofpart still supports the old syntax like: > spi-flash@0 { > compatible = "jedec,spi-nor"; > reg = <0x0>; > > partition@0 { > label = "bootloader"; > reg = <0x0 0x100000>; > }; > }; > (without "partitions" subnode). > > There is no reason however to support nested partitions without a clear > "compatible" string like: > partitions { > compatible = "fixed-partitions"; > #address-cells = <1>; > #size-cells = <1>; > > partition@0 { > label = "bootloader"; > reg = <0x0 0x100000>; > > partition@0 { > label = "config"; > reg = <0x80000 0x80000>; > }; > }; > }; > (we never officially supported or documented that). > > Make sure ofpart doesn't attempt to parse above. > I have one question. From what I can see in the new code, in this example "something" partition won't be parsed with the fixed-partition parser. Correct? partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>; partition@0 { label = "bootloader"; reg = <0x0 0x100000>; compatible = "random-parser"; partition@0 { label = "something"; reg = <0x0 0x100>; }; }; };
On 02.03.2021 20:38, ansuelsmth@gmail.com wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> For backward compatibility ofpart still supports the old syntax like: >> spi-flash@0 { >> compatible = "jedec,spi-nor"; >> reg = <0x0>; >> >> partition@0 { >> label = "bootloader"; >> reg = <0x0 0x100000>; >> }; >> }; >> (without "partitions" subnode). >> >> There is no reason however to support nested partitions without a clear >> "compatible" string like: >> partitions { >> compatible = "fixed-partitions"; >> #address-cells = <1>; >> #size-cells = <1>; >> >> partition@0 { >> label = "bootloader"; >> reg = <0x0 0x100000>; >> >> partition@0 { >> label = "config"; >> reg = <0x80000 0x80000>; >> }; >> }; >> }; >> (we never officially supported or documented that). >> >> Make sure ofpart doesn't attempt to parse above. >> > > I have one question. > From what I can see in the new code, in this example "something" > partition won't be parsed with the fixed-partition parser. Correct? > > partitions { > compatible = "fixed-partitions"; > #address-cells = <1>; > #size-cells = <1>; > > partition@0 { > label = "bootloader"; > reg = <0x0 0x100000>; > compatible = "random-parser"; > > partition@0 { > label = "something"; > reg = <0x0 0x100>; > }; > }; > }; Correct. Both: "bootloader" and "something" partitions in your example will have master->parent set to their parents. Parser "ofpart" will NOT attempt to parse "bootloader" subnodes or "something" subnodes.
On 02.03.2021 21:23, Rafał Miłecki wrote: > On 02.03.2021 20:38, ansuelsmth@gmail.com wrote: >>> From: Rafał Miłecki <rafal@milecki.pl> >>> >>> For backward compatibility ofpart still supports the old syntax like: >>> spi-flash@0 { >>> compatible = "jedec,spi-nor"; >>> reg = <0x0>; >>> >>> partition@0 { >>> label = "bootloader"; >>> reg = <0x0 0x100000>; >>> }; >>> }; >>> (without "partitions" subnode). >>> >>> There is no reason however to support nested partitions without a clear >>> "compatible" string like: >>> partitions { >>> compatible = "fixed-partitions"; >>> #address-cells = <1>; >>> #size-cells = <1>; >>> >>> partition@0 { >>> label = "bootloader"; >>> reg = <0x0 0x100000>; >>> >>> partition@0 { >>> label = "config"; >>> reg = <0x80000 0x80000>; >>> }; >>> }; >>> }; >>> (we never officially supported or documented that). >>> >>> Make sure ofpart doesn't attempt to parse above. >>> >> >> I have one question. >> From what I can see in the new code, in this example "something" >> partition won't be parsed with the fixed-partition parser. Correct? >> >> partitions { >> compatible = "fixed-partitions"; >> #address-cells = <1>; >> #size-cells = <1>; >> >> partition@0 { >> label = "bootloader"; >> reg = <0x0 0x100000>; >> compatible = "random-parser"; >> >> partition@0 { >> label = "something"; >> reg = <0x0 0x100>; >> }; >> }; >> }; > > Correct. > > Both: "bootloader" and "something" partitions in your example will have > master->parent set to their parents. Parser "ofpart" will NOT attempt to > parse "bootloader" subnodes or "something" subnodes. I even verified that with CONFIG_MTD_PARTITIONED_MASTER=y as well as with # CONFIG_MTD_PARTITIONED_MASTER is not set :)
On Tue, 2021-03-02 at 19:00:12 UTC, =?utf-8?b?UmFmYcWCIE1pxYJlY2tp?= wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > For backward compatibility ofpart still supports the old syntax like: > spi-flash@0 { > compatible = "jedec,spi-nor"; > reg = <0x0>; > > partition@0 { > label = "bootloader"; > reg = <0x0 0x100000>; > }; > }; > (without "partitions" subnode). > > There is no reason however to support nested partitions without a clear > "compatible" string like: > partitions { > compatible = "fixed-partitions"; > #address-cells = <1>; > #size-cells = <1>; > > partition@0 { > label = "bootloader"; > reg = <0x0 0x100000>; > > partition@0 { > label = "config"; > reg = <0x80000 0x80000>; > }; > }; > }; > (we never officially supported or documented that). > > Make sure ofpart doesn't attempt to parse above. > > Cc: Ansuel Smith <ansuelsmth@gmail.com> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks. Miquel
diff --git a/drivers/mtd/parsers/ofpart_core.c b/drivers/mtd/parsers/ofpart_core.c index 258c06a42283..02658298d99a 100644 --- a/drivers/mtd/parsers/ofpart_core.c +++ b/drivers/mtd/parsers/ofpart_core.c @@ -53,7 +53,7 @@ static int parse_fixed_partitions(struct mtd_info *master, return 0; ofpart_node = of_get_child_by_name(mtd_node, "partitions"); - if (!ofpart_node) { + if (!ofpart_node && !master->parent) { /* * We might get here even when ofpart isn't used at all (e.g., * when using another parser), so don't be louder than @@ -64,6 +64,8 @@ static int parse_fixed_partitions(struct mtd_info *master, ofpart_node = mtd_node; dedicated = false; } + if (!ofpart_node) + return 0; of_id = of_match_node(parse_ofpart_match_table, ofpart_node); if (dedicated && !of_id) {