[RFC,3/3] net: cdc_ncm: Add ACPI MAC address pass through functionality

Commit Message

[e-mail] Aug. 23, 2019, 10:29 p.m. UTC

This change adds support to cdc_ncm for ACPI MAC address pass through
functionality that also exists in the Realtek r8152 driver.  This is in
support of Dell's Universal Dock D6000, to give it the same feature
capability as is currently available in Windows and advertized on Dell's
product web site.

Signed-off-by: Charles Hyde <[e-mail]>
Cc: Mario Limonciello <[e-mail]>
Cc: Oliver Neukum <[e-mail]>
Cc: [e-mail]
---
 drivers/net/usb/cdc_ncm.c | 47 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 3 deletions(-)

Comments

Bjørn Mork Aug. 25, 2019, 3:11 p.m. UTC | #1

<[e-mail]> writes:

> +static int cdc_ncm_resume (struct usb_interface *intf)
> +{
> +	struct usbnet *dev = usb_get_intfdata(intf);
> +	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
> +	int ret;
> +
> +	ret = usbnet_resume(intf);
> +	if (ret != 0)
> +		goto error2;
> +
> +	if (ctx->ether_desc) {
> +		struct sockaddr sa;
> +
> +		if (cdc_ncm_get_ethernet_address(dev, ctx)) {
> +			dev_dbg(&intf->dev, "failed to get mac address\n");
> +			goto error2;
> +		}
> +		if (get_acpi_mac_passthru(&intf->dev, &sa) == 0) {
> +			if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) != 0) {
> +				if (cdc_ncm_set_ethernet_address(dev, &sa) == 0) {
> +					memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
> +				}
> +			}
> +		}
> +		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
> +	}
> +
> +error2:
> +	return ret;
> +}


This is wrong.  dev->net->dev_addr will hold the correct address, and it
does not have to match any USB descriptor or ACPI table entry. It's a
user managed address.

You should simply make sure the device is syncronized with the
dev->net->dev_addr field.  But you probably want to avoid doing that if
it failed initially.  There is no need to issue control requests you
know will fail.

Why the dev_info()? It logs info the user can retrieve with a simple
"ip link" or similar.  Seems pretty useless even as a debug statement?


Bjørn

Patch

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 6fbe88fd7896..6f138ec3f362 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -52,6 +52,7 @@ 
 #include <linux/usb/usbnet.h>
 #include <linux/usb/cdc.h>
 #include <linux/usb/cdc_ncm.h>
+#include <acpi/acpi_mac_passthru.h>
 
 #if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
 static bool prefer_mbim = true;
@@ -984,11 +985,20 @@  int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	usb_set_intfdata(ctx->control, dev);
 
 	if (ctx->ether_desc) {
-		temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
+		struct sockaddr sa;
+
+		temp = cdc_ncm_get_ethernet_address(dev, ctx);
 		if (temp) {
 			dev_dbg(&intf->dev, "failed to get mac address\n");
 			goto error2;
 		}
+		if (get_acpi_mac_passthru(&intf->dev, &sa) == 0) {
+			if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) != 0) {
+				if (cdc_ncm_set_ethernet_address(dev, &sa) == 0) {
+					memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
+				}
+			}
+		}
 		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
 	}
 
@@ -1716,6 +1726,37 @@  static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 	}
 }
 
+static int cdc_ncm_resume (struct usb_interface *intf)
+{
+	struct usbnet *dev = usb_get_intfdata(intf);
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	int ret;
+
+	ret = usbnet_resume(intf);
+	if (ret != 0)
+		goto error2;
+
+	if (ctx->ether_desc) {
+		struct sockaddr sa;
+
+		if (cdc_ncm_get_ethernet_address(dev, ctx)) {
+			dev_dbg(&intf->dev, "failed to get mac address\n");
+			goto error2;
+		}
+		if (get_acpi_mac_passthru(&intf->dev, &sa) == 0) {
+			if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) != 0) {
+				if (cdc_ncm_set_ethernet_address(dev, &sa) == 0) {
+					memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
+				}
+			}
+		}
+		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
+	}
+
+error2:
+	return ret;
+}
+
 static const struct driver_info cdc_ncm_info = {
 	.description = "CDC NCM",
 	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
@@ -1848,8 +1889,8 @@  static struct usb_driver cdc_ncm_driver = {
 	.probe = usbnet_probe,
 	.disconnect = usbnet_disconnect,
 	.suspend = usbnet_suspend,
-	.resume = usbnet_resume,
-	.reset_resume =	usbnet_resume,
+	.resume = cdc_ncm_resume,
+	.reset_resume =	cdc_ncm_resume,
 	.supports_autosuspend = 1,
 	.disable_hub_initiated_lpm = 1,
 };