From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933143AbcFBScq (ORCPT ); Thu, 2 Jun 2016 14:32:46 -0400 Received: from ausxipps301.us.dell.com ([143.166.148.223]:49359 "EHLO ausxipps301.us.dell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932339AbcFBSco convert rfc822-to-8bit (ORCPT ); Thu, 2 Jun 2016 14:32:44 -0400 DomainKey-Signature: s=smtpout; d=dell.com; c=nofws; q=dns; h=X-LoopCount0:X-IronPort-AV:From:To:CC:Subject: Thread-Topic:Thread-Index:Date:Message-ID:References: In-Reply-To:Accept-Language:Content-Language: X-MS-Has-Attach:X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader:x-originating-ip: Content-Type:Content-Transfer-Encoding:MIME-Version: Return-Path; b=MQ+GaoDoXek6z01zz0g9VZ+mzVOL7TJy1A1+JtWQouNDadmBSy23yVMC CcCoIDupVn8BHZLVC8k61+CJc9jZ3GLekG/RCGbvhi/qPlUawF/r/57mL gUd5+4TRycB5lSRESReDDskRdrDGkRoBk/6bGA5FWk70V0ROVc9KRlCZs w=; X-LoopCount0: from 10.170.28.39 X-IronPort-AV: E=Sophos;i="5.26,407,1459832400"; d="scan'208";a="801397035" From: To: CC: , , , , , Subject: RE: [PATCH v2] r8152: Add support for setting MAC to system's Auxiliary MAC address Thread-Topic: [PATCH v2] r8152: Add support for setting MAC to system's Auxiliary MAC address Thread-Index: AQHRvO/4qOkaZaVerUSL1iyCPJzWv5/WudCAgAAN+QD//6/tUA== Date: Thu, 2 Jun 2016 18:32:42 +0000 Message-ID: <2d81e70165b8447d92b873eafeaeded7@ausx13mpc124.AMER.DELL.COM> References: <1464886687-16284-1-git-send-email-mario_limonciello@dell.com> <1464886687-16284-2-git-send-email-mario_limonciello@dell.com> <20160602174808.GA32131@kroah.com> In-Reply-To: <20160602174808.GA32131@kroah.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.11.181.4] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > -----Original Message----- > From: Greg KH [mailto:gregkh@linuxfoundation.org] > Sent: Thursday, June 2, 2016 12:48 PM > To: Limonciello, Mario > Cc: hayeswang@realtek.com; LKML ; Netdev > ; Linux USB ; > pali.rohar@gmail.com; anthony.wong@canonical.com > 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 > > --- > > 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 > > #include > > #include > > +#include > > +#include > > > > /* 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.