[RESEND PATCH v3 0/2] Add nvmem support for dynamic partitions

[RESEND PATCH v3 0/2] Add nvmem support for dynamic partitions

[Ansuel Smith]

This very small series comes to fix the very annyoing problem of
partitions declared by parser at runtime NOT supporting nvmem cells
definition.

The current implementation is very generic. The idea is to provide an of
node if defined for everyone and not strictly limit this to nvmem stuff.
But still the actual change is done only for nvmem-cells mtd. (just to
make sure) This can totally change by removing the compatible check.

The idea here is that a user can still use these dynamic parsers
instead of declaring a fixed-partition and also declare how nvmem-cells
are defined for the partition.
This live with the assumption that dynamic partition have always the
same name and they are known. (this is the case for smem-part partition
that would require a bootloader reflash to change and for parsers like
cmdlinepart where the name is always the same.)
With this assumption, it's easy to fix this problem. Just introduce a
new partition node that will declare just these special partition.
Mtdcore then will check if these special declaration are present and
connect the dynamic partition with the OF node present in the dts. Nvmem
will automagically fin the OF node and cells will be works based on the
data provided by the parser.

The initial idea was to create a special nvmem driver with a special
compatible where a user would declare the mtd partition name and this
driver would search it and register the nvmem cells but that became
difficult really fast, mtd notifier system is problematic for this kind
of stuff. So here is the better implementation. A variant of this is
already tested on openwrt where we have devices that use cmdlinepart.
(that current variant have defined in the dts the exact copy of
cmdlinepart in the fixed-partition scheme and we patched the cmdlinepart
parser to scan this fixed-partition node (that is ignored as cmdlinepart
have priority) and connect the dynamic partition with the dts node)

I provided an example of this in the documentation commit.
In short it's needed to add to the partitions where the compatible parser
is declared, a partition with just the label declared (instead of the reg).
Then declare some nvmem-cells and it will all work at runtime.
Mtdcore will check if a node with the same label is present and assign an
OF node to the MTD.

I currently tested this on my device that have smem-part and the
gmac driver use nvmem to get the mac-address. This works correctly and
the same address is provided.

v3:
- Fix warning from bot (function not declared as static)
- Updated code to support also node name
- Made partition label optional
v2:
- Simplify this. Drop dynamic-partition
- Fix problem with parser with ko
- Do not pollude mtd_get_of_node
- Fix problem with Documentation

Ansuel Smith (2):
  dt-bindings: mtd: partitions: Document new partition-dynamic nodes
  mtd: core: introduce of support for dynamic partitions

 .../mtd/partitions/partition-dynamic.yaml     | 56 +++++++++++++++++++
 .../mtd/partitions/qcom,smem-part.yaml        |  4 ++
 drivers/mtd/mtdcore.c                         | 56 +++++++++++++++++++
 3 files changed, 116 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml

-- 
2.34.1

[RESEND PATCH v3 1/2] dt-bindings: mtd: partitions: Document new partition-dynamic nodes

[Ansuel Smith]

Document new partition-dynamic nodes used to provide an OF node for
partition registred at runtime by parsers. This is required for nvmem
system to declare and detect nvmem-cells.

With these special partitions, the reg / offset is not required.
The node name must be in the form of "partition name"-dynamic.
If the partition can't be displayed using the node name, it's possible
to use the label binding that will be used instead of the node name.
The node name or the label binding is used to match the partition
allocated by the parser at runtime and the parser will provide reg
and offset of the mtd.

NVMEM will use the data from the parser and provide the NVMEM cells
declared in the DTS, "connecting" the dynamic partition with a
static declaration of cells in them.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../mtd/partitions/partition-dynamic.yaml     | 56 +++++++++++++++++++
 .../mtd/partitions/qcom,smem-part.yaml        |  4 ++
 2 files changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml

diff --git a/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
new file mode 100644
index 000000000000..e0efa58e4fac
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/partitions/partition-dynamic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dynamic Partition
+
+description: |
+  This binding describes a single flash partition that is dynamically allocated
+  by a dedicated parser that is not a fixed-partition parser.
+
+  A dynamic partition require the node ending with the "-dynamic" tag and if the
+  dynamic partition name can't be displayed using the node name, the label
+  properties can be used. The node name or the label have to match the dynamic
+  partition allocated by the parser.
+
+  These special partition definition can be used to give a dynamic partition
+  an OF node to declare NVMEM cells. An example is declaring the partition
+  label and all the NVMEM cells in it. The parser will detect the correct reg
+  and offset and the NVMEM will register the cells in it based on the data
+  extracted by the parser.
+
+maintainers:
+  - Ansuel Smith <ansuelsmth@gmail.com>
+
+properties:
+  label:
+    description: The label / name for the partition assigned by the parser at
+      runtime. This is needed for sybsystem like NVMEM to define cells and
+      register with this OF node.
+
+additionalProperties: true
+
+examples:
+  - |
+    flash {
+      partitions {
+        compatible = "qcom,smem-part";
+
+        art-dynamic {
+          compatible = "nvmem-cells";
+          #address-cells = <1>;
+          #size-cells = <1>;
+          label = "0:art";
+
+          macaddr_art_0: macaddr@0 {
+            reg = <0x0 0x6>;
+          };
+
+          macaddr_art_6: macaddr@6 {
+            reg = <0x6 0x6>;
+          };
+        };
+      };
+    };
diff --git a/Documentation/devicetree/bindings/mtd/partitions/qcom,smem-part.yaml b/Documentation/devicetree/bindings/mtd/partitions/qcom,smem-part.yaml
index cf3f8c1e035d..1d7d9b3378c3 100644
--- a/Documentation/devicetree/bindings/mtd/partitions/qcom,smem-part.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/qcom,smem-part.yaml
@@ -19,6 +19,10 @@ properties:
   compatible:
     const: qcom,smem-part
 
+patternProperties:
+  "-dynamic$":
+    $ref: "partition-dynamic.yaml#"
+
 required:
   - compatible
 
-- 
2.34.1

[RESEND PATCH v3 2/2] mtd: core: introduce of support for dynamic partitions

[Ansuel Smith]

We have many parser that register mtd partitions at runtime. One example
is the cmdlinepart or the smem-part parser where the compatible is defined
in the dts and the partitions gets detected and registered by the
parser. This is problematic for the NVMEM subsystem that requires an OF node
to detect NVMEM cells.

To fix this problem, introduce an additional logic that will try to
assign an OF node to the MTD if declared.

On MTD addition, it will be checked if the MTD has an OF node and if
not declared will check if a partition with the same name / label is
declared in DTS. If an exact match is found, the partition dynamically
allocated by the parser will have a connected OF node.

The NVMEM subsystem will detect the OF node and register any NVMEM cells
declared statically in the DTS.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/mtd/mtdcore.c | 56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 7731796024e0..ffca53fcc19a 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -546,6 +546,61 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
 	return 0;
 }
 
+static void mtd_check_of_node(struct mtd_info *mtd)
+{
+	struct device_node *partitions, *parent_dn, *mtd_dn = NULL;
+	const char *mtd_name, *suffix = "-dynamic";
+	int plen, suffix_len = 8;
+	struct mtd_info *parent;
+	bool found = false;
+
+	/* Check if MTD already has a device node */
+	if (dev_of_node(&mtd->dev))
+		return;
+
+	/* Check if a partitions node exist */
+	parent = mtd->parent;
+	parent_dn = dev_of_node(&parent->dev);
+	if (!parent_dn)
+		return;
+
+	partitions = of_get_child_by_name(parent_dn, "partitions");
+	if (!partitions)
+		goto exit_parent;
+
+	/* Search if a partition is defined with the same name */
+	for_each_child_of_node(partitions, mtd_dn) {
+		mtd_name = of_get_property(mtd_dn, "label", &plen);
+		if (!mtd_name) {
+			mtd_name = of_get_property(mtd_dn, "name", &plen);
+
+			/* Check correct suffix */
+			if (plen <= suffix_len ||
+			    strncmp(mtd_name + plen - suffix_len, suffix, suffix_len))
+				continue;
+
+			plen = plen - suffix_len;
+		}
+
+		if (!strncmp(mtd->name, mtd_name, plen)) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
+		goto exit_partitions;
+
+	/* Set of_node only for nvmem */
+	if (of_device_is_compatible(mtd_dn, "nvmem-cells"))
+		mtd_set_of_node(mtd, mtd_dn);
+
+exit_partitions:
+	of_node_put(partitions);
+exit_parent:
+	of_node_put(parent_dn);
+}
+
 /**
  *	add_mtd_device - register an MTD device
  *	@mtd: pointer to new MTD device info structure
@@ -651,6 +706,7 @@ int add_mtd_device(struct mtd_info *mtd)
 	mtd->dev.devt = MTD_DEVT(i);
 	dev_set_name(&mtd->dev, "mtd%d", i);
 	dev_set_drvdata(&mtd->dev, mtd);
+	mtd_check_of_node(mtd);
 	of_node_get(mtd_get_of_node(mtd));
 	error = device_register(&mtd->dev);
 	if (error)
-- 
2.34.1

Re: [RESEND PATCH v3 0/2] Add nvmem support for dynamic partitions

[Rafał Miłecki]

On 29.04.2022 14:48, Ansuel Smith wrote:
> This very small series comes to fix the very annyoing problem of
> partitions declared by parser at runtime NOT supporting nvmem cells
> definition.
> 
> The current implementation is very generic. The idea is to provide an of
> node if defined for everyone and not strictly limit this to nvmem stuff.
> But still the actual change is done only for nvmem-cells mtd. (just to
> make sure) This can totally change by removing the compatible check.
> 
> The idea here is that a user can still use these dynamic parsers
> instead of declaring a fixed-partition and also declare how nvmem-cells
> are defined for the partition.
> This live with the assumption that dynamic partition have always the
> same name and they are known. (this is the case for smem-part partition
> that would require a bootloader reflash to change and for parsers like
> cmdlinepart where the name is always the same.)
> With this assumption, it's easy to fix this problem. Just introduce a
> new partition node that will declare just these special partition.
> Mtdcore then will check if these special declaration are present and
> connect the dynamic partition with the OF node present in the dts. Nvmem
> will automagically fin the OF node and cells will be works based on the
> data provided by the parser.
> 
> The initial idea was to create a special nvmem driver with a special
> compatible where a user would declare the mtd partition name and this
> driver would search it and register the nvmem cells but that became
> difficult really fast, mtd notifier system is problematic for this kind
> of stuff. So here is the better implementation. A variant of this is
> already tested on openwrt where we have devices that use cmdlinepart.
> (that current variant have defined in the dts the exact copy of
> cmdlinepart in the fixed-partition scheme and we patched the cmdlinepart
> parser to scan this fixed-partition node (that is ignored as cmdlinepart
> have priority) and connect the dynamic partition with the dts node)
> 
> I provided an example of this in the documentation commit.
> In short it's needed to add to the partitions where the compatible parser
> is declared, a partition with just the label declared (instead of the reg).
> Then declare some nvmem-cells and it will all work at runtime.
> Mtdcore will check if a node with the same label is present and assign an
> OF node to the MTD.
> 
> I currently tested this on my device that have smem-part and the
> gmac driver use nvmem to get the mac-address. This works correctly and
> the same address is provided.

Thanks a lot for working on this.

Another case (that I need this work for) is cmdline parser. Some
partitions passed by U-Boot may require extra handling and that needs to
be described in DT.

Re: [RESEND PATCH v3 1/2] dt-bindings: mtd: partitions: Document new partition-dynamic nodes

[Rafał Miłecki]

On 29.04.2022 14:48, Ansuel Smith wrote:
> Document new partition-dynamic nodes used to provide an OF node for
> partition registred at runtime by parsers. This is required for nvmem
> system to declare and detect nvmem-cells.
> 
> With these special partitions, the reg / offset is not required.
> The node name must be in the form of "partition name"-dynamic.
> If the partition can't be displayed using the node name, it's possible
> to use the label binding that will be used instead of the node name.
> The node name or the label binding is used to match the partition
> allocated by the parser at runtime and the parser will provide reg
> and offset of the mtd.
> 
> NVMEM will use the data from the parser and provide the NVMEM cells
> declared in the DTS, "connecting" the dynamic partition with a
> static declaration of cells in them.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>   .../mtd/partitions/partition-dynamic.yaml     | 56 +++++++++++++++++++
>   .../mtd/partitions/qcom,smem-part.yaml        |  4 ++
>   2 files changed, 60 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
> new file mode 100644
> index 000000000000..e0efa58e4fac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/partitions/partition-dynamic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dynamic Partition

I'm not native but that "Dynamic Partition" sounds pretty natural and
I'm wondering if you shouldn't make that binding dynamic-partition.yaml

Any natives to comment on this? :)


> +description: |
> +  This binding describes a single flash partition that is dynamically allocated
> +  by a dedicated parser that is not a fixed-partition parser.
> +
> +  A dynamic partition require the node ending with the "-dynamic" tag and if the
> +  dynamic partition name can't be displayed using the node name, the label
> +  properties can be used. The node name or the label have to match the dynamic
> +  partition allocated by the parser.
> +
> +  These special partition definition can be used to give a dynamic partition
> +  an OF node to declare NVMEM cells. An example is declaring the partition
> +  label and all the NVMEM cells in it. The parser will detect the correct reg
> +  and offset and the NVMEM will register the cells in it based on the data
> +  extracted by the parser.
> +
> +maintainers:
> +  - Ansuel Smith <ansuelsmth@gmail.com>
> +
> +properties:
> +  label:
> +    description: The label / name for the partition assigned by the parser at
> +      runtime. This is needed for sybsystem like NVMEM to define cells and
> +      register with this OF node.
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    flash {
> +      partitions {
> +        compatible = "qcom,smem-part";
> +
> +        art-dynamic {
> +          compatible = "nvmem-cells";
> +          #address-cells = <1>;
> +          #size-cells = <1>;
> +          label = "0:art";
> +
> +          macaddr_art_0: macaddr@0 {
> +            reg = <0x0 0x6>;
> +          };
> +
> +          macaddr_art_6: macaddr@6 {
> +            reg = <0x6 0x6>;
> +          };
> +        };
> +      };
> +    };

I see that we need a property (like "label") for storing partition name
as it may contain characters not allowed in $nodename.

Is there a reason to play with all that foo-dynamic $nodename then? With
fallback from "label" to extracting foo from *-dynamic pattern?

Could we just be lazy, keep things simple and require "label" property?

Then we could e.g. require $nodename to be pattern ^partition-[0-9a-f]+$
It's what leds-gpio.yaml does for reference.

Example:

partitions {
	compatible = "foo";

	partition-1 {
		label = "bootloader";
	};

	partition-2 {
		label = "0:art";
	};
};

Re: [RESEND PATCH v3 1/2] dt-bindings: mtd: partitions: Document new partition-dynamic nodes

[Ansuel Smith]

On Wed, May 04, 2022 at 10:39:14PM +0200, Rafał Miłecki wrote:
> On 29.04.2022 14:48, Ansuel Smith wrote:
> > Document new partition-dynamic nodes used to provide an OF node for
> > partition registred at runtime by parsers. This is required for nvmem
> > system to declare and detect nvmem-cells.
> > 
> > With these special partitions, the reg / offset is not required.
> > The node name must be in the form of "partition name"-dynamic.
> > If the partition can't be displayed using the node name, it's possible
> > to use the label binding that will be used instead of the node name.
> > The node name or the label binding is used to match the partition
> > allocated by the parser at runtime and the parser will provide reg
> > and offset of the mtd.
> > 
> > NVMEM will use the data from the parser and provide the NVMEM cells
> > declared in the DTS, "connecting" the dynamic partition with a
> > static declaration of cells in them.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >   .../mtd/partitions/partition-dynamic.yaml     | 56 +++++++++++++++++++
> >   .../mtd/partitions/qcom,smem-part.yaml        |  4 ++
> >   2 files changed, 60 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
> > new file mode 100644
> > index 000000000000..e0efa58e4fac
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
> > @@ -0,0 +1,56 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mtd/partitions/partition-dynamic.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dynamic Partition
> 
> I'm not native but that "Dynamic Partition" sounds pretty natural and
> I'm wondering if you shouldn't make that binding dynamic-partition.yaml
> 
> Any natives to comment on this? :)
> 
>

The naming for the file is used to keep the standard of
[parser]-partition.yaml. Agree that we should find a better naming for
all of this.

> > +description: |
> > +  This binding describes a single flash partition that is dynamically allocated
> > +  by a dedicated parser that is not a fixed-partition parser.
> > +
> > +  A dynamic partition require the node ending with the "-dynamic" tag and if the
> > +  dynamic partition name can't be displayed using the node name, the label
> > +  properties can be used. The node name or the label have to match the dynamic
> > +  partition allocated by the parser.
> > +
> > +  These special partition definition can be used to give a dynamic partition
> > +  an OF node to declare NVMEM cells. An example is declaring the partition
> > +  label and all the NVMEM cells in it. The parser will detect the correct reg
> > +  and offset and the NVMEM will register the cells in it based on the data
> > +  extracted by the parser.
> > +
> > +maintainers:
> > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > +
> > +properties:
> > +  label:
> > +    description: The label / name for the partition assigned by the parser at
> > +      runtime. This is needed for sybsystem like NVMEM to define cells and
> > +      register with this OF node.
> > +
> > +additionalProperties: true
> > +
> > +examples:
> > +  - |
> > +    flash {
> > +      partitions {
> > +        compatible = "qcom,smem-part";
> > +
> > +        art-dynamic {
> > +          compatible = "nvmem-cells";
> > +          #address-cells = <1>;
> > +          #size-cells = <1>;
> > +          label = "0:art";
> > +
> > +          macaddr_art_0: macaddr@0 {
> > +            reg = <0x0 0x6>;
> > +          };
> > +
> > +          macaddr_art_6: macaddr@6 {
> > +            reg = <0x6 0x6>;
> > +          };
> > +        };
> > +      };
> > +    };
> 
> I see that we need a property (like "label") for storing partition name
> as it may contain characters not allowed in $nodename.
> 
> Is there a reason to play with all that foo-dynamic $nodename then? With
> fallback from "label" to extracting foo from *-dynamic pattern?
> 

Honestly the "-dynamic" thing is to correctly handle this ""strange""
Documentation. At times using the pattern caused tons of problems with
pattern so I had this bright idea of using the suffix "-dynamic" to
cleary differentiate these special partition from fixed one.

> Could we just be lazy, keep things simple and require "label" property?
> 

This is problematic to correctly assign a patternProperties to any user
or this parser.

> Then we could e.g. require $nodename to be pattern ^partition-[0-9a-f]+$
> It's what leds-gpio.yaml does for reference.
> 

Mhhh ok I can totally make this change. My concern is that someone would
get confused thinking they are fixed partition declared on top of the
parser. But yhea this can also work... It's really a similar
implementation of what I already to with dynamic. If you want I can do
this change and send a v4.

> Example:
> 
> partitions {
> 	compatible = "foo";
> 
> 	partition-1 {
> 		label = "bootloader";
> 	};
> 
> 	partition-2 {
> 		label = "0:art";
> 	};
> };

-- 
	Ansuel

Re: [RESEND PATCH v3 0/2] Add nvmem support for dynamic partitions

[Ansuel Smith]

On Wed, May 04, 2022 at 10:23:16PM +0200, Rafał Miłecki wrote:
> On 29.04.2022 14:48, Ansuel Smith wrote:
> > This very small series comes to fix the very annyoing problem of
> > partitions declared by parser at runtime NOT supporting nvmem cells
> > definition.
> > 
> > The current implementation is very generic. The idea is to provide an of
> > node if defined for everyone and not strictly limit this to nvmem stuff.
> > But still the actual change is done only for nvmem-cells mtd. (just to
> > make sure) This can totally change by removing the compatible check.
> > 
> > The idea here is that a user can still use these dynamic parsers
> > instead of declaring a fixed-partition and also declare how nvmem-cells
> > are defined for the partition.
> > This live with the assumption that dynamic partition have always the
> > same name and they are known. (this is the case for smem-part partition
> > that would require a bootloader reflash to change and for parsers like
> > cmdlinepart where the name is always the same.)
> > With this assumption, it's easy to fix this problem. Just introduce a
> > new partition node that will declare just these special partition.
> > Mtdcore then will check if these special declaration are present and
> > connect the dynamic partition with the OF node present in the dts. Nvmem
> > will automagically fin the OF node and cells will be works based on the
> > data provided by the parser.
> > 
> > The initial idea was to create a special nvmem driver with a special
> > compatible where a user would declare the mtd partition name and this
> > driver would search it and register the nvmem cells but that became
> > difficult really fast, mtd notifier system is problematic for this kind
> > of stuff. So here is the better implementation. A variant of this is
> > already tested on openwrt where we have devices that use cmdlinepart.
> > (that current variant have defined in the dts the exact copy of
> > cmdlinepart in the fixed-partition scheme and we patched the cmdlinepart
> > parser to scan this fixed-partition node (that is ignored as cmdlinepart
> > have priority) and connect the dynamic partition with the dts node)
> > 
> > I provided an example of this in the documentation commit.
> > In short it's needed to add to the partitions where the compatible parser
> > is declared, a partition with just the label declared (instead of the reg).
> > Then declare some nvmem-cells and it will all work at runtime.
> > Mtdcore will check if a node with the same label is present and assign an
> > OF node to the MTD.
> > 
> > I currently tested this on my device that have smem-part and the
> > gmac driver use nvmem to get the mac-address. This works correctly and
> > the same address is provided.
> 
> Thanks a lot for working on this.
> 
> Another case (that I need this work for) is cmdline parser. Some
> partitions passed by U-Boot may require extra handling and that needs to
> be described in DT.

I see more and more OEMs and SoC using special parser to declare
partition so this is starting to become necessary.

-- 
	Ansuel

Re: [RESEND PATCH v3 1/2] dt-bindings: mtd: partitions: Document new partition-dynamic nodes

[Rafał Miłecki]

On 4.05.2022 22:52, Ansuel Smith wrote:
> On Wed, May 04, 2022 at 10:39:14PM +0200, Rafał Miłecki wrote:
>> On 29.04.2022 14:48, Ansuel Smith wrote:
>>> Document new partition-dynamic nodes used to provide an OF node for
>>> partition registred at runtime by parsers. This is required for nvmem
>>> system to declare and detect nvmem-cells.
>>>
>>> With these special partitions, the reg / offset is not required.
>>> The node name must be in the form of "partition name"-dynamic.
>>> If the partition can't be displayed using the node name, it's possible
>>> to use the label binding that will be used instead of the node name.
>>> The node name or the label binding is used to match the partition
>>> allocated by the parser at runtime and the parser will provide reg
>>> and offset of the mtd.
>>>
>>> NVMEM will use the data from the parser and provide the NVMEM cells
>>> declared in the DTS, "connecting" the dynamic partition with a
>>> static declaration of cells in them.
>>>
>>> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
>>> ---
>>>    .../mtd/partitions/partition-dynamic.yaml     | 56 +++++++++++++++++++
>>>    .../mtd/partitions/qcom,smem-part.yaml        |  4 ++
>>>    2 files changed, 60 insertions(+)
>>>    create mode 100644 Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
>>> new file mode 100644
>>> index 000000000000..e0efa58e4fac
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
>>> @@ -0,0 +1,56 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mtd/partitions/partition-dynamic.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Dynamic Partition
>>
>> I'm not native but that "Dynamic Partition" sounds pretty natural and
>> I'm wondering if you shouldn't make that binding dynamic-partition.yaml
>>
>> Any natives to comment on this? :)
>>
>>
> 
> The naming for the file is used to keep the standard of
> [parser]-partition.yaml. Agree that we should find a better naming for
> all of this.

Someone has to break the pattern and start naming things sane ;)


>>> +description: |
>>> +  This binding describes a single flash partition that is dynamically allocated
>>> +  by a dedicated parser that is not a fixed-partition parser.
>>> +
>>> +  A dynamic partition require the node ending with the "-dynamic" tag and if the
>>> +  dynamic partition name can't be displayed using the node name, the label
>>> +  properties can be used. The node name or the label have to match the dynamic
>>> +  partition allocated by the parser.
>>> +
>>> +  These special partition definition can be used to give a dynamic partition
>>> +  an OF node to declare NVMEM cells. An example is declaring the partition
>>> +  label and all the NVMEM cells in it. The parser will detect the correct reg
>>> +  and offset and the NVMEM will register the cells in it based on the data
>>> +  extracted by the parser.
>>> +
>>> +maintainers:
>>> +  - Ansuel Smith <ansuelsmth@gmail.com>
>>> +
>>> +properties:
>>> +  label:
>>> +    description: The label / name for the partition assigned by the parser at
>>> +      runtime. This is needed for sybsystem like NVMEM to define cells and
>>> +      register with this OF node.
>>> +
>>> +additionalProperties: true
>>> +
>>> +examples:
>>> +  - |
>>> +    flash {
>>> +      partitions {
>>> +        compatible = "qcom,smem-part";
>>> +
>>> +        art-dynamic {
>>> +          compatible = "nvmem-cells";
>>> +          #address-cells = <1>;
>>> +          #size-cells = <1>;
>>> +          label = "0:art";
>>> +
>>> +          macaddr_art_0: macaddr@0 {
>>> +            reg = <0x0 0x6>;
>>> +          };
>>> +
>>> +          macaddr_art_6: macaddr@6 {
>>> +            reg = <0x6 0x6>;
>>> +          };
>>> +        };
>>> +      };
>>> +    };
>>
>> I see that we need a property (like "label") for storing partition name
>> as it may contain characters not allowed in $nodename.
>>
>> Is there a reason to play with all that foo-dynamic $nodename then? With
>> fallback from "label" to extracting foo from *-dynamic pattern?
>>
> 
> Honestly the "-dynamic" thing is to correctly handle this ""strange""
> Documentation. At times using the pattern caused tons of problems with
> pattern so I had this bright idea of using the suffix "-dynamic" to
> cleary differentiate these special partition from fixed one.
> 
>> Could we just be lazy, keep things simple and require "label" property?
>>
> 
> This is problematic to correctly assign a patternProperties to any user
> or this parser.
> 
>> Then we could e.g. require $nodename to be pattern ^partition-[0-9a-f]+$
>> It's what leds-gpio.yaml does for reference.
>>
> 
> Mhhh ok I can totally make this change. My concern is that someone would
> get confused thinking they are fixed partition declared on top of the
> parser. But yhea this can also work... It's really a similar
> implementation of what I already to with dynamic. If you want I can do
> this change and send a v4.

We can wait for more reviews, in case other developers think my idea
may be confusing. For me seeing partition without address is easy to
interpret as dynamic partition but let's see what others think.


>> Example:
>>
>> partitions {
>> 	compatible = "foo";
>>
>> 	partition-1 {
>> 		label = "bootloader";
>> 	};
>>
>> 	partition-2 {
>> 		label = "0:art";
>> 	};
>> };
> 

Re: [RESEND PATCH v3 1/2] dt-bindings: mtd: partitions: Document new partition-dynamic nodes

[Rob Herring]

On Wed, May 04, 2022 at 10:59:44PM +0200, Rafał Miłecki wrote:
> On 4.05.2022 22:52, Ansuel Smith wrote:
> > On Wed, May 04, 2022 at 10:39:14PM +0200, Rafał Miłecki wrote:
> > > On 29.04.2022 14:48, Ansuel Smith wrote:
> > > > Document new partition-dynamic nodes used to provide an OF node for
> > > > partition registred at runtime by parsers. This is required for nvmem
> > > > system to declare and detect nvmem-cells.
> > > > 
> > > > With these special partitions, the reg / offset is not required.
> > > > The node name must be in the form of "partition name"-dynamic.
> > > > If the partition can't be displayed using the node name, it's possible
> > > > to use the label binding that will be used instead of the node name.
> > > > The node name or the label binding is used to match the partition
> > > > allocated by the parser at runtime and the parser will provide reg
> > > > and offset of the mtd.
> > > > 
> > > > NVMEM will use the data from the parser and provide the NVMEM cells
> > > > declared in the DTS, "connecting" the dynamic partition with a
> > > > static declaration of cells in them.
> > > > 
> > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > > ---
> > > >    .../mtd/partitions/partition-dynamic.yaml     | 56 +++++++++++++++++++
> > > >    .../mtd/partitions/qcom,smem-part.yaml        |  4 ++
> > > >    2 files changed, 60 insertions(+)
> > > >    create mode 100644 Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
> > > > new file mode 100644
> > > > index 000000000000..e0efa58e4fac
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mtd/partitions/partition-dynamic.yaml
> > > > @@ -0,0 +1,56 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/mtd/partitions/partition-dynamic.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Dynamic Partition
> > > 
> > > I'm not native but that "Dynamic Partition" sounds pretty natural and
> > > I'm wondering if you shouldn't make that binding dynamic-partition.yaml
> > > 
> > > Any natives to comment on this? :)
> > > 
> > > 
> > 
> > The naming for the file is used to keep the standard of
> > [parser]-partition.yaml. Agree that we should find a better naming for
> > all of this.
> 
> Someone has to break the pattern and start naming things sane ;)
> 
> 
> > > > +description: |
> > > > +  This binding describes a single flash partition that is dynamically allocated
> > > > +  by a dedicated parser that is not a fixed-partition parser.
> > > > +
> > > > +  A dynamic partition require the node ending with the "-dynamic" tag and if the
> > > > +  dynamic partition name can't be displayed using the node name, the label
> > > > +  properties can be used. The node name or the label have to match the dynamic
> > > > +  partition allocated by the parser.
> > > > +
> > > > +  These special partition definition can be used to give a dynamic partition
> > > > +  an OF node to declare NVMEM cells. An example is declaring the partition
> > > > +  label and all the NVMEM cells in it. The parser will detect the correct reg
> > > > +  and offset and the NVMEM will register the cells in it based on the data
> > > > +  extracted by the parser.
> > > > +
> > > > +maintainers:
> > > > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > > > +
> > > > +properties:
> > > > +  label:
> > > > +    description: The label / name for the partition assigned by the parser at
> > > > +      runtime. This is needed for sybsystem like NVMEM to define cells and
> > > > +      register with this OF node.
> > > > +
> > > > +additionalProperties: true
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    flash {
> > > > +      partitions {
> > > > +        compatible = "qcom,smem-part";
> > > > +
> > > > +        art-dynamic {
> > > > +          compatible = "nvmem-cells";
> > > > +          #address-cells = <1>;
> > > > +          #size-cells = <1>;
> > > > +          label = "0:art";
> > > > +
> > > > +          macaddr_art_0: macaddr@0 {
> > > > +            reg = <0x0 0x6>;
> > > > +          };
> > > > +
> > > > +          macaddr_art_6: macaddr@6 {
> > > > +            reg = <0x6 0x6>;
> > > > +          };
> > > > +        };
> > > > +      };
> > > > +    };
> > > 
> > > I see that we need a property (like "label") for storing partition name
> > > as it may contain characters not allowed in $nodename.
> > > 
> > > Is there a reason to play with all that foo-dynamic $nodename then? With
> > > fallback from "label" to extracting foo from *-dynamic pattern?
> > > 
> > 
> > Honestly the "-dynamic" thing is to correctly handle this ""strange""
> > Documentation. At times using the pattern caused tons of problems with
> > pattern so I had this bright idea of using the suffix "-dynamic" to
> > cleary differentiate these special partition from fixed one.
> > 
> > > Could we just be lazy, keep things simple and require "label" property?
> > > 
> > 
> > This is problematic to correctly assign a patternProperties to any user
> > or this parser.
> > 
> > > Then we could e.g. require $nodename to be pattern ^partition-[0-9a-f]+$
> > > It's what leds-gpio.yaml does for reference.
> > > 
> > 
> > Mhhh ok I can totally make this change. My concern is that someone would
> > get confused thinking they are fixed partition declared on top of the
> > parser. But yhea this can also work... It's really a similar
> > implementation of what I already to with dynamic. If you want I can do
> > this change and send a v4.
> 
> We can wait for more reviews, in case other developers think my idea
> may be confusing. For me seeing partition without address is easy to
> interpret as dynamic partition but let's see what others think.

That sounds good to me.

Rob