Le Fri, 29 Oct 2021 11:35:43 +0000,
Vladimir Oltean [off-list ref] a écrit :

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

quoted

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.

Agreed, additionally the driver uses the "mac-adress" property from the
switch node to initialize the base mac.
Anyway, I changed my patch to use mac-adress for each port and modified
the yaml bindings to include ethernet-controller.yaml and use a
"mac-address" property per port.

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

quoted

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  

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