[PATCH net-next 0/3] net: add support for an offset of a nvmem provided MAC address

[PATCH net-next 0/3] net: add support for an offset of a nvmem provided MAC address

[Michael Walle]

Boards with multiple ethernet ports might store their MAC addresses not
individually per port but just store one base MAC address. To get the
MAC address of a specific network port we have to add an offset.

This series adds a new device tree property "nvmem-mac-address-offset".

Michael Walle (3):
  dt-bindings: net: add nvmem-mac-address-offset property
  net: add helper eth_addr_add()
  net: implement nvmem-mac-address-offset DT property

 .../bindings/net/ethernet-controller.yaml          |  6 ++++++
 drivers/of/of_net.c                                |  4 ++++
 include/linux/etherdevice.h                        | 14 ++++++++++++++
 net/ethernet/eth.c                                 |  5 +++++
 4 files changed, 29 insertions(+)

-- 
2.20.1

[PATCH net-next 1/3] dt-bindings: net: add nvmem-mac-address-offset property

[Michael Walle]

It is already possible to read the MAC address via a NVMEM provider. But
there are boards, esp. with many ports, which only have a base MAC
address stored. Thus we need to have a way to provide an offset per
network device.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 .../devicetree/bindings/net/ethernet-controller.yaml        | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index e8f04687a3e0..1a8517b0e445 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -52,6 +52,12 @@ properties:
   nvmem-cell-names:
     const: mac-address
 
+  nvmem-mac-address-offset:
+    maxItems: 1
+    description:
+      Specifies an offset which will be added to the MAC address when
+      fetched from a nvmem cell.
+
   phy-connection-type:
     description:
       Specifies interface type between the Ethernet device and a physical
-- 
2.20.1

Re: [PATCH net-next 1/3] dt-bindings: net: add nvmem-mac-address-offset property

[Andrew Lunn]

On Wed, Apr 14, 2021 at 05:26:55PM +0200, Michael Walle wrote:
> It is already possible to read the MAC address via a NVMEM provider. But
> there are boards, esp. with many ports, which only have a base MAC
> address stored. Thus we need to have a way to provide an offset per
> network device.

We need to see what Rob thinks of this. There was recently a patchset
to support swapping the byte order of the MAC address in a NVMEM. Rob
said the NVMEM provider should have the property, not the MAC driver.
This does seems more ethernet specific, so maybe it should be an
Ethernet property?

	 Andrew

Re: [PATCH net-next 1/3] dt-bindings: net: add nvmem-mac-address-offset property

[Rob Herring]

On Wed, Apr 14, 2021 at 05:43:49PM +0200, Andrew Lunn wrote:
> On Wed, Apr 14, 2021 at 05:26:55PM +0200, Michael Walle wrote:
> > It is already possible to read the MAC address via a NVMEM provider. But
> > there are boards, esp. with many ports, which only have a base MAC
> > address stored. Thus we need to have a way to provide an offset per
> > network device.
> 
> We need to see what Rob thinks of this. There was recently a patchset
> to support swapping the byte order of the MAC address in a NVMEM. Rob
> said the NVMEM provider should have the property, not the MAC driver.
> This does seems more ethernet specific, so maybe it should be an
> Ethernet property?

There was also this one[1]. I'm not totally opposed, but don't want to 
see a never ending addition of properties to try to describe any 
possible transformation.

Rob

[1] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20200920095724.8251-4-ansuelsmth@gmail.com/

Re: [PATCH net-next 1/3] dt-bindings: net: add nvmem-mac-address-offset property

[Michael Walle]

Am 2021-04-15 23:59, schrieb Rob Herring:
> On Wed, Apr 14, 2021 at 05:43:49PM +0200, Andrew Lunn wrote:
>> On Wed, Apr 14, 2021 at 05:26:55PM +0200, Michael Walle wrote:
>> > It is already possible to read the MAC address via a NVMEM provider. But
>> > there are boards, esp. with many ports, which only have a base MAC
>> > address stored. Thus we need to have a way to provide an offset per
>> > network device.
>> 
>> We need to see what Rob thinks of this. There was recently a patchset
>> to support swapping the byte order of the MAC address in a NVMEM. Rob
>> said the NVMEM provider should have the property, not the MAC driver.
>> This does seems more ethernet specific, so maybe it should be an
>> Ethernet property?
> 
> There was also this one[1]. I'm not totally opposed, but don't want to
> see a never ending addition of properties to try to describe any
> possible transformation.

Agreed, that stuff like ASCII MAC address parsing should be done
elsewhere. But IMHO adding an offset is a pretty common one (as also
pointed out in [1]). And it also need to be a per ethernet device
property.

-michael

[1] 
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20200920095724.8251-4-ansuelsmth@gmail.com/

Re: [PATCH net-next 1/3] dt-bindings: net: add nvmem-mac-address-offset property

[Michael Walle]

[adding Srinivas Kandagatla and Ansuel Smith]

Am 2021-04-16 00:27, schrieb Michael Walle:
> Am 2021-04-15 23:59, schrieb Rob Herring:
>> On Wed, Apr 14, 2021 at 05:43:49PM +0200, Andrew Lunn wrote:
>>> On Wed, Apr 14, 2021 at 05:26:55PM +0200, Michael Walle wrote:
>>> > It is already possible to read the MAC address via a NVMEM provider. But
>>> > there are boards, esp. with many ports, which only have a base MAC
>>> > address stored. Thus we need to have a way to provide an offset per
>>> > network device.
>>> 
>>> We need to see what Rob thinks of this. There was recently a patchset
>>> to support swapping the byte order of the MAC address in a NVMEM. Rob
>>> said the NVMEM provider should have the property, not the MAC driver.
>>> This does seems more ethernet specific, so maybe it should be an
>>> Ethernet property?
>> 
>> There was also this one[1]. I'm not totally opposed, but don't want to
>> see a never ending addition of properties to try to describe any
>> possible transformation.
> 
> Agreed, that stuff like ASCII MAC address parsing should be done
> elsewhere. But IMHO adding an offset is a pretty common one (as also
> pointed out in [1]). And it also need to be a per ethernet device
> property.

I'm a bit up in the air on this, as I don't know how to proceed here.

To cite Rob from IRC:
   Not really up to me. All the people that care need to come up with
   something flexible enough for common/simple cases and that's not
   going to get extended with every new variation. What I don't want is
   a one-off that's then extended with another one-off.

I already pointed out that this property is per consumer as opposed
to something like endianess swap or parsing a given format. The latter
operates on the nvmem cell.

One random idea is to have a nvmem-cells-transformation (in the lack of
a better name) property for consumers, where you can have some kind of
simple operations like add:
   nvmem-cells-transformation = <NVMEM_ADD 1>
But is that something we really want to have? I'm not sure.

btw. given that there might be other means where a base mac address can
come from in the future, it might make sense to drop the "nvmem-"
prefix and just use "mac-address-offset" (or 
"base-mac-address-offset"?).

> [1] 
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20200920095724.8251-4-ansuelsmth@gmail.com/

-michael

Re: [PATCH net-next 1/3] dt-bindings: net: add nvmem-mac-address-offset property

[Florian Fainelli]

On 4/15/2021 2:59 PM, Rob Herring wrote:
> On Wed, Apr 14, 2021 at 05:43:49PM +0200, Andrew Lunn wrote:
>> On Wed, Apr 14, 2021 at 05:26:55PM +0200, Michael Walle wrote:
>>> It is already possible to read the MAC address via a NVMEM provider. But
>>> there are boards, esp. with many ports, which only have a base MAC
>>> address stored. Thus we need to have a way to provide an offset per
>>> network device.
>>
>> We need to see what Rob thinks of this. There was recently a patchset
>> to support swapping the byte order of the MAC address in a NVMEM. Rob
>> said the NVMEM provider should have the property, not the MAC driver.
>> This does seems more ethernet specific, so maybe it should be an
>> Ethernet property?
> 
> There was also this one[1]. I'm not totally opposed, but don't want to 
> see a never ending addition of properties to try to describe any 
> possible transformation.

If only we could load eBPF bytecode embedded into Device Tree ;)
-- 
Florian

[PATCH net-next 2/3] net: add helper eth_addr_add()

[Michael Walle]

Sometimes you need to add an offset to a base ethernet address. Add a
helper for that.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 include/linux/etherdevice.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 330345b1be54..6ec62c501d3f 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -466,6 +466,20 @@ static inline void eth_addr_inc(u8 *addr)
 	u64_to_ether_addr(u, addr);
 }
 
+/**
+ * eth_addr_add() - Add (or subtract) and offset to/from the given MAC address.
+ *
+ * @offset: Offset to add.
+ * @addr: Pointer to a six-byte array containing Ethernet address to increment.
+ */
+static inline void eth_addr_add(u8 *addr, long offset)
+{
+	u64 u = ether_addr_to_u64(addr);
+
+	u += offset;
+	u64_to_ether_addr(u, addr);
+}
+
 /**
  * is_etherdev_addr - Tell if given Ethernet address belongs to the device.
  * @dev: Pointer to a device structure
-- 
2.20.1

[PATCH net-next 3/3] net: implement nvmem-mac-address-offset DT property

[Michael Walle]

The MAC address fetched by an NVMEM provider might have an offset to it.
Add the support for it in nvmem_get_mac_address().

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/of/of_net.c | 4 ++++
 net/ethernet/eth.c  | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index dbac3a172a11..60c6048a823a 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -63,6 +63,7 @@ static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
 	struct nvmem_cell *cell;
 	const void *mac;
 	size_t len;
+	u32 offset;
 	int ret;
 
 	/* Try lookup by device first, there might be a nvmem_cell_lookup
@@ -92,6 +93,9 @@ static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
 	memcpy(addr, mac, ETH_ALEN);
 	kfree(mac);
 
+	if (!of_property_read_u32(np, "nvmem-mac-address-offset", &offset))
+		eth_addr_add(addr, offset);
+
 	return 0;
 }
 
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 9cce612e8976..fe5311f614fe 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -541,6 +541,7 @@ int nvmem_get_mac_address(struct device *dev, void *addrbuf)
 {
 	struct nvmem_cell *cell;
 	const void *mac;
+	u32 offset;
 	size_t len;
 
 	cell = nvmem_cell_get(dev, "mac-address");
@@ -561,6 +562,10 @@ int nvmem_get_mac_address(struct device *dev, void *addrbuf)
 	ether_addr_copy(addrbuf, mac);
 	kfree(mac);
 
+	if (!device_property_read_u32(dev, "nvmem-mac-address-offset",
+				      &offset))
+		eth_addr_add(addrbuf, offset);
+
 	return 0;
 }
 EXPORT_SYMBOL(nvmem_get_mac_address);
-- 
2.20.1