diff mbox series

mtd: parsers: ofpart: limit parsing of deprecated DT syntax

Message ID 20210302190012.1255-1-zajec5@gmail.com
State Accepted
Headers show
Series mtd: parsers: ofpart: limit parsing of deprecated DT syntax | expand

Commit Message

Rafał Miłecki March 2, 2021, 7 p.m. UTC
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>
---
 drivers/mtd/parsers/ofpart_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Ansuel Smith March 2, 2021, 7:38 p.m. UTC | #1
> 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>;
		};
	};
};
Rafał Miłecki March 2, 2021, 8:23 p.m. UTC | #2
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.
Rafał Miłecki March 2, 2021, 8:27 p.m. UTC | #3
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

:)
Miquel Raynal March 11, 2021, 11:42 a.m. UTC | #4
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 mbox series

Patch

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) {