linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax
@ 2021-03-02 19:00 Rafał Miłecki
  2021-03-02 19:38 ` R: " ansuelsmth
  2021-03-11 11:42 ` Miquel Raynal
  0 siblings, 2 replies; 5+ messages in thread
From: Rafał Miłecki @ 2021-03-02 19:00 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: Boris Brezillon, linux-mtd, Rafał Miłecki, Ansuel Smith

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

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


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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* R: [PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax
  2021-03-02 19:00 [PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax Rafał Miłecki
@ 2021-03-02 19:38 ` ansuelsmth
  2021-03-02 20:23   ` Rafał Miłecki
  2021-03-11 11:42 ` Miquel Raynal
  1 sibling, 1 reply; 5+ messages in thread
From: ansuelsmth @ 2021-03-02 19:38 UTC (permalink / raw)
  To: 'Rafał Miłecki', 'Miquel Raynal',
	'Richard Weinberger', 'Vignesh Raghavendra'
  Cc: 'Boris Brezillon', linux-mtd, 'Rafał Miłecki'

> 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>;
		};
	};
};



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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: R: [PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax
  2021-03-02 19:38 ` R: " ansuelsmth
@ 2021-03-02 20:23   ` Rafał Miłecki
  2021-03-02 20:27     ` Rafał Miłecki
  0 siblings, 1 reply; 5+ messages in thread
From: Rafał Miłecki @ 2021-03-02 20:23 UTC (permalink / raw)
  To: ansuelsmth, 'Rafał Miłecki',
	'Miquel Raynal', 'Richard Weinberger',
	'Vignesh Raghavendra'
  Cc: 'Boris Brezillon', linux-mtd

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.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: R: [PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax
  2021-03-02 20:23   ` Rafał Miłecki
@ 2021-03-02 20:27     ` Rafał Miłecki
  0 siblings, 0 replies; 5+ messages in thread
From: Rafał Miłecki @ 2021-03-02 20:27 UTC (permalink / raw)
  To: ansuelsmth, 'Rafał Miłecki',
	'Miquel Raynal', 'Richard Weinberger',
	'Vignesh Raghavendra'
  Cc: 'Boris Brezillon', linux-mtd

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

:)

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax
  2021-03-02 19:00 [PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax Rafał Miłecki
  2021-03-02 19:38 ` R: " ansuelsmth
@ 2021-03-11 11:42 ` Miquel Raynal
  1 sibling, 0 replies; 5+ messages in thread
From: Miquel Raynal @ 2021-03-11 11:42 UTC (permalink / raw)
  To: Rafał Miłecki, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra
  Cc: Boris Brezillon, linux-mtd, Rafał Miłecki, Ansuel Smith

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

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-11 11:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 19:00 [PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax Rafał Miłecki
2021-03-02 19:38 ` R: " ansuelsmth
2021-03-02 20:23   ` Rafał Miłecki
2021-03-02 20:27     ` Rafał Miłecki
2021-03-11 11:42 ` Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).