[PATCH 0/6] Marvell nvmem mac addresses support

[PATCH 0/6] Marvell nvmem mac addresses support

[Miquel Raynal]

Hello,

Now that we are aligned on how to make information available from static
storage media to drivers like Ethernet controller drivers or switch
drivers by using nvmem cells and going through the whole nvmem
infrastructure, here are two driver updates to reflect these changes.

Prior to the driver updates, I propose:
* Reverting binding changes which should have never been accepted like
  that.
* A conversion of the (old) Prestera and DFX server bindings (optional,
  can be dropped if not considered necessary).
* A better description of the more recent Prestera PCI switch.

Please mind that this series cannot break anything since retrieving the
MAC address Prestera driver has never worked upstream, because the (ONIE
tlv) driver supposed to export the MAC address has not been accepted in
its original form and has been updated to the nvmem-layout
infrastructure (bindings have been merged, the code remains to be
applied).

Thanks,
Miquèl

Miquel Raynal (6):
  Revert "dt-bindings: marvell,prestera: Add description for device-tree
    bindings"
  dt-bindings: net: marvell,dfx-server: Convert to yaml
  dt-bindings: net: marvell,prestera: Convert to yaml
  dt-bindings: net: marvell,prestera: Describe PCI devices of the
    prestera family
  net: marvell: prestera: Avoid unnecessary DT lookups
  net: mvpp2: Consider NVMEM cells as possible MAC address source

 .../bindings/net/marvell,dfx-server.yaml      | 60 +++++++++++++
 .../bindings/net/marvell,prestera.txt         | 81 -----------------
 .../bindings/net/marvell,prestera.yaml        | 86 +++++++++++++++++++
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |  6 ++
 .../ethernet/marvell/prestera/prestera_main.c | 15 +---
 5 files changed, 156 insertions(+), 92 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,dfx-server.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/marvell,prestera.txt
 create mode 100644 Documentation/devicetree/bindings/net/marvell,prestera.yaml

-- 
2.34.1

[PATCH 1/6] Revert "dt-bindings: marvell,prestera: Add description for device-tree bindings"

[Miquel Raynal]

This reverts commit 40acc05271abc2852c32622edbebd75698736b9b.

marvell,prestera.txt is an old file describing the old Alleycat3
standalone switches. The commit mentioned above actually hacked these
bindings to add support for a device tree property for a more modern
version of the IP connected over PCI, using only the generic compatible
in order to retrieve the device node from the prestera driver to read
one static property.

The problematic property discussed here is "base-mac-provider". The
original intent was to point to a nvmem device which could produce the
relevant nvmem-cell. This property has never been acked by DT
maintainers and fails all the layering that has been brought with the nvmem
bindings by pointing at a nvmem producer, bypassing the existing nvmem
bindings, rather than a nvmem cell directly. Furthermore, the property
cannot even be used upstream because it expected the ONIE tlv driver to
produce a specific cell, driver which used nacked bindings and thus was
never merged, replaced by a more integrated concept: the nvmem-layout.

So let's forget about this temporary addition, safely avoiding the need
for any backward compatibility handling. A new (yaml) binding file will
be brought with the prestera bindings, and there we will actually
include a description of the modern IP over PCI, including the right way
to point to a nvmem cell.

Cc: Vadym Kochan <vadym.kochan@plvision.eu>
Cc: Taras Chornyi <tchornyi@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/net/marvell,prestera.txt         | 34 -------------------
 1 file changed, 34 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell,prestera.txt b/Documentation/devicetree/bindings/net/marvell,prestera.txt
index e28938ddfdf5..83370ebf5b89 100644
--- a/Documentation/devicetree/bindings/net/marvell,prestera.txt
+++ b/Documentation/devicetree/bindings/net/marvell,prestera.txt
@@ -45,37 +45,3 @@ dfx-server {
 	ranges = <0 MBUS_ID(0x08, 0x00) 0 0x100000>;
 	reg = <MBUS_ID(0x08, 0x00) 0 0x100000>;
 };
-
-Marvell Prestera SwitchDev bindings
------------------------------------
-Optional properties:
-- compatible: must be "marvell,prestera"
-- base-mac-provider: describes handle to node which provides base mac address,
-	might be a static base mac address or nvme cell provider.
-
-Example:
-
-eeprom_mac_addr: eeprom-mac-addr {
-       compatible = "eeprom,mac-addr-cell";
-       status = "okay";
-
-       nvmem = <&eeprom_at24>;
-};
-
-prestera {
-       compatible = "marvell,prestera";
-       status = "okay";
-
-       base-mac-provider = <&eeprom_mac_addr>;
-};
-
-The current implementation of Prestera Switchdev PCI interface driver requires
-that BAR2 is assigned to 0xf6000000 as base address from the PCI IO range:
-
-&cp0_pcie0 {
-	ranges = <0x81000000 0x0 0xfb000000 0x0 0xfb000000 0x0 0xf0000
-		0x82000000 0x0 0xf6000000 0x0 0xf6000000 0x0 0x2000000
-		0x82000000 0x0 0xf9000000 0x0 0xf9000000 0x0 0x100000>;
-	phys = <&cp0_comphy0 0>;
-	status = "okay";
-};
-- 
2.34.1

[PATCH 2/6] dt-bindings: net: marvell,dfx-server: Convert to yaml

[Miquel Raynal]

Even though this description is not used anywhere upstream (no matching
driver), while on this file I decided I would try a conversion to yaml
in order to clarify the prestera family description.

I cannot keep the nodename dfx-server@xxxx so I switched to dfx-bus@xxxx
which matches simple-bus.yaml. Otherwise I took the example context from
the only user of this compatible: armada-xp-98dx3236.dtsi, which is a
rather old and not perfect DT.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
I am fine dropping this file entirely as well, if judged useless.
---
 .../bindings/net/marvell,dfx-server.yaml      | 60 +++++++++++++++++++
 .../bindings/net/marvell,prestera.txt         | 18 ------
 2 files changed, 60 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,dfx-server.yaml

diff --git a/Documentation/devicetree/bindings/net/marvell,dfx-server.yaml b/Documentation/devicetree/bindings/net/marvell,dfx-server.yaml
new file mode 100644
index 000000000000..72151a78396f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell,dfx-server.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/marvell,dfx-server.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Prestera DFX server
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+select:
+  properties:
+    compatible:
+      contains:
+        const: marvell,dfx-server
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - const: marvell,dfx-server
+      - const: simple-bus
+
+  reg: true
+
+  ranges: true
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - ranges
+
+# The DFX server may expose clocks described as subnodes
+additionalProperties: true
+
+examples:
+  - |
+
+    #define MBUS_ID(target,attributes) (((target) << 24) | ((attributes) << 16))
+    bus@0 {
+        reg = <0 0>;
+        #address-cells = <2>;
+        #size-cells = <1>;
+
+        dfx-bus@ac000000 {
+            compatible = "marvell,dfx-server", "simple-bus";
+            #address-cells = <1>;
+            #size-cells = <1>;
+            ranges = <0 MBUS_ID(0x08, 0x00) 0 0x100000>;
+            reg = <MBUS_ID(0x08, 0x00) 0 0x100000>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/net/marvell,prestera.txt b/Documentation/devicetree/bindings/net/marvell,prestera.txt
index 83370ebf5b89..8868d774da67 100644
--- a/Documentation/devicetree/bindings/net/marvell,prestera.txt
+++ b/Documentation/devicetree/bindings/net/marvell,prestera.txt
@@ -27,21 +27,3 @@ switch {
 		dfx = <&dfx>;
 	};
 };
-
-DFX Server bindings
--------------------
-
-Required properties:
-- compatible: must be "marvell,dfx-server", "simple-bus"
-- ranges: describes the address mapping of a memory-mapped bus.
-- reg: address and length of the register set for the device.
-
-Example:
-
-dfx-server {
-	compatible = "marvell,dfx-server", "simple-bus";
-	#address-cells = <1>;
-	#size-cells = <1>;
-	ranges = <0 MBUS_ID(0x08, 0x00) 0 0x100000>;
-	reg = <MBUS_ID(0x08, 0x00) 0 0x100000>;
-};
-- 
2.34.1

[PATCH 3/6] dt-bindings: net: marvell,prestera: Convert to yaml

[Miquel Raynal]

The currently described switch family is named AlleyCat3, it is a memory
mapped switch found on Armada XP boards.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
This patch (and the original txt file) can also be dropped if judged not
worth the conversion as anyway in both cases there is no driver upstream
for these devices.
---
 .../bindings/net/marvell,prestera.txt         | 29 ------------
 .../bindings/net/marvell,prestera.yaml        | 45 +++++++++++++++++++
 2 files changed, 45 insertions(+), 29 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/marvell,prestera.txt
 create mode 100644 Documentation/devicetree/bindings/net/marvell,prestera.yaml

diff --git a/Documentation/devicetree/bindings/net/marvell,prestera.txt b/Documentation/devicetree/bindings/net/marvell,prestera.txt
deleted file mode 100644
index 8868d774da67..000000000000
--- a/Documentation/devicetree/bindings/net/marvell,prestera.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-Marvell Prestera Switch Chip bindings
--------------------------------------
-
-Required properties:
-- compatible: must be "marvell,prestera" and one of the following
-	"marvell,prestera-98dx3236",
-	"marvell,prestera-98dx3336",
-	"marvell,prestera-98dx4251",
-- reg: address and length of the register set for the device.
-- interrupts: interrupt for the device
-
-Optional properties:
-- dfx: phandle reference to the "DFX Server" node
-
-Example:
-
-switch {
-	compatible = "simple-bus";
-	#address-cells = <1>;
-	#size-cells = <1>;
-	ranges = <0 MBUS_ID(0x03, 0x00) 0 0x100000>;
-
-	packet-processor@0 {
-		compatible = "marvell,prestera-98dx3236", "marvell,prestera";
-		reg = <0 0x4000000>;
-		interrupts = <33>, <34>, <35>;
-		dfx = <&dfx>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/net/marvell,prestera.yaml b/Documentation/devicetree/bindings/net/marvell,prestera.yaml
new file mode 100644
index 000000000000..b0a3ecca406e
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell,prestera.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/marvell,prestera.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Prestera AlleyCat3 switch
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - marvell,prestera-98dx3236
+          - marvell,prestera-98dx3336
+          - marvell,prestera-98dx4251
+      - const: marvell,prestera
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 3
+
+  dfx:
+    description: Reference to the DFX Server bus node.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    packet-processor@0 {
+        compatible = "marvell,prestera-98dx3236", "marvell,prestera";
+        reg = <0 0x4000000>;
+        interrupts = <33>, <34>, <35>;
+        dfx = <&dfx>;
+    };
-- 
2.34.1

[PATCH 4/6] dt-bindings: net: marvell,prestera: Describe PCI devices of the prestera family

[Miquel Raynal]

Even though the devices have very little in common beside the name and
the main "switch" feature, Marvell Prestera switch family is also
composed of PCI-only devices which can receive additional static
properties, like nvmem cells to point at MAC addresses, for
instance. Let's describe them.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/net/marvell,prestera.yaml        | 55 ++++++++++++++++---
 1 file changed, 48 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell,prestera.yaml b/Documentation/devicetree/bindings/net/marvell,prestera.yaml
index b0a3ecca406e..f159fadf86ec 100644
--- a/Documentation/devicetree/bindings/net/marvell,prestera.yaml
+++ b/Documentation/devicetree/bindings/net/marvell,prestera.yaml
@@ -4,19 +4,24 @@
 $id: http://devicetree.org/schemas/net/marvell,prestera.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Marvell Prestera AlleyCat3 switch
+title: Marvell Prestera switch family
 
 maintainers:
   - Miquel Raynal <miquel.raynal@bootlin.com>
 
 properties:
   compatible:
-    items:
+    oneOf:
+      - items:
+          - enum:
+              - marvell,prestera-98dx3236
+              - marvell,prestera-98dx3336
+              - marvell,prestera-98dx4251
+          - const: marvell,prestera
       - enum:
-          - marvell,prestera-98dx3236
-          - marvell,prestera-98dx3336
-          - marvell,prestera-98dx4251
-      - const: marvell,prestera
+          - pci11ab,c804
+          - pci11ab,c80c
+          - pci11ab,cc1e
 
   reg:
     maxItems: 1
@@ -28,10 +33,33 @@ properties:
     description: Reference to the DFX Server bus node.
     $ref: /schemas/types.yaml#/definitions/phandle
 
+  nvmem-cells: true
+
+  nvmem-cell-names: true
+
+if:
+  properties:
+    compatible:
+      contains:
+        const: marvell,prestera
+
+# Memory mapped AlleyCat3 family
+then:
+  properties:
+    nvmem-cells: false
+    nvmem-cell-names: false
+  required:
+    - interrupts
+
+# PCI Aldrin family
+else:
+  properties:
+    interrupts: false
+    dfx: false
+
 required:
   - compatible
   - reg
-  - interrupts
 
 additionalProperties: false
 
@@ -43,3 +71,16 @@ examples:
         interrupts = <33>, <34>, <35>;
         dfx = <&dfx>;
     };
+
+  - |
+    pcie {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        switch@0,0 {
+            reg = <0x0 0x0 0x0 0x0 0x0>;
+            compatible = "pci11ab,c80c";
+            nvmem-cells = <&mac_address 0>;
+            nvmem-cell-names = "mac-address";
+        };
+    };
-- 
2.34.1

[PATCH net-next 5/6] net: marvell: prestera: Avoid unnecessary DT lookups

[Miquel Raynal]

This driver fist makes an expensive DT lookup to retrieve its DT node
(this is a PCI driver) in order to later search for the
base-mac-provider property. This property has no reality upstream and
this code should not have been accepted like this in the first
place. Instead, there is a proper nvmem interface that should be
used. Let's avoid these extra lookups and rely on the nvmem internal
logic.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../net/ethernet/marvell/prestera/prestera_main.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index 24f9d6024745..d4b48f674a88 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -862,17 +862,10 @@ static void prestera_event_handlers_unregister(struct prestera_switch *sw)
 
 static int prestera_switch_set_base_mac_addr(struct prestera_switch *sw)
 {
-	struct device_node *base_mac_np;
-	int ret = 0;
-
-	if (sw->np) {
-		base_mac_np = of_parse_phandle(sw->np, "base-mac-provider", 0);
-		if (base_mac_np) {
-			ret = of_get_mac_address(base_mac_np, sw->base_mac);
-			of_node_put(base_mac_np);
-		}
-	}
+	int ret;
 
+	if (sw->np)
+		ret = of_get_mac_address(sw->np, sw->base_mac);
 	if (!is_valid_ether_addr(sw->base_mac) || ret) {
 		eth_random_addr(sw->base_mac);
 		dev_info(prestera_dev(sw), "using random base mac address\n");
@@ -1376,7 +1369,7 @@ static int prestera_switch_init(struct prestera_switch *sw)
 {
 	int err;
 
-	sw->np = of_find_compatible_node(NULL, NULL, "marvell,prestera");
+	sw->np = sw->dev->dev->of_node;
 
 	err = prestera_hw_switch_init(sw);
 	if (err) {
-- 
2.34.1

[PATCH net-next 6/6] 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>
---
 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 net-next 6/6] net: mvpp2: Consider NVMEM cells as possible MAC address source

[Marcin Wojtas]

Hi Miquel,


czw., 17 lis 2022 o 22:56 Miquel Raynal <miquel.raynal@bootlin.com> napisał(a):
>
> 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

Thanks for the patch. Did you manage to test in on a real HW? I am curious about

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  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)) {

Unfortunately, nvmem cells seem to be not supported with ACPI yet, so
we cannot extend fwnode_get_mac_address - I think it should be,
however, an end solution.

As of now, I'd prefer to use of_get_mac_addr_nvmem directly, to avoid
parsing the DT again (after fwnode_get_mac_address) and relying
implicitly on falling back to nvmem stuff (currently, without any
comment it is not obvious).

Best regards,
Marcin

> +               *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 net-next 6/6] net: mvpp2: Consider NVMEM cells as possible MAC address source

[Miquel Raynal]

Hi Marcin,

mw@semihalf.com wrote on Sat, 19 Nov 2022 09:18:34 +0100:

> Hi Miquel,
> 
> 
> czw., 17 lis 2022 o 22:56 Miquel Raynal <miquel.raynal@bootlin.com> napisał(a):
> >
> > 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  
> 
> Thanks for the patch. Did you manage to test in on a real HW? I am curious about

Yes, I have a Replica switch on which the commercial ports use the
replica PCI IP while the config "OOB" port is running with mvpp2:
[   16.737759] mvpp2 f2000000.ethernet eth52: Using nvmem cell mac address 18:be:92:13:9a:00

> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  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)) {  
> 
> Unfortunately, nvmem cells seem to be not supported with ACPI yet, so
> we cannot extend fwnode_get_mac_address - I think it should be,
> however, an end solution.

Agreed.

> As of now, I'd prefer to use of_get_mac_addr_nvmem directly, to avoid
> parsing the DT again (after fwnode_get_mac_address) and relying
> implicitly on falling back to nvmem stuff (currently, without any
> comment it is not obvious).

I did not do that in the first place because of_get_mac_addr_nvmem()
was not exported, but I agree it would be the cleanest (and quickest)
approach, so I'll attempt to export the function first, and then use it
directly from the driver.

> Best regards,
> Marcin
> 
> > +               *mac_from = "nvmem cell";
> > +               eth_hw_addr_set(dev, hw_mac_addr);
> > +               return;
> > +       }
> > +
> >         *mac_from = "random";
> >         eth_hw_addr_random(dev);
> >  }
> > --
> > 2.34.1
> >  

Thanks a lot,
Miquèl

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

[Marcin Wojtas]

pon., 21 lis 2022 o 10:29 Miquel Raynal <miquel.raynal@bootlin.com> napisał(a):
>
> Hi Marcin,
>
> mw@semihalf.com wrote on Sat, 19 Nov 2022 09:18:34 +0100:
>
> > Hi Miquel,
> >
> >
> > czw., 17 lis 2022 o 22:56 Miquel Raynal <miquel.raynal@bootlin.com> napisał(a):
> > >
> > > 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
> >
> > Thanks for the patch. Did you manage to test in on a real HW? I am curious about
>
> Yes, I have a Replica switch on which the commercial ports use the
> replica PCI IP while the config "OOB" port is running with mvpp2:
> [   16.737759] mvpp2 f2000000.ethernet eth52: Using nvmem cell mac address 18:be:92:13:9a:00
>

Nice. Do you have a DT snippet that can possibly be shared? I'd like
to recreate this locally and eventually leverage EDK2 firmware to
expose that.

> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  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)) {
> >
> > Unfortunately, nvmem cells seem to be not supported with ACPI yet, so
> > we cannot extend fwnode_get_mac_address - I think it should be,
> > however, an end solution.
>
> Agreed.
>
> > As of now, I'd prefer to use of_get_mac_addr_nvmem directly, to avoid
> > parsing the DT again (after fwnode_get_mac_address) and relying
> > implicitly on falling back to nvmem stuff (currently, without any
> > comment it is not obvious).
>
> I did not do that in the first place because of_get_mac_addr_nvmem()
> was not exported, but I agree it would be the cleanest (and quickest)
> approach, so I'll attempt to export the function first, and then use it
> directly from the driver.
>

That would be great, thank you. Please add one-comment in the
mvpp2_main.c, that this is valid for now only in DT world.

Best regards,
Marcin

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

[Miquel Raynal]

Hi Marcin,

mw@semihalf.com wrote on Mon, 21 Nov 2022 15:46:44 +0100:

> pon., 21 lis 2022 o 10:29 Miquel Raynal <miquel.raynal@bootlin.com> napisał(a):
> >
> > Hi Marcin,
> >
> > mw@semihalf.com wrote on Sat, 19 Nov 2022 09:18:34 +0100:
> >  
> > > Hi Miquel,
> > >
> > >
> > > czw., 17 lis 2022 o 22:56 Miquel Raynal <miquel.raynal@bootlin.com> napisał(a):  
> > > >
> > > > 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  
> > >
> > > Thanks for the patch. Did you manage to test in on a real HW? I am curious about  
> >
> > Yes, I have a Replica switch on which the commercial ports use the
> > replica PCI IP while the config "OOB" port is running with mvpp2:
> > [   16.737759] mvpp2 f2000000.ethernet eth52: Using nvmem cell mac address 18:be:92:13:9a:00
> >  
> 
> Nice. Do you have a DT snippet that can possibly be shared? I'd like
> to recreate this locally and eventually leverage EDK2 firmware to
> expose that.

Yes of course, the DT is public on Sartura's Github, but here is the
exact file I used showing the diff cleaning the Armada 7040 TN48M DT
eeprom description and its use as an nvmem-cell provider):
https://github.com/miquelraynal/linux/commit/230ee68728799454e2f07f61792e11724e731d6d

> 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  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)) {  
> > >
> > > Unfortunately, nvmem cells seem to be not supported with ACPI yet, so
> > > we cannot extend fwnode_get_mac_address - I think it should be,
> > > however, an end solution.  
> >
> > Agreed.
> >  
> > > As of now, I'd prefer to use of_get_mac_addr_nvmem directly, to avoid
> > > parsing the DT again (after fwnode_get_mac_address) and relying
> > > implicitly on falling back to nvmem stuff (currently, without any
> > > comment it is not obvious).  
> >
> > I did not do that in the first place because of_get_mac_addr_nvmem()
> > was not exported, but I agree it would be the cleanest (and quickest)
> > approach, so I'll attempt to export the function first, and then use it
> > directly from the driver.
> >  
> 
> That would be great, thank you. Please add one-comment in the
> mvpp2_main.c, that this is valid for now only in DT world.

Ack.

Thanks,
Miquèl

Re: [PATCH 1/6] Revert "dt-bindings: marvell,prestera: Add description for device-tree bindings"

[Rob Herring]

On Thu, 17 Nov 2022 22:55:52 +0100, Miquel Raynal wrote:
> This reverts commit 40acc05271abc2852c32622edbebd75698736b9b.
> 
> marvell,prestera.txt is an old file describing the old Alleycat3
> standalone switches. The commit mentioned above actually hacked these
> bindings to add support for a device tree property for a more modern
> version of the IP connected over PCI, using only the generic compatible
> in order to retrieve the device node from the prestera driver to read
> one static property.
> 
> The problematic property discussed here is "base-mac-provider". The
> original intent was to point to a nvmem device which could produce the
> relevant nvmem-cell. This property has never been acked by DT
> maintainers and fails all the layering that has been brought with the nvmem
> bindings by pointing at a nvmem producer, bypassing the existing nvmem
> bindings, rather than a nvmem cell directly. Furthermore, the property
> cannot even be used upstream because it expected the ONIE tlv driver to
> produce a specific cell, driver which used nacked bindings and thus was
> never merged, replaced by a more integrated concept: the nvmem-layout.
> 
> So let's forget about this temporary addition, safely avoiding the need
> for any backward compatibility handling. A new (yaml) binding file will
> be brought with the prestera bindings, and there we will actually
> include a description of the modern IP over PCI, including the right way
> to point to a nvmem cell.
> 
> Cc: Vadym Kochan <vadym.kochan@plvision.eu>
> Cc: Taras Chornyi <tchornyi@marvell.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/net/marvell,prestera.txt         | 34 -------------------
>  1 file changed, 34 deletions(-)
> 

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

Re: [PATCH 2/6] dt-bindings: net: marvell,dfx-server: Convert to yaml

[Rob Herring]

On Thu, Nov 17, 2022 at 10:55:53PM +0100, Miquel Raynal wrote:
> Even though this description is not used anywhere upstream (no matching
> driver), while on this file I decided I would try a conversion to yaml
> in order to clarify the prestera family description.
> 
> I cannot keep the nodename dfx-server@xxxx so I switched to dfx-bus@xxxx
> which matches simple-bus.yaml. Otherwise I took the example context from
> the only user of this compatible: armada-xp-98dx3236.dtsi, which is a
> rather old and not perfect DT.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> I am fine dropping this file entirely as well, if judged useless.
> ---
>  .../bindings/net/marvell,dfx-server.yaml      | 60 +++++++++++++++++++
>  .../bindings/net/marvell,prestera.txt         | 18 ------
>  2 files changed, 60 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,dfx-server.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,dfx-server.yaml b/Documentation/devicetree/bindings/net/marvell,dfx-server.yaml
> new file mode 100644
> index 000000000000..72151a78396f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,dfx-server.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,dfx-server.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Prestera DFX server
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        const: marvell,dfx-server
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: marvell,dfx-server
> +      - const: simple-bus
> +
> +  reg: true

How many entries?

> +
> +  ranges: true
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - ranges
> +
> +# The DFX server may expose clocks described as subnodes
> +additionalProperties: true

addtionalProperties:
  type: object

So that only nodes can be added.

> +
> +examples:
> +  - |
> +
> +    #define MBUS_ID(target,attributes) (((target) << 24) | ((attributes) << 16))
> +    bus@0 {
> +        reg = <0 0>;
> +        #address-cells = <2>;
> +        #size-cells = <1>;
> +
> +        dfx-bus@ac000000 {
> +            compatible = "marvell,dfx-server", "simple-bus";
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            ranges = <0 MBUS_ID(0x08, 0x00) 0 0x100000>;
> +            reg = <MBUS_ID(0x08, 0x00) 0 0x100000>;
> +        };
> +    };

Re: [PATCH 3/6] dt-bindings: net: marvell,prestera: Convert to yaml

[Rob Herring]

On Thu, 17 Nov 2022 22:55:54 +0100, Miquel Raynal wrote:
> The currently described switch family is named AlleyCat3, it is a memory
> mapped switch found on Armada XP boards.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> This patch (and the original txt file) can also be dropped if judged not
> worth the conversion as anyway in both cases there is no driver upstream
> for these devices.
> ---
>  .../bindings/net/marvell,prestera.txt         | 29 ------------
>  .../bindings/net/marvell,prestera.yaml        | 45 +++++++++++++++++++
>  2 files changed, 45 insertions(+), 29 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/marvell,prestera.txt
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,prestera.yaml
> 

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

Re: [PATCH 4/6] dt-bindings: net: marvell,prestera: Describe PCI devices of the prestera family

[Rob Herring]

On Thu, Nov 17, 2022 at 10:55:55PM +0100, Miquel Raynal wrote:
> Even though the devices have very little in common beside the name and
> the main "switch" feature, Marvell Prestera switch family is also
> composed of PCI-only devices which can receive additional static
> properties, like nvmem cells to point at MAC addresses, for
> instance. Let's describe them.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/net/marvell,prestera.yaml        | 55 ++++++++++++++++---
>  1 file changed, 48 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,prestera.yaml b/Documentation/devicetree/bindings/net/marvell,prestera.yaml
> index b0a3ecca406e..f159fadf86ec 100644
> --- a/Documentation/devicetree/bindings/net/marvell,prestera.yaml
> +++ b/Documentation/devicetree/bindings/net/marvell,prestera.yaml
> @@ -4,19 +4,24 @@
>  $id: http://devicetree.org/schemas/net/marvell,prestera.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Marvell Prestera AlleyCat3 switch
> +title: Marvell Prestera switch family
>  
>  maintainers:
>    - Miquel Raynal <miquel.raynal@bootlin.com>
>  
>  properties:
>    compatible:
> -    items:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - marvell,prestera-98dx3236
> +              - marvell,prestera-98dx3336
> +              - marvell,prestera-98dx4251
> +          - const: marvell,prestera
>        - enum:
> -          - marvell,prestera-98dx3236
> -          - marvell,prestera-98dx3336
> -          - marvell,prestera-98dx4251
> -      - const: marvell,prestera
> +          - pci11ab,c804
> +          - pci11ab,c80c
> +          - pci11ab,cc1e
>  
>    reg:
>      maxItems: 1
> @@ -28,10 +33,33 @@ properties:
>      description: Reference to the DFX Server bus node.
>      $ref: /schemas/types.yaml#/definitions/phandle
>  
> +  nvmem-cells: true
> +
> +  nvmem-cell-names: true
> +
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: marvell,prestera
> +
> +# Memory mapped AlleyCat3 family
> +then:
> +  properties:
> +    nvmem-cells: false
> +    nvmem-cell-names: false
> +  required:
> +    - interrupts
> +
> +# PCI Aldrin family
> +else:
> +  properties:
> +    interrupts: false
> +    dfx: false
> +
>  required:
>    - compatible
>    - reg
> -  - interrupts
>  
>  additionalProperties: false
>  
> @@ -43,3 +71,16 @@ examples:
>          interrupts = <33>, <34>, <35>;
>          dfx = <&dfx>;
>      };
> +
> +  - |
> +    pcie {
> +        #address-cells = <3>;
> +        #size-cells = <2>;

           ranges;
           device_type = "pci";

With that,

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

> +
> +        switch@0,0 {
> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> +            compatible = "pci11ab,c80c";
> +            nvmem-cells = <&mac_address 0>;
> +            nvmem-cell-names = "mac-address";
> +        };
> +    };
> -- 
> 2.34.1
> 
> 

Re: [PATCH 2/6] dt-bindings: net: marvell,dfx-server: Convert to yaml

[Miquel Raynal]

Hi Rob,

robh@kernel.org wrote on Wed, 23 Nov 2022 16:10:23 -0600:

> On Thu, Nov 17, 2022 at 10:55:53PM +0100, Miquel Raynal wrote:
> > Even though this description is not used anywhere upstream (no matching
> > driver), while on this file I decided I would try a conversion to yaml
> > in order to clarify the prestera family description.
> > 
> > I cannot keep the nodename dfx-server@xxxx so I switched to dfx-bus@xxxx
> > which matches simple-bus.yaml. Otherwise I took the example context from
> > the only user of this compatible: armada-xp-98dx3236.dtsi, which is a
> > rather old and not perfect DT.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > I am fine dropping this file entirely as well, if judged useless.
> > ---
> >  .../bindings/net/marvell,dfx-server.yaml      | 60 +++++++++++++++++++
> >  .../bindings/net/marvell,prestera.txt         | 18 ------
> >  2 files changed, 60 insertions(+), 18 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/net/marvell,dfx-server.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/marvell,dfx-server.yaml b/Documentation/devicetree/bindings/net/marvell,dfx-server.yaml
> > new file mode 100644
> > index 000000000000..72151a78396f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/marvell,dfx-server.yaml
> > @@ -0,0 +1,60 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/marvell,dfx-server.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell Prestera DFX server
> > +
> > +maintainers:
> > +  - Miquel Raynal <miquel.raynal@bootlin.com>
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        const: marvell,dfx-server
> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: marvell,dfx-server
> > +      - const: simple-bus
> > +
> > +  reg: true  
> 
> How many entries?

Right, there is a single one, I'll constrain reg properly in v2.

> > +
> > +  ranges: true
> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - ranges
> > +
> > +# The DFX server may expose clocks described as subnodes
> > +additionalProperties: true  
> 
> addtionalProperties:
>   type: object
> 
> So that only nodes can be added.

Excellent, I never thought about this possibility, but of course that
works. Thanks a lot!

> 
> > +
> > +examples:
> > +  - |
> > +
> > +    #define MBUS_ID(target,attributes) (((target) << 24) | ((attributes) << 16))
> > +    bus@0 {
> > +        reg = <0 0>;
> > +        #address-cells = <2>;
> > +        #size-cells = <1>;
> > +
> > +        dfx-bus@ac000000 {
> > +            compatible = "marvell,dfx-server", "simple-bus";
> > +            #address-cells = <1>;
> > +            #size-cells = <1>;
> > +            ranges = <0 MBUS_ID(0x08, 0x00) 0 0x100000>;
> > +            reg = <MBUS_ID(0x08, 0x00) 0 0x100000>;
> > +        };
> > +    };  


Thanks,
Miquèl