[PATCH] dt-bindings: nvmem: fixed-cell: add compatibles for MAC cells

[PATCH] dt-bindings: nvmem: fixed-cell: add compatibles for MAC cells

[Rafał Miłecki]

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

A lot of home routers have NVMEM fixed cells containing MAC address that
need some further processing. In ~99% cases MAC needs to be:
1. Optionally parsed from ASCII format
2. Increased by a vendor-picked value

There was already an attempt to design a binding for that at NVMEM
device level in the past. It wasn't accepted though as it didn't really
fit NVMEM device layer.

The introduction of NVMEM fixed-cells layout seems to be an opportunity
to provide a relevant binding in a clean way.

This commit adds two *generic* compatible strings: "mac-base" and
"mac-ascii". As always those need to be carefully reviewed.

OpenWrt project currently supports ~300 home routers that would benefit
from the "mac-base" binding. Those devices are manufactured by multiple
vendors. There are TP-Link devices (76 of them), Netgear (19),
D-Link (11), OpenMesh (9), EnGenius (8), GL.iNet (8), ZTE (7),
Xiaomi (5), Ubiquiti (6) and more. Those devices don't share an
architecture or SoC.

Amount of devices to benefit from the "mac-ascii" is hard to determine
as not all of them were converted to DT yet. There are at least 200 of
such devices.

It would be impractical to provide unique "compatible" strings for NVMEM
layouts of all those devices. It seems like a valid case for allowing a
generic binding instead. Even if this binding will not be sufficient for
some further devices it seems to be useful enough as it is.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
If this binding gets approved I will still need a minor help with YAML.

For some reason my conditions in fixed-cell.yaml don't seem to work as
expected. I tried to make "#nvmem-cell-cells" required only for the
"mac-base" but it seems it got required for all cells:

  DTC_CHK Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.example.dtb
Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.example.dtb: nvmem-layout: calibration@4000: '#nvmem-cell-cells' is a required property
        From schema: Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml

Cell "calibration" doesn't have any "compatible" so it shouldn't require
"#nvmem-cell-cells".
Can someone hint me what I did wrong, please?
---
 .../bindings/nvmem/layouts/fixed-cell.yaml    | 35 +++++++++++++++++++
 .../bindings/nvmem/layouts/fixed-layout.yaml  | 12 +++++++
 .../devicetree/bindings/nvmem/nvmem.yaml      |  5 ++-
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
index e698098450e1..047e42438a4f 100644
--- a/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
+++ b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
@@ -11,6 +11,17 @@ maintainers:
   - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
 
 properties:
+  compatible:
+    oneOf:
+      - const: mac-base
+        description: >
+          Cell with base MAC address to be used for calculating extra relative
+          addresses.
+      - const: mac-ascii
+        description: >
+          Cell with base MAC address stored in an ASCII format (like
+          "00:11:22:33:44:55").
+
   reg:
     maxItems: 1
 
@@ -25,6 +36,30 @@ properties:
         description:
           Size in bit within the address range specified by reg.
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mac-base
+    then:
+      properties:
+        "#nvmem-cell-cells":
+          description: The first argument is a MAC address offset.
+          const: 1
+      required:
+        - "#nvmem-cell-cells"
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mac-ascii
+    then:
+      properties:
+        "#nvmem-cell-cells":
+          description: The first argument is a MAC address offset.
+          const: 1
+
 required:
   - reg
 
diff --git a/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml b/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
index c271537d0714..05b8230cd18c 100644
--- a/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
+++ b/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
@@ -44,6 +44,18 @@ examples:
         #address-cells = <1>;
         #size-cells = <1>;
 
+        mac@100 {
+            compatible = "mac-base";
+            reg = <0x100 0xc>;
+            #nvmem-cell-cells = <1>;
+        };
+
+        mac@110 {
+            compatible = "mac-ascii";
+            reg = <0x110 0x11>;
+            #nvmem-cell-cells = <1>;
+        };
+
         calibration@4000 {
             reg = <0x4000 0x100>;
         };
diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 980244100690..9f921d940142 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -49,7 +49,10 @@ properties:
 patternProperties:
   "@[0-9a-f]+(,[0-7])?$":
     type: object
-    $ref: layouts/fixed-cell.yaml
+    allOf:
+      - $ref: layouts/fixed-cell.yaml
+      - properties:
+          compatible: false
     deprecated: true
 
 additionalProperties: true
-- 
2.35.3

Re: [PATCH] dt-bindings: nvmem: fixed-cell: add compatibles for MAC cells

[Rob Herring]

On Fri, Jun 16, 2023 at 11:30:33PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> A lot of home routers have NVMEM fixed cells containing MAC address that
> need some further processing. In ~99% cases MAC needs to be:
> 1. Optionally parsed from ASCII format
> 2. Increased by a vendor-picked value
> 
> There was already an attempt to design a binding for that at NVMEM
> device level in the past. It wasn't accepted though as it didn't really
> fit NVMEM device layer.
> 
> The introduction of NVMEM fixed-cells layout seems to be an opportunity
> to provide a relevant binding in a clean way.
> 
> This commit adds two *generic* compatible strings: "mac-base" and
> "mac-ascii". As always those need to be carefully reviewed.
> 
> OpenWrt project currently supports ~300 home routers that would benefit
> from the "mac-base" binding. Those devices are manufactured by multiple
> vendors. There are TP-Link devices (76 of them), Netgear (19),
> D-Link (11), OpenMesh (9), EnGenius (8), GL.iNet (8), ZTE (7),
> Xiaomi (5), Ubiquiti (6) and more. Those devices don't share an
> architecture or SoC.
> 
> Amount of devices to benefit from the "mac-ascii" is hard to determine
> as not all of them were converted to DT yet. There are at least 200 of
> such devices.
> 
> It would be impractical to provide unique "compatible" strings for NVMEM
> layouts of all those devices. It seems like a valid case for allowing a
> generic binding instead. Even if this binding will not be sufficient for
> some further devices it seems to be useful enough as it is.

I'm generally okay with this approach as it's not trying to handle all 
permutations with properties. Anything odd can have a specific 
compatible easily.

> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> If this binding gets approved I will still need a minor help with YAML.
> 
> For some reason my conditions in fixed-cell.yaml don't seem to work as
> expected. I tried to make "#nvmem-cell-cells" required only for the
> "mac-base" but it seems it got required for all cells:

Answer below.

>   DTC_CHK Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.example.dtb
> Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.example.dtb: nvmem-layout: calibration@4000: '#nvmem-cell-cells' is a required property
>         From schema: Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
> 
> Cell "calibration" doesn't have any "compatible" so it shouldn't require
> "#nvmem-cell-cells".
> Can someone hint me what I did wrong, please?
> ---
>  .../bindings/nvmem/layouts/fixed-cell.yaml    | 35 +++++++++++++++++++
>  .../bindings/nvmem/layouts/fixed-layout.yaml  | 12 +++++++
>  .../devicetree/bindings/nvmem/nvmem.yaml      |  5 ++-
>  3 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
> index e698098450e1..047e42438a4f 100644
> --- a/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
> @@ -11,6 +11,17 @@ maintainers:
>    - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>  
>  properties:
> +  compatible:
> +    oneOf:
> +      - const: mac-base
> +        description: >
> +          Cell with base MAC address to be used for calculating extra relative
> +          addresses.
> +      - const: mac-ascii
> +        description: >
> +          Cell with base MAC address stored in an ASCII format (like
> +          "00:11:22:33:44:55").

Isn't ASCII detectable? Just look at the length or that all values are 
0x3?. Though I can't make sense of the lengths your examples have.

> +
>    reg:
>      maxItems: 1
>  
> @@ -25,6 +36,30 @@ properties:
>          description:
>            Size in bit within the address range specified by reg.
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mac-base

This will be true with no compatible property. You need 'required: [ 
compatible ]' in addition.

> +    then:
> +      properties:
> +        "#nvmem-cell-cells":
> +          description: The first argument is a MAC address offset.
> +          const: 1
> +      required:
> +        - "#nvmem-cell-cells"
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mac-ascii
> +    then:
> +      properties:
> +        "#nvmem-cell-cells":
> +          description: The first argument is a MAC address offset.
> +          const: 1
> +
>  required:
>    - reg
>  
> diff --git a/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml b/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
> index c271537d0714..05b8230cd18c 100644
> --- a/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
> @@ -44,6 +44,18 @@ examples:
>          #address-cells = <1>;
>          #size-cells = <1>;
>  
> +        mac@100 {
> +            compatible = "mac-base";
> +            reg = <0x100 0xc>;
> +            #nvmem-cell-cells = <1>;
> +        };
> +
> +        mac@110 {
> +            compatible = "mac-ascii";
> +            reg = <0x110 0x11>;
> +            #nvmem-cell-cells = <1>;
> +        };
> +
>          calibration@4000 {
>              reg = <0x4000 0x100>;
>          };
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> index 980244100690..9f921d940142 100644
> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> @@ -49,7 +49,10 @@ properties:
>  patternProperties:
>    "@[0-9a-f]+(,[0-7])?$":
>      type: object
> -    $ref: layouts/fixed-cell.yaml
> +    allOf:
> +      - $ref: layouts/fixed-cell.yaml
> +      - properties:
> +          compatible: false
>      deprecated: true
>  
>  additionalProperties: true
> -- 
> 2.35.3
> 

Re: [PATCH] dt-bindings: nvmem: fixed-cell: add compatibles for MAC cells

[Srinivas Kandagatla]

On Fri, 16 Jun 2023 23:30:33 +0200, Rafał Miłecki wrote:
> A lot of home routers have NVMEM fixed cells containing MAC address that
> need some further processing. In ~99% cases MAC needs to be:
> 1. Optionally parsed from ASCII format
> 2. Increased by a vendor-picked value
> 
> There was already an attempt to design a binding for that at NVMEM
> device level in the past. It wasn't accepted though as it didn't really
> fit NVMEM device layer.
> 
> [...]

Applied, thanks!

[1/1] dt-bindings: nvmem: fixed-cell: add compatibles for MAC cells
      commit: a7964674427bdf5aa9ff342e4dfb8a4d345851a1

Best regards,
-- 
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>