Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752542AbdI1AHJ (ORCPT ); Wed, 27 Sep 2017 20:07:09 -0400 Received: from mail-yw0-f178.google.com ([209.85.161.178]:51923 "EHLO mail-yw0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752453AbdI1AHH (ORCPT ); Wed, 27 Sep 2017 20:07:07 -0400 X-Google-Smtp-Source: AOwi7QDwTCe9RZ26v224qtoYmbiBXBAj+Rjq2skoswtVa/TjJoIjfBzTDu66SScjyVqjHWRck04YmO0q2E6pWdFfXoQ= MIME-Version: 1.0 In-Reply-To: References: <20170927172802.80654-1-grundler@chromium.org> From: Grant Grundler Date: Wed, 27 Sep 2017 17:07:05 -0700 X-Google-Sender-Auth: giNAScY51XHi8bMtLOffrcjGGOc Message-ID: Subject: Re: [PATCH V3] r8152: add Linksys USB3GIGV1 id To: Doug Anderson Cc: Grant Grundler , Hayes Wang , Oliver Neukum , linux-usb , "David S . Miller" , LKML , netdev Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2504 Lines: 73 Hi Doug! On Wed, Sep 27, 2017 at 4:47 PM, Doug Anderson wrote: > Hi, > > On Wed, Sep 27, 2017 at 10:28 AM, Grant Grundler 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 >> --- >> 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