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

From: Rafał Miłecki
Date: Wed May 04 2022 - 16:59:53 EST


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@xxxxxxxxx>
---
.../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@xxxxxxxxx>
+
+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";
};
};