| Message ID | 20211028134932.658167-2-clement.leger@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add FDMA support on ocelot switch driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger wrote: > 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. > + if (ret) > + eth_random_addr(ocelot->base_mac); > + > ocelot->base_mac[5] &= 0xf0; > > return 0; > -- > 2.33.0 >
Le Thu, 28 Oct 2021 14:06:12 +0000, Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger wrote: > > 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 > > > + if (ret) > > + eth_random_addr(ocelot->base_mac); > > + > > ocelot->base_mac[5] &= 0xf0; > > > > return 0; > > -- > > 2.33.0 >
On Thu, Oct 28, 2021 at 04:15:22PM +0200, Clément Léger wrote: > Le Thu, 28 Oct 2021 14:06:12 +0000, > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger wrote: > > > 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; } > > > + if (ret) > > > + eth_random_addr(ocelot->base_mac); > > > + > > > ocelot->base_mac[5] &= 0xf0; > > > > > > return 0; > > > -- > > > 2.33.0 > > >
Le Thu, 28 Oct 2021 14:22:55 +0000, Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > On Thu, Oct 28, 2021 at 04:15:22PM +0200, Clément Léger wrote: > > Le Thu, 28 Oct 2021 14:06:12 +0000, > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger wrote: > > > > 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 ? > > > > > + if (ret) > > > > + eth_random_addr(ocelot->base_mac); > > > > + > > > > ocelot->base_mac[5] &= 0xf0; > > > > > > > > return 0; > > > > -- > > > > 2.33.0 > > > >
On Thu, Oct 28, 2021 at 04:38:25PM +0200, Clément Léger wrote: > Le Thu, 28 Oct 2021 14:22:55 +0000, > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > On Thu, Oct 28, 2021 at 04:15:22PM +0200, Clément Léger wrote: > > > Le Thu, 28 Oct 2021 14:06:12 +0000, > > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > > > On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger wrote: > > > > > 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? > > > > > + if (ret) > > > > > + eth_random_addr(ocelot->base_mac); > > > > > + > > > > > ocelot->base_mac[5] &= 0xf0; > > > > > > > > > > return 0; > > > > > -- > > > > > 2.33.0 > > > > > > >
Le Thu, 28 Oct 2021 14:51:43 +0000, Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > On Thu, Oct 28, 2021 at 04:38:25PM +0200, Clément Léger wrote: > > Le Thu, 28 Oct 2021 14:22:55 +0000, > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > On Thu, Oct 28, 2021 at 04:15:22PM +0200, Clément Léger wrote: > > > > Le Thu, 28 Oct 2021 14:06:12 +0000, > > > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > > > > > On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger > > > > > wrote: > > > > > > 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? Sorry, guess I'm a bit tired and missed out the base_mac = random... Acked for the whole modification requested. > > > > > > > + if (ret) > > > > > > + eth_random_addr(ocelot->base_mac); > > > > > > + > > > > > > ocelot->base_mac[5] &= 0xf0; > > > > > > > > > > > > return 0; > > > > > > -- > > > > > > 2.33.0 > > > > > > > > >
Le Thu, 28 Oct 2021 14:51:43 +0000, Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > On Thu, Oct 28, 2021 at 04:38:25PM +0200, Clément Léger wrote: > > Le Thu, 28 Oct 2021 14:22:55 +0000, > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > On Thu, Oct 28, 2021 at 04:15:22PM +0200, Clément Léger wrote: > > > > Le Thu, 28 Oct 2021 14:06:12 +0000, > > > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > > > > > On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger > > > > > wrote: > > > > > > 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. > > > > > > > + if (ret) > > > > > > + eth_random_addr(ocelot->base_mac); > > > > > > + > > > > > > ocelot->base_mac[5] &= 0xf0; > > > > > > > > > > > > return 0; > > > > > > -- > > > > > > 2.33.0 > > > > > > > > >
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 <vladimir.oltean@nxp.com> a écrit : > > > On Thu, Oct 28, 2021 at 04:38:25PM +0200, Clément Léger wrote: > > > Le Thu, 28 Oct 2021 14:22:55 +0000, > > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > > > On Thu, Oct 28, 2021 at 04:15:22PM +0200, Clément Léger wrote: > > > > > Le Thu, 28 Oct 2021 14:06:12 +0000, > > > > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > > > > > > > On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger > > > > > > wrote: > > > > > > > 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. > > > > > > > + 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/
Le Fri, 29 Oct 2021 11:35:43 +0000, Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > 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 <vladimir.oltean@nxp.com> a écrit : > > > > > On Thu, Oct 28, 2021 at 04:38:25PM +0200, Clément Léger wrote: > > > > Le Thu, 28 Oct 2021 14:22:55 +0000, > > > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > > > > > On Thu, Oct 28, 2021 at 04:15:22PM +0200, Clément Léger > > > > > wrote: > > > > > > Le Thu, 28 Oct 2021 14:06:12 +0000, > > > > > > Vladimir Oltean <vladimir.oltean@nxp.com> a écrit : > > > > > > > > > > > > > On Thu, Oct 28, 2021 at 03:49:30PM +0200, Clément Léger > > > > > > > wrote: > > > > > > > > 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. > > > > > > > > > + 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
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); + if (ret) + eth_random_addr(ocelot->base_mac); + ocelot->base_mac[5] &= 0xf0; return 0;
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(-)