Re: [PATCH 2/2] r8152: Add macpassthru support for ThinkPad Thunderbolt 3 Dock Gen 2

[Kai-Heng Feng]

> On Oct 28, 2019, at 11:58, Hayes Wang <hayeswang@xxxxxxxxxxx> wrote:
> 
> Kai-Heng Feng [mailto:kai.heng.feng@xxxxxxxxxxxxx]
>> Sent: Friday, October 25, 2019 6:59 PM
> [...]
>> @@ -6626,6 +6648,9 @@ static int rtl8152_probe(struct usb_interface *intf,
>> 		netdev->hw_features &= ~NETIF_F_RXCSUM;
>> 	}
>> 
>> +	if (id->driver_info & R8152_QUIRK_LENOVO_MACPASSTHRU)
> 
> Do you really need this?
> It seems the information of idVendor and idProduct is enough. 

Both idVendor and idProduct are just part of "struct usb_device_id".
IMO it's clearer to add quirks there.

> 
>> +		set_bit(LENOVO_MACPASSTHRU, &tp->flags);
>> +
>> 	if (le16_to_cpu(udev->descriptor.bcdDevice) == 0x3011 && udev->serial
>> &&
>> 	    (!strcmp(udev->serial, "000001000000") ||
>> 	     !strcmp(udev->serial, "000002000000"))) {
>> @@ -6754,6 +6779,8 @@ static const struct usb_device_id rtl8152_table[] = {
>> 	{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x304f, 0)},
>> 	{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x3062, 0)},
>> 	{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x3069, 0)},
>> +	{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x3082,
>> +			R8152_QUIRK_LENOVO_MACPASSTHRU)},
> 
> This limits the usage of driver_info. For example, I couldn't
> use it to store a pointer address anymore.

But will the driver ever use .driver_info for pointers?
There are many driver use it for quirks and quirks only.

Kai-Heng

> 
>> 	{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7205, 0)},
>> 	{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x720c, 0)},
>> 	{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7214, 0)},
>> --
>> 2.17.1
>