| Message ID | 20220127100109.12979-1-aaron.ma@canonical.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: usb: r8152: Add MAC passthrough support for RTL8153BL | expand |
On Thu, Jan 27, 2022 at 06:01:09PM +0800, Aaron Ma wrote: > RTL8153-BL is used in Lenovo Thunderbolt4 dock. > Add the support of MAC passthrough. > This is ported from Realtek Outbox driver r8152.53.56-2.15.0. > > There are 2 kinds of rules for MAC passthrough of Lenovo products, > 1st USB vendor ID belongs to Lenovo, 2nd the chip of RTL8153-BL > is dedicated for Lenovo. Check the ocp data first then set ACPI object > names. > > Suggested-by: Hayes Wang <hayeswang@realtek.com> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com> > --- > drivers/net/usb/r8152.c | 44 ++++++++++++++++++++++------------------- > 1 file changed, 24 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c > index ee41088c5251..df997b330ee4 100644 > --- a/drivers/net/usb/r8152.c > +++ b/drivers/net/usb/r8152.c > @@ -718,6 +718,7 @@ enum spd_duplex { > #define AD_MASK 0xfee0 > #define BND_MASK 0x0004 > #define BD_MASK 0x0001 > +#define BL_MASK BIT(3) No tab? :(
On 1/27/22 22:19, Andrew Lunn wrote: > On Thu, Jan 27, 2022 at 06:01:09PM +0800, Aaron Ma wrote: >> RTL8153-BL is used in Lenovo Thunderbolt4 dock. >> Add the support of MAC passthrough. >> This is ported from Realtek Outbox driver r8152.53.56-2.15.0. >> >> There are 2 kinds of rules for MAC passthrough of Lenovo products, >> 1st USB vendor ID belongs to Lenovo, 2nd the chip of RTL8153-BL >> is dedicated for Lenovo. Check the ocp data first then set ACPI object >> names. >> >> Suggested-by: Hayes Wang <hayeswang@realtek.com> >> Signed-off-by: Aaron Ma <aaron.ma@canonical.com> >> --- >> drivers/net/usb/r8152.c | 44 ++++++++++++++++++++++------------------- >> 1 file changed, 24 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c >> index ee41088c5251..df997b330ee4 100644 >> --- a/drivers/net/usb/r8152.c >> +++ b/drivers/net/usb/r8152.c >> @@ -718,6 +718,7 @@ enum spd_duplex { >> #define AD_MASK 0xfee0 >> #define BND_MASK 0x0004 >> #define BD_MASK 0x0001 >> +#define BL_MASK BIT(3) > > Just to be sure, this is defined by Realtek? This is not just Lenovo > just misusing a reserved bit? > From what I know, It's defined by Realtek to identify customized product. It shouldn't be misusing. Also it's used by Realtek outbox driver. Aaron > Andrew
On 1/27/22 19:07, Hayes Wang wrote: > I think the devices with the VID/PID of Lenovo, such as 0x17EF/0x3082 and 0x17EF/0xA387, > would always return -ENODEV here. Is it what you want? > This was a mistake. Try to fix it in V3. Thanks, Aaron > > Best Regards, > Hayes
I am still very much against any patches in that direction. The feature as the vendors envision it does not seem to be really understood or even explained. Just narrowing down the device matching caters for vendor lock-in and confusion when that pass through is happening and when not. And seems to lead to unmaintainable spaghetti-code. People that use this very dock today will see an unexpected mac-change once they update to a kernel with this patch applied. But given that some people seem to want that feature in the kernel, i will stop here and simply disable the feature in the bios. And i will make sure _not_ get any lenovo gear for my lenovo laptop, not sure that matches the vendor-vision. Henning Am Fri, 28 Jan 2022 12:32:07 +0800 schrieb Aaron Ma <aaron.ma@canonical.com>: > RTL8153-BL is used in Lenovo Thunderbolt4 dock. > Add the support of MAC passthrough. > This is ported from Realtek Outbox driver r8152.53.56-2.15.0. > > There are 2 kinds of rules for MAC passthrough of Lenovo products, > 1st USB vendor ID belongs to Lenovo, 2nd the chip of RTL8153-BL > is dedicated for Lenovo. Check flag and the ocp data first, > then set ACPI object names. > > Suggested-by: Hayes Wang <hayeswang@realtek.com> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com> > --- > v1 -> v2: fix whitespace in definition. > v2 -> v3: check flag of vendor/product ID to avoid it return error > drivers/net/usb/r8152.c | 45 > +++++++++++++++++++++++------------------ 1 file changed, 25 > insertions(+), 20 deletions(-) > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c > index ee41088c5251..d8350d229f5c 100644 > --- a/drivers/net/usb/r8152.c > +++ b/drivers/net/usb/r8152.c > @@ -718,6 +718,7 @@ enum spd_duplex { > #define AD_MASK 0xfee0 > #define BND_MASK 0x0004 > #define BD_MASK 0x0001 > +#define BL_MASK BIT(3) > #define EFUSE 0xcfdb > #define PASS_THRU_MASK 0x1 > > @@ -1606,31 +1607,35 @@ static int > vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa) > acpi_object_type mac_obj_type; int mac_strlen; > > + /* test for -AD variant of RTL8153 */ > + ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0); > + if ((ocp_data & AD_MASK) == 0x1000) { > + /* test for MAC address pass-through bit */ > + ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, EFUSE); > + if ((ocp_data & PASS_THRU_MASK) != 1) { > + netif_dbg(tp, probe, tp->netdev, > + "No efuse for RTL8153-AD MAC > pass through\n"); > + return -ENODEV; > + } > + } else { > + ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, > USB_MISC_1); > + if (tp->lenovo_macpassthru || > + (tp->version == RTL_VER_09 && > (ocp_data & BL_MASK))) { > + /* test for Lenovo vender/product ID or > RTL8153BL */ > + tp->lenovo_macpassthru = 1; > + } else if ((ocp_data & BND_MASK) == 0 && (ocp_data & > BD_MASK) == 0) { > + /* test for RTL8153-BND and RTL8153-BD */ > + netif_dbg(tp, probe, tp->netdev, > + "Invalid variant for MAC > pass through\n"); > + return -ENODEV; > + } > + } > + > if (tp->lenovo_macpassthru) { > mac_obj_name = "\\MACA"; > mac_obj_type = ACPI_TYPE_STRING; > mac_strlen = 0x16; > } else { > - /* test for -AD variant of RTL8153 */ > - ocp_data = ocp_read_word(tp, MCU_TYPE_USB, > USB_MISC_0); > - if ((ocp_data & AD_MASK) == 0x1000) { > - /* test for MAC address pass-through bit */ > - ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, > EFUSE); > - if ((ocp_data & PASS_THRU_MASK) != 1) { > - netif_dbg(tp, probe, tp->netdev, > - "No efuse for > RTL8153-AD MAC pass through\n"); > - return -ENODEV; > - } > - } else { > - /* test for RTL8153-BND and RTL8153-BD */ > - ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, > USB_MISC_1); > - if ((ocp_data & BND_MASK) == 0 && (ocp_data > & BD_MASK) == 0) { > - netif_dbg(tp, probe, tp->netdev, > - "Invalid variant for > MAC pass through\n"); > - return -ENODEV; > - } > - } > - > mac_obj_name = "\\_SB.AMAC"; > mac_obj_type = ACPI_TYPE_BUFFER; > mac_strlen = 0x17;
On Fri, Jan 28, 2022 at 09:21:03AM +0100, Henning Schild wrote: > I am still very much against any patches in that direction. The feature > as the vendors envision it does not seem to be really understood or > even explained. > Just narrowing down the device matching caters for vendor lock-in and > confusion when that pass through is happening and when not. And seems > to lead to unmaintainable spaghetti-code. > People that use this very dock today will see an unexpected mac-change > once they update to a kernel with this patch applied. I've not yet been convinced by replies that the proposed code really does only match the given dock, and not random USB dongles. To be convinced i would probably like to see code which positively identifies the dock, and that the USB device is on the correct port of the USB hub within the dock. I doubt you can actually do that in a sane way inside an Ethernet driver. As you say, it will likely lead to unmaintainable spaghetti-code. I also don't really think the vendor would be keen on adding code which they know will get reverted as soon as it is shown to cause a regression. So i would prefer to NACK this, and push it to udev rules where you have a complete picture of the hardware and really can identify with 100% certainty it really is the docks NIC. Andrew
[Public] > -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Friday, January 28, 2022 12:07 > To: Henning Schild <henning.schild@siemens.com> > Cc: Aaron Ma <aaron.ma@canonical.com>; Limonciello, Mario > <Mario.Limonciello@amd.com>; kuba@kernel.org; linux-usb@vger.kernel.org; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > gregkh@linuxfoundation.org; davem@davemloft.net; > hayeswang@realtek.com; tiwai@suse.de > Subject: Re: [PATCH v3] net: usb: r8152: Add MAC passthrough support for > RTL8153BL > > On Fri, Jan 28, 2022 at 09:21:03AM +0100, Henning Schild wrote: > > I am still very much against any patches in that direction. The feature > > as the vendors envision it does not seem to be really understood or > > even explained. > > Just narrowing down the device matching caters for vendor lock-in and > > confusion when that pass through is happening and when not. And seems > > to lead to unmaintainable spaghetti-code. > > People that use this very dock today will see an unexpected mac-change > > once they update to a kernel with this patch applied. > > I've not yet been convinced by replies that the proposed code really > does only match the given dock, and not random USB dongles. Didn't Realtek confirm this bit is used to identify the Lenovo devices? > To be > convinced i would probably like to see code which positively > identifies the dock, and that the USB device is on the correct port of > the USB hub within the dock. I doubt you can actually do that in a > sane way inside an Ethernet driver. As you say, it will likely lead to > unmaintainable spaghetti-code. > > I also don't really think the vendor would be keen on adding code > which they know will get reverted as soon as it is shown to cause a > regression. > > So i would prefer to NACK this, and push it to udev rules where you > have a complete picture of the hardware and really can identify with > 100% certainty it really is the docks NIC. I remember when I did the Dell implementation I tried userspace first. Pushing this out to udev has a few other implications I remember hitting: 1) You need to also get the value you're supposed to use from ACPI BIOS exported some way in userland too. 2) You can run into race conditions with other device or MAC renaming rules. My first try I did it with NM and hit that continually. So you would probably need to land this in systemd or so. > > Andrew
Am Fri, 28 Jan 2022 18:41:16 +0000 schrieb "Limonciello, Mario" <Mario.Limonciello@amd.com>: > [Public] > > > > > -----Original Message----- > > From: Andrew Lunn <andrew@lunn.ch> > > Sent: Friday, January 28, 2022 12:07 > > To: Henning Schild <henning.schild@siemens.com> > > Cc: Aaron Ma <aaron.ma@canonical.com>; Limonciello, Mario > > <Mario.Limonciello@amd.com>; kuba@kernel.org; > > linux-usb@vger.kernel.org; netdev@vger.kernel.org; > > linux-kernel@vger.kernel.org; gregkh@linuxfoundation.org; > > davem@davemloft.net; hayeswang@realtek.com; tiwai@suse.de > > Subject: Re: [PATCH v3] net: usb: r8152: Add MAC passthrough > > support for RTL8153BL > > > > On Fri, Jan 28, 2022 at 09:21:03AM +0100, Henning Schild wrote: > > > I am still very much against any patches in that direction. The > > > feature as the vendors envision it does not seem to be really > > > understood or even explained. > > > Just narrowing down the device matching caters for vendor lock-in > > > and confusion when that pass through is happening and when not. > > > And seems to lead to unmaintainable spaghetti-code. > > > People that use this very dock today will see an unexpected > > > mac-change once they update to a kernel with this patch applied. > > > > I've not yet been convinced by replies that the proposed code really > > does only match the given dock, and not random USB dongles. > > Didn't Realtek confirm this bit is used to identify the Lenovo > devices? The question is really why we do all that and that answer really seems missing. Lenovo might have a very different answer than Dell. All i saw was Lenovo docks that suggest PXE. While that could be a good case, bootloaders and OSs should then inherit the MAC of a NIC that is already up. But sure not try and guess what to do and spoof on their own. (based on weird bits and topology guesses) I am willing to bet that grub/pxelinux/uboot/systemd-boot would all fail a PXE run even if the OS to boot would spoof/inherit. PXE uses spoofed MAC and bootloader will dhcp again and not spoof. dhcp would time out or fall back to somewhere (MAC not known), Linux kernel would not come, PXE boot failed. In fact i have seen first hand how PXE Windows install failed on a Lenovo dock and the real NIC worked. That was within my company and i sure do not know the BIOS settings or what bootloader was involved and if that Windows installer did active spoofing. But i would say that the PXE story probably does not really "work" for Windows either. In fact i am willing to bet the BIOS setting for spoofing was turned on, because it seems to be the factory default. And all stories beyond PXE-bootstrap should probably be answered with "a MAC does not identify a machine". So people that care to ident a machine should use something like x509, or allow network access in any other way not relying on a MAC. If "Linux" cares to spoof for the lazy ones, udev is a better place than the kernel. > > To be > > convinced i would probably like to see code which positively > > identifies the dock, and that the USB device is on the correct port > > of the USB hub within the dock. I doubt you can actually do that in > > a sane way inside an Ethernet driver. As you say, it will likely > > lead to unmaintainable spaghetti-code. > > > > I also don't really think the vendor would be keen on adding code > > which they know will get reverted as soon as it is shown to cause a > > regression. > > > > So i would prefer to NACK this, and push it to udev rules where you > > have a complete picture of the hardware and really can identify with > > 100% certainty it really is the docks NIC. > > I remember when I did the Dell implementation I tried userspace first. > > Pushing this out to udev has a few other implications I remember > hitting: 1) You need to also get the value you're supposed to use > from ACPI BIOS exported some way in userland too. Sounds like a problem with ACPI in userspace. So the kernel could expose ACPI in a better shape. Or you will simply need to see what systemd thinks about a funny "sed | grep | awk | bc" patch to parse binary. DMI might contain bits, but without clear vendor instructions we would be guessing (like on ACPI?). > 2) You can run into race conditions with other device or MAC renaming > rules. My first try I did it with NM and hit that continually. So > you would probably need to land this in systemd or so. For sure you would end up in systemd (udev). NM is just one of many options and would be the wrong place. You might quickly find yourself in mdev (busybox) as well because of that PXE case or because of an initrd. If it was my call i would revert all mac passthough patches and request Lenovo/Dell and others to present their case first hand. (no canonical/amd/suse proxies) The "feature" causes MAC collisions and is not well understood/argued by anyone. https://en.wikipedia.org/wiki/MAC_address > A media access control address (MAC address) is a unique identifier > assigned to a network interface controller (NIC) ... unique and NIC, as opposed to colliding and machine Henning > > > > Andre
[Public] > > > I've not yet been convinced by replies that the proposed code really > > > does only match the given dock, and not random USB dongles. > > > > Didn't Realtek confirm this bit is used to identify the Lenovo > > devices? > > The question is really why we do all that and that answer really seems > missing. Lenovo might have a very different answer than Dell. All i saw > was Lenovo docks that suggest PXE. While that could be a good case, > bootloaders and OSs should then inherit the MAC of a NIC that is > already up. But sure not try and guess what to do and spoof on their > own. (based on weird bits and topology guesses) I am willing to bet that > grub/pxelinux/uboot/systemd-boot would all fail a PXE run even if the > OS to boot would spoof/inherit. > > PXE uses spoofed MAC and bootloader will dhcp again and not spoof. dhcp > would time out or fall back to somewhere (MAC not known), Linux kernel > would not come, PXE boot failed. > > In fact i have seen first hand how PXE Windows install failed on a > Lenovo dock and the real NIC worked. That was within my company and i > sure do not know the BIOS settings or what bootloader was involved and > if that Windows installer did active spoofing. But i would say that the > PXE story probably does not really "work" for Windows either. > In fact i am willing to bet the BIOS setting for spoofing was turned > on, because it seems to be the factory default. > > And all stories beyond PXE-bootstrap should probably be answered with > "a MAC does not identify a machine". So people that care to ident a > machine should use something like x509, or allow network access in any > other way not relying on a MAC. If "Linux" cares to spoof for the lazy > ones, udev is a better place than the kernel. > > > > To be > > > convinced i would probably like to see code which positively > > > identifies the dock, and that the USB device is on the correct port > > > of the USB hub within the dock. I doubt you can actually do that in > > > a sane way inside an Ethernet driver. As you say, it will likely > > > lead to unmaintainable spaghetti-code. > > > > > > I also don't really think the vendor would be keen on adding code > > > which they know will get reverted as soon as it is shown to cause a > > > regression. > > > > > > So i would prefer to NACK this, and push it to udev rules where you > > > have a complete picture of the hardware and really can identify with > > > 100% certainty it really is the docks NIC. > > > > I remember when I did the Dell implementation I tried userspace first. > > > > Pushing this out to udev has a few other implications I remember > > hitting: 1) You need to also get the value you're supposed to use > > from ACPI BIOS exported some way in userland too. > > Sounds like a problem with ACPI in userspace. So the kernel could > expose ACPI in a better shape. Or you will simply need to see what > systemd thinks about a funny "sed | grep | awk | bc" patch to parse > binary. DMI might contain bits, but without clear vendor instructions > we would be guessing (like on ACPI?). Yeah I think if this is to be reverted, step 1 is going to be to export that data into sysfs from some Dell and Lenovo drivers so userspace can get it. No funny sed/grep/awk to parse binary. DMI doesn't contain it (at least for Dell). > > > 2) You can run into race conditions with other device or MAC renaming > > rules. My first try I did it with NM and hit that continually. So > > you would probably need to land this in systemd or so. > > For sure you would end up in systemd (udev). NM is just one of many > options and would be the wrong place. You might quickly find yourself > in mdev (busybox) as well because of that PXE case or because of an > initrd. > > If it was my call i would revert all mac passthough patches and request > Lenovo/Dell and others to present their case first hand. (no > canonical/amd/suse proxies) > The "feature" causes MAC collisions and is not well understood/argued by > anyone. Reverting it has the possibility to really mess up machines people have in the field that have behavior built around it. I think a clear set of rules for what is allow to use it is the only safe way forward. You need to "clearly identify the device" or something. Just FYI I'm not intentionally acting as a proxy to anyone at Dell on behalf of AMD, my only involvement here is because I did the original implementation for Dell when I was there and so I can speak the how things were and thought processes at that time. If the consensus is to revert this then someone from Dell probably does need to speak up. > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.wikip > edia.org%2Fwiki%2FMAC_address&data=04%7C01%7CMario.Limonciello% > 40amd.com%7C385b6dd02672490749d208d9e29ba192%7C3dd8961fe4884e60 > 8e11a82d994e183d%7C0%7C0%7C637789980357612909%7CUnknown%7CTWF > pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > Mn0%3D%7C3000&sdata=9vmPi1%2FF%2BxrkqpjoYoLmmupAJ1vBMckLLo > 5hDMqTuZA%3D&reserved=0 > > > A media access control address (MAC address) is a unique identifier > > assigned to a network interface controller (NIC) ... > > unique and NIC, as opposed to colliding and machine > > Henning > > > > > > > Andre
Am Fri, 28 Jan 2022 20:29:58 +0000 schrieb "Limonciello, Mario" <Mario.Limonciello@amd.com>: > [Public] > > > > > I've not yet been convinced by replies that the proposed code > > > > really does only match the given dock, and not random USB > > > > dongles. > > > > > > Didn't Realtek confirm this bit is used to identify the Lenovo > > > devices? > > > > The question is really why we do all that and that answer really > > seems missing. Lenovo might have a very different answer than Dell. > > All i saw was Lenovo docks that suggest PXE. While that could be a > > good case, bootloaders and OSs should then inherit the MAC of a NIC > > that is already up. But sure not try and guess what to do and spoof > > on their own. (based on weird bits and topology guesses) I am > > willing to bet that grub/pxelinux/uboot/systemd-boot would all fail > > a PXE run even if the OS to boot would spoof/inherit. > > > > PXE uses spoofed MAC and bootloader will dhcp again and not spoof. > > dhcp would time out or fall back to somewhere (MAC not known), > > Linux kernel would not come, PXE boot failed. > > > > In fact i have seen first hand how PXE Windows install failed on a > > Lenovo dock and the real NIC worked. That was within my company and > > i sure do not know the BIOS settings or what bootloader was > > involved and if that Windows installer did active spoofing. But i > > would say that the PXE story probably does not really "work" for > > Windows either. In fact i am willing to bet the BIOS setting for > > spoofing was turned on, because it seems to be the factory default. > > > > And all stories beyond PXE-bootstrap should probably be answered > > with "a MAC does not identify a machine". So people that care to > > ident a machine should use something like x509, or allow network > > access in any other way not relying on a MAC. If "Linux" cares to > > spoof for the lazy ones, udev is a better place than the kernel. > > > > > > To be > > > > convinced i would probably like to see code which positively > > > > identifies the dock, and that the USB device is on the correct > > > > port of the USB hub within the dock. I doubt you can actually > > > > do that in a sane way inside an Ethernet driver. As you say, it > > > > will likely lead to unmaintainable spaghetti-code. > > > > > > > > I also don't really think the vendor would be keen on adding > > > > code which they know will get reverted as soon as it is shown > > > > to cause a regression. > > > > > > > > So i would prefer to NACK this, and push it to udev rules where > > > > you have a complete picture of the hardware and really can > > > > identify with 100% certainty it really is the docks NIC. > > > > > > I remember when I did the Dell implementation I tried userspace > > > first. > > > > > > Pushing this out to udev has a few other implications I remember > > > hitting: 1) You need to also get the value you're supposed to use > > > from ACPI BIOS exported some way in userland too. > > > > Sounds like a problem with ACPI in userspace. So the kernel could > > expose ACPI in a better shape. Or you will simply need to see what > > systemd thinks about a funny "sed | grep | awk | bc" patch to parse > > binary. DMI might contain bits, but without clear vendor > > instructions we would be guessing (like on ACPI?). > > Yeah I think if this is to be reverted, step 1 is going to be to > export that data into sysfs from some Dell and Lenovo drivers so > userspace can get it. No funny sed/grep/awk to parse binary. > > DMI doesn't contain it (at least for Dell). Would still cause a nasty vendor-lockin where passthrough would only happen if peripherals match the laptop. In the end users would end up very confused or Linux would cater for vendors selling their docks. > > > > > 2) You can run into race conditions with other device or MAC > > > renaming rules. My first try I did it with NM and hit that > > > continually. So you would probably need to land this in systemd > > > or so. > > > > For sure you would end up in systemd (udev). NM is just one of many > > options and would be the wrong place. You might quickly find > > yourself in mdev (busybox) as well because of that PXE case or > > because of an initrd. > > > > If it was my call i would revert all mac passthough patches and > > request Lenovo/Dell and others to present their case first hand. (no > > canonical/amd/suse proxies) > > The "feature" causes MAC collisions and is not well > > understood/argued by anyone. > > Reverting it has the possibility to really mess up machines people > have in the field that have behavior built around it. I think a > clear set of rules for what is allow to use it is the only safe way > forward. You need to "clearly identify the device" or something. Very true, i was kind of hardlining it. But IMHO the clear rule is that a MAC does not ident a machine. And while that might break already deployed setups, it will make clear that passthrough is a clear no-go. If you do not draw that line, many more patches might follow. Claiming their rightful place next to Dell hacks. The revert that did already happen actually has potential to "break" already deployed setups. It was still done because i brought up the case and people agreed. I can not speak for the Dell patches, they are in there for much longer and will be more widely deployed. So a revert will potentially affect for people. But they sure seem equally questionable and not well explained. > Just FYI I'm not intentionally acting as a proxy to anyone at Dell on > behalf of AMD, my only involvement here is because I did the original > implementation for Dell when I was there and so I can speak the how > things were and thought processes at that time. I bet nobody wanted to act as a proxy, it just happened somehow. If that proxy can explain in detail why and how that is fine. Since you have been at Dell and started that, do you happen to know the real story? Like real ... not "i had to solve jira ticket 42" > If the consensus is to revert this then someone from Dell probably > does need to speak up. If you do not cater for Dell any longer, maybe you would be just the person to revert and tell them to reveal what "jira 42" was about. Merge windows are wide open and distros will be slow to follow, so enough time to react after a revert. regards, Henning > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.wikip > > edia.org%2Fwiki%2FMAC_address&data=04%7C01%7CMario.Limonciello% > > 40amd.com%7C385b6dd02672490749d208d9e29ba192%7C3dd8961fe4884e60 > > 8e11a82d994e183d%7C0%7C0%7C637789980357612909%7CUnknown%7CTWF > > pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > > Mn0%3D%7C3000&sdata=9vmPi1%2FF%2BxrkqpjoYoLmmupAJ1vBMckLLo > > 5hDMqTuZA%3D&reserved=0 > > > > > A media access control address (MAC address) is a unique > > > identifier assigned to a network interface controller (NIC) ... > > > > unique and NIC, as opposed to colliding and machine > > > > Henning > > > > > > > > > > Andre
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index ee41088c5251..df997b330ee4 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -718,6 +718,7 @@ enum spd_duplex { #define AD_MASK 0xfee0 #define BND_MASK 0x0004 #define BD_MASK 0x0001 +#define BL_MASK BIT(3) #define EFUSE 0xcfdb #define PASS_THRU_MASK 0x1 @@ -1606,31 +1607,34 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa) acpi_object_type mac_obj_type; int mac_strlen; + /* test for -AD variant of RTL8153 */ + ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0); + if ((ocp_data & AD_MASK) == 0x1000) { + /* test for MAC address pass-through bit */ + ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, EFUSE); + if ((ocp_data & PASS_THRU_MASK) != 1) { + netif_dbg(tp, probe, tp->netdev, + "No efuse for RTL8153-AD MAC pass through\n"); + return -ENODEV; + } + } else { + ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, USB_MISC_1); + if (tp->version == RTL_VER_09 && (ocp_data & BL_MASK)) { + /* test for RTL8153BL for Lenovo */ + tp->lenovo_macpassthru = 1; + } else if ((ocp_data & BND_MASK) == 0 && (ocp_data & BD_MASK) == 0) { + /* test for RTL8153-BND and RTL8153-BD */ + netif_dbg(tp, probe, tp->netdev, + "Invalid variant for MAC pass through\n"); + return -ENODEV; + } + } + if (tp->lenovo_macpassthru) { mac_obj_name = "\\MACA"; mac_obj_type = ACPI_TYPE_STRING; mac_strlen = 0x16; } else { - /* test for -AD variant of RTL8153 */ - ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0); - if ((ocp_data & AD_MASK) == 0x1000) { - /* test for MAC address pass-through bit */ - ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, EFUSE); - if ((ocp_data & PASS_THRU_MASK) != 1) { - netif_dbg(tp, probe, tp->netdev, - "No efuse for RTL8153-AD MAC pass through\n"); - return -ENODEV; - } - } else { - /* test for RTL8153-BND and RTL8153-BD */ - ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, USB_MISC_1); - if ((ocp_data & BND_MASK) == 0 && (ocp_data & BD_MASK) == 0) { - netif_dbg(tp, probe, tp->netdev, - "Invalid variant for MAC pass through\n"); - return -ENODEV; - } - } - mac_obj_name = "\\_SB.AMAC"; mac_obj_type = ACPI_TYPE_BUFFER; mac_strlen = 0x17;
RTL8153-BL is used in Lenovo Thunderbolt4 dock. Add the support of MAC passthrough. This is ported from Realtek Outbox driver r8152.53.56-2.15.0. There are 2 kinds of rules for MAC passthrough of Lenovo products, 1st USB vendor ID belongs to Lenovo, 2nd the chip of RTL8153-BL is dedicated for Lenovo. Check the ocp data first then set ACPI object names. Suggested-by: Hayes Wang <hayeswang@realtek.com> Signed-off-by: Aaron Ma <aaron.ma@canonical.com> --- drivers/net/usb/r8152.c | 44 ++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 20 deletions(-)