| Message ID | 20221028092337.822840-6-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ONIE tlv nvmem layout support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
Am 2022-10-28 11:23, schrieb Miquel Raynal: > 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. -michael > + *mac_from = "nvmem cell"; > + eth_hw_addr_set(dev, hw_mac_addr); > + return; > + } > + > *mac_from = "random"; > eth_hw_addr_random(dev); > }
Hi Michael, michael@walle.cc wrote on Fri, 28 Oct 2022 15:33:31 +0200: > Am 2022-10-28 11:23, schrieb Miquel Raynal: > > 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
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)) { + *mac_from = "nvmem cell"; + eth_hw_addr_set(dev, hw_mac_addr); + return; + } + *mac_from = "random"; eth_hw_addr_random(dev); }
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(+)