[PATCH 0/5] nvmem: support more NVMEM cells variants

From: Rafał Miłecki <rafal@milecki.pl>

Some NVMEM devices don't have NVMEM cells at hardcoded offsets and they
can't be strictly specified in a binding. Those devices usually store
NVMEM cells in some internal format. We still need a way of referencing
such hidden / dynamic NVMEM cells.

This patchset adds support for bindings like:

nvram@1eff0000 {
	compatible = "brcm,nvram";
	reg = <0x1eff0000 0x10000>;

	mac_addr: cell-0 {
		label = "et0macaddr";
	};
};

ethernet@18024000 {
	compatible = "brcm,amac";
	reg = <0x18024000 0x800>;

	nvmem-cells = <&mac_addr>;
	nvmem-cell-names = "mac-address";
};

Rafał Miłecki (5):
  dt-bindings: nvmem: add "label" property to allow more flexible cells
    names
  nvmem: core: read OF defined NVMEM cell name from "label" property
  dt-bindings: nvmem: allow referencing device defined cells by names
  dt-bindings: nvmem: brcm,nvram: add NVMEM cell to example
  nvmem: core: add cell name based matching of DT cell nodes

 .../devicetree/bindings/nvmem/brcm,nvram.yaml |  8 +++--
 .../devicetree/bindings/nvmem/nvmem.yaml      | 16 +++++++--
 drivers/nvmem/core.c                          | 36 ++++++++++++++++++-
 3 files changed, 55 insertions(+), 5 deletions(-)

-- 
2.31.1

From: Rafał Miłecki <rafal@milecki.pl>

So far NVMEM cells names were indicated by DT $nodename. That didn't
allow fancy names with characters that are not allowed there.

That wasn't a big problem for cells fully defined in DT. One could just
adjust a name slightly if needed.

This is a problem a however for NVMEM devices with cells defined at
device level. Such vendor defined names can be more fancy and DT needs a
way to match them strictly.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 Documentation/devicetree/bindings/nvmem/nvmem.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 456fb808100a..3392405ee010 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml

@@ -49,6 +49,9 @@ patternProperties:
         description:
           Offset and size in bytes within the storage device.
 
+      label:
+        description: name of NVMEM cell
+
       bits:
         maxItems: 1
         items:

-- 
2.31.1

From: Rafał Miłecki <rafal@milecki.pl>

Prefer it over $nodename. Property "label" allows more fancy /
customized names with special characters.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/nvmem/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 23a38dcf0fc4..45c39ac401bd 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c

@@ -685,6 +685,7 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
 	struct device *dev = &nvmem->dev;
 	struct nvmem_cell_entry *cell;
 	const __be32 *addr;
+	const char *label;
 	int len;
 
 	parent = dev->of_node;

@@ -708,7 +709,11 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
 		cell->nvmem = nvmem;
 		cell->offset = be32_to_cpup(addr++);
 		cell->bytes = be32_to_cpup(addr);
-		cell->name = kasprintf(GFP_KERNEL, "%pOFn", child);
+
+		if (!of_property_read_string(child, "label", &label))
+			cell->name = kasprintf(GFP_KERNEL, "%s", label);
+		else
+			cell->name = kasprintf(GFP_KERNEL, "%pOFn", child);
 
 		addr = of_get_property(child, "bits", &len);
 		if (addr && len == (2 * sizeof(u32))) {

-- 
2.31.1

From: Rafał Miłecki <rafal@milecki.pl>

When adding NVMEM cells defined by driver it's important to match them
with DT nodes that specify matching names. That way other bindings &
drivers can reference such "dynamic" NVMEM cells.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/nvmem/core.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 45c39ac401bd..5fe92751645a 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c

@@ -499,6 +499,33 @@ static int nvmem_cell_info_to_nvmem_cell_entry(struct nvmem_device *nvmem,
 	return 0;
 }
 
+/**
+ * nvmem_find_cell_of_node() - Find DT node matching nvmem cell
+ *
+ * @nvmem: nvmem device to add cells to.
+ * @name: nvmem cell name
+ *
+ * Runtime created nvmem cells (those not coming from DT) may still need to be
+ * referenced in DT. This function allows finding DT node referencing nvmem cell
+ * by its name. Such a DT node can be used by nvmem consumers.
+ *
+ * Return: NULL or pointer to DT node
+ */
+static struct device_node *nvmem_find_cell_of_node(struct nvmem_device *nvmem,
+						   const char *name)
+{
+	struct device_node *child;
+	const char *label;
+
+	for_each_child_of_node(nvmem->dev.of_node, child) {
+		if (!of_property_read_string(child, "label", &label) &&
+		    !strcmp(label, name))
+			return child;
+	}
+
+	return NULL;
+}
+
 /**
  * nvmem_add_cells() - Add cell information to an nvmem device
  *

@@ -532,6 +559,8 @@ static int nvmem_add_cells(struct nvmem_device *nvmem,
 			goto err;
 		}
 
+		cells[i]->np = nvmem_find_cell_of_node(nvmem, cells[i]->name);
+
 		nvmem_cell_entry_add(cells[i]);
 	}
 

-- 
2.31.1

From: Rafał Miłecki <rafal@milecki.pl>

NVRAM doesn't have cells at hardcoded addresses. They are stored in
internal struct. One of cells set in almost every device is "et0macaddr"
containing MAC address. Add example that show how it can be referenced.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 Documentation/devicetree/bindings/nvmem/brcm,nvram.yaml | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/brcm,nvram.yaml b/Documentation/devicetree/bindings/nvmem/brcm,nvram.yaml
index 8c3f0cd22821..840a4559b2fd 100644
--- a/Documentation/devicetree/bindings/nvmem/brcm,nvram.yaml
+++ b/Documentation/devicetree/bindings/nvmem/brcm,nvram.yaml

@@ -32,6 +32,10 @@ unevaluatedProperties: false
 examples:
   - |
     nvram@1eff0000 {
-            compatible = "brcm,nvram";
-            reg = <0x1eff0000 0x10000>;
+        compatible = "brcm,nvram";
+        reg = <0x1eff0000 0x10000>;
+
+        mac: cell-0 {
+            label = "et0macaddr";
+        };
     };

-- 
2.31.1

From: Rafał Miłecki <rafal@milecki.pl>

Not every NVMEM has predefined cells at hardcoded addresses. Some
devices store cells in internal structs and custom formats. Referencing
such cells is still required to let other bindings use them.

Modify binding to require "reg" xor "label". The later one can be used
to match "dynamic" NVMEM cells by their names.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 Documentation/devicetree/bindings/nvmem/nvmem.yaml | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 3392405ee010..83154df25c27 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml

@@ -43,6 +43,12 @@ patternProperties:
   "@[0-9a-f]+(,[0-7])?$":
     type: object
 
+    description: |
+      NVMEM cell - a part of NVMEM containing one specific information.
+
+      Cells can be fully defined by a binding or stored in NVMEM device specific
+      data and just referenced in DT by a name (label).
+
     properties:
       reg:
         maxItems: 1

@@ -64,8 +70,11 @@ patternProperties:
               description:
                 Size in bit within the address range specified by reg.
 
-    required:
-      - reg
+    oneOf:
+      - required:
+          - reg
+      - required:
+          - label
 
 additionalProperties: true
 

-- 
2.31.1

On Thu, Dec 23, 2021 at 7:08 AM Rafał Miłecki [off-list ref] wrote:

From: Rafał Miłecki <rafal@milecki.pl>

Not every NVMEM has predefined cells at hardcoded addresses. Some
devices store cells in internal structs and custom formats. Referencing
such cells is still required to let other bindings use them.

Modify binding to require "reg" xor "label". The later one can be used
to match "dynamic" NVMEM cells by their names.

'label' is supposed to correspond to a sticker on a port or something
human identifiable. It generally should be something optional to
making the OS functional. Yes, there are already some abuses of that,
but this case is too far for me.

Rob

On 23.12.2021 22:18, Rob Herring wrote:

On Thu, Dec 23, 2021 at 7:08 AM Rafał Miłecki [off-list ref] wrote:

quoted

From: Rafał Miłecki <rafal@milecki.pl>

Not every NVMEM has predefined cells at hardcoded addresses. Some
devices store cells in internal structs and custom formats. Referencing
such cells is still required to let other bindings use them.

Modify binding to require "reg" xor "label". The later one can be used
to match "dynamic" NVMEM cells by their names.

'label' is supposed to correspond to a sticker on a port or something
human identifiable. It generally should be something optional to
making the OS functional. Yes, there are already some abuses of that,
but this case is too far for me.

Good to learn that!

"name" is special & not allowed I think.

Any suggestion what to use? I'm not native. What about "title"? Or maybe
"term", "entity", "tag"?

On Thu, Dec 23, 2021 at 10:58:56PM +0100, Rafał Miłecki wrote:

On 23.12.2021 22:18, Rob Herring wrote:

quoted

On Thu, Dec 23, 2021 at 7:08 AM Rafał Miłecki [off-list ref] wrote:

quoted

From: Rafał Miłecki <rafal@milecki.pl>

Not every NVMEM has predefined cells at hardcoded addresses. Some
devices store cells in internal structs and custom formats. Referencing
such cells is still required to let other bindings use them.

Modify binding to require "reg" xor "label". The later one can be used
to match "dynamic" NVMEM cells by their names.

'label' is supposed to correspond to a sticker on a port or something
human identifiable. It generally should be something optional to
making the OS functional. Yes, there are already some abuses of that,
but this case is too far for me.

Good to learn that!

"name" is special & not allowed I think.

It's the node name essentially. Why is using node names not sufficient? 
Do you have some specific examples?

Rob

On 4.01.2022 21:16, Rob Herring wrote:

On Thu, Dec 23, 2021 at 10:58:56PM +0100, Rafał Miłecki wrote:

quoted

On 23.12.2021 22:18, Rob Herring wrote:

quoted

On Thu, Dec 23, 2021 at 7:08 AM Rafał Miłecki [off-list ref] wrote:

quoted

From: Rafał Miłecki <rafal@milecki.pl>

Not every NVMEM has predefined cells at hardcoded addresses. Some
devices store cells in internal structs and custom formats. Referencing
such cells is still required to let other bindings use them.

Modify binding to require "reg" xor "label". The later one can be used
to match "dynamic" NVMEM cells by their names.

'label' is supposed to correspond to a sticker on a port or something
human identifiable. It generally should be something optional to
making the OS functional. Yes, there are already some abuses of that,
but this case is too far for me.

Good to learn that!

"name" is special & not allowed I think.

It's the node name essentially. Why is using node names not sufficient?
Do you have some specific examples?

I tried to explain in
[PATCH 1/5] dt-bindings: nvmem: add "label" property to allow more flexible cells names
that some vendors come with fancy names that can't fit node names.

Broadcom's NVRAM examples:
0:macaddr
1:macaddr
2:macaddr
0:ccode
1:ccode
2:ccode
0:regrev

On 4.01.2022 21:50, Rafał Miłecki wrote:

On 4.01.2022 21:16, Rob Herring wrote:

quoted

On Thu, Dec 23, 2021 at 10:58:56PM +0100, Rafał Miłecki wrote:

quoted

On 23.12.2021 22:18, Rob Herring wrote:

quoted

On Thu, Dec 23, 2021 at 7:08 AM Rafał Miłecki [off-list ref] wrote:

quoted

From: Rafał Miłecki <rafal@milecki.pl>

Not every NVMEM has predefined cells at hardcoded addresses. Some
devices store cells in internal structs and custom formats. Referencing
such cells is still required to let other bindings use them.

Modify binding to require "reg" xor "label". The later one can be used
to match "dynamic" NVMEM cells by their names.

'label' is supposed to correspond to a sticker on a port or something
human identifiable. It generally should be something optional to
making the OS functional. Yes, there are already some abuses of that,
but this case is too far for me.

Good to learn that!

"name" is special & not allowed I think.

It's the node name essentially. Why is using node names not sufficient?
Do you have some specific examples?

I tried to explain in
[PATCH 1/5] dt-bindings: nvmem: add "label" property to allow more flexible cells names
that some vendors come with fancy names that can't fit node names.

Broadcom's NVRAM examples:
0:macaddr
1:macaddr
2:macaddr
0:ccode
1:ccode
2:ccode
0:regrev

In other words I'd like to have something like:

nvram@1eff0000 {
	compatible = "brcm,nvram";
	reg = <0x1eff0000 0x10000>;

	mac: cell-0 {
		label = "1:macaddr";
	};
};

ethernet@1000 {
	compatible = "brcm,ethernet";
	reg = <0x1000 0x1000>;
	nvmem-cells = <&mac>;
	nvmem-cell-names = "mac-address";
};

On Tue, Jan 04, 2022 at 09:56:01PM +0100, Rafał Miłecki wrote:

On 4.01.2022 21:50, Rafał Miłecki wrote:

quoted

On 4.01.2022 21:16, Rob Herring wrote:

quoted

On Thu, Dec 23, 2021 at 10:58:56PM +0100, Rafał Miłecki wrote:

quoted

On 23.12.2021 22:18, Rob Herring wrote:

quoted

On Thu, Dec 23, 2021 at 7:08 AM Rafał Miłecki [off-list ref] wrote:

quoted

From: Rafał Miłecki <rafal@milecki.pl>

Not every NVMEM has predefined cells at hardcoded addresses. Some
devices store cells in internal structs and custom formats. Referencing
such cells is still required to let other bindings use them.

Modify binding to require "reg" xor "label". The later one can be used
to match "dynamic" NVMEM cells by their names.

'label' is supposed to correspond to a sticker on a port or something
human identifiable. It generally should be something optional to
making the OS functional. Yes, there are already some abuses of that,
but this case is too far for me.

Good to learn that!

"name" is special & not allowed I think.

It's the node name essentially. Why is using node names not sufficient?
Do you have some specific examples?

I tried to explain in
[PATCH 1/5] dt-bindings: nvmem: add "label" property to allow more flexible cells names
that some vendors come with fancy names that can't fit node names.

I still don't see the issue. Why do you need 'more flexible cells 
names'? What problem does that solve?

quoted

Broadcom's NVRAM examples:
0:macaddr
1:macaddr
2:macaddr
0:ccode
1:ccode
2:ccode
0:regrev

In other words I'd like to have something like:

nvram@1eff0000 {
	compatible = "brcm,nvram";
	reg = <0x1eff0000 0x10000>;

	mac: cell-0 {
		label = "1:macaddr";
	};
};

ethernet@1000 {
	compatible = "brcm,ethernet";
	reg = <0x1000 0x1000>;
	nvmem-cells = <&mac>;
	nvmem-cell-names = "mac-address";
};

How does 'label' help here?

Note there's some other efforts around multiple mac addresses and how to 
interpret the nvmem data. Maybe that helps solve your problem.

Rob