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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 28 Oct 2019 12:58:11 +0800
From:   Kai-Heng Feng <kai.heng.feng@...onical.com>
To:     Hayes Wang <hayeswang@...ltek.com>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        "oliver@...kum.org" <oliver@...kum.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        nic_swsd <nic_swsd@...ltek.com>
Subject: Re: [PATCH 2/2] r8152: Add macpassthru support for ThinkPad
 Thunderbolt 3 Dock Gen 2


> On Oct 28, 2019, at 11:58, Hayes Wang <hayeswang@...ltek.com> wrote:
> 
> Kai-Heng Feng [mailto:kai.heng.feng@...onical.com]
>> 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
>