[PATCH] of_net: Implement of_get_nvmem_mac_address helper

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Signed-off-by: Mike Looijmans <redacted>
---
 drivers/of/of_net.c    | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_net.h |  6 ++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index d820f3e..316a537 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c

@@ -7,6 +7,7 @@
  */
 #include <linux/etherdevice.h>
 #include <linux/kernel.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/export.h>

@@ -80,3 +81,50 @@ const void *of_get_mac_address(struct device_node *np)
 	return of_get_mac_addr(np, "address");
 }
 EXPORT_SYMBOL(of_get_mac_address);
+
+/**
+ * Search the device tree for a MAC address, by calling of_get_mac_address
+ * and if that doesn't provide an address, fetch it from an nvmem provider
+ * using the name 'mac-address'.
+ * On success, copies the new address is into memory pointed to by addr and
+ * returns 0. Returns a negative error code otherwise.
+ * @dev:	Pointer to the device containing the device_node
+ * @addr:	Pointer to receive the MAC address using ether_addr_copy()
+ */
+int of_get_nvmem_mac_address(struct device *dev, char *addr)
+{
+	const char *mac;
+	struct nvmem_cell *cell;
+	size_t len;
+	int ret;
+
+	mac = of_get_mac_address(dev->of_node);
+	if (mac) {
+		ether_addr_copy(addr, mac);
+		return 0;
+	}
+
+	cell = nvmem_cell_get(dev, "mac-address");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	mac = (const char *)nvmem_cell_read(cell, &len);
+
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(mac))
+		return PTR_ERR(mac);
+
+	if (len < 6 || !is_valid_ether_addr(mac)) {
+		dev_err(dev, "MAC address from NVMEM is invalid\n");
+		ret = -EINVAL;
+	} else {
+		ether_addr_copy(addr, mac);
+		ret = 0;
+	}
+
+	kfree(mac);
+
+	return ret;
+}
+EXPORT_SYMBOL(of_get_nvmem_mac_address);

diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index 9cd72aa..0d52e1d 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h

@@ -13,6 +13,7 @@
 struct net_device;
 extern int of_get_phy_mode(struct device_node *np);
 extern const void *of_get_mac_address(struct device_node *np);
+extern int of_get_nvmem_mac_address(struct device *dev, char *addr);
 extern struct net_device *of_find_net_device_by_node(struct device_node *np);
 #else
 static inline int of_get_phy_mode(struct device_node *np)

@@ -25,6 +26,11 @@ static inline const void *of_get_mac_address(struct device_node *np)
 	return NULL;
 }
 
+static inline int of_get_nvmem_mac_address(struct device *dev, char *addr)
+{
+	return -ENODEV;
+}
+
 static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
 {
 	return NULL;

-- 
1.9.1

On Fri, Mar 23, 2018 at 03:24:34PM +0100, Mike Looijmans wrote:

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Hi Mike

Please can you document the device tree binding. I assume you are
adding a nvmen-cells and nvmem-cell-names to the Ethernet node in
device tree.

Is there a need to add a new API? Could of_get_mac_address() be
extended to look in NVMEM? The MAC driver does not care. It is saying,
using OF get me a MAC address. One API seems sufficient, and would
mean you don't need to change the MAC drivers.

     Andrew

On 23-3-2018 16:11, Andrew Lunn wrote:

On Fri, Mar 23, 2018 at 03:24:34PM +0100, Mike Looijmans wrote:

quoted

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Hi Mike

Please can you document the device tree binding. I assume you are
adding a nvmen-cells and nvmem-cell-names to the Ethernet node in
device tree.

Indeed. I'll add my settings as an example. Where should I put this documentation, in the commit comment or somewhere in Documents/devicetree/bindings?

quoted

+/**
+ * Search the device tree for a MAC address, by calling of_get_mac_address
+ * and if that doesn't provide an address, fetch it from an nvmem provider
+ * using the name 'mac-address'.
+ * On success, copies the new address is into memory pointed to by addr and
+ * returns 0. Returns a negative error code otherwise.
+ * @dev:	Pointer to the device containing the device_node
+ * @addr:	Pointer to receive the MAC address using ether_addr_copy()
+ */
+int of_get_nvmem_mac_address(struct device *dev, char *addr)
+{
+	const char *mac;
+	struct nvmem_cell *cell;
+	size_t len;
+	int ret;
+
+	mac = of_get_mac_address(dev->of_node);
+	if (mac) {
+		ether_addr_copy(addr, mac);
+		return 0;
+	}

Is there a need to add a new API? Could of_get_mac_address() be
extended to look in NVMEM? The MAC driver does not care. It is saying,
using OF get me a MAC address. One API seems sufficient, and would
mean you don't need to change the MAC drivers.

It's what I intended to do, but there were two problems with that:
- of_get_mac_address() returns a pointer to constant data in memory, but the nvmem functions return an allocated memory object that must be freed after use. This changes the way the call is to be made.
- The nvmem functions need the "struct device" pointer as well, while of_get_mac_address() only gets passed the DT node.

One approach would be to deprecate the of_get_mac_address() interface and migrate existing drivers to the of_get_nvmem_mac_address() interface.


-- 
Mike Looijmans

On 03/23/2018 12:20 PM, Mike Looijmans wrote:

On 23-3-2018 16:11, Andrew Lunn wrote:

quoted

On Fri, Mar 23, 2018 at 03:24:34PM +0100, Mike Looijmans wrote:

quoted

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface
cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Hi Mike

Please can you document the device tree binding. I assume you are
adding a nvmen-cells and nvmem-cell-names to the Ethernet node in
device tree.

Indeed. I'll add my settings as an example. Where should I put this
documentation, in the commit comment or somewhere in
Documents/devicetree/bindings?

quoted

quoted

+/**
+ * Search the device tree for a MAC address, by calling
of_get_mac_address
+ * and if that doesn't provide an address, fetch it from an nvmem
provider
+ * using the name 'mac-address'.
+ * On success, copies the new address is into memory pointed to by
addr and
+ * returns 0. Returns a negative error code otherwise.
+ * @dev:    Pointer to the device containing the device_node
+ * @addr:    Pointer to receive the MAC address using ether_addr_copy()
+ */
+int of_get_nvmem_mac_address(struct device *dev, char *addr)
+{
+    const char *mac;
+    struct nvmem_cell *cell;
+    size_t len;
+    int ret;
+
+    mac = of_get_mac_address(dev->of_node);
+    if (mac) {
+        ether_addr_copy(addr, mac);
+        return 0;
+    }

Is there a need to add a new API? Could of_get_mac_address() be
extended to look in NVMEM? The MAC driver does not care. It is saying,
using OF get me a MAC address. One API seems sufficient, and would
mean you don't need to change the MAC drivers.

It's what I intended to do, but there were two problems with that:
- of_get_mac_address() returns a pointer to constant data in memory, but
the nvmem functions return an allocated memory object that must be freed
after use. This changes the way the call is to be made.

Yeah...

- The nvmem functions need the "struct device" pointer as well, while
of_get_mac_address() only gets passed the DT node.

Bummer, you can't assume there is always a struct device associated with
a struct device_node. Also, bigger question is, how can we make this
work, for e.g: ACPI systems and therefore use an abstract fw_node handle?

One approach would be to deprecate the of_get_mac_address() interface
and migrate existing drivers to the of_get_nvmem_mac_address() interface.

Humm maybe, but clearly making of_get_mac_address() look for a nvmem is
less error prone and does not require people to opt-in for the new
helper, that seems beneficial to me.
-- 
Florian

Indeed. I'll add my settings as an example. Where should I put this
documentation, in the commit comment or somewhere in
Documents/devicetree/bindings?

Documention/devicetree/bindings/net/ethernet.txt

It's what I intended to do, but there were two problems with that:
- of_get_mac_address() returns a pointer to constant data in memory, but the
nvmem functions return an allocated memory object that must be freed after
use. This changes the way the call is to be made.
- The nvmem functions need the "struct device" pointer as well, while
of_get_mac_address() only gets passed the DT node.

Does of_nvmem_cell_get() help?

      Andrew

On 23-03-18 20:42, Andrew Lunn wrote:

quoted

Indeed. I'll add my settings as an example. Where should I put this
documentation, in the commit comment or somewhere in
Documents/devicetree/bindings?

Documention/devicetree/bindings/net/ethernet.txt

Ok

quoted

It's what I intended to do, but there were two problems with that:
- of_get_mac_address() returns a pointer to constant data in memory, but the
nvmem functions return an allocated memory object that must be freed after
use. This changes the way the call is to be made.
- The nvmem functions need the "struct device" pointer as well, while
of_get_mac_address() only gets passed the DT node.

Does of_nvmem_cell_get() help?

Yes, looking at the interface, it would.

-- 
Mike Looijmans

On 23-03-18 20:33, Florian Fainelli wrote:

On 03/23/2018 12:20 PM, Mike Looijmans wrote:

quoted

On 23-3-2018 16:11, Andrew Lunn wrote:

quoted

On Fri, Mar 23, 2018 at 03:24:34PM +0100, Mike Looijmans wrote:

quoted

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface
cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Hi Mike

Please can you document the device tree binding. I assume you are
adding a nvmen-cells and nvmem-cell-names to the Ethernet node in
device tree.

Indeed. I'll add my settings as an example. Where should I put this
documentation, in the commit comment or somewhere in
Documents/devicetree/bindings?

quoted

quoted

+/**
+ * Search the device tree for a MAC address, by calling
of_get_mac_address
+ * and if that doesn't provide an address, fetch it from an nvmem
provider
+ * using the name 'mac-address'.
+ * On success, copies the new address is into memory pointed to by
addr and
+ * returns 0. Returns a negative error code otherwise.
+ * @dev:    Pointer to the device containing the device_node
+ * @addr:    Pointer to receive the MAC address using ether_addr_copy()
+ */
+int of_get_nvmem_mac_address(struct device *dev, char *addr)
+{
+    const char *mac;
+    struct nvmem_cell *cell;
+    size_t len;
+    int ret;
+
+    mac = of_get_mac_address(dev->of_node);
+    if (mac) {
+        ether_addr_copy(addr, mac);
+        return 0;
+    }

Is there a need to add a new API? Could of_get_mac_address() be
extended to look in NVMEM? The MAC driver does not care. It is saying,
using OF get me a MAC address. One API seems sufficient, and would
mean you don't need to change the MAC drivers.

It's what I intended to do, but there were two problems with that:
- of_get_mac_address() returns a pointer to constant data in memory, but
the nvmem functions return an allocated memory object that must be freed
after use. This changes the way the call is to be made.

Yeah...

quoted

- The nvmem functions need the "struct device" pointer as well, while
of_get_mac_address() only gets passed the DT node.

Bummer, you can't assume there is always a struct device associated with
a struct device_node. Also, bigger question is, how can we make this
work, for e.g: ACPI systems and therefore use an abstract fw_node handle?

Andrew Lunn's suggestion of using "of_nvmem_cell_get()" should solve this.

quoted

One approach would be to deprecate the of_get_mac_address() interface
and migrate existing drivers to the of_get_nvmem_mac_address() interface.

Humm maybe, but clearly making of_get_mac_address() look for a nvmem is
less error prone and does not require people to opt-in for the new
helper, that seems beneficial to me.

Totally agree. But I can't think of a way that doesn't change the method's signature. At some point the allocated nvmem buffer must be freed.

A quick survey for the of_get_mac_address users learns that most of them do a memcpy (or similar) right after it, so for these drivers the "of_get_nvmem_mac_address" style signature that performs the memcpy (or better, ether_addr_copy) is a better fit, e.g.:

int of_get_mac_address(struct device_node *np, void *addr)


-- 
Mike Looijmans

A quick survey for the of_get_mac_address users learns that most of them do
a memcpy (or similar) right after it, so for these drivers the
"of_get_nvmem_mac_address" style signature that performs the memcpy (or
better, ether_addr_copy) is a better fit, e.g.:

int of_get_mac_address(struct device_node *np, void *addr)

Hi Mike

This is a nicer solution, but it is quite a lot of work, there are a
lot of users. Maybe Coccinelle can help?

    Andrew

On 24-03-18 19:53, Andrew Lunn wrote:

quoted

A quick survey for the of_get_mac_address users learns that most of them do
a memcpy (or similar) right after it, so for these drivers the
"of_get_nvmem_mac_address" style signature that performs the memcpy (or
better, ether_addr_copy) is a better fit, e.g.:

int of_get_mac_address(struct device_node *np, void *addr)

Hi Mike

This is a nicer solution, but it is quite a lot of work, there are a
lot of users. Maybe Coccinelle can help?

About 58 of them, yeah. And this looked like such a simple thing when I started it...
https://elixir.bootlin.com/linux/v4.16-rc6/ident/of_get_mac_address
I have no experience with Coccinelle though.



-- 
Mike Looijmans

I have no experience with Coccinelle though.

Hi Mike

I've very little either. But all the interactions i've had with
Coccinelle people have been very friendly and helpful. It could be, if
you can describe in words what you need help with, they can write the
script to do it.

       Andrew

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Signed-off-by: Mike Looijmans <redacted>
---
v2: Use of_nvmem_cell_get to avoid needing the assiciated device
    Use void* instead of char*
    Add devicetree binding doc
 Documentation/devicetree/bindings/net/ethernet.txt |  2 +
 drivers/of/of_net.c                                | 47 ++++++++++++++++++++++
 include/linux/of_net.h                             |  6 +++
 3 files changed, 55 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 2974e63..cfc376b 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt

@@ -10,6 +10,8 @@ Documentation/devicetree/bindings/phy/phy-bindings.txt.
   the boot program; should be used in cases where the MAC address assigned to
   the device by the boot program is different from the "local-mac-address"
   property;
+- nvmem-cells: phandle, reference to an nvmem node for the MAC address;
+- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used;
 - max-speed: number, specifies maximum speed in Mbit/s supported by the device;
 - max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
   the maximum frame size (there's contradiction in the Devicetree

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index d820f3e..8999745 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c

@@ -7,6 +7,7 @@
  */
 #include <linux/etherdevice.h>
 #include <linux/kernel.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/export.h>

@@ -80,3 +81,49 @@ const void *of_get_mac_address(struct device_node *np)
 	return of_get_mac_addr(np, "address");
 }
 EXPORT_SYMBOL(of_get_mac_address);
+
+/**
+ * Search the device tree for a MAC address, by calling of_get_mac_address
+ * and if that doesn't provide an address, fetch it from an nvmem provider
+ * using the name 'mac-address'.
+ * On success, copies the new address is into memory pointed to by addr and
+ * returns 0. Returns a negative error code otherwise.
+ * @dev:	Pointer to the device containing the device_node
+ * @addr:	Pointer to receive the MAC address using ether_addr_copy()
+ */
+int of_get_nvmem_mac_address(struct device_node *np, void *addr)
+{
+	const void *mac;
+	struct nvmem_cell *cell;
+	size_t len;
+	int ret;
+
+	mac = of_get_mac_address(np);
+	if (mac) {
+		ether_addr_copy(addr, mac);
+		return 0;
+	}
+
+	cell = of_nvmem_cell_get(np, "mac-address");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	mac = nvmem_cell_read(cell, &len);
+
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(mac))
+		return PTR_ERR(mac);
+
+	if (len < 6 || !is_valid_ether_addr(mac)) {
+		ret = -EINVAL;
+	} else {
+		ether_addr_copy(addr, mac);
+		ret = 0;
+	}
+
+	kfree(mac);
+
+	return ret;
+}
+EXPORT_SYMBOL(of_get_nvmem_mac_address);

diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index 9cd72aa..90d81ee 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h

@@ -13,6 +13,7 @@
 struct net_device;
 extern int of_get_phy_mode(struct device_node *np);
 extern const void *of_get_mac_address(struct device_node *np);
+extern int of_get_nvmem_mac_address(struct device_node *np, void *addr);
 extern struct net_device *of_find_net_device_by_node(struct device_node *np);
 #else
 static inline int of_get_phy_mode(struct device_node *np)

@@ -25,6 +26,11 @@ static inline const void *of_get_mac_address(struct device_node *np)
 	return NULL;
 }
 
+static inline int of_get_nvmem_mac_address(struct device_node *np, void *addr)
+{
+	return -ENODEV;
+}
+
 static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
 {
 	return NULL;

-- 
1.9.1

On Mon, Mar 26, 2018 at 08:41:29AM +0200, Mike Looijmans wrote:

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Hi Mike

I can understand you not wanting to modify all the call sites for
of_get_mac_address().

However, the name of_get_nvmem_mac_address() suggests it gets the MAC
address from NVMEM. I think people are going to be surprised when they
find it first tries for a MAC address directly in device tree. I would
drop the call to of_get_mac_address(), and have the MAC driver call
both.

You could also maybe take a look at fwnode_get_mac_address(). It
should work for both OF and ACPI. It fits better because is passes a
char * for the address. You could make that do both, and call it from
the macb driver. dev_fwnode() probably does what you want.

    Andrew

From: Mike Looijmans <redacted>
Date: Mon, 26 Mar 2018 08:41:29 +0200

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Signed-off-by: Mike Looijmans <redacted>
---
v2: Use of_nvmem_cell_get to avoid needing the assiciated device
    Use void* instead of char*
    Add devicetree binding doc

Like Andrew, I think you should add a new interface for getting the MAC
address from nvmem.

And drivers can call both of them if they want OF device tree and
NVMEM probing for MAC addresses.

Later you can add a consolidated interface, if necessary, which does
both and also take a reference to the MAC address buffer of the driver
in order to deal with the resource allocation issues.

Thanks.

On 03/26/2018 09:58 AM, David Miller wrote:

From: Mike Looijmans <redacted>
Date: Mon, 26 Mar 2018 08:41:29 +0200

quoted

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Signed-off-by: Mike Looijmans <redacted>
---
v2: Use of_nvmem_cell_get to avoid needing the assiciated device
    Use void* instead of char*
    Add devicetree binding doc

Like Andrew, I think you should add a new interface for getting the MAC
address from nvmem.

And drivers can call both of them if they want OF device tree and
NVMEM probing for MAC addresses.

Later you can add a consolidated interface, if necessary, which does
both and also take a reference to the MAC address buffer of the driver
in order to deal with the resource allocation issues.

Agreed, also, how does this fit with Alban's patch series here:

https://lkml.org/lkml/2018/3/24/312

do you depend on those changes at all?
-- 
Florian

On 26-03-18 19:05, Florian Fainelli wrote:

On 03/26/2018 09:58 AM, David Miller wrote:

quoted

From: Mike Looijmans <redacted>
Date: Mon, 26 Mar 2018 08:41:29 +0200

quoted

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Signed-off-by: Mike Looijmans <redacted>
---
v2: Use of_nvmem_cell_get to avoid needing the assiciated device
     Use void* instead of char*
     Add devicetree binding doc

Like Andrew, I think you should add a new interface for getting the MAC
address from nvmem.

And drivers can call both of them if they want OF device tree and
NVMEM probing for MAC addresses.

Later you can add a consolidated interface, if necessary, which does
both and also take a reference to the MAC address buffer of the driver
in order to deal with the resource allocation issues.

Agreed, also, how does this fit with Alban's patch series here:

Ok, makes sense. I'll cook up a v3.

https://lkml.org/lkml/2018/3/24/312

do you depend on those changes at all?

As far as I can tell, there's no dependency, I'm adding an nvmem "consumer" while Alban's patch adds a "provider". I'm mainly interested in storing the MAC address into an I2C EEPROM, which you can also buy with a pre-programmed MAC address in the first 6 bytes, so there's no production cost for managing that.


Alban's patch might some day collide with my idea of making hardware that has some unique ID into nvmem providers, like 1-wire chips, some NOR flash chips (there's the conflict) and various other devices. That would allow a board to obtain a random yet constant MAC address without any additional hardware. I'll cross that bridge when I find it.


-- 
Mike Looijmans

On 26-03-18 17:50, Andrew Lunn wrote:

On Mon, Mar 26, 2018 at 08:41:29AM +0200, Mike Looijmans wrote:

quoted

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Tested by adapting the cadence macb driver to call this instead of
of_get_mac_address().

Hi Mike

I can understand you not wanting to modify all the call sites for
of_get_mac_address().

However, the name of_get_nvmem_mac_address() suggests it gets the MAC
address from NVMEM. I think people are going to be surprised when they
find it first tries for a MAC address directly in device tree. I would
drop the call to of_get_mac_address(), and have the MAC driver call
both.

You could also maybe take a look at fwnode_get_mac_address(). It
should work for both OF and ACPI. It fits better because is passes a
char * for the address. You could make that do both, and call it from
the macb driver. dev_fwnode() probably does what you want.

fwnode_get_mac_address looks really new, there's only one user so far. Is it the intention that all drivers eventually migrate to that?

(It also means I cannot backport it to the kernel I'm actually using, 'cause I haven't got the Zynq to work with the mainline macb driver yet. But that's just my problem...)

-- 
Mike Looijmans

fwnode_get_mac_address looks really new, there's only one user so far. Is it
the intention that all drivers eventually migrate to that?

Hi Mike

Probably not. But any driver which needs to work with both ACPI and OF
is likely to use this API. So server class ARM64 chips for example.

   Andrew

Call of_get_nvmem_mac_address() to fetch the MAC address from an nvmem
cell, if one is provided in the device tree. This allows the address to
be stored in an I2C EEPROM device for example.

Signed-off-by: Mike Looijmans <redacted>
---
 drivers/net/ethernet/cadence/macb_main.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf..eabe14f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c

@@ -3950,10 +3950,16 @@ static int macb_probe(struct platform_device *pdev)
 		dev->max_mtu = ETH_DATA_LEN;
 
 	mac = of_get_mac_address(np);
-	if (mac)
+	if (mac) {
 		ether_addr_copy(bp->dev->dev_addr, mac);
-	else
-		macb_get_hwaddr(bp);
+	} else {
+		err = of_get_nvmem_mac_address(np, bp->dev->dev_addr);
+		if (err) {
+			if (err == -EPROBE_DEFER)
+				goto err_out_free_netdev;
+			macb_get_hwaddr(bp);
+		}
+	}
 
 	err = of_get_phy_mode(np);
 	if (err < 0) {

-- 
1.9.1

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Signed-off-by: Mike Looijmans <redacted>
---
 Documentation/devicetree/bindings/net/ethernet.txt |  2 ++
 drivers/of/of_net.c                                | 40 ++++++++++++++++++++++
 include/linux/of_net.h                             |  6 ++++
 3 files changed, 48 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 2974e63..cfc376b 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt

@@ -10,6 +10,8 @@ Documentation/devicetree/bindings/phy/phy-bindings.txt.
   the boot program; should be used in cases where the MAC address assigned to
   the device by the boot program is different from the "local-mac-address"
   property;
+- nvmem-cells: phandle, reference to an nvmem node for the MAC address;
+- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used;
 - max-speed: number, specifies maximum speed in Mbit/s supported by the device;
 - max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
   the maximum frame size (there's contradiction in the Devicetree

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index d820f3e..1c5d372 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c

@@ -7,6 +7,7 @@
  */
 #include <linux/etherdevice.h>
 #include <linux/kernel.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/export.h>

@@ -80,3 +81,42 @@ const void *of_get_mac_address(struct device_node *np)
 	return of_get_mac_addr(np, "address");
 }
 EXPORT_SYMBOL(of_get_mac_address);
+
+/**
+ * Obtain the MAC address from an nvmem provider named 'mac-address' through
+ * device tree.
+ * On success, copies the new address into memory pointed to by addr and
+ * returns 0. Returns a negative error code otherwise.
+ * @np:		Device tree node containing the nvmem-cells phandle
+ * @addr:	Pointer to receive the MAC address using ether_addr_copy()
+ */
+int of_get_nvmem_mac_address(struct device_node *np, void *addr)
+{
+	struct nvmem_cell *cell;
+	const void *mac;
+	size_t len;
+	int ret;
+
+	cell = of_nvmem_cell_get(np, "mac-address");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	mac = nvmem_cell_read(cell, &len);
+
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(mac))
+		return PTR_ERR(mac);
+
+	if (len < 6 || !is_valid_ether_addr(mac)) {
+		ret = -EINVAL;
+	} else {
+		ether_addr_copy(addr, mac);
+		ret = 0;
+	}
+
+	kfree(mac);
+
+	return ret;
+}
+EXPORT_SYMBOL(of_get_nvmem_mac_address);

diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index 9cd72aa..90d81ee 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h

@@ -13,6 +13,7 @@
 struct net_device;
 extern int of_get_phy_mode(struct device_node *np);
 extern const void *of_get_mac_address(struct device_node *np);
+extern int of_get_nvmem_mac_address(struct device_node *np, void *addr);
 extern struct net_device *of_find_net_device_by_node(struct device_node *np);
 #else
 static inline int of_get_phy_mode(struct device_node *np)

@@ -25,6 +26,11 @@ static inline const void *of_get_mac_address(struct device_node *np)
 	return NULL;
 }
 
+static inline int of_get_nvmem_mac_address(struct device_node *np, void *addr)
+{
+	return -ENODEV;
+}
+
 static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
 {
 	return NULL;

-- 
1.9.1

Posted this as a small set now, with an (optional) second patch that shows
how the changes work and what I've used to test the code on a Topic Miami board.

v3: Add patch that implements mac in nvmem for the Cadence MACB controller
    Remove the integrated of_get_mac_address call

v2: Use of_nvmem_cell_get to avoid needing the assiciated device
    Use void* instead of char*
    Add devicetree binding doc

Mike Looijmans (2):
  of_net: Implement of_get_nvmem_mac_address helper
  net: macb: Try to retrieve MAC addess from nvmem provider

 Documentation/devicetree/bindings/net/ethernet.txt |  2 ++
 drivers/net/ethernet/cadence/macb_main.c           | 12 +++++--
 drivers/of/of_net.c                                | 40 ++++++++++++++++++++++
 include/linux/of_net.h                             |  6 ++++
 4 files changed, 57 insertions(+), 3 deletions(-)

-- 
1.9.1

On Tue, Mar 27, 2018 at 11:52:24AM +0200, Mike Looijmans wrote:

Posted this as a small set now, with an (optional) second patch that shows
how the changes work and what I've used to test the code on a Topic Miami board.

v3: Add patch that implements mac in nvmem for the Cadence MACB controller
    Remove the integrated of_get_mac_address call

v2: Use of_nvmem_cell_get to avoid needing the assiciated device
    Use void* instead of char*
    Add devicetree binding doc

Hi Mike

This looks good now, thanks.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

On 03/27/2018 02:52 AM, Mike Looijmans wrote:

Just one nit here, can you use ETH_ALEN instead of 6? With that fixed:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

On 27/03/2018 at 11:52, Mike Looijmans wrote:

Call of_get_nvmem_mac_address() to fetch the MAC address from an nvmem
cell, if one is provided in the device tree. This allows the address to
be stored in an I2C EEPROM device for example.

Signed-off-by: Mike Looijmans <redacted>

For this part:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

-- 
Nicolas Ferre

Posted this as a small set now, with an (optional) second patch that shows
how the changes work and what I've used to test the code on a Topic Miami board.
I've taken the liberty to add appropriate "Acked" and "Review" tags.

v4: Replaced "6" with ETH_ALEN

v3: Add patch that implements mac in nvmem for the Cadence MACB controller
    Remove the integrated of_get_mac_address call

v2: Use of_nvmem_cell_get to avoid needing the assiciated device
    Use void* instead of char*
    Add devicetree binding doc


Mike Looijmans (2):
  of_net: Implement of_get_nvmem_mac_address helper
  net: macb: Try to retrieve MAC addess from nvmem provider

 Documentation/devicetree/bindings/net/ethernet.txt |  2 ++
 drivers/net/ethernet/cadence/macb_main.c           | 12 +++++--
 drivers/of/of_net.c                                | 40 ++++++++++++++++++++++
 include/linux/of_net.h                             |  6 ++++
 4 files changed, 57 insertions(+), 3 deletions(-)

-- 
1.9.1

Call of_get_nvmem_mac_address() to fetch the MAC address from an nvmem
cell, if one is provided in the device tree. This allows the address to
be stored in an I2C EEPROM device for example.

Signed-off-by: Mike Looijmans <redacted>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e84afcf..eabe14f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c

@@ -3950,10 +3950,16 @@ static int macb_probe(struct platform_device *pdev)
 		dev->max_mtu = ETH_DATA_LEN;
 
 	mac = of_get_mac_address(np);
-	if (mac)
+	if (mac) {
 		ether_addr_copy(bp->dev->dev_addr, mac);
-	else
-		macb_get_hwaddr(bp);
+	} else {
+		err = of_get_nvmem_mac_address(np, bp->dev->dev_addr);
+		if (err) {
+			if (err == -EPROBE_DEFER)
+				goto err_out_free_netdev;
+			macb_get_hwaddr(bp);
+		}
+	}
 
 	err = of_get_phy_mode(np);
 	if (err < 0) {

-- 
1.9.1

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Signed-off-by: Mike Looijmans <redacted>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 Documentation/devicetree/bindings/net/ethernet.txt |  2 ++
 drivers/of/of_net.c                                | 40 ++++++++++++++++++++++
 include/linux/of_net.h                             |  6 ++++
 3 files changed, 48 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 2974e63..cfc376b 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt

@@ -10,6 +10,8 @@ Documentation/devicetree/bindings/phy/phy-bindings.txt.
   the boot program; should be used in cases where the MAC address assigned to
   the device by the boot program is different from the "local-mac-address"
   property;
+- nvmem-cells: phandle, reference to an nvmem node for the MAC address;
+- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used;
 - max-speed: number, specifies maximum speed in Mbit/s supported by the device;
 - max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
   the maximum frame size (there's contradiction in the Devicetree

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index d820f3e..53189d4 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c

@@ -7,6 +7,7 @@
  */
 #include <linux/etherdevice.h>
 #include <linux/kernel.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/export.h>

@@ -80,3 +81,42 @@ const void *of_get_mac_address(struct device_node *np)
 	return of_get_mac_addr(np, "address");
 }
 EXPORT_SYMBOL(of_get_mac_address);
+
+/**
+ * Obtain the MAC address from an nvmem provider named 'mac-address' through
+ * device tree.
+ * On success, copies the new address into memory pointed to by addr and
+ * returns 0. Returns a negative error code otherwise.
+ * @np:		Device tree node containing the nvmem-cells phandle
+ * @addr:	Pointer to receive the MAC address using ether_addr_copy()
+ */
+int of_get_nvmem_mac_address(struct device_node *np, void *addr)
+{
+	struct nvmem_cell *cell;
+	const void *mac;
+	size_t len;
+	int ret;
+
+	cell = of_nvmem_cell_get(np, "mac-address");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	mac = nvmem_cell_read(cell, &len);
+
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(mac))
+		return PTR_ERR(mac);
+
+	if (len < ETH_ALEN || !is_valid_ether_addr(mac)) {
+		ret = -EINVAL;
+	} else {
+		ether_addr_copy(addr, mac);
+		ret = 0;
+	}
+
+	kfree(mac);
+
+	return ret;
+}
+EXPORT_SYMBOL(of_get_nvmem_mac_address);

diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index 9cd72aa..90d81ee 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h

@@ -13,6 +13,7 @@
 struct net_device;
 extern int of_get_phy_mode(struct device_node *np);
 extern const void *of_get_mac_address(struct device_node *np);
+extern int of_get_nvmem_mac_address(struct device_node *np, void *addr);
 extern struct net_device *of_find_net_device_by_node(struct device_node *np);
 #else
 static inline int of_get_phy_mode(struct device_node *np)

@@ -25,6 +26,11 @@ static inline const void *of_get_mac_address(struct device_node *np)
 	return NULL;
 }
 
+static inline int of_get_nvmem_mac_address(struct device_node *np, void *addr)
+{
+	return -ENODEV;
+}
+
 static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
 {
 	return NULL;

-- 
1.9.1

On 28-03-18 00:50, Florian Fainelli wrote:

On 03/27/2018 02:52 AM, Mike Looijmans wrote:

quoted

It's common practice to store MAC addresses for network interfaces into
nvmem devices. However the code to actually do this in the kernel lacks,
so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
address from an nvmem cell provider.

This is particulary useful on devices where the ethernet interface cannot
be configured by the bootloader, for example because it's in an FPGA.

Signed-off-by: Mike Looijmans <redacted>
---
  Documentation/devicetree/bindings/net/ethernet.txt |  2 ++
  drivers/of/of_net.c                                | 40 ++++++++++++++++++++++
  include/linux/of_net.h                             |  6 ++++
  3 files changed, 48 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 2974e63..cfc376b 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt

@@ -10,6 +10,8 @@ Documentation/devicetree/bindings/phy/phy-bindings.txt.
    the boot program; should be used in cases where the MAC address assigned to
    the device by the boot program is different from the "local-mac-address"
    property;
+- nvmem-cells: phandle, reference to an nvmem node for the MAC address;
+- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used;
  - max-speed: number, specifies maximum speed in Mbit/s supported by the device;
  - max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
    the maximum frame size (there's contradiction in the Devicetree

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index d820f3e..1c5d372 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c

@@ -7,6 +7,7 @@
   */
  #include <linux/etherdevice.h>
  #include <linux/kernel.h>
+#include <linux/nvmem-consumer.h>
  #include <linux/of_net.h>
  #include <linux/phy.h>
  #include <linux/export.h>

@@ -80,3 +81,42 @@ const void *of_get_mac_address(struct device_node *np)
  	return of_get_mac_addr(np, "address");
  }
  EXPORT_SYMBOL(of_get_mac_address);
+
+/**
+ * Obtain the MAC address from an nvmem provider named 'mac-address' through
+ * device tree.
+ * On success, copies the new address into memory pointed to by addr and
+ * returns 0. Returns a negative error code otherwise.
+ * @np:		Device tree node containing the nvmem-cells phandle
+ * @addr:	Pointer to receive the MAC address using ether_addr_copy()
+ */
+int of_get_nvmem_mac_address(struct device_node *np, void *addr)
+{
+	struct nvmem_cell *cell;
+	const void *mac;
+	size_t len;
+	int ret;
+
+	cell = of_nvmem_cell_get(np, "mac-address");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	mac = nvmem_cell_read(cell, &len);
+
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(mac))
+		return PTR_ERR(mac);
+
+	if (len < 6 || !is_valid_ether_addr(mac)) {
+		ret = -EINVAL;

Just one nit here, can you use ETH_ALEN instead of 6? With that fixed:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Ok, implemented and tested it, and v4 is on the way with that change.


Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

On 25-03-18 10:17, Mike Looijmans wrote:

On 24-03-18 19:53, Andrew Lunn wrote:

quoted

quoted

A quick survey for the of_get_mac_address users learns that most of them do
a memcpy (or similar) right after it, so for these drivers the
"of_get_nvmem_mac_address" style signature that performs the memcpy (or
better, ether_addr_copy) is a better fit, e.g.:

int of_get_mac_address(struct device_node *np, void *addr)

Hi Mike

This is a nicer solution, but it is quite a lot of work, there are a
lot of users. Maybe Coccinelle can help?

About 58 of them, yeah. And this looked like such a simple thing when I started it...
https://elixir.bootlin.com/linux/v4.16-rc6/ident/of_get_mac_address
I have no experience with Coccinelle though.

I'll at least post a v2 patch with what we've discussed so far.

Another thing that just occurred to me is that the nvmem interface would also allow a MAC address to be written to NV storage. Haven't looked at that path, but it would be nice to be able to permanently program the MAC address using standard interfaces.


Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

From: Mike Looijmans <redacted>
Date: Thu, 29 Mar 2018 07:29:47 +0200

Posted this as a small set now, with an (optional) second patch that shows
how the changes work and what I've used to test the code on a Topic Miami board.
I've taken the liberty to add appropriate "Acked" and "Review" tags.

v4: Replaced "6" with ETH_ALEN

v3: Add patch that implements mac in nvmem for the Cadence MACB controller
    Remove the integrated of_get_mac_address call

v2: Use of_nvmem_cell_get to avoid needing the assiciated device
    Use void* instead of char*
    Add devicetree binding doc

Series applied to net-next, thank you.

On 30-03-18 16:40, David Miller wrote:

From: Mike Looijmans <redacted>
Date: Thu, 29 Mar 2018 07:29:47 +0200

quoted

Posted this as a small set now, with an (optional) second patch that shows
how the changes work and what I've used to test the code on a Topic Miami board.
I've taken the liberty to add appropriate "Acked" and "Review" tags.

v4: Replaced "6" with ETH_ALEN

v3: Add patch that implements mac in nvmem for the Cadence MACB controller
     Remove the integrated of_get_mac_address call

v2: Use of_nvmem_cell_get to avoid needing the assiciated device
     Use void* instead of char*
     Add devicetree binding doc

Series applied to net-next, thank you.

Got a kbuild bot error message in the e-mail, with errors like: "undefined reference to `of_nvmem_cell_get'"

Should I add an "#if CONFIG_NVMEM" guard around the new code and make a v5 patch?

-- 
Mike Looijmans

On Sat, Mar 31, 2018 at 07:20:30PM +0200, Mike Looijmans wrote:

On 30-03-18 16:40, David Miller wrote:

quoted

From: Mike Looijmans <redacted>
Date: Thu, 29 Mar 2018 07:29:47 +0200

quoted

Posted this as a small set now, with an (optional) second patch that shows
how the changes work and what I've used to test the code on a Topic Miami board.
I've taken the liberty to add appropriate "Acked" and "Review" tags.

v4: Replaced "6" with ETH_ALEN

v3: Add patch that implements mac in nvmem for the Cadence MACB controller
    Remove the integrated of_get_mac_address call

v2: Use of_nvmem_cell_get to avoid needing the assiciated device
    Use void* instead of char*
    Add devicetree binding doc

Series applied to net-next, thank you.

Got a kbuild bot error message in the e-mail, with errors like: "undefined
reference to `of_nvmem_cell_get'"

Should I add an "#if CONFIG_NVMEM" guard around the new code and make a v5
patch?

Hi Mike

You need to understand what is actually causing the problem. If it no
NVMEM, or no OF? The kernel config was part of the email, so you
should be able to reproduce the failure.

We don't normally add #ifdef in code. Instead we implement stub
functions in the header file, which would in this case return
something like -ENODEV.

Dave has already take this patch. So you need to do an incremental
fix.

	  Andrew