> -----Original Message----- > From: Greg KH [mailto:gregkh@xxxxxxxxxxxxxxxxxxx] > Sent: Thursday, June 2, 2016 12:48 PM > To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx> > Cc: hayeswang@xxxxxxxxxxx; LKML <linux-kernel@xxxxxxxxxxxxxxx>; Netdev > <netdev@xxxxxxxxxxxxxxx>; Linux USB <linux-usb@xxxxxxxxxxxxxxx>; > pali.rohar@xxxxxxxxx; anthony.wong@xxxxxxxxxxxxx > Subject: Re: [PATCH v2] r8152: Add support for setting MAC to system's > Auxiliary MAC address > > On Thu, Jun 02, 2016 at 11:58:07AM -0500, Mario Limonciello wrote: > > Dell systems with Type-C ports have support for a persistent system > > specific MAC address when used with Dell Type-C docks and dongles. > > This means a dock plugged into two different systems will show different > > (but persistent) MAC addresses. Dell Type-C docks and dongles use the > > r8152 driver. > > > > This information for the system's persistent MAC address is burned in > when > > the HW is built and available under _SB\AMAC in the DSDT at runtime. > > > > More information about the technology is available here: > > http://www.dell.com/support/article/us/en/04/SLN301147 > > > > Signed-off-by: Mario Limonciello <mario_limonciello@xxxxxxxx> > > --- > > drivers/net/usb/r8152.c | 53 > +++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 53 insertions(+) > > > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c > > index 3f9f6ed..6dea542 100644 > > --- a/drivers/net/usb/r8152.c > > +++ b/drivers/net/usb/r8152.c > > @@ -26,6 +26,8 @@ > > #include <linux/mdio.h> > > #include <linux/usb/cdc.h> > > #include <linux/suspend.h> > > +#include <linux/acpi.h> > > +#include <linux/dmi.h> > > > > /* Information for net-next */ > > #define NETNEXT_VERSION "08" > > @@ -500,6 +502,7 @@ enum rtl8152_flags { > > SELECTIVE_SUSPEND, > > PHY_RESET, > > SCHEDULE_NAPI, > > + MAC_PASSTHRU = 0, > > Does setting that to 0 really work? You just did this for two enum > values, what is the compiler supposed to do? Very silly of me. I was rushing to send a v2. I'm surprised this worked. Shouldn't be assigned to anything. > > > }; > > > > /* Define these values to match your device */ > > @@ -653,6 +656,7 @@ enum tx_csum_stat { > > */ > > static const int multicast_filter_limit = 32; > > static unsigned int agg_buf_sz = 16384; > > +static bool mac_passthru_active; > > very generic name for a platform-specific feature :( Once this is broken up into an x86 platform provided method I'll rename this to platform_mac_active (or something similar). > > > > > > #define RTL_LIMITED_TSO_SIZE (agg_buf_sz - sizeof(struct tx_desc) - > \ > > VLAN_ETH_HLEN - VLAN_HLEN) > > @@ -1030,6 +1034,49 @@ out1: > > return ret; > > } > > > > +static int get_auxiliary_addr(struct r8152 *tp, struct sockaddr *sa) > > What about the platform mac address api that was pointed out? I mentioned this in the cover letter - I haven't gotten a chance to move it over there yet. I sent v2 before I did so that you can see what I've been doing as it was relevant to your other comments. > > > +{ > > + acpi_status status; > > + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > > + union acpi_object *obj; > > + int ret = -1; > > + unsigned char buf[6]; > > + > > + if (!dmi_name_in_vendors("Dell Inc.") || mac_passthru_active) > > + return -1; > > Don't make up random error values, please use "real" ones. OK. > > And you want to check this for all Dell devices? Please be model > specific, I doubt a bunch of Dell servers wants to run this code... > Tracking model specific is really going to turn into a giant list never ending list. To drill down more specifically, I can match on chassis too. > > + > > + /* returns _AUXMAC_#AABBCCDDEEFF# */ > > + status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer); > > + obj = (union acpi_object *)buffer.pointer; > > + if (ACPI_SUCCESS(status)) { > > + if (obj->type != ACPI_TYPE_BUFFER || > > + obj->string.length != 0x17) { > > + pr_warn("r8152: get_auxiliary_addr: Invalid buffer"); > > + goto amacout; > > + } > > + if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0) { > > + pr_warn("r8152: get_auxiliary_addr: Invalid header"); > > + goto amacout; > > + } > > + ret = hex2bin(buf, obj->string.pointer + 9, 6); > > + if (ret < 0) { > > + pr_warn("r8152: get_auxiliary_addr: Invalid MAC"); > > + goto amacout; > > + } > > + memcpy(sa->sa_data, buf, 6); > > + ether_addr_copy(tp->netdev->dev_addr, sa->sa_data); > > + netdev_info(tp->netdev, "Using system MAC address > %pM\n", > > + sa->sa_data); > > + set_bit(MAC_PASSTHRU, &tp->flags); > > + mac_passthru_active = true; > > + ret = 1; > > 1 is not a "all is good" return value. OK will switch to 0. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
