[PATCH 0/5] ONIE tlv nvmem layout support

[PATCH 0/5] ONIE tlv nvmem layout support

[Miquel Raynal]

Hello,

Here is a series bringing support for an NVMEM layout parser. The table
that will get processed has been standardized by the ONIE project [1]
and its content is highly dependent on the manufacturer choices. There
is a dedicated process to read it, but in no case we can define the
nvmem cells location/length statically in the DT like other NVMEM
cells. Instead, we need what the "layout" abstraction proposed here [2]
brings: a dynamic way to find and export NVMEM cells. So this series is
actually dependent on [2] and cannot be merged without it.

The mvpp2 patch is an example of use which was useful to me during my
test runs, so I figured out it might make sense to upstream it. I am not
100% convinced this is the right way so reviews there are welcome.

[1] https://opencomputeproject.github.io/onie/design-spec/hw_requirements.html
[2] https://lore.kernel.org/linux-arm-kernel/20220921115813.208ff789@xps-13/T/

Cheers,
Miquèl

Miquel Raynal (5):
  dt-bindings: vendor-prefixes: Add ONIE
  dt-bindings: nvmem: add YAML schema for the ONIE tlv layout
  nvmem: layouts: Add ONIE tlv layout driver
  MAINTAINERS: Add myself as ONIE tlv NVMEM layout maintainer
  net: mvpp2: Consider NVMEM cells as possible MAC address source

 .../nvmem/layouts/onie,tlv-layout.yaml        |  96 +++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   6 +
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |   6 +
 drivers/nvmem/layouts/Kconfig                 |   9 +
 drivers/nvmem/layouts/Makefile                |   1 +
 drivers/nvmem/layouts/onie-tlv.c              | 240 ++++++++++++++++++
 7 files changed, 360 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.yaml
 create mode 100644 drivers/nvmem/layouts/onie-tlv.c

-- 
2.34.1

[PATCH 1/5] dt-bindings: vendor-prefixes: Add ONIE

[Miquel Raynal]

As described on their website (see link below),

   "The Open Network Install Environment (ONIE) is an open source
    initiative that defines an open “install environment” for modern
    networking hardware."

It is not a proper corporation per-se but rather more a group which
tries to spread the use of open source standards in the networking
hardware world.

Link: https://opencomputeproject.github.io/onie/
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---

Please note ONIE is not a "company" but rather more an open source
group. I don't know if there will be other uses of this prefix but I
figured out it would be best to describe it to avoid warnings, but I'm
open to other solutions otherwise.

 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6e323a380294..65a74026cf2b 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -927,6 +927,8 @@ patternProperties:
     description: One Laptop Per Child
   "^oneplus,.*":
     description: OnePlus Technology (Shenzhen) Co., Ltd.
+  "^onie,.*":
+    description: Open Network Install Environment group
   "^onion,.*":
     description: Onion Corporation
   "^onnn,.*":
-- 
2.34.1

Re: [PATCH 1/5] dt-bindings: vendor-prefixes: Add ONIE

[Rob Herring]

On Fri, 28 Oct 2022 11:23:33 +0200, Miquel Raynal wrote:
> As described on their website (see link below),
> 
>    "The Open Network Install Environment (ONIE) is an open source
>     initiative that defines an open “install environment” for modern
>     networking hardware."
> 
> It is not a proper corporation per-se but rather more a group which
> tries to spread the use of open source standards in the networking
> hardware world.
> 
> Link: https://opencomputeproject.github.io/onie/
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> 
> Please note ONIE is not a "company" but rather more an open source
> group. I don't know if there will be other uses of this prefix but I
> figured out it would be best to describe it to avoid warnings, but I'm
> open to other solutions otherwise.
> 
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 2/5] dt-bindings: nvmem: add YAML schema for the ONIE tlv layout

[Miquel Raynal]

Add a schema for the ONIE tlv NVMEM layout that can be found on any ONIE
compatible networking device.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../nvmem/layouts/onie,tlv-layout.yaml        | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.yaml

diff --git a/Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.yaml b/Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.yaml
new file mode 100644
index 000000000000..388547d46646
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/layouts/onie,tlv-layout.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVMEM layout of the ONIE tlv table
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+description:
+  Modern networking hardware implementing the Open Compute Project ONIE
+  infrastructure shall provide a non-volatile memory with a table whose the
+  content is well specified and gives many information about the manufacturer
+  (name, country of manufacture, etc) as well as device caracteristics (serial
+  number, hardware version, mac addresses, etc). The underlaying device type
+  (flash, EEPROM,...) is not specified. The exact location of each value is also
+  dynamic and should be discovered at run time because it depends on the
+  parameters the manufacturer decided to embed.
+
+allOf:
+  - $ref: "../nvmem.yaml#"
+
+select:
+  properties:
+    compatible:
+      contains:
+        const: onie,tlv-layout
+  required:
+    - compatible
+
+properties:
+  compatible: true
+
+  product-name: true
+
+  part-number: true
+
+  serial-number: true
+
+  mac-address:
+    type: object
+    description:
+      Base MAC address for all on-module network interfaces. The first
+      argument of the phandle will be treated as an offset.
+
+    properties:
+      "#nvmem-cell-cells":
+        const: 1
+
+    additionalProperties: false
+
+  manufacture-date: true
+
+  device-version: true
+
+  label-revision: true
+
+  platforn-name: true
+
+  onie-version: true
+
+  num-macs: true
+
+  manufacturer: true
+
+  country-code: true
+
+  vendor: true
+
+  diag-version: true
+
+  service-tag: true
+
+  vendor-extension: true
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+        onie {
+            compatible = "onie,tlv-layout", "vendor,device";
+
+            serial_number: serial-number {
+            };
+
+            mac_address: mac-address {
+                #nvmem-cell-cells = <1>;
+            };
+        };
+
+...
-- 
2.34.1

Re: [PATCH 2/5] dt-bindings: nvmem: add YAML schema for the ONIE tlv layout

[Rob Herring]

On Fri, 28 Oct 2022 11:23:34 +0200, Miquel Raynal wrote:
> Add a schema for the ONIE tlv NVMEM layout that can be found on any ONIE
> compatible networking device.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../nvmem/layouts/onie,tlv-layout.yaml        | 96 +++++++++++++++++++
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.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:
Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.example.dtb:0:0: /example-0/onie: failed to match any schema with compatible: ['onie,tlv-layout', 'vendor,device']

doc reference errors (make refcheckdocs):

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

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.

Re: [PATCH 2/5] dt-bindings: nvmem: add YAML schema for the ONIE tlv layout

[Miquel Raynal]

Hi Rob & Krzysztof,

robh@kernel.org wrote on Fri, 28 Oct 2022 07:20:05 -0500:

> On Fri, 28 Oct 2022 11:23:34 +0200, Miquel Raynal wrote:
> > Add a schema for the ONIE tlv NVMEM layout that can be found on any ONIE
> > compatible networking device.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  .../nvmem/layouts/onie,tlv-layout.yaml        | 96 +++++++++++++++++++
> >  1 file changed, 96 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.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:
> Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.example.dtb:0:0: /example-0/onie: failed to match any schema with compatible: ['onie,tlv-layout', 'vendor,device']

Oh right, I wanted to ask about this under the three --- but I forgot.
Here was my question:

How do we make the checker happy with an example where the second
compatible can be almost anything (any nvmem-compatible device) but the
first one should be the layout? (this is currently what Michael's
proposal uses).

> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/patch/
> 
> 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.
> 


Thanks,
Miquèl

Re: [PATCH 2/5] dt-bindings: nvmem: add YAML schema for the ONIE tlv layout

[Rob Herring]

On Fri, Oct 28, 2022 at 03:44:31PM +0200, Miquel Raynal wrote:
> Hi Rob & Krzysztof,
> 
> robh@kernel.org wrote on Fri, 28 Oct 2022 07:20:05 -0500:
> 
> > On Fri, 28 Oct 2022 11:23:34 +0200, Miquel Raynal wrote:
> > > Add a schema for the ONIE tlv NVMEM layout that can be found on any ONIE
> > > compatible networking device.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  .../nvmem/layouts/onie,tlv-layout.yaml        | 96 +++++++++++++++++++
> > >  1 file changed, 96 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.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:
> > Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.example.dtb:0:0: /example-0/onie: failed to match any schema with compatible: ['onie,tlv-layout', 'vendor,device']
> 
> Oh right, I wanted to ask about this under the three --- but I forgot.
> Here was my question:
> 
> How do we make the checker happy with an example where the second
> compatible can be almost anything (any nvmem-compatible device) but the
> first one should be the layout? (this is currently what Michael's
> proposal uses).

That seems like mixing 2 different meanings for compatibles. Perhaps 
that should be split with the nvmem stuff going into a child container 
node.

Rob

P.S. Any compatible string starting with 'foo' will pass, but I probably 
won't be happy to see that used.

Re: [PATCH 2/5] dt-bindings: nvmem: add YAML schema for the ONIE tlv layout

[Miquel Raynal]

Hi Rob,

robh@kernel.org wrote on Fri, 28 Oct 2022 16:35:56 -0500:

> On Fri, Oct 28, 2022 at 03:44:31PM +0200, Miquel Raynal wrote:
> > Hi Rob & Krzysztof,
> > 
> > robh@kernel.org wrote on Fri, 28 Oct 2022 07:20:05 -0500:
> >   
> > > On Fri, 28 Oct 2022 11:23:34 +0200, Miquel Raynal wrote:  
> > > > Add a schema for the ONIE tlv NVMEM layout that can be found on any ONIE
> > > > compatible networking device.
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  .../nvmem/layouts/onie,tlv-layout.yaml        | 96 +++++++++++++++++++
> > > >  1 file changed, 96 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.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:
> > > Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.example.dtb:0:0: /example-0/onie: failed to match any schema with compatible: ['onie,tlv-layout', 'vendor,device']  
> > 
> > Oh right, I wanted to ask about this under the three --- but I forgot.
> > Here was my question:
> > 
> > How do we make the checker happy with an example where the second
> > compatible can be almost anything (any nvmem-compatible device) but the
> > first one should be the layout? (this is currently what Michael's
> > proposal uses).  
> 
> That seems like mixing 2 different meanings for compatibles. Perhaps 
> that should be split with the nvmem stuff going into a child container 
> node.
> 
> Rob
> 
> P.S. Any compatible string starting with 'foo' will pass, but I probably 
> won't be happy to see that used.

Ok, I've scratched my forehead a little bit and came with something (I
hope) better. I've taken over the binding patches from Michael's
original series to show how they conform with my changes. Basically
I've introduced an nvmem-layout container node which will improve a lot
the description without mixing everything. More details in the upcoming
series.

Thanks, Miquèl

[PATCH 3/5] nvmem: layouts: Add ONIE tlv layout driver

[Miquel Raynal]

This layout applies no top of any non volatile storage device containing
an ONIE table factory flashed. This table follows the tlv
(type-length-value) organization described in the link below. We cannot
afford using regular parsers because the content of these tables is
manufacturer specific and must be dynamically discovered.

Link: https://opencomputeproject.github.io/onie/design-spec/hw_requirements.html
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/nvmem/layouts/Kconfig    |   9 ++
 drivers/nvmem/layouts/Makefile   |   1 +
 drivers/nvmem/layouts/onie-tlv.c | 240 +++++++++++++++++++++++++++++++
 3 files changed, 250 insertions(+)
 create mode 100644 drivers/nvmem/layouts/onie-tlv.c

diff --git a/drivers/nvmem/layouts/Kconfig b/drivers/nvmem/layouts/Kconfig
index 3db0c139a8b7..ff346f9f9273 100644
--- a/drivers/nvmem/layouts/Kconfig
+++ b/drivers/nvmem/layouts/Kconfig
@@ -19,4 +19,13 @@ config NVMEM_LAYOUT_U_BOOT_ENV
 
 	  If unsure, say N.
 
+config NVMEM_LAYOUT_ONIE_TLV
+	bool "ONIE tlv support"
+	select CRC32
+	help
+	  Say Y here if you want to support the Open Compute Project ONIE
+	  Type-Length-Value standard table.
+
+	  If unsure, say N.
+
 endmenu
diff --git a/drivers/nvmem/layouts/Makefile b/drivers/nvmem/layouts/Makefile
index dae93fff2b85..0ec076cf541d 100644
--- a/drivers/nvmem/layouts/Makefile
+++ b/drivers/nvmem/layouts/Makefile
@@ -5,3 +5,4 @@
 
 obj-$(CONFIG_NVMEM_LAYOUT_SL28_VPD) += sl28vpd.o
 obj-$(CONFIG_NVMEM_LAYOUT_U_BOOT_ENV) += u-boot-env.o
+obj-$(CONFIG_NVMEM_LAYOUT_ONIE_TLV) += onie-tlv.o
diff --git a/drivers/nvmem/layouts/onie-tlv.c b/drivers/nvmem/layouts/onie-tlv.c
new file mode 100644
index 000000000000..5b33f283d8dc
--- /dev/null
+++ b/drivers/nvmem/layouts/onie-tlv.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ONIE tlv NVMEM cells provider
+ *
+ * Copyright (C) 2022 Open Compute Group ONIE
+ * Author: Miquel Raynal <miquel.raynal@bootlin.com>
+ * Based on the nvmem driver written by: Vadym Kochan <vadym.kochan@plvision.eu>
+ * Inspired by the first layout written by: Rafał Miłecki <rafal@milecki.pl>
+ */
+
+#include <linux/crc32.h>
+#include <linux/etherdevice.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of.h>
+
+#define ONIE_TLV_MAX_LEN 2048
+#define ONIE_TLV_CRC_FIELD_SZ 6
+#define ONIE_TLV_CRC_SZ 4
+#define ONIE_TLV_HDR_ID	"TlvInfo"
+
+struct onie_tlv_hdr {
+	u8 id[8];
+	u8 version;
+	__be16 data_len;
+} __packed;
+
+struct onie_tlv {
+	u8 type;
+	u8 len;
+} __packed;
+
+static const char *onie_tlv_cell_name(u8 type)
+{
+	switch (type) {
+	case 0x21:
+		return "product-name";
+	case 0x22:
+		return "part-number";
+	case 0x23:
+		return "serial-number";
+	case 0x24:
+		return "mac-address";
+	case 0x25:
+		return "manufacture-date";
+	case 0x26:
+		return "device-version";
+	case 0x27:
+		return "label-revision";
+	case 0x28:
+		return "platforn-name";
+	case 0x29:
+		return "onie-version";
+	case 0x2A:
+		return "num-macs";
+	case 0x2B:
+		return "manufacturer";
+	case 0x2C:
+		return "country-code";
+	case 0x2D:
+		return "vendor";
+	case 0x2E:
+		return "diag-version";
+	case 0x2F:
+		return "service-tag";
+	case 0xFD:
+		return "vendor-extension";
+	case 0xFE:
+		return "crc32";
+	default:
+		break;
+	}
+
+	return NULL;
+}
+
+static int onie_tlv_mac_read_cb(const char *id, int index,
+				unsigned int offset, void *buf,
+				size_t bytes)
+{
+	eth_addr_add(buf, index);
+
+	return 0;
+}
+
+static nvmem_cell_post_process_t onie_tlv_read_cb(u8 type, u8 *buf)
+{
+	switch (type) {
+	case 0x24:
+		return &onie_tlv_mac_read_cb;
+	default:
+		break;
+	}
+
+	return NULL;
+}
+
+static int onie_tlv_add_cells(struct device *dev, struct nvmem_device *nvmem,
+			      size_t data_len, u8 *data)
+{
+	struct nvmem_cell_info cell = {};
+	struct onie_tlv tlv;
+	unsigned int hdr_len = sizeof(struct onie_tlv_hdr);
+	unsigned int offset = 0;
+	int ret;
+
+	while (offset < data_len) {
+		memcpy(&tlv, data + offset, sizeof(tlv));
+		if (offset + tlv.len >= data_len) {
+			dev_err(dev, "Out of bounds field (0x%x bytes at 0x%x)\n",
+				tlv.len, hdr_len + offset);
+			break;
+		}
+
+		cell.name = onie_tlv_cell_name(tlv.type);
+		if (!cell.name)
+			continue;
+
+		cell.offset = hdr_len + offset + sizeof(tlv.type) + sizeof(tlv.len);
+		cell.bytes = tlv.len;
+		cell.np = of_get_child_by_name(dev->of_node, cell.name);
+		cell.read_post_process = onie_tlv_read_cb(tlv.type, data + offset + sizeof(tlv));
+
+		ret = nvmem_add_one_cell(nvmem, &cell);
+		if (ret)
+			return ret;
+
+		offset += sizeof(tlv) + tlv.len;
+	}
+
+	return 0;
+}
+
+static bool onie_tlv_hdr_is_valid(struct device *dev, struct onie_tlv_hdr *hdr)
+{
+	if (memcmp(hdr->id, ONIE_TLV_HDR_ID, sizeof(hdr->id))) {
+		dev_err(dev, "Invalid header\n");
+		return false;
+	}
+
+	if (hdr->version != 0x1) {
+		dev_err(dev, "Invalid version number\n");
+		return false;
+	}
+
+	return true;
+}
+
+static bool onie_tlv_crc_is_valid(struct device *dev, size_t table_len, u8 *table)
+{
+	struct onie_tlv crc_hdr;
+	u32 read_crc, calc_crc;
+	__be32 crc_be;
+
+	memcpy(&crc_hdr, table + table_len - ONIE_TLV_CRC_FIELD_SZ, sizeof(crc_hdr));
+	if (crc_hdr.type != 0xfe || crc_hdr.len != ONIE_TLV_CRC_SZ) {
+		dev_err(dev, "Invalid CRC field\n");
+		return false;
+	}
+
+	/* The table contains a JAMCRC, which is XOR'ed compared to the original
+	 * CRC32 implementation as known in the Ethernet world.
+	 */
+	memcpy(&crc_be, table + table_len - ONIE_TLV_CRC_SZ, ONIE_TLV_CRC_SZ);
+	read_crc = be32_to_cpu(crc_be);
+	calc_crc = crc32(~0, table, table_len - ONIE_TLV_CRC_SZ) ^ 0xFFFFFFFF;
+	if (read_crc != calc_crc) {
+		dev_err(dev, "Invalid CRC read: 0x%08x, expected: 0x%08x\n",
+			read_crc, calc_crc);
+		return false;
+	}
+
+	return true;
+}
+
+static int onie_tlv_parse_table(struct device *dev, struct nvmem_device *nvmem,
+				struct nvmem_layout *layout)
+{
+	struct onie_tlv_hdr hdr;
+	size_t table_len, data_len, hdr_len;
+	u8 *table, *data;
+	int ret;
+
+	ret = nvmem_device_read(nvmem, 0, sizeof(hdr), &hdr);
+	if (ret < 0)
+		return ret;
+
+	if (!onie_tlv_hdr_is_valid(dev, &hdr)) {
+		dev_err(dev, "Invalid ONIE TLV header\n");
+		return -EINVAL;
+	}
+
+	hdr_len = sizeof(hdr.id) + sizeof(hdr.version) + sizeof(hdr.data_len);
+	data_len = be16_to_cpu(hdr.data_len);
+	table_len = hdr_len + data_len;
+	if (table_len > ONIE_TLV_MAX_LEN) {
+		dev_err(dev, "Invalid ONIE TLV data length\n");
+		return -EINVAL;
+	}
+
+	table = devm_kmalloc(dev, table_len, GFP_KERNEL);
+	if (!table)
+		return -ENOMEM;
+
+	ret = nvmem_device_read(nvmem, 0, table_len, table);
+	if (ret != table_len)
+		goto free_data_buf;
+
+	if (!onie_tlv_crc_is_valid(dev, table_len, table)) {
+		ret = -EINVAL;
+		goto free_data_buf;
+	}
+
+	data = table + hdr_len;
+	ret = onie_tlv_add_cells(dev, nvmem, data_len, data);
+	if (ret)
+		goto free_data_buf;
+
+free_data_buf:
+	kfree(table);
+
+	return ret;
+}
+
+static const struct of_device_id onie_tlv_of_match_table[] = {
+	{ .compatible = "onie,tlv-layout", },
+	{},
+};
+
+static struct nvmem_layout onie_tlv_layout = {
+	.name = "ONIE tlv layout",
+	.of_match_table = onie_tlv_of_match_table,
+	.add_cells = onie_tlv_parse_table,
+};
+
+static int __init onie_tlv_init(void)
+{
+	return nvmem_layout_register(&onie_tlv_layout);
+}
+subsys_initcall(onie_tlv_init);
-- 
2.34.1

[PATCH 4/5] MAINTAINERS: Add myself as ONIE tlv NVMEM layout maintainer

[Miquel Raynal]

Following the introduction of the bindings for this NVMEM parser and the
layout driver, add myself as maintainer.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index cf0f18502372..be387e7e33a1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15328,6 +15328,12 @@ S:	Maintained
 F:	drivers/mtd/nand/onenand/
 F:	include/linux/mtd/onenand*.h
 
+ONIE TLV NVMEM LAYOUT DRIVER
+M:	Miquel Raynal <miquel.raynal@bootlin.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/nvmem/layouts/onie,tlv-layout.yaml
+F:	drivers/nvmem/layouts/onie-tlv.c
+
 ONION OMEGA2+ BOARD
 M:	Harvey Hunt <harveyhuntnexus@gmail.com>
 L:	linux-mips@vger.kernel.org
-- 
2.34.1

[PATCH 5/5] net: mvpp2: Consider NVMEM cells as possible MAC address source

[Miquel Raynal]

The ONIE standard describes the organization of tlv (type-length-value)
arrays commonly stored within NVMEM devices on common networking
hardware.

Several drivers already make use of NVMEM cells for purposes like
retrieving a default MAC address provided by the manufacturer.

What made ONIE tables unusable so far was the fact that the information
where "dynamically" located within the table depending on the
manufacturer wishes, while Linux NVMEM support only allowed statically
defined NVMEM cells. Fortunately, this limitation was eventually tackled
with the introduction of discoverable cells through the use of NVMEM
layouts, making it possible to extract and consistently use the content
of tables like ONIE's tlv arrays.

Parsing this table at runtime in order to get various information is now
possible. So, because many Marvell networking switches already follow
this standard, let's consider using NVMEM cells as a new valid source of
information when looking for a base MAC address, which is one of the
primary uses of these new fields. Indeed, manufacturers following the
ONIE standard are encouraged to provide a default MAC address there, so
let's eventually use it if no other MAC address has been found using the
existing methods.

Link: https://opencomputeproject.github.io/onie/design-spec/hw_requirements.html
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---

Hello, I suppose my change is safe but I don't want to break existing
setups so a review on this would be welcome!

 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index eb0fb8128096..7c8c323f4411 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -6104,6 +6104,12 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
 		}
 	}
 
+	if (!of_get_mac_address(to_of_node(fwnode), hw_mac_addr)) {
+		*mac_from = "nvmem cell";
+		eth_hw_addr_set(dev, hw_mac_addr);
+		return;
+	}
+
 	*mac_from = "random";
 	eth_hw_addr_random(dev);
 }
-- 
2.34.1

Re: [PATCH 5/5] net: mvpp2: Consider NVMEM cells as possible MAC address source

[Michael Walle]

Am 2022-10-28 11:23, schrieb Miquel Raynal:
> The ONIE standard describes the organization of tlv (type-length-value)
> arrays commonly stored within NVMEM devices on common networking
> hardware.
> 
> Several drivers already make use of NVMEM cells for purposes like
> retrieving a default MAC address provided by the manufacturer.
> 
> What made ONIE tables unusable so far was the fact that the information
> where "dynamically" located within the table depending on the
> manufacturer wishes, while Linux NVMEM support only allowed statically
> defined NVMEM cells. Fortunately, this limitation was eventually 
> tackled
> with the introduction of discoverable cells through the use of NVMEM
> layouts, making it possible to extract and consistently use the content
> of tables like ONIE's tlv arrays.
> 
> Parsing this table at runtime in order to get various information is 
> now
> possible. So, because many Marvell networking switches already follow
> this standard, let's consider using NVMEM cells as a new valid source 
> of
> information when looking for a base MAC address, which is one of the
> primary uses of these new fields. Indeed, manufacturers following the
> ONIE standard are encouraged to provide a default MAC address there, so
> let's eventually use it if no other MAC address has been found using 
> the
> existing methods.
> 
> Link: 
> https://opencomputeproject.github.io/onie/design-spec/hw_requirements.html
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> 
> Hello, I suppose my change is safe but I don't want to break existing
> setups so a review on this would be welcome!
> 
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index eb0fb8128096..7c8c323f4411 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -6104,6 +6104,12 @@ static void mvpp2_port_copy_mac_addr(struct
> net_device *dev, struct mvpp2 *priv,
>  		}
>  	}
> 
> +	if (!of_get_mac_address(to_of_node(fwnode), hw_mac_addr)) {

Mh, the driver already does a fwnode_get_mac_address() which might
fetch it from OF. But that variant doesn't try to get the mac address
via nvmem; in contrast to the of_get_mac_address() variant which will
also try NVMEM.
Maybe it would be better to just use device_get_ethdev_address() and
extend that one to also try the nvmem store. Just to align all the
different variants to get a mac address.

-michael

> +		*mac_from = "nvmem cell";
> +		eth_hw_addr_set(dev, hw_mac_addr);
> +		return;
> +	}
> +
>  	*mac_from = "random";
>  	eth_hw_addr_random(dev);
>  }

Re: [PATCH 5/5] net: mvpp2: Consider NVMEM cells as possible MAC address source

[Miquel Raynal]

Hi Michael,

michael@walle.cc wrote on Fri, 28 Oct 2022 15:33:31 +0200:

> Am 2022-10-28 11:23, schrieb Miquel Raynal:
> > The ONIE standard describes the organization of tlv (type-length-value)
> > arrays commonly stored within NVMEM devices on common networking
> > hardware.
> > 
> > Several drivers already make use of NVMEM cells for purposes like
> > retrieving a default MAC address provided by the manufacturer.
> > 
> > What made ONIE tables unusable so far was the fact that the information
> > where "dynamically" located within the table depending on the
> > manufacturer wishes, while Linux NVMEM support only allowed statically
> > defined NVMEM cells. Fortunately, this limitation was eventually > tackled
> > with the introduction of discoverable cells through the use of NVMEM
> > layouts, making it possible to extract and consistently use the content
> > of tables like ONIE's tlv arrays.
> > 
> > Parsing this table at runtime in order to get various information is > now
> > possible. So, because many Marvell networking switches already follow
> > this standard, let's consider using NVMEM cells as a new valid source > of
> > information when looking for a base MAC address, which is one of the
> > primary uses of these new fields. Indeed, manufacturers following the
> > ONIE standard are encouraged to provide a default MAC address there, so
> > let's eventually use it if no other MAC address has been found using > the
> > existing methods.
> > 
> > Link: > https://opencomputeproject.github.io/onie/design-spec/hw_requirements.html
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > 
> > Hello, I suppose my change is safe but I don't want to break existing
> > setups so a review on this would be welcome!
> > 
> >  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > index eb0fb8128096..7c8c323f4411 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -6104,6 +6104,12 @@ static void mvpp2_port_copy_mac_addr(struct
> > net_device *dev, struct mvpp2 *priv,
> >  		}
> >  	}
> > 
> > +	if (!of_get_mac_address(to_of_node(fwnode), hw_mac_addr)) {  
> 
> Mh, the driver already does a fwnode_get_mac_address() which might
> fetch it from OF. But that variant doesn't try to get the mac address
> via nvmem; in contrast to the of_get_mac_address() variant which will
> also try NVMEM.
> Maybe it would be better to just use device_get_ethdev_address() and
> extend that one to also try the nvmem store. Just to align all the
> different variants to get a mac address.

Actually this choice was made on purpose: I am adding this method to
retrieve the MAC address only if no other way has succeeded. I don't
know if the MAC addresses are expected to remain stable over time, I
assumed it was somehow part of the ABI.

Using device_get_ethdev_address() with support for MAC addresses in
nvmem cells would possibly change the MAC address of many existing
devices after an update because we found a MAC address in the tlv table
before checking the device's own registers (as in this driver)

So I assumed it was better avoiding changing the MAC address providers
order in the probe...

Thanks,
Miquèl