| Message ID | 1567717304186.90134@Dellteam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
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, > };
<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
>> + 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.
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
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, };
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(-)