* [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver
@ 2014-02-24 23:13 Keith Packard
2014-02-24 23:42 ` Kyle McMartin
2014-02-25 9:15 ` David Laight
0 siblings, 2 replies; 6+ messages in thread
From: Keith Packard @ 2014-02-24 23:13 UTC (permalink / raw)
To: linux-kernel, linux-usb; +Cc: Keith Packard
The Lenovo OneLink dock includes a USB ethernet adapter using the
AX88179 chip, but with a different USB ID. Add this new USB id to the
driver so that it will autodetect the adapter correctly.
Signed-off-by: Keith Packard <keithp@keithp.com>
Tested-by: Carl Worth <cworth@cworth.org>
---
drivers/net/usb/ax88179_178a.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 8e8d0fc..dcf974f 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1418,6 +1418,19 @@ static const struct driver_info samsung_info = {
.tx_fixup = ax88179_tx_fixup,
};
+static const struct driver_info lenovo_info = {
+ .description = "ThinkPad OneLink Dock USB GigaLAN",
+ .bind = ax88179_bind,
+ .unbind = ax88179_unbind,
+ .status = ax88179_status,
+ .link_reset = ax88179_link_reset,
+ .reset = ax88179_reset,
+ .stop = ax88179_stop,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .rx_fixup = ax88179_rx_fixup,
+ .tx_fixup = ax88179_tx_fixup,
+};
+
static const struct usb_device_id products[] = {
{
/* ASIX AX88179 10/100/1000 */
@@ -1435,6 +1448,10 @@ static const struct usb_device_id products[] = {
/* Samsung USB Ethernet Adapter */
USB_DEVICE(0x04e8, 0xa100),
.driver_info = (unsigned long)&samsung_info,
+}, {
+ /* Lenovo ThinkPad OneLink GigaLAN */
+ USB_DEVICE(0x17ef, 0x304b),
+ .driver_info = (unsigned long)&samsung_info,
},
{ },
};
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver
2014-02-24 23:13 [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver Keith Packard
@ 2014-02-24 23:42 ` Kyle McMartin
2014-02-24 23:56 ` Keith Packard
2014-02-25 9:15 ` David Laight
1 sibling, 1 reply; 6+ messages in thread
From: Kyle McMartin @ 2014-02-24 23:42 UTC (permalink / raw)
To: Keith Packard; +Cc: linux-kernel, linux-usb
On Mon, Feb 24, 2014 at 03:13:49PM -0800, Keith Packard wrote:
> +static const struct driver_info lenovo_info = {
<snip>
> +}, {
> + /* Lenovo ThinkPad OneLink GigaLAN */
> + USB_DEVICE(0x17ef, 0x304b),
> + .driver_info = (unsigned long)&samsung_info,
&lenovo_info, surely.
--Kyle
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver
2014-02-24 23:42 ` Kyle McMartin
@ 2014-02-24 23:56 ` Keith Packard
2014-02-25 8:55 ` Bjørn Mork
0 siblings, 1 reply; 6+ messages in thread
From: Keith Packard @ 2014-02-24 23:56 UTC (permalink / raw)
To: linux-kernel, linux-usb; +Cc: Keith Packard, Kyle McMartin
The Lenovo OneLink dock includes a USB ethernet adapter using the
AX88179 chip, but with a different USB ID. Add this new USB id to the
driver so that it will autodetect the adapter correctly.
Signed-off-by: Keith Packard <keithp@keithp.com>
Tested-by: Carl Worth <cworth@cworth.org>
---
Kyle McMartin says:
> &lenovo_info, surely.
Yeah, Daniel Stone caught the same bug. Not a big deal; the only
difference between any of these elements is the string produced by the
kernel. And that's odd because when we tested it, we saw the expected
name...
drivers/net/usb/ax88179_178a.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 8e8d0fc..7707b4e 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1418,6 +1418,19 @@ static const struct driver_info samsung_info = {
.tx_fixup = ax88179_tx_fixup,
};
+static const struct driver_info lenovo_info = {
+ .description = "ThinkPad OneLink Dock USB GigaLAN",
+ .bind = ax88179_bind,
+ .unbind = ax88179_unbind,
+ .status = ax88179_status,
+ .link_reset = ax88179_link_reset,
+ .reset = ax88179_reset,
+ .stop = ax88179_stop,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .rx_fixup = ax88179_rx_fixup,
+ .tx_fixup = ax88179_tx_fixup,
+};
+
static const struct usb_device_id products[] = {
{
/* ASIX AX88179 10/100/1000 */
@@ -1435,6 +1448,10 @@ static const struct usb_device_id products[] = {
/* Samsung USB Ethernet Adapter */
USB_DEVICE(0x04e8, 0xa100),
.driver_info = (unsigned long)&samsung_info,
+}, {
+ /* Lenovo ThinkPad OneLink GigaLAN */
+ USB_DEVICE(0x17ef, 0x304b),
+ .driver_info = (unsigned long)&lenovo_info,
},
{ },
};
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver
2014-02-24 23:56 ` Keith Packard
@ 2014-02-25 8:55 ` Bjørn Mork
0 siblings, 0 replies; 6+ messages in thread
From: Bjørn Mork @ 2014-02-25 8:55 UTC (permalink / raw)
To: Keith Packard; +Cc: linux-kernel, linux-usb, Kyle McMartin
Keith Packard <keithp@keithp.com> writes:
> Kyle McMartin says:
>
>> &lenovo_info, surely.
>
> Yeah, Daniel Stone caught the same bug. Not a big deal; the only
> difference between any of these elements is the string produced by the
> kernel.
Yes, and that mess should really be cleaned up in this driver (and
probably others).
The .description field describes the *driver*, not the device. The
device is described by the manufacturer, using the iManufacturer,
iProduct and iSerialstring descriptors, which the USB core takes care of
logging and storing. There is absolutely no reason to add additional,
possibly conflicting, product descriptions in these drivers.
Some additional static data does of course not matter much, but the
additional code is really an unnecessary source of subtle bugs. I'm
pretty sure the entry you copied has bugs in the .flags field
(ax88179_rx_fixup clones the skb and returns 1, which means that
FLAG_MULTI_PACKET should be set). Keeping numerous copies of the exact
same driver definition is just waiting for someone to forget
syncronizing any fixups for such bugs.
I propose replacing samsung_info, lenovo_info and the other entries
differing only by description with a generic driver entry, and trust the
vendors to add the appropriate string descriptors to their devices.
The separate generic entries for USB2 and USB3 versions of the chip is
also more confusing than helpful IMHO, given that the driver really is
identical. I don't think it helps anyone to see "USB 3.0 Gigabit
Ethernet" reported if the adapter is connected to a USB 2.0 host and
vice versa. Something lik "ASIX AX88179/178A" would do just fine as
description regardless of device ID.
Bjørn
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver
2014-02-24 23:13 [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver Keith Packard
2014-02-24 23:42 ` Kyle McMartin
@ 2014-02-25 9:15 ` David Laight
2014-02-25 18:25 ` Keith Packard
1 sibling, 1 reply; 6+ messages in thread
From: David Laight @ 2014-02-25 9:15 UTC (permalink / raw)
To: 'Keith Packard', linux-kernel, linux-usb
From: Keith Packard
> The Lenovo OneLink dock includes a USB ethernet adapter using the
> AX88179 chip, but with a different USB ID. Add this new USB id to the
> driver so that it will autodetect the adapter correctly.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Tested-by: Carl Worth <cworth@cworth.org>
> ---
> drivers/net/usb/ax88179_178a.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index 8e8d0fc..dcf974f 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -1418,6 +1418,19 @@ static const struct driver_info samsung_info = {
> .tx_fixup = ax88179_tx_fixup,
> };
>
> +static const struct driver_info lenovo_info = {
> + .description = "ThinkPad OneLink Dock USB GigaLAN",
> + .bind = ax88179_bind,
> + .unbind = ax88179_unbind,
> + .status = ax88179_status,
> + .link_reset = ax88179_link_reset,
> + .reset = ax88179_reset,
> + .stop = ax88179_stop,
> + .flags = FLAG_ETHER | FLAG_FRAMING_AX,
> + .rx_fixup = ax88179_rx_fixup,
> + .tx_fixup = ax88179_tx_fixup,
> +};
> +
> static const struct usb_device_id products[] = {
> {
> /* ASIX AX88179 10/100/1000 */
> @@ -1435,6 +1448,10 @@ static const struct usb_device_id products[] = {
> /* Samsung USB Ethernet Adapter */
> USB_DEVICE(0x04e8, 0xa100),
> .driver_info = (unsigned long)&samsung_info,
> +}, {
> + /* Lenovo ThinkPad OneLink GigaLAN */
> + USB_DEVICE(0x17ef, 0x304b),
> + .driver_info = (unsigned long)&samsung_info,
^^^^^^^^^^^^
I think you meant lenovo_info.
Actually it looks like the initialiser should be factored somehow
to that the list of functions and flags doesn't need repeating for
every clone.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver
2014-02-25 9:15 ` David Laight
@ 2014-02-25 18:25 ` Keith Packard
0 siblings, 0 replies; 6+ messages in thread
From: Keith Packard @ 2014-02-25 18:25 UTC (permalink / raw)
To: David Laight, linux-kernel, linux-usb
[-- Attachment #1: Type: text/plain, Size: 225 bytes --]
David Laight <David.Laight@ACULAB.COM> writes:
> I think you meant lenovo_info.
I posted an updated patch already; nice to know that so many people are
looking at this kernel patch :-)
--
keith.packard@intel.com
[-- Attachment #2: Type: application/pgp-signature, Size: 810 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-25 22:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-24 23:13 [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver Keith Packard
2014-02-24 23:42 ` Kyle McMartin
2014-02-24 23:56 ` Keith Packard
2014-02-25 8:55 ` Bjørn Mork
2014-02-25 9:15 ` David Laight
2014-02-25 18:25 ` Keith Packard
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.