* [PATCH v2] r8152: Add macpassthru support for ThinkPad Thunderbolt 3 Dock Gen 2
@ 2019-11-05 8:15 Kai-Heng Feng
2019-11-05 8:55 ` Hayes Wang
0 siblings, 1 reply; 4+ messages in thread
From: Kai-Heng Feng @ 2019-11-05 8:15 UTC (permalink / raw)
To: davem, oliver; +Cc: hayeswang, linux-usb, netdev, linux-kernel, Kai-Heng Feng
ThinkPad Thunderbolt 3 Dock Gen 2 is another docking station that uses
RTL8153 based USB ethernet.
The device supports macpassthru, but it failed to pass the test of -AD,
-BND and -BD. Simply bypass these tests since the device supports this
feature just fine.
Also the ACPI objects have some differences between Dell's and Lenovo's,
so make those ACPI infos no longer hardcoded.
BugLink: https://bugs.launchpad.net/bugs/1827961
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Use idVendor and idProduct directly.
drivers/net/usb/cdc_ether.c | 7 +++++
drivers/net/usb/r8152.c | 63 ++++++++++++++++++++++++++-----------
2 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index fe630438f67b..0cdb2ce47645 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -766,6 +766,13 @@ static const struct usb_device_id products[] = {
.driver_info = 0,
},
+/* ThinkPad Thunderbolt 3 Dock Gen 2 (based on Realtek RTL8153) */
+{
+ USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x3082, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+ .driver_info = 0,
+},
+
/* Lenovo Thinkpad USB 3.0 Ethernet Adapters (based on Realtek RTL8153) */
{
USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x7205, USB_CLASS_COMM,
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index b9f526ed0d30..ff9fe45901e4 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -670,6 +670,7 @@ enum rtl8152_flags {
SCHEDULE_TASKLET,
GREEN_ETHERNET,
DELL_TB_RX_AGG_BUG,
+ LENOVO_MACPASSTHRU,
};
/* Define these values to match your device */
@@ -1408,38 +1409,57 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
int ret = -EINVAL;
u32 ocp_data;
unsigned char buf[6];
-
- /* 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;
- }
+ bool bypass_test;
+ char *mac_obj_name;
+ acpi_object_type mac_obj_type;
+ int mac_strlen;
+
+ if (test_bit(LENOVO_MACPASSTHRU, &tp->flags)) {
+ bypass_test = true;
+ mac_obj_name = "\\MACA";
+ mac_obj_type = ACPI_TYPE_STRING;
+ mac_strlen = 0x16;
} 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;
+ bypass_test = false;
+ mac_obj_name = "\\_SB.AMAC";
+ mac_obj_type = ACPI_TYPE_BUFFER;
+ mac_strlen = 0x17;
+ }
+
+ if (!bypass_test) {
+ /* 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;
+ }
}
}
/* returns _AUXMAC_#AABBCCDDEEFF# */
- status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
+ status = acpi_evaluate_object(NULL, mac_obj_name, NULL, &buffer);
obj = (union acpi_object *)buffer.pointer;
if (!ACPI_SUCCESS(status))
return -ENODEV;
- if (obj->type != ACPI_TYPE_BUFFER || obj->string.length != 0x17) {
+ if (obj->type != mac_obj_type || obj->string.length != mac_strlen) {
netif_warn(tp, probe, tp->netdev,
"Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
obj->type, obj->string.length);
goto amacout;
}
+
if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
netif_warn(tp, probe, tp->netdev,
@@ -6629,6 +6649,10 @@ static int rtl8152_probe(struct usb_interface *intf,
netdev->hw_features &= ~NETIF_F_RXCSUM;
}
+ if (le16_to_cpu(udev->descriptor.idVendor) == VENDOR_ID_LENOVO &&
+ le16_to_cpu(udev->descriptor.idProduct) == 0x3082)
+ set_bit(LENOVO_MACPASSTHRU, &tp->flags);
+
if (le16_to_cpu(udev->descriptor.bcdDevice) == 0x3011 && udev->serial &&
(!strcmp(udev->serial, "000001000000") ||
!strcmp(udev->serial, "000002000000"))) {
@@ -6755,6 +6779,7 @@ static const struct usb_device_id rtl8152_table[] = {
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x304f)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3062)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3069)},
+ {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3082)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7205)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x720c)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7214)},
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH v2] r8152: Add macpassthru support for ThinkPad Thunderbolt 3 Dock Gen 2
2019-11-05 8:15 [PATCH v2] r8152: Add macpassthru support for ThinkPad Thunderbolt 3 Dock Gen 2 Kai-Heng Feng
@ 2019-11-05 8:55 ` Hayes Wang
2019-11-05 11:18 ` Kai-Heng Feng
0 siblings, 1 reply; 4+ messages in thread
From: Hayes Wang @ 2019-11-05 8:55 UTC (permalink / raw)
To: Kai-Heng Feng, davem, oliver; +Cc: linux-usb, netdev, linux-kernel
Kai-Heng Feng [mailto:kai.heng.feng@canonical.com]
> Sent: Tuesday, November 05, 2019 4:15 PM
> To: davem@davemloft.net; oliver@neukum.org
[...]
> + if (test_bit(LENOVO_MACPASSTHRU, &tp->flags)) {
> + bypass_test = true;
> + mac_obj_name = "\\MACA";
> + mac_obj_type = ACPI_TYPE_STRING;
> + mac_strlen = 0x16;
> } 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;
> + bypass_test = false;
> + mac_obj_name = "\\_SB.AMAC";
> + mac_obj_type = ACPI_TYPE_BUFFER;
> + mac_strlen = 0x17;
> + }
> +
> + if (!bypass_test) {
Maybe you could combine this with the "else" above.
Then, the variable "bypass_test" could be removed.
And the declaration of "ocp_data" could be moved after the "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;
> + }
> }
> }
>
> /* returns _AUXMAC_#AABBCCDDEEFF# */
> - status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
> + status = acpi_evaluate_object(NULL, mac_obj_name, NULL, &buffer);
> obj = (union acpi_object *)buffer.pointer;
> if (!ACPI_SUCCESS(status))
> return -ENODEV;
> - if (obj->type != ACPI_TYPE_BUFFER || obj->string.length != 0x17) {
> + if (obj->type != mac_obj_type || obj->string.length != mac_strlen) {
> netif_warn(tp, probe, tp->netdev,
> "Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
> obj->type, obj->string.length);
> goto amacout;
> }
> +
> if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
> strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
> netif_warn(tp, probe, tp->netdev,
> @@ -6629,6 +6649,10 @@ static int rtl8152_probe(struct usb_interface *intf,
> netdev->hw_features &= ~NETIF_F_RXCSUM;
> }
>
> + if (le16_to_cpu(udev->descriptor.idVendor) == VENDOR_ID_LENOVO &&
> + le16_to_cpu(udev->descriptor.idProduct) == 0x3082)
> + set_bit(LENOVO_MACPASSTHRU, &tp->flags);
> +
> if (le16_to_cpu(udev->descriptor.bcdDevice) == 0x3011 && udev->serial
> &&
> (!strcmp(udev->serial, "000001000000") ||
> !strcmp(udev->serial, "000002000000"))) {
> @@ -6755,6 +6779,7 @@ static const struct usb_device_id rtl8152_table[] = {
> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x304f)},
> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3062)},
> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3069)},
> + {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3082)},
> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7205)},
> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x720c)},
> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7214)},
> --
> 2.17.1
Best Regards,
Hayes
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] r8152: Add macpassthru support for ThinkPad Thunderbolt 3 Dock Gen 2
2019-11-05 8:55 ` Hayes Wang
@ 2019-11-05 11:18 ` Kai-Heng Feng
2019-11-05 11:48 ` Hayes Wang
0 siblings, 1 reply; 4+ messages in thread
From: Kai-Heng Feng @ 2019-11-05 11:18 UTC (permalink / raw)
To: Hayes Wang; +Cc: davem, oliver, linux-usb, netdev, linux-kernel
> On Nov 5, 2019, at 16:55, Hayes Wang <hayeswang@realtek.com> wrote:
>
> Kai-Heng Feng [mailto:kai.heng.feng@canonical.com]
>> Sent: Tuesday, November 05, 2019 4:15 PM
>> To: davem@davemloft.net; oliver@neukum.org
> [...]
>> + if (test_bit(LENOVO_MACPASSTHRU, &tp->flags)) {
>> + bypass_test = true;
>> + mac_obj_name = "\\MACA";
>> + mac_obj_type = ACPI_TYPE_STRING;
>> + mac_strlen = 0x16;
>> } 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;
>> + bypass_test = false;
>> + mac_obj_name = "\\_SB.AMAC";
>> + mac_obj_type = ACPI_TYPE_BUFFER;
>> + mac_strlen = 0x17;
>> + }
>> +
>> + if (!bypass_test) {
>
> Maybe you could combine this with the "else" above.
> Then, the variable "bypass_test" could be removed.
Ok, will do in V3.
> And the declaration of "ocp_data" could be moved after the "else".
Isn't putting declarations at the top of the function the preferred way?
Kai-Heng
>
>> + /* 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;
>> + }
>> }
>> }
>>
>> /* returns _AUXMAC_#AABBCCDDEEFF# */
>> - status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
>> + status = acpi_evaluate_object(NULL, mac_obj_name, NULL, &buffer);
>> obj = (union acpi_object *)buffer.pointer;
>> if (!ACPI_SUCCESS(status))
>> return -ENODEV;
>> - if (obj->type != ACPI_TYPE_BUFFER || obj->string.length != 0x17) {
>> + if (obj->type != mac_obj_type || obj->string.length != mac_strlen) {
>> netif_warn(tp, probe, tp->netdev,
>> "Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
>> obj->type, obj->string.length);
>> goto amacout;
>> }
>> +
>> if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
>> strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
>> netif_warn(tp, probe, tp->netdev,
>> @@ -6629,6 +6649,10 @@ static int rtl8152_probe(struct usb_interface *intf,
>> netdev->hw_features &= ~NETIF_F_RXCSUM;
>> }
>>
>> + if (le16_to_cpu(udev->descriptor.idVendor) == VENDOR_ID_LENOVO &&
>> + le16_to_cpu(udev->descriptor.idProduct) == 0x3082)
>> + set_bit(LENOVO_MACPASSTHRU, &tp->flags);
>> +
>> if (le16_to_cpu(udev->descriptor.bcdDevice) == 0x3011 && udev->serial
>> &&
>> (!strcmp(udev->serial, "000001000000") ||
>> !strcmp(udev->serial, "000002000000"))) {
>> @@ -6755,6 +6779,7 @@ static const struct usb_device_id rtl8152_table[] = {
>> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x304f)},
>> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3062)},
>> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3069)},
>> + {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3082)},
>> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7205)},
>> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x720c)},
>> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7214)},
>> --
>> 2.17.1
>
>
> Best Regards,
> Hayes
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] r8152: Add macpassthru support for ThinkPad Thunderbolt 3 Dock Gen 2
2019-11-05 11:18 ` Kai-Heng Feng
@ 2019-11-05 11:48 ` Hayes Wang
0 siblings, 0 replies; 4+ messages in thread
From: Hayes Wang @ 2019-11-05 11:48 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: davem, oliver, linux-usb, netdev, linux-kernel
Kai-Heng Feng [mailto:kai.heng.feng@canonical.com]
> Sent: Tuesday, November 05, 2019 7:18 PM
[...]
> >> } 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;
> >> + bypass_test = false;
> >> + mac_obj_name = "\\_SB.AMAC";
> >> + mac_obj_type = ACPI_TYPE_BUFFER;
> >> + mac_strlen = 0x17;
> >> + }
> >> +
> >> + if (!bypass_test) {
> >
> > Maybe you could combine this with the "else" above.
> > Then, the variable "bypass_test" could be removed.
>
> Ok, will do in V3.
>
> > And the declaration of "ocp_data" could be moved after the "else".
>
> Isn't putting declarations at the top of the function the preferred way?
I mean the ocp_data wouldn't be used out of the else,
so you could move the declaration to the inside of the else.
However, I don't think you have to send another patch for this.
Thanks.
Best Regards,
Hayes
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-05 11:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05 8:15 [PATCH v2] r8152: Add macpassthru support for ThinkPad Thunderbolt 3 Dock Gen 2 Kai-Heng Feng
2019-11-05 8:55 ` Hayes Wang
2019-11-05 11:18 ` Kai-Heng Feng
2019-11-05 11:48 ` Hayes Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).