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

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 [off-list ref] napisał(a):

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  

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

quoted

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

quoted

+               *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