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

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

[Charles.Hyde]

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 <charles.hyde@dellteam.com>
Cc: Mario Limonciello <mario.limonciello@dell.com>
Cc: Oliver Neukum <oliver@neukum.org>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-usb@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
---
 drivers/net/usb/cdc_ncm.c | 67 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 85093579612f..11a04dc2298d 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,30 @@ 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;
 		}
+
+		/* Check for a Dell Universal Dock D6000 before checking if
+		 * ACPI supports MAC address pass through.
+		 */
+		if (!strstr(dev->udev->product, "D6000"))
+			goto skip_acpi_mapt_in_bind;
+
+		if (get_acpi_mac_passthru(sa.sa_data) != 0)
+			goto skip_acpi_mapt_in_bind;
+
+		if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) == 0)
+			goto skip_acpi_mapt_in_bind;
+
+		if (cdc_ncm_set_ethernet_address(dev, &sa) == 0)
+			memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
+
+skip_acpi_mapt_in_bind:
 		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
 	}
 
@@ -1716,6 +1736,47 @@ 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;
+		}
+
+		/* Check for a Dell Universal Dock D6000 before checking if
+		 * ACPI supports MAC address pass through.
+		 */
+		if (!strstr(dev->udev->product, "D6000"))
+			goto skip_acpi_mapt_in_resume;
+
+		if (get_acpi_mac_passthru(sa.sa_data) != 0)
+			goto skip_acpi_mapt_in_resume;
+
+		if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) == 0)
+			goto skip_acpi_mapt_in_resume;
+
+		if (cdc_ncm_set_ethernet_address(dev, &sa) == 0)
+			memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
+
+skip_acpi_mapt_in_resume:
+		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 +1909,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,
 };
-- 
2.20.1

Re: [PATCH 3/3] net: cdc_ncm: Add ACPI MAC address pass through functionality

[Oliver Neukum]

Am Freitag, den 30.08.2019, 19:38 +0000 schrieb
Charles.Hyde@dellteam.com:
> 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 <charles.hyde@dellteam.com>
> Cc: Mario Limonciello <mario.limonciello@dell.com>
> Cc: Oliver Neukum <oliver@neukum.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-usb@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> ---
>  drivers/net/usb/cdc_ncm.c | 67 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 64 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 85093579612f..11a04dc2298d 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,30 @@ 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;
>  		}
> +
> +		/* Check for a Dell Universal Dock D6000 before checking if
> +		 * ACPI supports MAC address pass through.
> +		 */
> +		if (!strstr(dev->udev->product, "D6000"))
> +			goto skip_acpi_mapt_in_bind;
> +
> +		if (get_acpi_mac_passthru(sa.sa_data) != 0)
> +			goto skip_acpi_mapt_in_bind;
> +
> +		if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) == 0)
> +			goto skip_acpi_mapt_in_bind;
> +
> +		if (cdc_ncm_set_ethernet_address(dev, &sa) == 0)
> +			memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
> +
> +skip_acpi_mapt_in_bind:
>  		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
>  	}
>  
> @@ -1716,6 +1736,47 @@ 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;
> +		}
> +
> +		/* Check for a Dell Universal Dock D6000 before checking if
> +		 * ACPI supports MAC address pass through.
> +		 */
> +		if (!strstr(dev->udev->product, "D6000"))
> +			goto skip_acpi_mapt_in_resume;

This is too ugly. Use a flag for the need to restore the MAC.
And have you consider the case somebody triggers a reset through
usbfs? It looks to me like you should encapsulate the restoration
of the MAC and also use it in post_reset()

	Regards
		Oliver

RE: [PATCH 3/3] net: cdc_ncm: Add ACPI MAC address pass through functionality

[Charles.Hyde]

> > 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 <charles.hyde@dellteam.com>
> > Cc: Mario Limonciello <mario.limonciello@dell.com>
> > Cc: Oliver Neukum <oliver@neukum.org>
> > Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: linux-usb@vger.kernel.org
> > Cc: linux-acpi@vger.kernel.org
> > ---
> >  drivers/net/usb/cdc_ncm.c | 67
> > +++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 64 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> > index 85093579612f..11a04dc2298d 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,30 @@ 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;
> >  		}
> > +
> > +		/* Check for a Dell Universal Dock D6000 before checking if
> > +		 * ACPI supports MAC address pass through.
> > +		 */
> > +		if (!strstr(dev->udev->product, "D6000"))
> > +			goto skip_acpi_mapt_in_bind;
> > +
> > +		if (get_acpi_mac_passthru(sa.sa_data) != 0)
> > +			goto skip_acpi_mapt_in_bind;
> > +
> > +		if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) == 0)
> > +			goto skip_acpi_mapt_in_bind;
> > +
> > +		if (cdc_ncm_set_ethernet_address(dev, &sa) == 0)
> > +			memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
> > +
> > +skip_acpi_mapt_in_bind:
> >  		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net-
> >dev_addr);
> >  	}
> >
> > @@ -1716,6 +1736,47 @@ 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;
> > +		}
> > +
> > +		/* Check for a Dell Universal Dock D6000 before checking if
> > +		 * ACPI supports MAC address pass through.
> > +		 */
> > +		if (!strstr(dev->udev->product, "D6000"))
> > +			goto skip_acpi_mapt_in_resume;
> 
> This is too ugly. Use a flag for the need to restore the MAC.

It is ugly to me, also, because of restrictions that Linux source cannot be longer than 80 columns.  I much prefer readability and maintainability over 80 column line length limits.   Besides, I have not used an 80 column keypunch machine in more than 35 years.

> And have you consider the case somebody triggers a reset through usbfs? It
> looks to me like you should encapsulate the restoration of the MAC and also
> use it in post_reset()
> 
> 	Regards
> 		Oliver

Will there need to be a pre_reset() function to go with the post_reset() you are suggesting?

Charles

RE: [PATCH 3/3] net: cdc_ncm: Add ACPI MAC address pass through functionality

[Mario.Limonciello]

> -----Original Message-----
> From: Hyde, Charles - Dell Team
> Sent: Tuesday, September 3, 2019 12:50 PM
> To: Oliver Neukum; lenb@kernel.org; rjw@rjwysocki.net
> Cc: Limonciello, Mario; nic_swsd@realtek.com; linux-acpi@vger.kernel.org; linux-
> usb@vger.kernel.org
> Subject: RE: [PATCH 3/3] net: cdc_ncm: Add ACPI MAC address pass through
> functionality
> 
> > > 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 <charles.hyde@dellteam.com>
> > > Cc: Mario Limonciello <mario.limonciello@dell.com>
> > > Cc: Oliver Neukum <oliver@neukum.org>
> > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> > > Cc: Len Brown <lenb@kernel.org>
> > > Cc: linux-usb@vger.kernel.org
> > > Cc: linux-acpi@vger.kernel.org
> > > ---
> > >  drivers/net/usb/cdc_ncm.c | 67
> > > +++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 64 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> > > index 85093579612f..11a04dc2298d 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,30 @@ 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;
> > >  		}
> > > +
> > > +		/* Check for a Dell Universal Dock D6000 before checking if
> > > +		 * ACPI supports MAC address pass through.
> > > +		 */
> > > +		if (!strstr(dev->udev->product, "D6000"))
> > > +			goto skip_acpi_mapt_in_bind;
> > > +
> > > +		if (get_acpi_mac_passthru(sa.sa_data) != 0)
> > > +			goto skip_acpi_mapt_in_bind;
> > > +
> > > +		if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) == 0)
> > > +			goto skip_acpi_mapt_in_bind;
> > > +
> > > +		if (cdc_ncm_set_ethernet_address(dev, &sa) == 0)
> > > +			memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
> > > +
> > > +skip_acpi_mapt_in_bind:
> > >  		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net-
> > >dev_addr);
> > >  	}
> > >
> > > @@ -1716,6 +1736,47 @@ 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;
> > > +		}
> > > +
> > > +		/* Check for a Dell Universal Dock D6000 before checking if
> > > +		 * ACPI supports MAC address pass through.
> > > +		 */
> > > +		if (!strstr(dev->udev->product, "D6000"))
> > > +			goto skip_acpi_mapt_in_resume;
> >
> > This is too ugly. Use a flag for the need to restore the MAC.
> 
> It is ugly to me, also, because of restrictions that Linux source cannot be longer
> than 80 columns.  I much prefer readability and maintainability over 80 column
> line length limits.   Besides, I have not used an 80 column keypunch machine in
> more than 35 years.

This is kernel style though.
https://www.kernel.org/doc/html/v5.2/process/coding-style.html

> 
> > And have you consider the case somebody triggers a reset through usbfs? It
> > looks to me like you should encapsulate the restoration of the MAC and also
> > use it in post_reset()
> >
> > 	Regards
> > 		Oliver
> 
> Will there need to be a pre_reset() function to go with the post_reset() you are
> suggesting?

It should match implementing what 25766271e42f6b15b72ba156cb42a3fea91b5b21
did for r8152.

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

[Charles.Hyde]

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 <charles.hyde@dellteam.com>
Cc: Mario Limonciello <mario.limonciello@dell.com>
Cc: chip.programmer@gmail.com
Cc: Oliver Neukum <oliver@neukum.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-usb@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
---
 drivers/net/usb/cdc_ncm.c | 74 +++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 10 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 85093579612f..e0152d44f5af 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;
@@ -833,6 +834,45 @@ static const struct net_device_ops cdc_ncm_netdev_ops = {
 	.ndo_validate_addr   = eth_validate_addr,
 };
 
+static int get_ethernet_addr(struct usb_interface *intf)
+{
+	struct sockaddr sa;
+	struct usbnet *dev = usb_get_intfdata(intf);
+	struct cdc_ncm_ctx *ctx;
+	int ret = 0;
+
+	if (!dev)
+		return 0;
+
+	ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	if (!ctx->ether_desc)
+		return 0;
+
+	ret = cdc_ncm_get_ethernet_address(dev, ctx);
+	if (ret) {
+		dev_dbg(&intf->dev, "failed to get mac address\n");
+		return ret;
+	}
+
+	/* Check for a Dell Universal Dock D6000 before checking if ACPI
+	 * supports MAC address pass through.
+	 */
+	if (strstr(dev->udev->product, "D6000")) {
+		sa.sa_family = dev->net->type;
+		if (get_acpi_mac_passthru(sa.sa_data)) {
+			if (!memcmp(dev->net->dev_addr, sa.sa_data,
+				    ETH_ALEN)) {
+				if (!cdc_ncm_set_ethernet_address(dev, &sa))
+					memcpy(dev->net->dev_addr, sa.sa_data,
+					       ETH_ALEN);
+			}
+		}
+	}
+	dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
+
+	return 0;
+}
+
 int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting, int drvflags)
 {
 	struct cdc_ncm_ctx *ctx;
@@ -983,14 +1023,8 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	usb_set_intfdata(ctx->data, dev);
 	usb_set_intfdata(ctx->control, dev);
 
-	if (ctx->ether_desc) {
-		temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
-		if (temp) {
-			dev_dbg(&intf->dev, "failed to get mac address\n");
-			goto error2;
-		}
-		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
-	}
+	if (get_ethernet_addr(intf))
+		goto error2;
 
 	/* finish setting up the device specific data */
 	cdc_ncm_setup(dev);
@@ -1716,6 +1750,25 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 	}
 }
 
+static int cdc_ncm_resume(struct usb_interface *intf)
+{
+	int ret;
+
+	ret = usbnet_resume(intf);
+	if (ret == 0)
+		get_ethernet_addr(intf);
+
+	return ret;
+}
+
+static int cdc_ncm_post_reset(struct usb_interface *intf)
+{
+	/* reset the MAC address in case of policy change */
+	get_ethernet_addr(intf);
+
+	return 0;
+}
+
 static const struct driver_info cdc_ncm_info = {
 	.description = "CDC NCM",
 	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
@@ -1848,8 +1901,9 @@ 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,
+	.post_reset = cdc_ncm_post_reset,
 	.supports_autosuspend = 1,
 	.disable_hub_initiated_lpm = 1,
 };
-- 
2.20.1

Re: [PATCH 3/3] net: cdc_ncm: Add ACPI MAC address pass through functionality

[Chunfeng Yun]

On Thu, 2019-09-05 at 21:01 +0000, Charles.Hyde@dellteam.com wrote:
> 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 <charles.hyde@dellteam.com>
> Cc: Mario Limonciello <mario.limonciello@dell.com>
> Cc: chip.programmer@gmail.com
> Cc: Oliver Neukum <oliver@neukum.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-usb@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> ---
>  drivers/net/usb/cdc_ncm.c | 74 +++++++++++++++++++++++++++++++++------
>  1 file changed, 64 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 85093579612f..e0152d44f5af 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;
> @@ -833,6 +834,45 @@ static const struct net_device_ops cdc_ncm_netdev_ops = {
>  	.ndo_validate_addr   = eth_validate_addr,
>  };
>  
> +static int get_ethernet_addr(struct usb_interface *intf)
> +{
> +	struct sockaddr sa;
> +	struct usbnet *dev = usb_get_intfdata(intf);
> +	struct cdc_ncm_ctx *ctx;
> +	int ret = 0;
> +
> +	if (!dev)
> +		return 0;
> +
> +	ctx = (struct cdc_ncm_ctx *)dev->data[0];
> +	if (!ctx->ether_desc)
> +		return 0;
> +
> +	ret = cdc_ncm_get_ethernet_address(dev, ctx);
> +	if (ret) {
> +		dev_dbg(&intf->dev, "failed to get mac address\n");
> +		return ret;
> +	}
> +
> +	/* Check for a Dell Universal Dock D6000 before checking if ACPI
> +	 * supports MAC address pass through.
> +	 */
> +	if (strstr(dev->udev->product, "D6000")) {
> +		sa.sa_family = dev->net->type;
> +		if (get_acpi_mac_passthru(sa.sa_data)) {
> +			if (!memcmp(dev->net->dev_addr, sa.sa_data,
> +				    ETH_ALEN)) {
> +				if (!cdc_ncm_set_ethernet_address(dev, &sa))
How about use one if-statement instead of these three if-statement?

> +					memcpy(dev->net->dev_addr, sa.sa_data,
> +					       ETH_ALEN);
> +			}
> +		}
> +	}
> +	dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
> +
> +	return 0;
> +}
> +
>  int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting, int drvflags)
>  {
>  	struct cdc_ncm_ctx *ctx;
> @@ -983,14 +1023,8 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
>  	usb_set_intfdata(ctx->data, dev);
>  	usb_set_intfdata(ctx->control, dev);
>  
> -	if (ctx->ether_desc) {
> -		temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
> -		if (temp) {
> -			dev_dbg(&intf->dev, "failed to get mac address\n");
> -			goto error2;
> -		}
> -		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
> -	}
> +	if (get_ethernet_addr(intf))
> +		goto error2;
>  
>  	/* finish setting up the device specific data */
>  	cdc_ncm_setup(dev);
> @@ -1716,6 +1750,25 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
>  	}
>  }
>  
> +static int cdc_ncm_resume(struct usb_interface *intf)
> +{
> +	int ret;
> +
> +	ret = usbnet_resume(intf);
> +	if (ret == 0)
> +		get_ethernet_addr(intf);
> +
> +	return ret;
> +}
> +
> +static int cdc_ncm_post_reset(struct usb_interface *intf)
> +{
> +	/* reset the MAC address in case of policy change */
> +	get_ethernet_addr(intf);
> +
> +	return 0;
> +}
> +
>  static const struct driver_info cdc_ncm_info = {
>  	.description = "CDC NCM",
>  	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
> @@ -1848,8 +1901,9 @@ 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,
> +	.post_reset = cdc_ncm_post_reset,
>  	.supports_autosuspend = 1,
>  	.disable_hub_initiated_lpm = 1,
>  };

Re: [PATCH 3/3] net: cdc_ncm: Add ACPI MAC address pass through functionality

[chip.programmer]

>> + if (strstr(dev->udev->product, "D6000")) {
>> + sa.sa_family = dev->net->type;
>> + if (get_acpi_mac_passthru(sa.sa_data)) {
>> + if (!memcmp(dev->net->dev_addr, sa.sa_data,
>> +     ETH_ALEN)) {
>> + if (!cdc_ncm_set_ethernet_address(dev, &sa))

> How about use one if-statement instead of these three if-statement?

While using a single compound if statement is possible, my experience is 
that using multiple if statements will make the code easier to debug and 
maintain by others who come after me.  What is possible to do with code is 
not necessarily what should be done.

Re: [PATCH 3/3] net: cdc_ncm: Add ACPI MAC address pass through functionality

[Bjørn Mork]

<Charles.Hyde@dellteam.com> writes:

> +	if (strstr(dev->udev->product, "D6000")) {

Huh? Can you please test that on all USB devices ever made?


Bjørn

Re: [PATCH 3/3] net: cdc_ncm: Add ACPI MAC address pass through functionality

[Dan Williams]

On Fri, 2019-09-06 at 09:59 +0200, Bjørn Mork wrote:
> <Charles.Hyde@dellteam.com> writes:
> 
> > +	if (strstr(dev->udev->product, "D6000")) {
> 
> Huh? Can you please test that on all USB devices ever made?

Yeah. Can't VID/PID be used as the filter here instead?

Dan