Re: [PATCH v3] net: usb: r8152: Add MAC passthrough support for RTL8153BL

[Henning Schild <henning.schild@siemens.com>]

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;