On Fri, Oct 29, 2021 at 08:09:37AM +0200, Clément Léger wrote:

Le Thu, 28 Oct 2021 14:51:43 +0000,
Vladimir Oltean [off-list ref] a écrit :

quoted

On Thu, Oct 28, 2021 at 04:38:25PM +0200, Clément Léger wrote:

quoted

Le Thu, 28 Oct 2021 14:22:55 +0000,
Vladimir Oltean [off-list ref] a écrit :

quoted

On Thu, Oct 28, 2021 at 04:15:22PM +0200, Clément Léger wrote:

quoted

Le Thu, 28 Oct 2021 14:06:12 +0000,
Vladimir Oltean [off-list ref] a écrit :

quoted

On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger
wrote:

quoted

Add support to get mac from device-tree using
of_get_mac_address.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c

b/drivers/net/ethernet/mscc/ocelot_vsc7514.c index
d51f799e4e86..c39118e5b3ee 100644 ---
a/drivers/net/ethernet/mscc/ocelot_vsc7514.c +++
b/drivers/net/ethernet/mscc/ocelot_vsc7514.c @@ -526,7
+526,10 @@ static int ocelot_chip_init(struct ocelot
*ocelot, const struct ocelot_ops *ops)
ocelot_pll5_init(ocelot);
-	eth_random_addr(ocelot->base_mac);
+	ret = of_get_mac_address(ocelot->dev->of_node,
ocelot->base_mac);

Why not per port? This is pretty strange, I think.

Hi Vladimir,

Currently, all ports share the same base mac address (5 first
bytes). The final mac address per port is computed in
ocelot_probe_port by adding the port number as the last byte of
the mac_address provided.

Clément

Yes, I know that, but that's not my point.
Every switch port should be pretty much compliant with
ethernet-controller.yaml, if it could inherit that it would be
even better. And since mac-address is an ethernet-controller.yaml
property, it is pretty much non-obvious at all that you put the
mac-address property directly under the switch, and manually add
0, 1, 2, 3 etc to it. My request was to parse the mac-address
property of each port. Like this:

base_mac = random;

for_each_port() {
	err = of_get_mac_address(port_dn, &port_mac);
	if (err)
		port_mac = base_mac + port;
}

Ok indeed. So I will parse each port for a mac-address property. Do
you also want a fallback to use the switch base mac if not
specified in port or should I keep the use of a default random mac
as the base address anyway ?

Isn't the pseudocode I posted above explicit enough? Sorry...
Keep doing what the driver is doing right now, with an optional
mac-address override per port.
Why would we read the mac-address property of the switch? Which other
switch driver does that? Are there device trees in circulation where
this is being done?

BTW, this is actually done on sparx5 switch driver.

Highly inconsistent, but true. I'm saying that because
Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml says
that "mac-address" should be under "switch", but then proceeds to put it
under "port@64" in the example.

Most likely not caught during review, I'm not sure if this could be
considered good practice.

quoted

quoted

quoted

quoted

quoted

quoted

+	if (ret)
+		eth_random_addr(ocelot->base_mac);
+
 	ocelot->base_mac[5] &= 0xf0;

 	return 0;
--
2.33.0

--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com/