[V3] r8152: add Linksys USB3GIGV1 id

This linksys dongle by default comes up in cdc_ether mode.
This patch allows r8152 to claim the device:
   Bus 002 Device 002: ID 13b1:0041 Linksys

Signed-off-by: Grant Grundler <grundler@chromium.org>
---
 drivers/net/usb/cdc_ether.c | 10 ++++++++++
 drivers/net/usb/r8152.c     |  2 ++
 2 files changed, 12 insertions(+)

V3: for backwards compat, add #ifdef CONFIG_USB_RTL8152 around
    the cdc_ether blacklist entry so the cdc_ether driver can
    still claim the device if r8152 driver isn't configured.

V2: add LINKSYS_VENDOR_ID to cdc_ether blacklist

Hi,

On Wed, Sep 27, 2017 at 10:28 AM, Grant Grundler <grundler@chromium.org> wrote:
> This linksys dongle by default comes up in cdc_ether mode.
> This patch allows r8152 to claim the device:
>    Bus 002 Device 002: ID 13b1:0041 Linksys
>
> Signed-off-by: Grant Grundler <grundler@chromium.org>
> ---
>  drivers/net/usb/cdc_ether.c | 10 ++++++++++
>  drivers/net/usb/r8152.c     |  2 ++
>  2 files changed, 12 insertions(+)
>
> V3: for backwards compat, add #ifdef CONFIG_USB_RTL8152 around
>     the cdc_ether blacklist entry so the cdc_ether driver can
>     still claim the device if r8152 driver isn't configured.
>
> V2: add LINKSYS_VENDOR_ID to cdc_ether blacklist
>
> diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
> index 8ab281b478f2..446dcc0f1f70 100644
> --- a/drivers/net/usb/cdc_ether.c
> +++ b/drivers/net/usb/cdc_ether.c
> @@ -546,6 +546,7 @@ static const struct driver_info wwan_info = {
>  #define DELL_VENDOR_ID         0x413C
>  #define REALTEK_VENDOR_ID      0x0bda
>  #define SAMSUNG_VENDOR_ID      0x04e8
> +#define LINKSYS_VENDOR_ID      0x13b1
>  #define LENOVO_VENDOR_ID       0x17ef

Slight nit that "LI" sorts after "LE".  You got it right in the other case...


>  #define NVIDIA_VENDOR_ID       0x0955
>  #define HP_VENDOR_ID           0x03f0
> @@ -737,6 +738,15 @@ static const struct usb_device_id  products[] = {
>         .driver_info = 0,
>  },
>
> +#ifdef CONFIG_USB_RTL8152
> +/* Linksys USB3GIGV1 Ethernet Adapter */
> +{
> +       USB_DEVICE_AND_INTERFACE_INFO(LINKSYS_VENDOR_ID, 0x0041, USB_CLASS_COMM,
> +                       USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
> +       .driver_info = 0,
> +},
> +#endif

I believe you want to use IS_ENABLED(), don't you?

There's still a weird esoteric side case where kernel modules don't
all need to be included in the filesystem just because they were built
at the same time.  ...but IMHO that seems like enough of a nit that we
can probably ignore it unless someone has a better idea.


-Doug

Hi Doug!

On Wed, Sep 27, 2017 at 4:47 PM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Wed, Sep 27, 2017 at 10:28 AM, Grant Grundler <grundler@chromium.org> wrote:
>> This linksys dongle by default comes up in cdc_ether mode.
>> This patch allows r8152 to claim the device:
>>    Bus 002 Device 002: ID 13b1:0041 Linksys
>>
>> Signed-off-by: Grant Grundler <grundler@chromium.org>
>> ---
>>  drivers/net/usb/cdc_ether.c | 10 ++++++++++
>>  drivers/net/usb/r8152.c     |  2 ++
>>  2 files changed, 12 insertions(+)
>>
>> V3: for backwards compat, add #ifdef CONFIG_USB_RTL8152 around
>>     the cdc_ether blacklist entry so the cdc_ether driver can
>>     still claim the device if r8152 driver isn't configured.
>>
>> V2: add LINKSYS_VENDOR_ID to cdc_ether blacklist
>>
>> diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
>> index 8ab281b478f2..446dcc0f1f70 100644
>> --- a/drivers/net/usb/cdc_ether.c
>> +++ b/drivers/net/usb/cdc_ether.c
>> @@ -546,6 +546,7 @@ static const struct driver_info wwan_info = {
>>  #define DELL_VENDOR_ID         0x413C
>>  #define REALTEK_VENDOR_ID      0x0bda
>>  #define SAMSUNG_VENDOR_ID      0x04e8
>> +#define LINKSYS_VENDOR_ID      0x13b1
>>  #define LENOVO_VENDOR_ID       0x17ef
>
> Slight nit that "LI" sorts after "LE".  You got it right in the other case...

The list isn't sorted by any rational thing I can see.  I managed to
check my OCD reaction to sort the list numerically. :)

>>  #define NVIDIA_VENDOR_ID       0x0955
>>  #define HP_VENDOR_ID           0x03f0
>> @@ -737,6 +738,15 @@ static const struct usb_device_id  products[] = {
>>         .driver_info = 0,
>>  },
>>
>> +#ifdef CONFIG_USB_RTL8152
>> +/* Linksys USB3GIGV1 Ethernet Adapter */
>> +{
>> +       USB_DEVICE_AND_INTERFACE_INFO(LINKSYS_VENDOR_ID, 0x0041, USB_CLASS_COMM,
>> +                       USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
>> +       .driver_info = 0,
>> +},
>> +#endif
>
> I believe you want to use IS_ENABLED(), don't you?

Ah yes - I wasn't aware IS_ENABLED existed.  Will respin V4 with this
if there isn't any other feedback.


> There's still a weird esoteric side case where kernel modules don't
> all need to be included in the filesystem just because they were built
> at the same time.  ...but IMHO that seems like enough of a nit that we
> can probably ignore it unless someone has a better idea.

I think that would require a run time check. I'm perfectly willing to
ignore that case. :)

thanks!
grant

>
>
> -Doug

Hi,

On Wed, Sep 27, 2017 at 5:07 PM, Grant Grundler <grundler@chromium.org> wrote:
>>>  #define DELL_VENDOR_ID         0x413C
>>>  #define REALTEK_VENDOR_ID      0x0bda
>>>  #define SAMSUNG_VENDOR_ID      0x04e8
>>> +#define LINKSYS_VENDOR_ID      0x13b1
>>>  #define LENOVO_VENDOR_ID       0x17ef
>>
>> Slight nit that "LI" sorts after "LE".  You got it right in the other case...
>
> The list isn't sorted by any rational thing I can see.  I managed to
> check my OCD reaction to sort the list numerically. :)

Whoops, you're right.  It seems to be in a random order.  I just saw
LE, LI, and N sorted properly and jumped to a conclusion.  In any
case, if it's all the same to you it'd be nice if you were consistent
between LENOVO/LINKSYS in the two files.  :-P

-Doug