All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] Add nvmem support for dynamic partitions
@ 2022-01-20 20:26 ` Ansuel Smith
  0 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-20 20:26 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, Ansuel Smith, linux-mtd, devicetree, linux-kernel

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 and be limited to restore the old
mtd_get_of_node and add a function just to fix the problem with nvmem.

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 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 ofnode 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 the dynamic-partitions node is defined at the same level of the
partitions node. The partition in the "dynamic-partitions" node require
to have the same label of the dynamic-partition that will be created at
runtime and then a user can use the usual nvmem-cells way to declare
nvmem cells. Only the needed partition has to be declared, all the other
dynamic-partitions can be ignored.

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

I hope we find a solution to this problem as currently we are trying to
transition to nvmem and we found this limitation. This seems to be a
good solution that doesn't looks to be too much of an hack.

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

 .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
 drivers/mtd/mtdcore.c                         | 45 ++++++++++++++
 include/linux/mtd/mtd.h                       |  6 +-
 3 files changed, 105 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml

-- 
2.33.1


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

* [RFC PATCH 0/2] Add nvmem support for dynamic partitions
@ 2022-01-20 20:26 ` Ansuel Smith
  0 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-20 20:26 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, Ansuel Smith, linux-mtd, devicetree, linux-kernel

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 and be limited to restore the old
mtd_get_of_node and add a function just to fix the problem with nvmem.

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 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 ofnode 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 the dynamic-partitions node is defined at the same level of the
partitions node. The partition in the "dynamic-partitions" node require
to have the same label of the dynamic-partition that will be created at
runtime and then a user can use the usual nvmem-cells way to declare
nvmem cells. Only the needed partition has to be declared, all the other
dynamic-partitions can be ignored.

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

I hope we find a solution to this problem as currently we are trying to
transition to nvmem and we found this limitation. This seems to be a
good solution that doesn't looks to be too much of an hack.

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

 .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
 drivers/mtd/mtdcore.c                         | 45 ++++++++++++++
 include/linux/mtd/mtd.h                       |  6 +-
 3 files changed, 105 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml

-- 
2.33.1


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

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

* [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
  2022-01-20 20:26 ` Ansuel Smith
@ 2022-01-20 20:26   ` Ansuel Smith
  -1 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-20 20:26 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, Ansuel Smith, linux-mtd, devicetree, linux-kernel

Document new dynamic-partitions node 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.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml

diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
new file mode 100644
index 000000000000..7528e49f2d7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dynamic partitions
+
+description: |
+  This binding can be used on platforms which have partitions registered at
+  runtime by parsers or partition table present on the flash. Example are
+  partitions declared from smem parser or cmdlinepart. This will create an
+  of node for these dynamic partition where systems like Nvmem can get a
+  reference to register nvmem-cells.
+
+  The partition table should be a node named "dynamic-partitions".
+  Partitions are then defined as subnodes. Only the label is required
+  as any other data will be taken from the parser.
+
+maintainers:
+  - Ansuel Smith <ansuelsmth@gmail.com>
+
+properties:
+  compatible:
+    const: dynamic-partitions
+
+patternProperties:
+  "@[0-9a-f]+$":
+    $ref: "partition.yaml#"
+
+additionalProperties: true
+
+examples:
+  - |
+    partitions {
+        compatible = "qcom,smem";
+        #address-cells = <1>;
+        #size-cells = <1>;
+    };
+
+    dynamic-partitions {
+      compatible = "dynamic-partitions";
+
+      art: art {
+        label = "0:art";
+        read-only;
+        compatible = "nvmem-cells";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        macaddr_art_0: macaddr@0 {
+          reg = <0x0 0x6>;
+        };
+
+        macaddr_art_6: macaddr@6 {
+          reg = <0x6 0x6>;
+        };
+      };
+    };
-- 
2.33.1


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

* [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
@ 2022-01-20 20:26   ` Ansuel Smith
  0 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-20 20:26 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, Ansuel Smith, linux-mtd, devicetree, linux-kernel

Document new dynamic-partitions node 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.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml

diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
new file mode 100644
index 000000000000..7528e49f2d7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dynamic partitions
+
+description: |
+  This binding can be used on platforms which have partitions registered at
+  runtime by parsers or partition table present on the flash. Example are
+  partitions declared from smem parser or cmdlinepart. This will create an
+  of node for these dynamic partition where systems like Nvmem can get a
+  reference to register nvmem-cells.
+
+  The partition table should be a node named "dynamic-partitions".
+  Partitions are then defined as subnodes. Only the label is required
+  as any other data will be taken from the parser.
+
+maintainers:
+  - Ansuel Smith <ansuelsmth@gmail.com>
+
+properties:
+  compatible:
+    const: dynamic-partitions
+
+patternProperties:
+  "@[0-9a-f]+$":
+    $ref: "partition.yaml#"
+
+additionalProperties: true
+
+examples:
+  - |
+    partitions {
+        compatible = "qcom,smem";
+        #address-cells = <1>;
+        #size-cells = <1>;
+    };
+
+    dynamic-partitions {
+      compatible = "dynamic-partitions";
+
+      art: art {
+        label = "0:art";
+        read-only;
+        compatible = "nvmem-cells";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        macaddr_art_0: macaddr@0 {
+          reg = <0x0 0x6>;
+        };
+
+        macaddr_art_6: macaddr@6 {
+          reg = <0x6 0x6>;
+        };
+      };
+    };
-- 
2.33.1


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

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

* [RFC PATCH 2/2] mtd: core: introduce of support for dynamic partitions
  2022-01-20 20:26 ` Ansuel Smith
@ 2022-01-20 20:26   ` Ansuel Smith
  -1 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-20 20:26 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, Ansuel Smith, linux-mtd, devicetree, linux-kernel

We have many parser that register mtd partitions at runtime. One example
is the cmdlinepart or the smem partition where the compatible is defined
in the dts and the partitions gets detected and registered by the
parser. This is problematic for the Nvmem system that requires an of node
to detect nvmem cells. To fix this problem, introduce an additional node
called "dynamic-partitions" that must be defined at the same level of
the "partitions" node that will contain all the required partitions
where a nvmem cell has to be declared. When a mtd_get_of_node() is
called, the function will first check the default dev_of_node() and then
check this alternative partitions node and optionally if a "nvmem-cells"
compatible is detected, sets the of node for the mtd.

The "dynamic-partitions" requires the label set to the mtd name from the
dynamic partition. All the nvmem-cells will be declared in this node and
nvmem will use this node to register the nvmem cells.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/mtd/mtdcore.c   | 45 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |  6 +-----
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 9186268d361b..ccf350337811 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -563,6 +563,51 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
 	return 0;
 }
 
+struct device_node *mtd_get_of_node(struct mtd_info *mtd)
+{
+	struct device_node *dynamic_partitions, *parent_dn, *dn, *mtd_dn = NULL;
+	struct mtd_info *parent;
+	const char *mtd_name;
+	int plen;
+
+	/* Check if mtd has a device node */
+	dn = dev_of_node(&mtd->dev);
+	if (dn)
+		return dn;
+
+	/* Check if a dynamic-partitions node exist */
+	parent = mtd->parent;
+	parent_dn = dev_of_node(&parent->dev);
+	if (!parent_dn)
+		return NULL;
+
+	dynamic_partitions = of_get_compatible_child(parent_dn, "dynamic-partitions");
+	if (!dynamic_partitions)
+		goto exit_parent;
+
+	/* Search if a dynamic partition is defined with the same name */
+	for_each_child_of_node(dynamic_partitions, dn) {
+		mtd_name = of_get_property(dn, "label", &plen);
+		if (!strncmp(mtd->name, mtd_name, plen)) {
+			mtd_dn = dn;
+			break;
+		}
+	}
+
+	if (!mtd_dn)
+		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(dynamic_partitions);
+exit_parent:
+	of_node_put(parent_dn);
+	return mtd_dn;
+}
+
 /**
  *	add_mtd_device - register an MTD device
  *	@mtd: pointer to new MTD device info structure
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index f5e7dfc2e4e9..f73d65817468 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -464,11 +464,6 @@ static inline void mtd_set_of_node(struct mtd_info *mtd,
 		of_property_read_string(np, "label", &mtd->name);
 }
 
-static inline struct device_node *mtd_get_of_node(struct mtd_info *mtd)
-{
-	return dev_of_node(&mtd->dev);
-}
-
 static inline u32 mtd_oobavail(struct mtd_info *mtd, struct mtd_oob_ops *ops)
 {
 	return ops->mode == MTD_OPS_AUTO_OOB ? mtd->oobavail : mtd->oobsize;
@@ -489,6 +484,7 @@ static inline int mtd_max_bad_blocks(struct mtd_info *mtd,
 				       len);
 }
 
+struct device_node *mtd_get_of_node(struct mtd_info *mtd);
 int mtd_wunit_to_pairing_info(struct mtd_info *mtd, int wunit,
 			      struct mtd_pairing_info *info);
 int mtd_pairing_info_to_wunit(struct mtd_info *mtd,
-- 
2.33.1


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

* [RFC PATCH 2/2] mtd: core: introduce of support for dynamic partitions
@ 2022-01-20 20:26   ` Ansuel Smith
  0 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-20 20:26 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, Ansuel Smith, linux-mtd, devicetree, linux-kernel

We have many parser that register mtd partitions at runtime. One example
is the cmdlinepart or the smem partition where the compatible is defined
in the dts and the partitions gets detected and registered by the
parser. This is problematic for the Nvmem system that requires an of node
to detect nvmem cells. To fix this problem, introduce an additional node
called "dynamic-partitions" that must be defined at the same level of
the "partitions" node that will contain all the required partitions
where a nvmem cell has to be declared. When a mtd_get_of_node() is
called, the function will first check the default dev_of_node() and then
check this alternative partitions node and optionally if a "nvmem-cells"
compatible is detected, sets the of node for the mtd.

The "dynamic-partitions" requires the label set to the mtd name from the
dynamic partition. All the nvmem-cells will be declared in this node and
nvmem will use this node to register the nvmem cells.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/mtd/mtdcore.c   | 45 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |  6 +-----
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 9186268d361b..ccf350337811 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -563,6 +563,51 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
 	return 0;
 }
 
+struct device_node *mtd_get_of_node(struct mtd_info *mtd)
+{
+	struct device_node *dynamic_partitions, *parent_dn, *dn, *mtd_dn = NULL;
+	struct mtd_info *parent;
+	const char *mtd_name;
+	int plen;
+
+	/* Check if mtd has a device node */
+	dn = dev_of_node(&mtd->dev);
+	if (dn)
+		return dn;
+
+	/* Check if a dynamic-partitions node exist */
+	parent = mtd->parent;
+	parent_dn = dev_of_node(&parent->dev);
+	if (!parent_dn)
+		return NULL;
+
+	dynamic_partitions = of_get_compatible_child(parent_dn, "dynamic-partitions");
+	if (!dynamic_partitions)
+		goto exit_parent;
+
+	/* Search if a dynamic partition is defined with the same name */
+	for_each_child_of_node(dynamic_partitions, dn) {
+		mtd_name = of_get_property(dn, "label", &plen);
+		if (!strncmp(mtd->name, mtd_name, plen)) {
+			mtd_dn = dn;
+			break;
+		}
+	}
+
+	if (!mtd_dn)
+		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(dynamic_partitions);
+exit_parent:
+	of_node_put(parent_dn);
+	return mtd_dn;
+}
+
 /**
  *	add_mtd_device - register an MTD device
  *	@mtd: pointer to new MTD device info structure
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index f5e7dfc2e4e9..f73d65817468 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -464,11 +464,6 @@ static inline void mtd_set_of_node(struct mtd_info *mtd,
 		of_property_read_string(np, "label", &mtd->name);
 }
 
-static inline struct device_node *mtd_get_of_node(struct mtd_info *mtd)
-{
-	return dev_of_node(&mtd->dev);
-}
-
 static inline u32 mtd_oobavail(struct mtd_info *mtd, struct mtd_oob_ops *ops)
 {
 	return ops->mode == MTD_OPS_AUTO_OOB ? mtd->oobavail : mtd->oobsize;
@@ -489,6 +484,7 @@ static inline int mtd_max_bad_blocks(struct mtd_info *mtd,
 				       len);
 }
 
+struct device_node *mtd_get_of_node(struct mtd_info *mtd);
 int mtd_wunit_to_pairing_info(struct mtd_info *mtd, int wunit,
 			      struct mtd_pairing_info *info);
 int mtd_pairing_info_to_wunit(struct mtd_info *mtd,
-- 
2.33.1


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

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
  2022-01-20 20:26   ` Ansuel Smith
@ 2022-01-20 22:32     ` Rob Herring
  -1 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2022-01-20 22:32 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Vignesh Raghavendra, Richard Weinberger, linux-mtd,
	Miquel Raynal, devicetree, Rob Herring, linux-kernel

On Thu, 20 Jan 2022 21:26:14 +0100, Ansuel Smith wrote:
> Document new dynamic-partitions node 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.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.example.dt.yaml: partitions: 'hwlocks' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,smem.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.example.dt.yaml: partitions: 'oneOf' conditional failed, one must be fixed:
	'reg' is a required property
	'no-map' is a required property
	'memory-region' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,smem.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.example.dt.yaml: partitions: '#address-cells', '#size-cells' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,smem.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1582321

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
@ 2022-01-20 22:32     ` Rob Herring
  0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2022-01-20 22:32 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Vignesh Raghavendra, Richard Weinberger, linux-mtd,
	Miquel Raynal, devicetree, Rob Herring, linux-kernel

On Thu, 20 Jan 2022 21:26:14 +0100, Ansuel Smith wrote:
> Document new dynamic-partitions node 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.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.example.dt.yaml: partitions: 'hwlocks' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,smem.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.example.dt.yaml: partitions: 'oneOf' conditional failed, one must be fixed:
	'reg' is a required property
	'no-map' is a required property
	'memory-region' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,smem.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.example.dt.yaml: partitions: '#address-cells', '#size-cells' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,smem.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1582321

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
  2022-01-20 20:26   ` Ansuel Smith
@ 2022-01-21  1:49     ` Rob Herring
  -1 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2022-01-21  1:49 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	linux-mtd, devicetree, linux-kernel

On Thu, Jan 20, 2022 at 09:26:14PM +0100, Ansuel Smith wrote:
> Document new dynamic-partitions node 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.

So you have some discoverable way to find all the partitions and the 
nvmem cells are at an unknown (to the DT) location, but still need to be 
described in DT?

> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> new file mode 100644
> index 000000000000..7528e49f2d7e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dynamic partitions
> +
> +description: |
> +  This binding can be used on platforms which have partitions registered at
> +  runtime by parsers or partition table present on the flash. Example are
> +  partitions declared from smem parser or cmdlinepart. This will create an

Some information in DT and some on the cmdline seems broken to me. Pick 
one or the other.

> +  of node for these dynamic partition where systems like Nvmem can get a
> +  reference to register nvmem-cells.
> +
> +  The partition table should be a node named "dynamic-partitions".
> +  Partitions are then defined as subnodes. Only the label is required
> +  as any other data will be taken from the parser.
> +
> +maintainers:
> +  - Ansuel Smith <ansuelsmth@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: dynamic-partitions

This is useless. This tells me nothing about the what's in the 
partitions.

> +
> +patternProperties:
> +  "@[0-9a-f]+$":
> +    $ref: "partition.yaml#"
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    partitions {
> +        compatible = "qcom,smem";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +    };
> +
> +    dynamic-partitions {
> +      compatible = "dynamic-partitions";
> +
> +      art: art {
> +        label = "0:art";
> +        read-only;
> +        compatible = "nvmem-cells";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        macaddr_art_0: macaddr@0 {
> +          reg = <0x0 0x6>;
> +        };
> +
> +        macaddr_art_6: macaddr@6 {
> +          reg = <0x6 0x6>;
> +        };
> +      };
> +    };
> -- 
> 2.33.1
> 
> 

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
@ 2022-01-21  1:49     ` Rob Herring
  0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2022-01-21  1:49 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	linux-mtd, devicetree, linux-kernel

On Thu, Jan 20, 2022 at 09:26:14PM +0100, Ansuel Smith wrote:
> Document new dynamic-partitions node 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.

So you have some discoverable way to find all the partitions and the 
nvmem cells are at an unknown (to the DT) location, but still need to be 
described in DT?

> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> new file mode 100644
> index 000000000000..7528e49f2d7e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dynamic partitions
> +
> +description: |
> +  This binding can be used on platforms which have partitions registered at
> +  runtime by parsers or partition table present on the flash. Example are
> +  partitions declared from smem parser or cmdlinepart. This will create an

Some information in DT and some on the cmdline seems broken to me. Pick 
one or the other.

> +  of node for these dynamic partition where systems like Nvmem can get a
> +  reference to register nvmem-cells.
> +
> +  The partition table should be a node named "dynamic-partitions".
> +  Partitions are then defined as subnodes. Only the label is required
> +  as any other data will be taken from the parser.
> +
> +maintainers:
> +  - Ansuel Smith <ansuelsmth@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: dynamic-partitions

This is useless. This tells me nothing about the what's in the 
partitions.

> +
> +patternProperties:
> +  "@[0-9a-f]+$":
> +    $ref: "partition.yaml#"
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    partitions {
> +        compatible = "qcom,smem";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +    };
> +
> +    dynamic-partitions {
> +      compatible = "dynamic-partitions";
> +
> +      art: art {
> +        label = "0:art";
> +        read-only;
> +        compatible = "nvmem-cells";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        macaddr_art_0: macaddr@0 {
> +          reg = <0x0 0x6>;
> +        };
> +
> +        macaddr_art_6: macaddr@6 {
> +          reg = <0x6 0x6>;
> +        };
> +      };
> +    };
> -- 
> 2.33.1
> 
> 

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

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

* Re: [RFC PATCH 2/2] mtd: core: introduce of support for dynamic partitions
  2022-01-20 20:26   ` Ansuel Smith
  (?)
@ 2022-01-21  2:38   ` kernel test robot
  -1 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2022-01-21  2:38 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1907 bytes --]

Hi Ansuel,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on mtd/mtd/next]
[also build test ERROR on mtd/mtd/fixes robh/for-next v5.16 next-20220120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ansuel-Smith/Add-nvmem-support-for-dynamic-partitions/20220121-042801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next
config: arc-randconfig-r043-20220120 (https://download.01.org/0day-ci/archive/20220121/202201211003.6NTRXP9H-lkp(a)intel.com/config)
compiler: arc-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/e0b2803ed83327a84dc5e92fbb2f2d704748e08f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ansuel-Smith/Add-nvmem-support-for-dynamic-partitions/20220121-042801
        git checkout e0b2803ed83327a84dc5e92fbb2f2d704748e08f
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "mtd_get_of_node" [drivers/mtd/nand/nandcore.ko] undefined!
>> ERROR: modpost: "mtd_get_of_node" [drivers/mtd/parsers/ofpart.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
  2022-01-21  1:49     ` Rob Herring
@ 2022-01-22  0:29       ` Ansuel Smith
  -1 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-22  0:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	linux-mtd, devicetree, linux-kernel

On Thu, Jan 20, 2022 at 07:49:26PM -0600, Rob Herring wrote:
> On Thu, Jan 20, 2022 at 09:26:14PM +0100, Ansuel Smith wrote:
> > Document new dynamic-partitions node 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.
> 
> So you have some discoverable way to find all the partitions and the 
> nvmem cells are at an unknown (to the DT) location, but still need to be 
> described in DT?
>

Example: smem partition layout is saved in the bootloader and static. To
know the layout just boot the device and extract it. Aside from this the
naming convention is ""standard"" (example the standard nvmem location
for this is named 0:art)

What needs to be described in the DT is the cell offset and the
partition name (the location)

NVMEM doesn't support this and honestly I can't think of a simple and
direct way to solve this. Considering partition in this case are just
filled at runtime and they doesn't change (or at worst the partition
offset change but NEVER the name) it seems a good way to fix this by
describing a nvmem cells partition name and assign a of node to the
runtime filled partition.

> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > new file mode 100644
> > index 000000000000..7528e49f2d7e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dynamic partitions
> > +
> > +description: |
> > +  This binding can be used on platforms which have partitions registered at
> > +  runtime by parsers or partition table present on the flash. Example are
> > +  partitions declared from smem parser or cmdlinepart. This will create an
> 
> Some information in DT and some on the cmdline seems broken to me. Pick 
> one or the other.
> 

Converting a system from cmdline to fixedpartition is problematic
if the cmdline is dynamic. Example some system have dual partition and
are handled by editing the cmdline partition description. In this
cmdline tho the nvmem cell of our interest doesn't change and we can use
this new implemenation to add support for nvmem cells.

So really there are some case where nvmem won't work and it's not
possible to provide a correct configuration for nvmem to work correctly.

Is it that bad to have information in the DT about nvmem cells for a
partition named with a particular label that won't change.

> > +  of node for these dynamic partition where systems like Nvmem can get a
> > +  reference to register nvmem-cells.
> > +
> > +  The partition table should be a node named "dynamic-partitions".
> > +  Partitions are then defined as subnodes. Only the label is required
> > +  as any other data will be taken from the parser.
> > +
> > +maintainers:
> > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: dynamic-partitions
> 
> This is useless. This tells me nothing about the what's in the 
> partitions.
> 
> > +
> > +patternProperties:
> > +  "@[0-9a-f]+$":
> > +    $ref: "partition.yaml#"
> > +
> > +additionalProperties: true
> > +
> > +examples:
> > +  - |
> > +    partitions {
> > +        compatible = "qcom,smem";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +    };
> > +
> > +    dynamic-partitions {
> > +      compatible = "dynamic-partitions";
> > +
> > +      art: art {
> > +        label = "0:art";
> > +        read-only;
> > +        compatible = "nvmem-cells";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        macaddr_art_0: macaddr@0 {
> > +          reg = <0x0 0x6>;
> > +        };
> > +
> > +        macaddr_art_6: macaddr@6 {
> > +          reg = <0x6 0x6>;
> > +        };
> > +      };
> > +    };
> > -- 
> > 2.33.1
> > 
> > 

-- 
	Ansuel

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
@ 2022-01-22  0:29       ` Ansuel Smith
  0 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-22  0:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	linux-mtd, devicetree, linux-kernel

On Thu, Jan 20, 2022 at 07:49:26PM -0600, Rob Herring wrote:
> On Thu, Jan 20, 2022 at 09:26:14PM +0100, Ansuel Smith wrote:
> > Document new dynamic-partitions node 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.
> 
> So you have some discoverable way to find all the partitions and the 
> nvmem cells are at an unknown (to the DT) location, but still need to be 
> described in DT?
>

Example: smem partition layout is saved in the bootloader and static. To
know the layout just boot the device and extract it. Aside from this the
naming convention is ""standard"" (example the standard nvmem location
for this is named 0:art)

What needs to be described in the DT is the cell offset and the
partition name (the location)

NVMEM doesn't support this and honestly I can't think of a simple and
direct way to solve this. Considering partition in this case are just
filled at runtime and they doesn't change (or at worst the partition
offset change but NEVER the name) it seems a good way to fix this by
describing a nvmem cells partition name and assign a of node to the
runtime filled partition.

> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > new file mode 100644
> > index 000000000000..7528e49f2d7e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dynamic partitions
> > +
> > +description: |
> > +  This binding can be used on platforms which have partitions registered at
> > +  runtime by parsers or partition table present on the flash. Example are
> > +  partitions declared from smem parser or cmdlinepart. This will create an
> 
> Some information in DT and some on the cmdline seems broken to me. Pick 
> one or the other.
> 

Converting a system from cmdline to fixedpartition is problematic
if the cmdline is dynamic. Example some system have dual partition and
are handled by editing the cmdline partition description. In this
cmdline tho the nvmem cell of our interest doesn't change and we can use
this new implemenation to add support for nvmem cells.

So really there are some case where nvmem won't work and it's not
possible to provide a correct configuration for nvmem to work correctly.

Is it that bad to have information in the DT about nvmem cells for a
partition named with a particular label that won't change.

> > +  of node for these dynamic partition where systems like Nvmem can get a
> > +  reference to register nvmem-cells.
> > +
> > +  The partition table should be a node named "dynamic-partitions".
> > +  Partitions are then defined as subnodes. Only the label is required
> > +  as any other data will be taken from the parser.
> > +
> > +maintainers:
> > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: dynamic-partitions
> 
> This is useless. This tells me nothing about the what's in the 
> partitions.
> 
> > +
> > +patternProperties:
> > +  "@[0-9a-f]+$":
> > +    $ref: "partition.yaml#"
> > +
> > +additionalProperties: true
> > +
> > +examples:
> > +  - |
> > +    partitions {
> > +        compatible = "qcom,smem";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +    };
> > +
> > +    dynamic-partitions {
> > +      compatible = "dynamic-partitions";
> > +
> > +      art: art {
> > +        label = "0:art";
> > +        read-only;
> > +        compatible = "nvmem-cells";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        macaddr_art_0: macaddr@0 {
> > +          reg = <0x0 0x6>;
> > +        };
> > +
> > +        macaddr_art_6: macaddr@6 {
> > +          reg = <0x6 0x6>;
> > +        };
> > +      };
> > +    };
> > -- 
> > 2.33.1
> > 
> > 

-- 
	Ansuel

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

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
  2022-01-20 20:26   ` Ansuel Smith
@ 2022-01-24 22:02     ` Rafał Miłecki
  -1 siblings, 0 replies; 21+ messages in thread
From: Rafał Miłecki @ 2022-01-24 22:02 UTC (permalink / raw)
  To: Ansuel Smith, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Rob Herring, linux-mtd, devicetree,
	linux-kernel

On 20.01.2022 21:26, Ansuel Smith wrote:
> Document new dynamic-partitions node 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.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>   .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
>   1 file changed, 59 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> new file mode 100644
> index 000000000000..7528e49f2d7e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dynamic partitions
> +
> +description: |
> +  This binding can be used on platforms which have partitions registered at
> +  runtime by parsers or partition table present on the flash. Example are
> +  partitions declared from smem parser or cmdlinepart. This will create an
> +  of node for these dynamic partition where systems like Nvmem can get a
> +  reference to register nvmem-cells.
> +
> +  The partition table should be a node named "dynamic-partitions".
> +  Partitions are then defined as subnodes. Only the label is required
> +  as any other data will be taken from the parser.
> +
> +maintainers:
> +  - Ansuel Smith <ansuelsmth@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: dynamic-partitions
> +
> +patternProperties:
> +  "@[0-9a-f]+$":
> +    $ref: "partition.yaml#"
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    partitions {
> +        compatible = "qcom,smem";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +    };
> +
> +    dynamic-partitions {
> +      compatible = "dynamic-partitions";
> +
> +      art: art {
> +        label = "0:art";
> +        read-only;
> +        compatible = "nvmem-cells";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        macaddr_art_0: macaddr@0 {
> +          reg = <0x0 0x6>;
> +        };
> +
> +        macaddr_art_6: macaddr@6 {
> +          reg = <0x6 0x6>;
> +        };
> +      };
> +    };

First of all: I fully support such a feature. I need it for Broadom
platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions.
In my case bootloader partition is created dynamically (it doesn't have
const offset and size). It contains NVMEM data however that needs to be
described in DT.

This binding however looks loose and confusing to me.

First of all did you really mean to use "qcom,smem"? My first guess is
you meant "qcom,smem-part".

Secondly can't we have partitions defined just as subnodes of the
partitions { ... }; node?


I think sth like below would make more sense:

partitions {
     compatible = "qcom,smem-part";

     art {
         label = "0:art";
         read-only;
         compatible = "nvmem-cells";
         #address-cells = <1>;
         #size-cells = <1>;

         macaddr_art_0: macaddr@0 {
             reg = <0x0 0x6>;
         };

         macaddr_art_6: macaddr@6 {
             reg = <0x6 0x6>;
         };
     };
};


Then I could also reuse that for something like:

partitions {
     compatible = "brcm,bcm947xx-cfe-partitions";

     partition-0 {
         compatible = "nvmem-cells";
         label = "boot";

         #address-cells = <1>;
         #size-cells = <1>;

         mac: macaddr@0 {
             reg = <0x100 0x6>;
         };
     }
};

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

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
@ 2022-01-24 22:02     ` Rafał Miłecki
  0 siblings, 0 replies; 21+ messages in thread
From: Rafał Miłecki @ 2022-01-24 22:02 UTC (permalink / raw)
  To: Ansuel Smith, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Rob Herring, linux-mtd, devicetree,
	linux-kernel

On 20.01.2022 21:26, Ansuel Smith wrote:
> Document new dynamic-partitions node 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.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>   .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
>   1 file changed, 59 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> new file mode 100644
> index 000000000000..7528e49f2d7e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dynamic partitions
> +
> +description: |
> +  This binding can be used on platforms which have partitions registered at
> +  runtime by parsers or partition table present on the flash. Example are
> +  partitions declared from smem parser or cmdlinepart. This will create an
> +  of node for these dynamic partition where systems like Nvmem can get a
> +  reference to register nvmem-cells.
> +
> +  The partition table should be a node named "dynamic-partitions".
> +  Partitions are then defined as subnodes. Only the label is required
> +  as any other data will be taken from the parser.
> +
> +maintainers:
> +  - Ansuel Smith <ansuelsmth@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: dynamic-partitions
> +
> +patternProperties:
> +  "@[0-9a-f]+$":
> +    $ref: "partition.yaml#"
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    partitions {
> +        compatible = "qcom,smem";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +    };
> +
> +    dynamic-partitions {
> +      compatible = "dynamic-partitions";
> +
> +      art: art {
> +        label = "0:art";
> +        read-only;
> +        compatible = "nvmem-cells";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        macaddr_art_0: macaddr@0 {
> +          reg = <0x0 0x6>;
> +        };
> +
> +        macaddr_art_6: macaddr@6 {
> +          reg = <0x6 0x6>;
> +        };
> +      };
> +    };

First of all: I fully support such a feature. I need it for Broadom
platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions.
In my case bootloader partition is created dynamically (it doesn't have
const offset and size). It contains NVMEM data however that needs to be
described in DT.

This binding however looks loose and confusing to me.

First of all did you really mean to use "qcom,smem"? My first guess is
you meant "qcom,smem-part".

Secondly can't we have partitions defined just as subnodes of the
partitions { ... }; node?


I think sth like below would make more sense:

partitions {
     compatible = "qcom,smem-part";

     art {
         label = "0:art";
         read-only;
         compatible = "nvmem-cells";
         #address-cells = <1>;
         #size-cells = <1>;

         macaddr_art_0: macaddr@0 {
             reg = <0x0 0x6>;
         };

         macaddr_art_6: macaddr@6 {
             reg = <0x6 0x6>;
         };
     };
};


Then I could also reuse that for something like:

partitions {
     compatible = "brcm,bcm947xx-cfe-partitions";

     partition-0 {
         compatible = "nvmem-cells";
         label = "boot";

         #address-cells = <1>;
         #size-cells = <1>;

         mac: macaddr@0 {
             reg = <0x100 0x6>;
         };
     }
};

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
  2022-01-24 22:02     ` Rafał Miłecki
@ 2022-01-24 22:12       ` Ansuel Smith
  -1 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-24 22:12 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, linux-mtd, devicetree, linux-kernel

On Mon, Jan 24, 2022 at 11:02:24PM +0100, Rafał Miłecki wrote:
> On 20.01.2022 21:26, Ansuel Smith wrote:
> > Document new dynamic-partitions node 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.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >   .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
> >   1 file changed, 59 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > new file mode 100644
> > index 000000000000..7528e49f2d7e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dynamic partitions
> > +
> > +description: |
> > +  This binding can be used on platforms which have partitions registered at
> > +  runtime by parsers or partition table present on the flash. Example are
> > +  partitions declared from smem parser or cmdlinepart. This will create an
> > +  of node for these dynamic partition where systems like Nvmem can get a
> > +  reference to register nvmem-cells.
> > +
> > +  The partition table should be a node named "dynamic-partitions".
> > +  Partitions are then defined as subnodes. Only the label is required
> > +  as any other data will be taken from the parser.
> > +
> > +maintainers:
> > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: dynamic-partitions
> > +
> > +patternProperties:
> > +  "@[0-9a-f]+$":
> > +    $ref: "partition.yaml#"
> > +
> > +additionalProperties: true
> > +
> > +examples:
> > +  - |
> > +    partitions {
> > +        compatible = "qcom,smem";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +    };
> > +
> > +    dynamic-partitions {
> > +      compatible = "dynamic-partitions";
> > +
> > +      art: art {
> > +        label = "0:art";
> > +        read-only;
> > +        compatible = "nvmem-cells";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        macaddr_art_0: macaddr@0 {
> > +          reg = <0x0 0x6>;
> > +        };
> > +
> > +        macaddr_art_6: macaddr@6 {
> > +          reg = <0x6 0x6>;
> > +        };
> > +      };
> > +    };
> 
> First of all: I fully support such a feature. I need it for Broadom
> platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions.
> In my case bootloader partition is created dynamically (it doesn't have
> const offset and size). It contains NVMEM data however that needs to be
> described in DT.
> 
> This binding however looks loose and confusing to me.
>

I agree.

> First of all did you really mean to use "qcom,smem"? My first guess is
> you meant "qcom,smem-part".
> 

Yes sorry, I was referring to the smem parser qcom,smem-part 

> Secondly can't we have partitions defined just as subnodes of the
> partitions { ... }; node?
> 

I would love to use it. My only concern is that due to the fact
that we have to support legacy partition declaring, wonder if this could
create some problem. I'm referring to declaring fixed partition without
using any compatible/standard binding name.

I remember we improved that with the introduction of the nvmem binding
by making the fixed-partition compatible mandatory. But I would like to
have extra check. Wonder if to be on the safe part we can consider
appending to the "dynamic parser" a compatible like "dynamic-partitions"
and use your way to declare them (aka keeping the dynamic-partition and
removing the extra parallel partitions list)

Feel free to tell me it's just a stupid and unnecessary idea. I just
have fear to introduce regression in the partition parsing logic.

> 
> I think sth like below would make more sense:
> 
> partitions {
>     compatible = "qcom,smem-part";
> 
>     art {
>         label = "0:art";
>         read-only;
>         compatible = "nvmem-cells";
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         macaddr_art_0: macaddr@0 {
>             reg = <0x0 0x6>;
>         };
> 
>         macaddr_art_6: macaddr@6 {
>             reg = <0x6 0x6>;
>         };
>     };
> };
> 
> 
> Then I could also reuse that for something like:
> 
> partitions {
>     compatible = "brcm,bcm947xx-cfe-partitions";
> 
>     partition-0 {
>         compatible = "nvmem-cells";
>         label = "boot";
> 
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         mac: macaddr@0 {
>             reg = <0x100 0x6>;
>         };
>     }
> };

-- 
	Ansuel

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

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
@ 2022-01-24 22:12       ` Ansuel Smith
  0 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-24 22:12 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, linux-mtd, devicetree, linux-kernel

On Mon, Jan 24, 2022 at 11:02:24PM +0100, Rafał Miłecki wrote:
> On 20.01.2022 21:26, Ansuel Smith wrote:
> > Document new dynamic-partitions node 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.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >   .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
> >   1 file changed, 59 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > new file mode 100644
> > index 000000000000..7528e49f2d7e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dynamic partitions
> > +
> > +description: |
> > +  This binding can be used on platforms which have partitions registered at
> > +  runtime by parsers or partition table present on the flash. Example are
> > +  partitions declared from smem parser or cmdlinepart. This will create an
> > +  of node for these dynamic partition where systems like Nvmem can get a
> > +  reference to register nvmem-cells.
> > +
> > +  The partition table should be a node named "dynamic-partitions".
> > +  Partitions are then defined as subnodes. Only the label is required
> > +  as any other data will be taken from the parser.
> > +
> > +maintainers:
> > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: dynamic-partitions
> > +
> > +patternProperties:
> > +  "@[0-9a-f]+$":
> > +    $ref: "partition.yaml#"
> > +
> > +additionalProperties: true
> > +
> > +examples:
> > +  - |
> > +    partitions {
> > +        compatible = "qcom,smem";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +    };
> > +
> > +    dynamic-partitions {
> > +      compatible = "dynamic-partitions";
> > +
> > +      art: art {
> > +        label = "0:art";
> > +        read-only;
> > +        compatible = "nvmem-cells";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        macaddr_art_0: macaddr@0 {
> > +          reg = <0x0 0x6>;
> > +        };
> > +
> > +        macaddr_art_6: macaddr@6 {
> > +          reg = <0x6 0x6>;
> > +        };
> > +      };
> > +    };
> 
> First of all: I fully support such a feature. I need it for Broadom
> platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions.
> In my case bootloader partition is created dynamically (it doesn't have
> const offset and size). It contains NVMEM data however that needs to be
> described in DT.
> 
> This binding however looks loose and confusing to me.
>

I agree.

> First of all did you really mean to use "qcom,smem"? My first guess is
> you meant "qcom,smem-part".
> 

Yes sorry, I was referring to the smem parser qcom,smem-part 

> Secondly can't we have partitions defined just as subnodes of the
> partitions { ... }; node?
> 

I would love to use it. My only concern is that due to the fact
that we have to support legacy partition declaring, wonder if this could
create some problem. I'm referring to declaring fixed partition without
using any compatible/standard binding name.

I remember we improved that with the introduction of the nvmem binding
by making the fixed-partition compatible mandatory. But I would like to
have extra check. Wonder if to be on the safe part we can consider
appending to the "dynamic parser" a compatible like "dynamic-partitions"
and use your way to declare them (aka keeping the dynamic-partition and
removing the extra parallel partitions list)

Feel free to tell me it's just a stupid and unnecessary idea. I just
have fear to introduce regression in the partition parsing logic.

> 
> I think sth like below would make more sense:
> 
> partitions {
>     compatible = "qcom,smem-part";
> 
>     art {
>         label = "0:art";
>         read-only;
>         compatible = "nvmem-cells";
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         macaddr_art_0: macaddr@0 {
>             reg = <0x0 0x6>;
>         };
> 
>         macaddr_art_6: macaddr@6 {
>             reg = <0x6 0x6>;
>         };
>     };
> };
> 
> 
> Then I could also reuse that for something like:
> 
> partitions {
>     compatible = "brcm,bcm947xx-cfe-partitions";
> 
>     partition-0 {
>         compatible = "nvmem-cells";
>         label = "boot";
> 
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         mac: macaddr@0 {
>             reg = <0x100 0x6>;
>         };
>     }
> };

-- 
	Ansuel

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
  2022-01-24 22:12       ` Ansuel Smith
@ 2022-01-25 20:21         ` Rafał Miłecki
  -1 siblings, 0 replies; 21+ messages in thread
From: Rafał Miłecki @ 2022-01-25 20:21 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, linux-mtd, devicetree, linux-kernel

On 24.01.2022 23:12, Ansuel Smith wrote:
> On Mon, Jan 24, 2022 at 11:02:24PM +0100, Rafał Miłecki wrote:
>> On 20.01.2022 21:26, Ansuel Smith wrote:
>>> Document new dynamic-partitions node 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.
>>>
>>> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
>>> ---
>>>    .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
>>>    1 file changed, 59 insertions(+)
>>>    create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
>>> new file mode 100644
>>> index 000000000000..7528e49f2d7e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
>>> @@ -0,0 +1,59 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Dynamic partitions
>>> +
>>> +description: |
>>> +  This binding can be used on platforms which have partitions registered at
>>> +  runtime by parsers or partition table present on the flash. Example are
>>> +  partitions declared from smem parser or cmdlinepart. This will create an
>>> +  of node for these dynamic partition where systems like Nvmem can get a
>>> +  reference to register nvmem-cells.
>>> +
>>> +  The partition table should be a node named "dynamic-partitions".
>>> +  Partitions are then defined as subnodes. Only the label is required
>>> +  as any other data will be taken from the parser.
>>> +
>>> +maintainers:
>>> +  - Ansuel Smith <ansuelsmth@gmail.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: dynamic-partitions
>>> +
>>> +patternProperties:
>>> +  "@[0-9a-f]+$":
>>> +    $ref: "partition.yaml#"
>>> +
>>> +additionalProperties: true
>>> +
>>> +examples:
>>> +  - |
>>> +    partitions {
>>> +        compatible = "qcom,smem";
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +    };
>>> +
>>> +    dynamic-partitions {
>>> +      compatible = "dynamic-partitions";
>>> +
>>> +      art: art {
>>> +        label = "0:art";
>>> +        read-only;
>>> +        compatible = "nvmem-cells";
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +
>>> +        macaddr_art_0: macaddr@0 {
>>> +          reg = <0x0 0x6>;
>>> +        };
>>> +
>>> +        macaddr_art_6: macaddr@6 {
>>> +          reg = <0x6 0x6>;
>>> +        };
>>> +      };
>>> +    };
>>
>> First of all: I fully support such a feature. I need it for Broadom
>> platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions.
>> In my case bootloader partition is created dynamically (it doesn't have
>> const offset and size). It contains NVMEM data however that needs to be
>> described in DT.
>>
>> This binding however looks loose and confusing to me.
>>
> 
> I agree.
> 
>> First of all did you really mean to use "qcom,smem"? My first guess is
>> you meant "qcom,smem-part".
>>
> 
> Yes sorry, I was referring to the smem parser qcom,smem-part
> 
>> Secondly can't we have partitions defined just as subnodes of the
>> partitions { ... }; node?
>>
> 
> I would love to use it. My only concern is that due to the fact
> that we have to support legacy partition declaring, wonder if this could
> create some problem. I'm referring to declaring fixed partition without
> using any compatible/standard binding name.

Legacy partitioning won't kick in if you have "partitions" with
"compatible" string. We're safe here. Just checked to be sure.


> I remember we improved that with the introduction of the nvmem binding
> by making the fixed-partition compatible mandatory. But I would like to
> have extra check. Wonder if to be on the safe part we can consider
> appending to the "dynamic parser" a compatible like "dynamic-partitions"
> and use your way to declare them (aka keeping the dynamic-partition and
> removing the extra parallel partitions list)
> 
> Feel free to tell me it's just a stupid and unnecessary idea. I just
> have fear to introduce regression in the partition parsing logic.

I'm confused. I think all dynamic partitioners already have a
"compatible" set.

Can you post an example of DT binging you described above, please?

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
@ 2022-01-25 20:21         ` Rafał Miłecki
  0 siblings, 0 replies; 21+ messages in thread
From: Rafał Miłecki @ 2022-01-25 20:21 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, linux-mtd, devicetree, linux-kernel

On 24.01.2022 23:12, Ansuel Smith wrote:
> On Mon, Jan 24, 2022 at 11:02:24PM +0100, Rafał Miłecki wrote:
>> On 20.01.2022 21:26, Ansuel Smith wrote:
>>> Document new dynamic-partitions node 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.
>>>
>>> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
>>> ---
>>>    .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
>>>    1 file changed, 59 insertions(+)
>>>    create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
>>> new file mode 100644
>>> index 000000000000..7528e49f2d7e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
>>> @@ -0,0 +1,59 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Dynamic partitions
>>> +
>>> +description: |
>>> +  This binding can be used on platforms which have partitions registered at
>>> +  runtime by parsers or partition table present on the flash. Example are
>>> +  partitions declared from smem parser or cmdlinepart. This will create an
>>> +  of node for these dynamic partition where systems like Nvmem can get a
>>> +  reference to register nvmem-cells.
>>> +
>>> +  The partition table should be a node named "dynamic-partitions".
>>> +  Partitions are then defined as subnodes. Only the label is required
>>> +  as any other data will be taken from the parser.
>>> +
>>> +maintainers:
>>> +  - Ansuel Smith <ansuelsmth@gmail.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: dynamic-partitions
>>> +
>>> +patternProperties:
>>> +  "@[0-9a-f]+$":
>>> +    $ref: "partition.yaml#"
>>> +
>>> +additionalProperties: true
>>> +
>>> +examples:
>>> +  - |
>>> +    partitions {
>>> +        compatible = "qcom,smem";
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +    };
>>> +
>>> +    dynamic-partitions {
>>> +      compatible = "dynamic-partitions";
>>> +
>>> +      art: art {
>>> +        label = "0:art";
>>> +        read-only;
>>> +        compatible = "nvmem-cells";
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +
>>> +        macaddr_art_0: macaddr@0 {
>>> +          reg = <0x0 0x6>;
>>> +        };
>>> +
>>> +        macaddr_art_6: macaddr@6 {
>>> +          reg = <0x6 0x6>;
>>> +        };
>>> +      };
>>> +    };
>>
>> First of all: I fully support such a feature. I need it for Broadom
>> platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions.
>> In my case bootloader partition is created dynamically (it doesn't have
>> const offset and size). It contains NVMEM data however that needs to be
>> described in DT.
>>
>> This binding however looks loose and confusing to me.
>>
> 
> I agree.
> 
>> First of all did you really mean to use "qcom,smem"? My first guess is
>> you meant "qcom,smem-part".
>>
> 
> Yes sorry, I was referring to the smem parser qcom,smem-part
> 
>> Secondly can't we have partitions defined just as subnodes of the
>> partitions { ... }; node?
>>
> 
> I would love to use it. My only concern is that due to the fact
> that we have to support legacy partition declaring, wonder if this could
> create some problem. I'm referring to declaring fixed partition without
> using any compatible/standard binding name.

Legacy partitioning won't kick in if you have "partitions" with
"compatible" string. We're safe here. Just checked to be sure.


> I remember we improved that with the introduction of the nvmem binding
> by making the fixed-partition compatible mandatory. But I would like to
> have extra check. Wonder if to be on the safe part we can consider
> appending to the "dynamic parser" a compatible like "dynamic-partitions"
> and use your way to declare them (aka keeping the dynamic-partition and
> removing the extra parallel partitions list)
> 
> Feel free to tell me it's just a stupid and unnecessary idea. I just
> have fear to introduce regression in the partition parsing logic.

I'm confused. I think all dynamic partitioners already have a
"compatible" set.

Can you post an example of DT binging you described above, please?

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

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
  2022-01-25 20:21         ` Rafał Miłecki
@ 2022-01-25 20:30           ` Ansuel Smith
  -1 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-25 20:30 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, linux-mtd, devicetree, linux-kernel

On Tue, Jan 25, 2022 at 09:21:04PM +0100, Rafał Miłecki wrote:
> On 24.01.2022 23:12, Ansuel Smith wrote:
> > On Mon, Jan 24, 2022 at 11:02:24PM +0100, Rafał Miłecki wrote:
> > > On 20.01.2022 21:26, Ansuel Smith wrote:
> > > > Document new dynamic-partitions node 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.
> > > > 
> > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > > ---
> > > >    .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
> > > >    1 file changed, 59 insertions(+)
> > > >    create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > > > new file mode 100644
> > > > index 000000000000..7528e49f2d7e
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > > > @@ -0,0 +1,59 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Dynamic partitions
> > > > +
> > > > +description: |
> > > > +  This binding can be used on platforms which have partitions registered at
> > > > +  runtime by parsers or partition table present on the flash. Example are
> > > > +  partitions declared from smem parser or cmdlinepart. This will create an
> > > > +  of node for these dynamic partition where systems like Nvmem can get a
> > > > +  reference to register nvmem-cells.
> > > > +
> > > > +  The partition table should be a node named "dynamic-partitions".
> > > > +  Partitions are then defined as subnodes. Only the label is required
> > > > +  as any other data will be taken from the parser.
> > > > +
> > > > +maintainers:
> > > > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: dynamic-partitions
> > > > +
> > > > +patternProperties:
> > > > +  "@[0-9a-f]+$":
> > > > +    $ref: "partition.yaml#"
> > > > +
> > > > +additionalProperties: true
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    partitions {
> > > > +        compatible = "qcom,smem";
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <1>;
> > > > +    };
> > > > +
> > > > +    dynamic-partitions {
> > > > +      compatible = "dynamic-partitions";
> > > > +
> > > > +      art: art {
> > > > +        label = "0:art";
> > > > +        read-only;
> > > > +        compatible = "nvmem-cells";
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <1>;
> > > > +
> > > > +        macaddr_art_0: macaddr@0 {
> > > > +          reg = <0x0 0x6>;
> > > > +        };
> > > > +
> > > > +        macaddr_art_6: macaddr@6 {
> > > > +          reg = <0x6 0x6>;
> > > > +        };
> > > > +      };
> > > > +    };
> > > 
> > > First of all: I fully support such a feature. I need it for Broadom
> > > platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions.
> > > In my case bootloader partition is created dynamically (it doesn't have
> > > const offset and size). It contains NVMEM data however that needs to be
> > > described in DT.
> > > 
> > > This binding however looks loose and confusing to me.
> > > 
> > 
> > I agree.
> > 
> > > First of all did you really mean to use "qcom,smem"? My first guess is
> > > you meant "qcom,smem-part".
> > > 
> > 
> > Yes sorry, I was referring to the smem parser qcom,smem-part
> > 
> > > Secondly can't we have partitions defined just as subnodes of the
> > > partitions { ... }; node?
> > > 
> > 
> > I would love to use it. My only concern is that due to the fact
> > that we have to support legacy partition declaring, wonder if this could
> > create some problem. I'm referring to declaring fixed partition without
> > using any compatible/standard binding name.
> 
> Legacy partitioning won't kick in if you have "partitions" with
> "compatible" string. We're safe here. Just checked to be sure.
>

Oh ok then the dynamic partition compatible stuff is not needed.
To make sure I will change the "connect" function part and skip the
of_node assign if a compatible is not present. (The of_node assign
should be done only with the nvmem-cell compatible currently.)

> 
> > I remember we improved that with the introduction of the nvmem binding
> > by making the fixed-partition compatible mandatory. But I would like to
> > have extra check. Wonder if to be on the safe part we can consider
> > appending to the "dynamic parser" a compatible like "dynamic-partitions"
> > and use your way to declare them (aka keeping the dynamic-partition and
> > removing the extra parallel partitions list)
> > 
> > Feel free to tell me it's just a stupid and unnecessary idea. I just
> > have fear to introduce regression in the partition parsing logic.
> 
> I'm confused. I think all dynamic partitioners already have a
> "compatible" set.

Now that I think about it you are right. If a dynamic partition is
present in the system, a compatible must be present to use the correct
parser. And as I said up, all the nvmem cells should have the
correct compatible.

> 
> Can you post an example of DT binging you described above, please?

Was thinking something like this. But not needed.

partitions {
     compatible = "brcm,bcm947xx-cfe-partitions", "dynamic-partitions";

     partition-0 {
         compatible = "nvmem-cells";
         label = "boot";

         #address-cells = <1>;
         #size-cells = <1>;

         mac: macaddr@0 {
             reg = <0x100 0x6>;
         };
     }
};

So in short, a scheme like this should NOT be handled/should not have
of_node assigned. (and is actually very wrong)

partitions {
     compatible = "brcm,bcm947xx-cfe-partitions";

     partition-0 {
         label = "boot";

         #address-cells = <1>;
         #size-cells = <1>;

         mac: macaddr@0 {
             reg = <0x100 0x6>;
         };
     }
};

-- 
	Ansuel

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

* Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
@ 2022-01-25 20:30           ` Ansuel Smith
  0 siblings, 0 replies; 21+ messages in thread
From: Ansuel Smith @ 2022-01-25 20:30 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Rob Herring, linux-mtd, devicetree, linux-kernel

On Tue, Jan 25, 2022 at 09:21:04PM +0100, Rafał Miłecki wrote:
> On 24.01.2022 23:12, Ansuel Smith wrote:
> > On Mon, Jan 24, 2022 at 11:02:24PM +0100, Rafał Miłecki wrote:
> > > On 20.01.2022 21:26, Ansuel Smith wrote:
> > > > Document new dynamic-partitions node 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.
> > > > 
> > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > > ---
> > > >    .../mtd/partitions/dynamic-partitions.yaml    | 59 +++++++++++++++++++
> > > >    1 file changed, 59 insertions(+)
> > > >    create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > > > new file mode 100644
> > > > index 000000000000..7528e49f2d7e
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
> > > > @@ -0,0 +1,59 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Dynamic partitions
> > > > +
> > > > +description: |
> > > > +  This binding can be used on platforms which have partitions registered at
> > > > +  runtime by parsers or partition table present on the flash. Example are
> > > > +  partitions declared from smem parser or cmdlinepart. This will create an
> > > > +  of node for these dynamic partition where systems like Nvmem can get a
> > > > +  reference to register nvmem-cells.
> > > > +
> > > > +  The partition table should be a node named "dynamic-partitions".
> > > > +  Partitions are then defined as subnodes. Only the label is required
> > > > +  as any other data will be taken from the parser.
> > > > +
> > > > +maintainers:
> > > > +  - Ansuel Smith <ansuelsmth@gmail.com>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: dynamic-partitions
> > > > +
> > > > +patternProperties:
> > > > +  "@[0-9a-f]+$":
> > > > +    $ref: "partition.yaml#"
> > > > +
> > > > +additionalProperties: true
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    partitions {
> > > > +        compatible = "qcom,smem";
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <1>;
> > > > +    };
> > > > +
> > > > +    dynamic-partitions {
> > > > +      compatible = "dynamic-partitions";
> > > > +
> > > > +      art: art {
> > > > +        label = "0:art";
> > > > +        read-only;
> > > > +        compatible = "nvmem-cells";
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <1>;
> > > > +
> > > > +        macaddr_art_0: macaddr@0 {
> > > > +          reg = <0x0 0x6>;
> > > > +        };
> > > > +
> > > > +        macaddr_art_6: macaddr@6 {
> > > > +          reg = <0x6 0x6>;
> > > > +        };
> > > > +      };
> > > > +    };
> > > 
> > > First of all: I fully support such a feature. I need it for Broadom
> > > platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions.
> > > In my case bootloader partition is created dynamically (it doesn't have
> > > const offset and size). It contains NVMEM data however that needs to be
> > > described in DT.
> > > 
> > > This binding however looks loose and confusing to me.
> > > 
> > 
> > I agree.
> > 
> > > First of all did you really mean to use "qcom,smem"? My first guess is
> > > you meant "qcom,smem-part".
> > > 
> > 
> > Yes sorry, I was referring to the smem parser qcom,smem-part
> > 
> > > Secondly can't we have partitions defined just as subnodes of the
> > > partitions { ... }; node?
> > > 
> > 
> > I would love to use it. My only concern is that due to the fact
> > that we have to support legacy partition declaring, wonder if this could
> > create some problem. I'm referring to declaring fixed partition without
> > using any compatible/standard binding name.
> 
> Legacy partitioning won't kick in if you have "partitions" with
> "compatible" string. We're safe here. Just checked to be sure.
>

Oh ok then the dynamic partition compatible stuff is not needed.
To make sure I will change the "connect" function part and skip the
of_node assign if a compatible is not present. (The of_node assign
should be done only with the nvmem-cell compatible currently.)

> 
> > I remember we improved that with the introduction of the nvmem binding
> > by making the fixed-partition compatible mandatory. But I would like to
> > have extra check. Wonder if to be on the safe part we can consider
> > appending to the "dynamic parser" a compatible like "dynamic-partitions"
> > and use your way to declare them (aka keeping the dynamic-partition and
> > removing the extra parallel partitions list)
> > 
> > Feel free to tell me it's just a stupid and unnecessary idea. I just
> > have fear to introduce regression in the partition parsing logic.
> 
> I'm confused. I think all dynamic partitioners already have a
> "compatible" set.

Now that I think about it you are right. If a dynamic partition is
present in the system, a compatible must be present to use the correct
parser. And as I said up, all the nvmem cells should have the
correct compatible.

> 
> Can you post an example of DT binging you described above, please?

Was thinking something like this. But not needed.

partitions {
     compatible = "brcm,bcm947xx-cfe-partitions", "dynamic-partitions";

     partition-0 {
         compatible = "nvmem-cells";
         label = "boot";

         #address-cells = <1>;
         #size-cells = <1>;

         mac: macaddr@0 {
             reg = <0x100 0x6>;
         };
     }
};

So in short, a scheme like this should NOT be handled/should not have
of_node assigned. (and is actually very wrong)

partitions {
     compatible = "brcm,bcm947xx-cfe-partitions";

     partition-0 {
         label = "boot";

         #address-cells = <1>;
         #size-cells = <1>;

         mac: macaddr@0 {
             reg = <0x100 0x6>;
         };
     }
};

-- 
	Ansuel

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

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

end of thread, other threads:[~2022-01-25 20:30 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-20 20:26 [RFC PATCH 0/2] Add nvmem support for dynamic partitions Ansuel Smith
2022-01-20 20:26 ` Ansuel Smith
2022-01-20 20:26 ` [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node Ansuel Smith
2022-01-20 20:26   ` Ansuel Smith
2022-01-20 22:32   ` Rob Herring
2022-01-20 22:32     ` Rob Herring
2022-01-21  1:49   ` Rob Herring
2022-01-21  1:49     ` Rob Herring
2022-01-22  0:29     ` Ansuel Smith
2022-01-22  0:29       ` Ansuel Smith
2022-01-24 22:02   ` Rafał Miłecki
2022-01-24 22:02     ` Rafał Miłecki
2022-01-24 22:12     ` Ansuel Smith
2022-01-24 22:12       ` Ansuel Smith
2022-01-25 20:21       ` Rafał Miłecki
2022-01-25 20:21         ` Rafał Miłecki
2022-01-25 20:30         ` Ansuel Smith
2022-01-25 20:30           ` Ansuel Smith
2022-01-20 20:26 ` [RFC PATCH 2/2] mtd: core: introduce of support for dynamic partitions Ansuel Smith
2022-01-20 20:26   ` Ansuel Smith
2022-01-21  2:38   ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.