| Message ID | b24465a706744d99a7c1682f05a24784@AUSX13MPS307.AMER.DELL.COM (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add get/set ethernet address functions and ACPI MAC address pass through functionality to cdc_ncm driver | expand |
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
> > 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
> -----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.
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, };
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(-)