linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] dt-bindings: nvmem: support describing cells
@ 2022-01-25 18:01 Rafał Miłecki
  2022-01-25 18:01 ` [PATCH 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rafał Miłecki
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Rafał Miłecki @ 2022-01-25 18:01 UTC (permalink / raw)
  To: Rob Herring, Srinivas Kandagatla, Michael Walle
  Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, netdev,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Shawn Guo, Li Yang, Frank Rowand, David S . Miller,
	Jakub Kicinski, Ansuel Smith, Andrew Lunn, Florian Fainelli,
	Hauke Mehrtens, Rafał Miłecki

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

Michael has recently posted a cleaned up patchset for NVMEM
transformations support:
[PATCH 0/8] nvmem: add ethernet address offset support
https://lore.kernel.org/lkml/20211228142549.1275412-1-michael@walle.cc/T/
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=278644&state=*

I find it very important & fully support it. In home routers we very
often deal with just one MAC address that:
1. Is a base for calculating multiple Ethernet addresses
2. Can be stored in binary as well as ASCII format

I'd like to suggest just a slightly different solution though. I think
that using something like:

otp-1 {
        compatible = "kontron,sl28-vpd", "user-otp";
        #address-cells = <1>;
        #size-cells = <1>;

        base_mac_address: base-mac-address@17 {
                #nvmem-cell-cells = <1>;
                reg = <17 6>;
        };
};

isn't clear enough and requires too much conditional code in Linux /
whatever implementation. DT doesn't make it clear which NVMEM cells
are used for what and how should be handled. That has to be hardcoded in
a Linux / whatever driver.

My idea is to add "compatible" & additional flags to NVMEM cells.
Example:

otp-1 {
        compatible = "user-otp";
        #address-cells = <1>;
        #size-cells = <1>;

        base_mac_address: base-mac-address@17 {
                compatible = "mac-address";
                reg = <17 6>;
                #nvmem-cell-cells = <1>;
        };
};

(for more examples see PATCH 2/2 and its mac-address.yaml .

Rafał Miłecki (2):
  dt-bindings: nvmem: extract NVMEM cell to separated file
  dt-bindings: nvmem: cells: add MAC address cell

 .../devicetree/bindings/nvmem/cells/cell.yaml | 35 +++++++
 .../bindings/nvmem/cells/mac-address.yaml     | 94 +++++++++++++++++++
 .../devicetree/bindings/nvmem/nvmem.yaml      | 25 +----
 3 files changed, 131 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/nvmem/cells/cell.yaml
 create mode 100644 Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml

-- 
2.31.1


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

* [PATCH 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file
  2022-01-25 18:01 [PATCH 0/2] dt-bindings: nvmem: support describing cells Rafał Miłecki
@ 2022-01-25 18:01 ` Rafał Miłecki
  2022-01-25 18:01 ` [PATCH 2/2] dt-bindings: nvmem: cells: add MAC address cell Rafał Miłecki
  2022-01-26  7:07 ` [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rafał Miłecki
  2 siblings, 0 replies; 12+ messages in thread
From: Rafał Miłecki @ 2022-01-25 18:01 UTC (permalink / raw)
  To: Rob Herring, Srinivas Kandagatla, Michael Walle
  Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, netdev,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Shawn Guo, Li Yang, Frank Rowand, David S . Miller,
	Jakub Kicinski, Ansuel Smith, Andrew Lunn, Florian Fainelli,
	Hauke Mehrtens, Rafał Miłecki

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

This will allow adding binding for more specific cells and reusing
(sharing) common code.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/nvmem/cells/cell.yaml | 35 +++++++++++++++++++
 .../devicetree/bindings/nvmem/nvmem.yaml      | 25 ++-----------
 2 files changed, 37 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/nvmem/cells/cell.yaml

diff --git a/Documentation/devicetree/bindings/nvmem/cells/cell.yaml b/Documentation/devicetree/bindings/nvmem/cells/cell.yaml
new file mode 100644
index 000000000000..5d62d0c8f1e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/cells/cell.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/cells/cell.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVMEM cell
+
+maintainers:
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+
+description: NVMEM cell is a data entry of NVMEM device.
+
+properties:
+  reg:
+    maxItems: 1
+    description:
+      Offset and size in bytes within the storage device.
+
+  bits:
+    maxItems: 1
+    items:
+      items:
+        - minimum: 0
+          maximum: 7
+          description:
+            Offset in bit within the address range specified by reg.
+        - minimum: 1
+          description:
+            Size in bit within the address range specified by reg.
+
+required:
+  - reg
+
+additionalProperties: true
diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 456fb808100a..6b075c1db446 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -40,29 +40,8 @@ properties:
     maxItems: 1
 
 patternProperties:
-  "@[0-9a-f]+(,[0-7])?$":
-    type: object
-
-    properties:
-      reg:
-        maxItems: 1
-        description:
-          Offset and size in bytes within the storage device.
-
-      bits:
-        maxItems: 1
-        items:
-          items:
-            - minimum: 0
-              maximum: 7
-              description:
-                Offset in bit within the address range specified by reg.
-            - minimum: 1
-              description:
-                Size in bit within the address range specified by reg.
-
-    required:
-      - reg
+  "@[0-9a-f]+$":
+    $ref: cells/cell.yaml#
 
 additionalProperties: true
 
-- 
2.31.1


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

* [PATCH 2/2] dt-bindings: nvmem: cells: add MAC address cell
  2022-01-25 18:01 [PATCH 0/2] dt-bindings: nvmem: support describing cells Rafał Miłecki
  2022-01-25 18:01 ` [PATCH 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rafał Miłecki
@ 2022-01-25 18:01 ` Rafał Miłecki
  2022-01-26  3:29   ` Rob Herring
  2022-01-26  7:07 ` [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rafał Miłecki
  2 siblings, 1 reply; 12+ messages in thread
From: Rafał Miłecki @ 2022-01-25 18:01 UTC (permalink / raw)
  To: Rob Herring, Srinivas Kandagatla, Michael Walle
  Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, netdev,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Shawn Guo, Li Yang, Frank Rowand, David S . Miller,
	Jakub Kicinski, Ansuel Smith, Andrew Lunn, Florian Fainelli,
	Hauke Mehrtens, Rafał Miłecki

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

This adds support for describing details of NVMEM cell containing MAC
address. Those are often device specific and could be nicely stored in
DT.

Initial documentation includes support for describing:
1. Cell data format (e.g. Broadcom's NVRAM uses ASCII to store MAC)
2. Reversed bytes flash (required for i.MX6/i.MX7 OCOTP support)
3. Source for multiple addresses (very common in home routers)

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../bindings/nvmem/cells/mac-address.yaml     | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml

diff --git a/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
new file mode 100644
index 000000000000..f8d19e87cdf0
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/cells/mac-address.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVMEM cell containing a MAC address
+
+maintainers:
+  - Rafał Miłecki <rafal@milecki.pl>
+
+properties:
+  compatible:
+    const: mac-address
+
+  format:
+    description: |
+      Some NVMEM cells contain MAC in a non-binary format.
+
+      ASCII should be specified if MAC is string formatted like:
+      - "01:23:45:67:89:AB" (30 31 3a 32 33 3a 34 35 3a 36 37 3a 38 39 3a 41 42)
+      - "01-23-45-67-89-AB"
+      - "0123456789AB"
+    enum:
+      - ascii
+
+  reversed-bytes:
+    type: boolean
+    description: |
+      MAC is stored in reversed bytes order. Example:
+      Stored value: AB 89 67 45 23 01
+      Actual MAC: 01 23 45 67 89 AB
+
+  base-address:
+    type: boolean
+    description: |
+      Marks NVMEM cell as provider of multiple addresses that are relative to
+      the one actually stored physically. Respective addresses can be requested
+      by specifying cell index of NVMEM cell.
+
+allOf:
+  - $ref: cell.yaml#
+  - if:
+      required:
+        - base-address
+    then:
+      properties:
+        "#nvmem-cell-cells":
+          const: 1
+      required:
+        - "#nvmem-cell-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    partitions {
+        compatible = "fixed-partitions";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        partition@f00000 {
+            compatible = "nvmem-cells";
+            label = "calibration";
+            reg = <0xf00000 0x100000>;
+            ranges = <0 0xf00000 0x100000>;
+            #address-cells = <1>;
+            #size-cells = <1>;
+
+            mac@100 {
+                compatible = "mac-address";
+                reg = <0x100 0x6>;
+            };
+
+            mac@200 {
+                compatible = "mac-address";
+                reg = <0x200 0x6>;
+                reversed-bytes;
+            };
+
+            mac@300 {
+                compatible = "mac-address";
+                reg = <0x300 0x11>;
+                format = "ascii";
+            };
+
+            mac@400 {
+                compatible = "mac-address";
+                reg = <0x400 0x6>;
+                base-address;
+                #nvmem-cell-cells = <1>;
+            };
+        };
+    };
-- 
2.31.1


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

* Re: [PATCH 2/2] dt-bindings: nvmem: cells: add MAC address cell
  2022-01-25 18:01 ` [PATCH 2/2] dt-bindings: nvmem: cells: add MAC address cell Rafał Miłecki
@ 2022-01-26  3:29   ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2022-01-26  3:29 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Richard Weinberger, Andrew Lunn, devicetree, Rob Herring,
	linux-arm-kernel, netdev, Shawn Guo, linux-mtd, David S . Miller,
	linux-kernel, Michael Walle, Miquel Raynal, Vignesh Raghavendra,
	Frank Rowand, Rafał Miłecki, Ansuel Smith,
	Hauke Mehrtens, Li Yang, Florian Fainelli, Jakub Kicinski,
	Srinivas Kandagatla

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4468 bytes --]

On Tue, 25 Jan 2022 19:01:14 +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This adds support for describing details of NVMEM cell containing MAC
> address. Those are often device specific and could be nicely stored in
> DT.
> 
> Initial documentation includes support for describing:
> 1. Cell data format (e.g. Broadcom's NVRAM uses ASCII to store MAC)
> 2. Reversed bytes flash (required for i.MX6/i.MX7 OCOTP support)
> 3. Source for multiple addresses (very common in home routers)
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../bindings/nvmem/cells/mac-address.yaml     | 94 +++++++++++++++++++
>  1 file changed, 94 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/cells/mac-address.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:
schemas/nvmem/cells/cell.yaml: ignoring, error parsing file
make[1]: *** Deleting file 'Documentation/devicetree/bindings/nvmem/cells/mac-address.example.dt.yaml'
schemas/nvmem/cells/cell.yaml: ignoring, error parsing file
Traceback (most recent call last):
  File "/usr/local/bin/dt-validate", line 170, in <module>
    sg.check_trees(filename, testtree)
  File "/usr/local/bin/dt-validate", line 119, in check_trees
    self.check_subtree(dt, subtree, False, "/", "/", filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  [Previous line repeated 1 more time]
  File "/usr/local/bin/dt-validate", line 105, in check_subtree
    self.check_node(tree, subtree, disabled, nodename, fullname, filename)
  File "/usr/local/bin/dt-validate", line 49, in check_node
    errors = sorted(dtschema.DTValidator(schema).iter_errors(node), key=lambda e: e.linecol)
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 229, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 362, in allOf
    yield from validator.descend(instance, subschema, schema_path=index)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 245, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 229, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 298, in ref
    yield from validator.descend(instance, resolved)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 245, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 219, in iter_errors
    scope = id_of(_schema)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 96, in _id_of
    return schema.get("$id", "")
AttributeError: 'NoneType' object has no attribute 'get'
make[1]: *** [scripts/Makefile.lib:378: Documentation/devicetree/bindings/nvmem/cells/mac-address.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1398: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

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

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] 12+ messages in thread

* [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file
  2022-01-25 18:01 [PATCH 0/2] dt-bindings: nvmem: support describing cells Rafał Miłecki
  2022-01-25 18:01 ` [PATCH 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rafał Miłecki
  2022-01-25 18:01 ` [PATCH 2/2] dt-bindings: nvmem: cells: add MAC address cell Rafał Miłecki
@ 2022-01-26  7:07 ` Rafał Miłecki
  2022-01-26  7:07   ` [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell Rafał Miłecki
  2022-02-11 12:50   ` [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rob Herring
  2 siblings, 2 replies; 12+ messages in thread
From: Rafał Miłecki @ 2022-01-26  7:07 UTC (permalink / raw)
  To: Rob Herring, Srinivas Kandagatla, Michael Walle
  Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, netdev,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Shawn Guo, Li Yang, Frank Rowand, David S . Miller,
	Jakub Kicinski, Ansuel Smith, Andrew Lunn, Florian Fainelli,
	Hauke Mehrtens, Rafał Miłecki

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

This will allow adding binding for more specific cells and reusing
(sharing) common code.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/nvmem/cells/cell.yaml | 34 +++++++++++++++++++
 .../devicetree/bindings/nvmem/nvmem.yaml      | 22 +-----------
 2 files changed, 35 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/nvmem/cells/cell.yaml

diff --git a/Documentation/devicetree/bindings/nvmem/cells/cell.yaml b/Documentation/devicetree/bindings/nvmem/cells/cell.yaml
new file mode 100644
index 000000000000..adfc2e639f43
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/cells/cell.yaml
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/cells/cell.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVMEM cell
+
+maintainers:
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+
+description: NVMEM cell is a data entry of NVMEM device.
+
+properties:
+  reg:
+    maxItems: 1
+    description:
+      Offset and size in bytes within the storage device.
+
+  bits:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - minimum: 0
+        maximum: 7
+        description:
+          Offset in bit within the address range specified by reg.
+      - minimum: 1
+        description:
+          Size in bit within the address range specified by reg.
+
+required:
+  - reg
+
+additionalProperties: true
diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 43ed7e32e5ac..b79b51e98ee8 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -41,27 +41,7 @@ properties:
 
 patternProperties:
   "@[0-9a-f]+(,[0-7])?$":
-    type: object
-
-    properties:
-      reg:
-        maxItems: 1
-        description:
-          Offset and size in bytes within the storage device.
-
-      bits:
-        $ref: /schemas/types.yaml#/definitions/uint32-array
-        items:
-          - minimum: 0
-            maximum: 7
-            description:
-              Offset in bit within the address range specified by reg.
-          - minimum: 1
-            description:
-              Size in bit within the address range specified by reg.
-
-    required:
-      - reg
+    $ref: cells/cell.yaml#
 
 additionalProperties: true
 
-- 
2.31.1


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

* [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell
  2022-01-26  7:07 ` [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rafał Miłecki
@ 2022-01-26  7:07   ` Rafał Miłecki
  2022-02-01 15:55     ` Rob Herring
  2022-02-11 12:50   ` [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Rafał Miłecki @ 2022-01-26  7:07 UTC (permalink / raw)
  To: Rob Herring, Srinivas Kandagatla, Michael Walle
  Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, netdev,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Shawn Guo, Li Yang, Frank Rowand, David S . Miller,
	Jakub Kicinski, Ansuel Smith, Andrew Lunn, Florian Fainelli,
	Hauke Mehrtens, Rafał Miłecki

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

This adds support for describing details of NVMEM cell containing MAC
address. Those are often device specific and could be nicely stored in
DT.

Initial documentation includes support for describing:
1. Cell data format (e.g. Broadcom's NVRAM uses ASCII to store MAC)
2. Reversed bytes flash (required for i.MX6/i.MX7 OCOTP support)
3. Source for multiple addresses (very common in home routers)

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../bindings/nvmem/cells/mac-address.yaml     | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml

diff --git a/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
new file mode 100644
index 000000000000..f8d19e87cdf0
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/cells/mac-address.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVMEM cell containing a MAC address
+
+maintainers:
+  - Rafał Miłecki <rafal@milecki.pl>
+
+properties:
+  compatible:
+    const: mac-address
+
+  format:
+    description: |
+      Some NVMEM cells contain MAC in a non-binary format.
+
+      ASCII should be specified if MAC is string formatted like:
+      - "01:23:45:67:89:AB" (30 31 3a 32 33 3a 34 35 3a 36 37 3a 38 39 3a 41 42)
+      - "01-23-45-67-89-AB"
+      - "0123456789AB"
+    enum:
+      - ascii
+
+  reversed-bytes:
+    type: boolean
+    description: |
+      MAC is stored in reversed bytes order. Example:
+      Stored value: AB 89 67 45 23 01
+      Actual MAC: 01 23 45 67 89 AB
+
+  base-address:
+    type: boolean
+    description: |
+      Marks NVMEM cell as provider of multiple addresses that are relative to
+      the one actually stored physically. Respective addresses can be requested
+      by specifying cell index of NVMEM cell.
+
+allOf:
+  - $ref: cell.yaml#
+  - if:
+      required:
+        - base-address
+    then:
+      properties:
+        "#nvmem-cell-cells":
+          const: 1
+      required:
+        - "#nvmem-cell-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    partitions {
+        compatible = "fixed-partitions";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        partition@f00000 {
+            compatible = "nvmem-cells";
+            label = "calibration";
+            reg = <0xf00000 0x100000>;
+            ranges = <0 0xf00000 0x100000>;
+            #address-cells = <1>;
+            #size-cells = <1>;
+
+            mac@100 {
+                compatible = "mac-address";
+                reg = <0x100 0x6>;
+            };
+
+            mac@200 {
+                compatible = "mac-address";
+                reg = <0x200 0x6>;
+                reversed-bytes;
+            };
+
+            mac@300 {
+                compatible = "mac-address";
+                reg = <0x300 0x11>;
+                format = "ascii";
+            };
+
+            mac@400 {
+                compatible = "mac-address";
+                reg = <0x400 0x6>;
+                base-address;
+                #nvmem-cell-cells = <1>;
+            };
+        };
+    };
-- 
2.31.1


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

* Re: [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell
  2022-01-26  7:07   ` [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell Rafał Miłecki
@ 2022-02-01 15:55     ` Rob Herring
  2022-02-01 16:49       ` Rafał Miłecki
  2022-02-01 17:01       ` Michael Walle
  0 siblings, 2 replies; 12+ messages in thread
From: Rob Herring @ 2022-02-01 15:55 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Srinivas Kandagatla, Michael Walle, linux-mtd, devicetree,
	linux-kernel, linux-arm-kernel, netdev, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Shawn Guo, Li Yang,
	Frank Rowand, David S . Miller, Jakub Kicinski, Ansuel Smith,
	Andrew Lunn, Florian Fainelli, Hauke Mehrtens,
	Rafał Miłecki

On Wed, Jan 26, 2022 at 08:07:45AM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This adds support for describing details of NVMEM cell containing MAC
> address. Those are often device specific and could be nicely stored in
> DT.
> 
> Initial documentation includes support for describing:
> 1. Cell data format (e.g. Broadcom's NVRAM uses ASCII to store MAC)
> 2. Reversed bytes flash (required for i.MX6/i.MX7 OCOTP support)
> 3. Source for multiple addresses (very common in home routers)
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../bindings/nvmem/cells/mac-address.yaml     | 94 +++++++++++++++++++
>  1 file changed, 94 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
> new file mode 100644
> index 000000000000..f8d19e87cdf0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
> @@ -0,0 +1,94 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/cells/mac-address.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVMEM cell containing a MAC address
> +
> +maintainers:
> +  - Rafał Miłecki <rafal@milecki.pl>
> +
> +properties:
> +  compatible:
> +    const: mac-address
> +
> +  format:
> +    description: |
> +      Some NVMEM cells contain MAC in a non-binary format.
> +
> +      ASCII should be specified if MAC is string formatted like:
> +      - "01:23:45:67:89:AB" (30 31 3a 32 33 3a 34 35 3a 36 37 3a 38 39 3a 41 42)
> +      - "01-23-45-67-89-AB"
> +      - "0123456789AB"
> +    enum:
> +      - ascii
> +
> +  reversed-bytes:
> +    type: boolean
> +    description: |
> +      MAC is stored in reversed bytes order. Example:
> +      Stored value: AB 89 67 45 23 01
> +      Actual MAC: 01 23 45 67 89 AB
> +
> +  base-address:
> +    type: boolean
> +    description: |
> +      Marks NVMEM cell as provider of multiple addresses that are relative to
> +      the one actually stored physically. Respective addresses can be requested
> +      by specifying cell index of NVMEM cell.

While a base address is common, aren't there different ways the base is 
modified. 

The problem with these properties is every new variation results in a 
new property and the end result is something not well designed. A unique
compatible string, "#nvmem-cell-cells" and code to interpret the data is 
more flexible.

For something like this to fly, I need some level of confidence this is 
enough for everyone for some time (IOW, find all the previous attempts 
and get those people's buy-in). You have found at least 3 cases, but I 
seem to recall more.

Rob

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

* Re: [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell
  2022-02-01 15:55     ` Rob Herring
@ 2022-02-01 16:49       ` Rafał Miłecki
  2022-02-25  9:09         ` Rafał Miłecki
  2022-02-01 17:01       ` Michael Walle
  1 sibling, 1 reply; 12+ messages in thread
From: Rafał Miłecki @ 2022-02-01 16:49 UTC (permalink / raw)
  To: Rob Herring, Michael Walle
  Cc: Rafał Miłecki, Srinivas Kandagatla, linux-mtd,
	devicetree, linux-kernel, linux-arm-kernel, netdev,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Shawn Guo, Li Yang, Frank Rowand, David S . Miller,
	Jakub Kicinski, Ansuel Smith, Andrew Lunn, Florian Fainelli,
	Hauke Mehrtens

On 2022-02-01 16:55, Rob Herring wrote:
> On Wed, Jan 26, 2022 at 08:07:45AM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>> 
>> This adds support for describing details of NVMEM cell containing MAC
>> address. Those are often device specific and could be nicely stored in
>> DT.
>> 
>> Initial documentation includes support for describing:
>> 1. Cell data format (e.g. Broadcom's NVRAM uses ASCII to store MAC)
>> 2. Reversed bytes flash (required for i.MX6/i.MX7 OCOTP support)
>> 3. Source for multiple addresses (very common in home routers)
>> 
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>  .../bindings/nvmem/cells/mac-address.yaml     | 94 
>> +++++++++++++++++++
>>  1 file changed, 94 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml 
>> b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>> new file mode 100644
>> index 000000000000..f8d19e87cdf0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>> @@ -0,0 +1,94 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/nvmem/cells/mac-address.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: NVMEM cell containing a MAC address
>> +
>> +maintainers:
>> +  - Rafał Miłecki <rafal@milecki.pl>
>> +
>> +properties:
>> +  compatible:
>> +    const: mac-address
>> +
>> +  format:
>> +    description: |
>> +      Some NVMEM cells contain MAC in a non-binary format.
>> +
>> +      ASCII should be specified if MAC is string formatted like:
>> +      - "01:23:45:67:89:AB" (30 31 3a 32 33 3a 34 35 3a 36 37 3a 38 
>> 39 3a 41 42)
>> +      - "01-23-45-67-89-AB"
>> +      - "0123456789AB"
>> +    enum:
>> +      - ascii
>> +
>> +  reversed-bytes:
>> +    type: boolean
>> +    description: |
>> +      MAC is stored in reversed bytes order. Example:
>> +      Stored value: AB 89 67 45 23 01
>> +      Actual MAC: 01 23 45 67 89 AB
>> +
>> +  base-address:
>> +    type: boolean
>> +    description: |
>> +      Marks NVMEM cell as provider of multiple addresses that are 
>> relative to
>> +      the one actually stored physically. Respective addresses can be 
>> requested
>> +      by specifying cell index of NVMEM cell.
> 
> While a base address is common, aren't there different ways the base is
> modified.
> 
> The problem with these properties is every new variation results in a
> new property and the end result is something not well designed. A 
> unique
> compatible string, "#nvmem-cell-cells" and code to interpret the data 
> is
> more flexible.
> 
> For something like this to fly, I need some level of confidence this is
> enough for everyone for some time (IOW, find all the previous attempts
> and get those people's buy-in). You have found at least 3 cases, but I
> seem to recall more.

For base address I thought of dealing with base + offset only. I'm not
sure what are other cases.

I read few old threads:
https://lore.kernel.org/lkml/20211228142549.1275412-1-michael@walle.cc/T/
https://lore.kernel.org/linux-devicetree/20211123134425.3875656-1-michael@walle.cc/
https://lore.kernel.org/all/20210414152657.12097-2-michael@walle.cc/
https://lore.kernel.org/linux-devicetree/362f1c6a8b0ec191b285ac6a604500da@walle.cc/

but didn't find other required /transformations/ except for offset and
format. Even "reversed-bytes" wasn't widely discussed (or I missed that)
and I just came with it on my own.

If anyone knows other cases: please share so we have a complete view.


I tried to Cc all previously invovled people but it seems only me and
Michael remained active in this subject. If anyone knows other
interested please Cc them and let us know.


Rob: instead of me and Michael sending patch after patch let me try to
gather solutions I can think of / I recall. Please kindly review them
and let us know what do you find the cleanest.


1. NVMEM specific "compatible" string

Example:

partition@f00000 {
     compatible = "brcm,foo-cells", "nvmem-cells";
     label = "calibration";
     reg = <0xf00000 0x100000>;
     ranges = <0 0xf00000 0x100000>;
     #address-cells = <1>;
     #size-cells = <1>;

     mac@100 {
         reg = <0x100 0x6>;
         [optional: #nvmem-cell-cells = <1>;]
     };
};

A minimalistic binding proposed by Michael. DT doesn't carry any
information on NVMEM cell format. Specific drivers (e.g. one handling
"brcm,foo-cells") have to know how to handle specific cell.

Cell handling conditional code can depend on cell node name ("mac" in
above case) OR on value of "nvmem-cell-names" in cell consumer (e.g.
nvmem-cell-names = "mac-address").


2. NVMEM specific "compatible" string + cells "compatible"s

Example:

partition@f00000 {
     compatible = "brcm,foo-cells", "nvmem-cells";
     label = "calibration";
     reg = <0xf00000 0x100000>;
     ranges = <0 0xf00000 0x100000>;
     #address-cells = <1>;
     #size-cells = <1>;

     mac@100 {
         compatible = "mac-address";
         reg = <0x100 0x6>;
         [optional: #nvmem-cell-cells = <1>;]
     };
};

Similar to the first case but cells that require special handling are
marked with NVMEM device specific "compatible" values. Details of 
handling
cells are still hardcoded in NVMEM driver. Different cells with
compatible = "mac-address";
may be handled differencly - depending on parent NVMEM device.


3. Flexible properties in NVMEM cells

Example:

partition@f00000 {
     compatible = "brcm,foo-cells", "nvmem-cells";
     label = "calibration";
     reg = <0xf00000 0x100000>;
     ranges = <0 0xf00000 0x100000>;
     #address-cells = <1>;
     #size-cells = <1>;

     mac@100 {
         compatible = "mac-address";
         reg = <0x100 0x6>;
         [optional: #nvmem-cell-cells = <1>;]
     };

     mac@200 {
         compatible = "mac-address";
         reg = <0x200 0x6>;
         reversed-bytes;
         [optional: #nvmem-cell-cells = <1>;]
     };

     mac@300 {
         compatible = "mac-address";
         reg = <0x300 0x11>;
         format = "ascii";
         [optional: #nvmem-cell-cells = <1>;]
     };
};

This moves details into DT and requires more shared properties. It helps
avoiding duplicated code for common cases (like base MAC address).

It's what I proposed in the
[PATCH 0/2] dt-bindings: nvmem: support describing cells

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

* Re: [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell
  2022-02-01 15:55     ` Rob Herring
  2022-02-01 16:49       ` Rafał Miłecki
@ 2022-02-01 17:01       ` Michael Walle
  2022-02-25  9:07         ` Rafał Miłecki
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Walle @ 2022-02-01 17:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: Rafał Miłecki, Srinivas Kandagatla, linux-mtd,
	devicetree, linux-kernel, linux-arm-kernel, netdev,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Shawn Guo, Li Yang, Frank Rowand, David S . Miller,
	Jakub Kicinski, Ansuel Smith, Andrew Lunn, Florian Fainelli,
	Hauke Mehrtens, Rafał Miłecki

Am 2022-02-01 16:55, schrieb Rob Herring:
> On Wed, Jan 26, 2022 at 08:07:45AM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>> 
>> This adds support for describing details of NVMEM cell containing MAC
>> address. Those are often device specific and could be nicely stored in
>> DT.
>> 
>> Initial documentation includes support for describing:
>> 1. Cell data format (e.g. Broadcom's NVRAM uses ASCII to store MAC)
>> 2. Reversed bytes flash (required for i.MX6/i.MX7 OCOTP support)
>> 3. Source for multiple addresses (very common in home routers)
>> 
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>  .../bindings/nvmem/cells/mac-address.yaml     | 94 
>> +++++++++++++++++++
>>  1 file changed, 94 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml 
>> b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>> new file mode 100644
>> index 000000000000..f8d19e87cdf0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>> @@ -0,0 +1,94 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/nvmem/cells/mac-address.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: NVMEM cell containing a MAC address
>> +
>> +maintainers:
>> +  - Rafał Miłecki <rafal@milecki.pl>
>> +
>> +properties:
>> +  compatible:
>> +    const: mac-address
>> +
>> +  format:
>> +    description: |
>> +      Some NVMEM cells contain MAC in a non-binary format.
>> +
>> +      ASCII should be specified if MAC is string formatted like:
>> +      - "01:23:45:67:89:AB" (30 31 3a 32 33 3a 34 35 3a 36 37 3a 38 
>> 39 3a 41 42)
>> +      - "01-23-45-67-89-AB"
>> +      - "0123456789AB"
>> +    enum:
>> +      - ascii
>> +
>> +  reversed-bytes:
>> +    type: boolean
>> +    description: |
>> +      MAC is stored in reversed bytes order. Example:
>> +      Stored value: AB 89 67 45 23 01
>> +      Actual MAC: 01 23 45 67 89 AB
>> +
>> +  base-address:
>> +    type: boolean
>> +    description: |
>> +      Marks NVMEM cell as provider of multiple addresses that are 
>> relative to
>> +      the one actually stored physically. Respective addresses can be 
>> requested
>> +      by specifying cell index of NVMEM cell.
> 
> While a base address is common, aren't there different ways the base is
> modified.
> 
> The problem with these properties is every new variation results in a
> new property and the end result is something not well designed. A 
> unique
> compatible string, "#nvmem-cell-cells" and code to interpret the data 
> is
> more flexible.

I actually like having a unique compatible for anything but the basic
operations. For example, the sl28 vpd area also has a checksum, which
could be handled if there is an own compatible. I don't think this is
possible with this proposal. Also there is a version field, what if
we change the layout of that thing? Am I supposed to change the
device tree? The more I think about Rob's proposal to have a compatible
the more I like it.

One of Rafałs concerns are code duplication. I.e. if everything needs
its own compatible string, the driver will also have to have all of 
these.
But I'd say, this is a common thing in most drivers.

That being said, I'd really like to have a consens here as this topic
is open like forever and I was under the impression that at least we
were clear on the device tree side.

-michael

> For something like this to fly, I need some level of confidence this is
> enough for everyone for some time (IOW, find all the previous attempts
> and get those people's buy-in). You have found at least 3 cases, but I
> seem to recall more.
> 
> Rob

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

* Re: [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file
  2022-01-26  7:07 ` [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rafał Miłecki
  2022-01-26  7:07   ` [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell Rafał Miłecki
@ 2022-02-11 12:50   ` Rob Herring
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring @ 2022-02-11 12:50 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Srinivas Kandagatla, Michael Walle, linux-mtd, devicetree,
	linux-kernel, linux-arm-kernel, netdev, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Shawn Guo, Li Yang,
	Frank Rowand, David S . Miller, Jakub Kicinski, Ansuel Smith,
	Andrew Lunn, Florian Fainelli, Hauke Mehrtens,
	Rafał Miłecki

On Wed, Jan 26, 2022 at 08:07:44AM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This will allow adding binding for more specific cells and reusing
> (sharing) common code.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/nvmem/cells/cell.yaml | 34 +++++++++++++++++++

Why the 'cells' subdir? cell.yaml is a bit generic for me as DT defines 
'cell' which is different from nvmem cell. While we have the path to 
distinguish, '$ref: cell.yaml' doesn't.


>  .../devicetree/bindings/nvmem/nvmem.yaml      | 22 +-----------
>  2 files changed, 35 insertions(+), 21 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/cells/cell.yaml
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/cells/cell.yaml b/Documentation/devicetree/bindings/nvmem/cells/cell.yaml
> new file mode 100644
> index 000000000000..adfc2e639f43
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/cells/cell.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/cells/cell.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVMEM cell
> +
> +maintainers:
> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> +
> +description: NVMEM cell is a data entry of NVMEM device.
> +
> +properties:
> +  reg:
> +    maxItems: 1
> +    description:
> +      Offset and size in bytes within the storage device.
> +
> +  bits:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    items:
> +      - minimum: 0
> +        maximum: 7
> +        description:
> +          Offset in bit within the address range specified by reg.
> +      - minimum: 1
> +        description:
> +          Size in bit within the address range specified by reg.
> +
> +required:
> +  - reg
> +
> +additionalProperties: true
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> index 43ed7e32e5ac..b79b51e98ee8 100644
> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> @@ -41,27 +41,7 @@ properties:
>  
>  patternProperties:
>    "@[0-9a-f]+(,[0-7])?$":
> -    type: object
> -
> -    properties:
> -      reg:
> -        maxItems: 1
> -        description:
> -          Offset and size in bytes within the storage device.
> -
> -      bits:
> -        $ref: /schemas/types.yaml#/definitions/uint32-array
> -        items:
> -          - minimum: 0
> -            maximum: 7
> -            description:
> -              Offset in bit within the address range specified by reg.
> -          - minimum: 1
> -            description:
> -              Size in bit within the address range specified by reg.
> -
> -    required:
> -      - reg
> +    $ref: cells/cell.yaml#
>  
>  additionalProperties: true
>  
> -- 
> 2.31.1
> 
> 

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

* Re: [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell
  2022-02-01 17:01       ` Michael Walle
@ 2022-02-25  9:07         ` Rafał Miłecki
  0 siblings, 0 replies; 12+ messages in thread
From: Rafał Miłecki @ 2022-02-25  9:07 UTC (permalink / raw)
  To: Michael Walle, Rob Herring
  Cc: Srinivas Kandagatla, linux-mtd, devicetree, linux-kernel,
	linux-arm-kernel, netdev, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Shawn Guo, Li Yang, Frank Rowand,
	David S . Miller, Jakub Kicinski, Ansuel Smith, Andrew Lunn,
	Florian Fainelli, Hauke Mehrtens, Rafał Miłecki

On 1.02.2022 18:01, Michael Walle wrote:
> Am 2022-02-01 16:55, schrieb Rob Herring:
>> On Wed, Jan 26, 2022 at 08:07:45AM +0100, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> This adds support for describing details of NVMEM cell containing MAC
>>> address. Those are often device specific and could be nicely stored in
>>> DT.
>>>
>>> Initial documentation includes support for describing:
>>> 1. Cell data format (e.g. Broadcom's NVRAM uses ASCII to store MAC)
>>> 2. Reversed bytes flash (required for i.MX6/i.MX7 OCOTP support)
>>> 3. Source for multiple addresses (very common in home routers)
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>>  .../bindings/nvmem/cells/mac-address.yaml     | 94 +++++++++++++++++++
>>>  1 file changed, 94 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>>> new file mode 100644
>>> index 000000000000..f8d19e87cdf0
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>>> @@ -0,0 +1,94 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/nvmem/cells/mac-address.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: NVMEM cell containing a MAC address
>>> +
>>> +maintainers:
>>> +  - Rafał Miłecki <rafal@milecki.pl>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: mac-address
>>> +
>>> +  format:
>>> +    description: |
>>> +      Some NVMEM cells contain MAC in a non-binary format.
>>> +
>>> +      ASCII should be specified if MAC is string formatted like:
>>> +      - "01:23:45:67:89:AB" (30 31 3a 32 33 3a 34 35 3a 36 37 3a 38 39 3a 41 42)
>>> +      - "01-23-45-67-89-AB"
>>> +      - "0123456789AB"
>>> +    enum:
>>> +      - ascii
>>> +
>>> +  reversed-bytes:
>>> +    type: boolean
>>> +    description: |
>>> +      MAC is stored in reversed bytes order. Example:
>>> +      Stored value: AB 89 67 45 23 01
>>> +      Actual MAC: 01 23 45 67 89 AB
>>> +
>>> +  base-address:
>>> +    type: boolean
>>> +    description: |
>>> +      Marks NVMEM cell as provider of multiple addresses that are relative to
>>> +      the one actually stored physically. Respective addresses can be requested
>>> +      by specifying cell index of NVMEM cell.
>>
>> While a base address is common, aren't there different ways the base is
>> modified.
>>
>> The problem with these properties is every new variation results in a
>> new property and the end result is something not well designed. A unique
>> compatible string, "#nvmem-cell-cells" and code to interpret the data is
>> more flexible.
> 
> I actually like having a unique compatible for anything but the basic
> operations. For example, the sl28 vpd area also has a checksum, which
> could be handled if there is an own compatible. I don't think this is
> possible with this proposal. Also there is a version field, what if
> we change the layout of that thing? Am I supposed to change the
> device tree? The more I think about Rob's proposal to have a compatible
> the more I like it.

Having more detailed binding for cells doesn't stop you from using NVMEM
device specific binding.

You could have e.g.

partition@f00000 {
      compatible = "foo,foo-cells", "nvmem-cells";
      (...)

      mac@100 {
          compatible = "mac-address";
          reg = <0x100 0x6>;
      };

      OR

      mac@100 {
          compatible = "mac-address";
          reg = <0x100 0x11>;
          format = "ascii";
      };
};

Then you can have "foo,foo-cells" driver handling checksum.

I'm not saying we have to use this solution, just saying it's possible.

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

* Re: [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell
  2022-02-01 16:49       ` Rafał Miłecki
@ 2022-02-25  9:09         ` Rafał Miłecki
  0 siblings, 0 replies; 12+ messages in thread
From: Rafał Miłecki @ 2022-02-25  9:09 UTC (permalink / raw)
  To: Rafał Miłecki, Rob Herring, Michael Walle
  Cc: Srinivas Kandagatla, linux-mtd, devicetree, linux-kernel,
	linux-arm-kernel, netdev, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Shawn Guo, Li Yang, Frank Rowand,
	David S . Miller, Jakub Kicinski, Ansuel Smith, Andrew Lunn,
	Florian Fainelli, Hauke Mehrtens

On 1.02.2022 17:49, Rafał Miłecki wrote:
> On 2022-02-01 16:55, Rob Herring wrote:
>> On Wed, Jan 26, 2022 at 08:07:45AM +0100, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> This adds support for describing details of NVMEM cell containing MAC
>>> address. Those are often device specific and could be nicely stored in
>>> DT.
>>>
>>> Initial documentation includes support for describing:
>>> 1. Cell data format (e.g. Broadcom's NVRAM uses ASCII to store MAC)
>>> 2. Reversed bytes flash (required for i.MX6/i.MX7 OCOTP support)
>>> 3. Source for multiple addresses (very common in home routers)
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>>  .../bindings/nvmem/cells/mac-address.yaml     | 94 +++++++++++++++++++
>>>  1 file changed, 94 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>>> new file mode 100644
>>> index 000000000000..f8d19e87cdf0
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/nvmem/cells/mac-address.yaml
>>> @@ -0,0 +1,94 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/nvmem/cells/mac-address.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: NVMEM cell containing a MAC address
>>> +
>>> +maintainers:
>>> +  - Rafał Miłecki <rafal@milecki.pl>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: mac-address
>>> +
>>> +  format:
>>> +    description: |
>>> +      Some NVMEM cells contain MAC in a non-binary format.
>>> +
>>> +      ASCII should be specified if MAC is string formatted like:
>>> +      - "01:23:45:67:89:AB" (30 31 3a 32 33 3a 34 35 3a 36 37 3a 38 39 3a 41 42)
>>> +      - "01-23-45-67-89-AB"
>>> +      - "0123456789AB"
>>> +    enum:
>>> +      - ascii
>>> +
>>> +  reversed-bytes:
>>> +    type: boolean
>>> +    description: |
>>> +      MAC is stored in reversed bytes order. Example:
>>> +      Stored value: AB 89 67 45 23 01
>>> +      Actual MAC: 01 23 45 67 89 AB
>>> +
>>> +  base-address:
>>> +    type: boolean
>>> +    description: |
>>> +      Marks NVMEM cell as provider of multiple addresses that are relative to
>>> +      the one actually stored physically. Respective addresses can be requested
>>> +      by specifying cell index of NVMEM cell.
>>
>> While a base address is common, aren't there different ways the base is
>> modified.
>>
>> The problem with these properties is every new variation results in a
>> new property and the end result is something not well designed. A unique
>> compatible string, "#nvmem-cell-cells" and code to interpret the data is
>> more flexible.
>>
>> For something like this to fly, I need some level of confidence this is
>> enough for everyone for some time (IOW, find all the previous attempts
>> and get those people's buy-in). You have found at least 3 cases, but I
>> seem to recall more.
> 
> For base address I thought of dealing with base + offset only. I'm not
> sure what are other cases.
> 
> I read few old threads:
> https://lore.kernel.org/lkml/20211228142549.1275412-1-michael@walle.cc/T/
> https://lore.kernel.org/linux-devicetree/20211123134425.3875656-1-michael@walle.cc/
> https://lore.kernel.org/all/20210414152657.12097-2-michael@walle.cc/
> https://lore.kernel.org/linux-devicetree/362f1c6a8b0ec191b285ac6a604500da@walle.cc/
> 
> but didn't find other required /transformations/ except for offset and
> format. Even "reversed-bytes" wasn't widely discussed (or I missed that)
> and I just came with it on my own.
> 
> If anyone knows other cases: please share so we have a complete view.
> 
> 
> I tried to Cc all previously invovled people but it seems only me and
> Michael remained active in this subject. If anyone knows other
> interested please Cc them and let us know.
> 
> 
> Rob: instead of me and Michael sending patch after patch let me try to
> gather solutions I can think of / I recall. Please kindly review them
> and let us know what do you find the cleanest.
> 
> 
> 1. NVMEM specific "compatible" string
> 
> Example:
> 
> partition@f00000 {
>      compatible = "brcm,foo-cells", "nvmem-cells";
>      label = "calibration";
>      reg = <0xf00000 0x100000>;
>      ranges = <0 0xf00000 0x100000>;
>      #address-cells = <1>;
>      #size-cells = <1>;
> 
>      mac@100 {
>          reg = <0x100 0x6>;
>          [optional: #nvmem-cell-cells = <1>;]
>      };
> };
> 
> A minimalistic binding proposed by Michael. DT doesn't carry any
> information on NVMEM cell format. Specific drivers (e.g. one handling
> "brcm,foo-cells") have to know how to handle specific cell.
> 
> Cell handling conditional code can depend on cell node name ("mac" in
> above case) OR on value of "nvmem-cell-names" in cell consumer (e.g.
> nvmem-cell-names = "mac-address").
> 
> 
> 2. NVMEM specific "compatible" string + cells "compatible"s
> 
> Example:
> 
> partition@f00000 {
>      compatible = "brcm,foo-cells", "nvmem-cells";
>      label = "calibration";
>      reg = <0xf00000 0x100000>;
>      ranges = <0 0xf00000 0x100000>;
>      #address-cells = <1>;
>      #size-cells = <1>;
> 
>      mac@100 {
>          compatible = "mac-address";
>          reg = <0x100 0x6>;
>          [optional: #nvmem-cell-cells = <1>;]
>      };
> };
> 
> Similar to the first case but cells that require special handling are
> marked with NVMEM device specific "compatible" values. Details of handling
> cells are still hardcoded in NVMEM driver. Different cells with
> compatible = "mac-address";
> may be handled differencly - depending on parent NVMEM device.
> 
> 
> 3. Flexible properties in NVMEM cells
> 
> Example:
> 
> partition@f00000 {
>      compatible = "brcm,foo-cells", "nvmem-cells";
>      label = "calibration";
>      reg = <0xf00000 0x100000>;
>      ranges = <0 0xf00000 0x100000>;
>      #address-cells = <1>;
>      #size-cells = <1>;
> 
>      mac@100 {
>          compatible = "mac-address";
>          reg = <0x100 0x6>;
>          [optional: #nvmem-cell-cells = <1>;]
>      };
> 
>      mac@200 {
>          compatible = "mac-address";
>          reg = <0x200 0x6>;
>          reversed-bytes;
>          [optional: #nvmem-cell-cells = <1>;]
>      };
> 
>      mac@300 {
>          compatible = "mac-address";
>          reg = <0x300 0x11>;
>          format = "ascii";
>          [optional: #nvmem-cell-cells = <1>;]
>      };
> };
> 
> This moves details into DT and requires more shared properties. It helps
> avoiding duplicated code for common cases (like base MAC address).
> 
> It's what I proposed in the
> [PATCH 0/2] dt-bindings: nvmem: support describing cells

Rob: could you review for us 3 above examples, please?

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

end of thread, other threads:[~2022-02-25  9:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-25 18:01 [PATCH 0/2] dt-bindings: nvmem: support describing cells Rafał Miłecki
2022-01-25 18:01 ` [PATCH 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rafał Miłecki
2022-01-25 18:01 ` [PATCH 2/2] dt-bindings: nvmem: cells: add MAC address cell Rafał Miłecki
2022-01-26  3:29   ` Rob Herring
2022-01-26  7:07 ` [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rafał Miłecki
2022-01-26  7:07   ` [PATCH REBASED 2/2] dt-bindings: nvmem: cells: add MAC address cell Rafał Miłecki
2022-02-01 15:55     ` Rob Herring
2022-02-01 16:49       ` Rafał Miłecki
2022-02-25  9:09         ` Rafał Miłecki
2022-02-01 17:01       ` Michael Walle
2022-02-25  9:07         ` Rafał Miłecki
2022-02-11 12:50   ` [PATCH REBASED 1/2] dt-bindings: nvmem: extract NVMEM cell to separated file Rob Herring

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