From: Rafał Miłecki <[email protected]>
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/[email protected]/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
From: Rafał Miłecki <[email protected]>
This will allow adding binding for more specific cells and reusing
(sharing) common code.
Signed-off-by: Rafał Miłecki <[email protected]>
---
.../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 <[email protected]>
+
+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
From: Rafał Miłecki <[email protected]>
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 <[email protected]>
---
.../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 <[email protected]>
+
+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
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 <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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
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 <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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/[email protected]/T/
https://lore.kernel.org/linux-devicetree/[email protected]/
https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/linux-devicetree/[email protected]/
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
On Wed, Jan 26, 2022 at 08:07:45AM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> 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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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
On Wed, Jan 26, 2022 at 08:07:44AM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> This will allow adding binding for more specific cells and reusing
> (sharing) common code.
>
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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
>
>
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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>> ---
>>> .../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 <[email protected]>
>>> +
>>> +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/[email protected]/T/
> https://lore.kernel.org/linux-devicetree/[email protected]/
> https://lore.kernel.org/all/[email protected]/
> https://lore.kernel.org/linux-devicetree/[email protected]/
>
> 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?
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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>> ---
>>> .../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 <[email protected]>
>>> +
>>> +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.