Hi Michael,

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

Am 2022-10-28 11:23, schrieb Miquel Raynal:

quoted

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