Message ID | 1355832626-3034-1-git-send-email-dev@lynxeye.de |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote: > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h > index 9bbeabf..8e9516f 100644 > --- a/include/linux/usb/usbnet.h > +++ b/include/linux/usb/usbnet.h > @@ -106,6 +106,7 @@ struct driver_info { > */ > #define FLAG_MULTI_PACKET 0x2000 > #define FLAG_RX_ASSEMBLE 0x4000 /* rx packets may span >1 frames */ > +#define FLAG_EEPROM_MAC 0x8000 /* initialize device MAC from eeprom */ Hi, this looks sensible, but why are you adding a flag unused in usbnet to usbnet.h? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum: > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote: > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h > > index 9bbeabf..8e9516f 100644 > > --- a/include/linux/usb/usbnet.h > > +++ b/include/linux/usb/usbnet.h > > @@ -106,6 +106,7 @@ struct driver_info { > > */ > > #define FLAG_MULTI_PACKET 0x2000 > > #define FLAG_RX_ASSEMBLE 0x4000 /* rx packets may span >1 frames */ > > +#define FLAG_EEPROM_MAC 0x8000 /* initialize device MAC from eeprom */ > > Hi, > > this looks sensible, but > why are you adding a flag unused in usbnet to usbnet.h? Right, this might not be the right place to add this. Could you point me to a more appropriate place? The data member of usbnet might be a good place to stuff this into, but why is this a plain long and not some kind of pointer? It is used for a different purpose on other ASIX chips already. Regards, Lucas -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 18 December 2012 14:24:32 Lucas Stach wrote: > Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum: > > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote: > > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h > > > index 9bbeabf..8e9516f 100644 > > > --- a/include/linux/usb/usbnet.h > > > +++ b/include/linux/usb/usbnet.h > > > @@ -106,6 +106,7 @@ struct driver_info { > > > */ > > > #define FLAG_MULTI_PACKET 0x2000 > > > #define FLAG_RX_ASSEMBLE 0x4000 /* rx packets may span >1 frames */ > > > +#define FLAG_EEPROM_MAC 0x8000 /* initialize device MAC from eeprom */ > > > > Hi, > > > > this looks sensible, but > > why are you adding a flag unused in usbnet to usbnet.h? > > Right, this might not be the right place to add this. Could you point me > to a more appropriate place? The data member of usbnet might be a good driver_priv is intended for such stuff > place to stuff this into, but why is this a plain long and not some kind > of pointer? It is used for a different purpose on other ASIX chips > already. But there is another pointer. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Dienstag, den 18.12.2012, 14:33 +0100 schrieb Oliver Neukum: > On Tuesday 18 December 2012 14:24:32 Lucas Stach wrote: > > Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum: > > > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote: > > > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h > > > > index 9bbeabf..8e9516f 100644 > > > > --- a/include/linux/usb/usbnet.h > > > > +++ b/include/linux/usb/usbnet.h > > > > @@ -106,6 +106,7 @@ struct driver_info { > > > > */ > > > > #define FLAG_MULTI_PACKET 0x2000 > > > > #define FLAG_RX_ASSEMBLE 0x4000 /* rx packets may span >1 frames */ > > > > +#define FLAG_EEPROM_MAC 0x8000 /* initialize device MAC from eeprom */ > > > > > > Hi, > > > > > > this looks sensible, but > > > why are you adding a flag unused in usbnet to usbnet.h? > > > > Right, this might not be the right place to add this. Could you point me > > to a more appropriate place? The data member of usbnet might be a good > > driver_priv is intended for such stuff > I'm not talking about the usbnet struct, but the driver_info struct. I need a way to pass this flag from the static driver info to the bind function. I don't even have a usbnet device at this point, where I could hang on driver_priv data. Regards, Lucas -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 18 December 2012 14:38:32 Lucas Stach wrote: > Am Dienstag, den 18.12.2012, 14:33 +0100 schrieb Oliver Neukum: > > On Tuesday 18 December 2012 14:24:32 Lucas Stach wrote: > > > Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum: > > > > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote: > > > > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h > > > > > index 9bbeabf..8e9516f 100644 > > > > > --- a/include/linux/usb/usbnet.h > > > > > +++ b/include/linux/usb/usbnet.h > > > > > @@ -106,6 +106,7 @@ struct driver_info { > > > > > */ > > > > > #define FLAG_MULTI_PACKET 0x2000 > > > > > #define FLAG_RX_ASSEMBLE 0x4000 /* rx packets may span >1 frames */ > > > > > +#define FLAG_EEPROM_MAC 0x8000 /* initialize device MAC from eeprom */ > > > > > > > > Hi, > > > > > > > > this looks sensible, but > > > > why are you adding a flag unused in usbnet to usbnet.h? > > > > > > Right, this might not be the right place to add this. Could you point me > > > to a more appropriate place? The data member of usbnet might be a good > > > > driver_priv is intended for such stuff > > > I'm not talking about the usbnet struct, but the driver_info struct. I > need a way to pass this flag from the static driver info to the bind > function. I don't even have a usbnet device at this point, where I could > hang on driver_priv data. True, sorry I misread the patch. You could split up ax88772_bind() implicitly encoding the flag. That would be reasonably clean. If you really don't want to do that, please add another private field. But we cannot pollute the flags. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c index 7a6e758..06f7f7cb 100644 --- a/drivers/net/usb/asix_devices.c +++ b/drivers/net/usb/asix_devices.c @@ -422,14 +422,25 @@ static const struct net_device_ops ax88772_netdev_ops = { static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf) { - int ret, embd_phy; + int ret, embd_phy, i; u8 buf[ETH_ALEN]; u32 phyid; usbnet_get_endpoints(dev,intf); /* Get the MAC address */ - ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf); + if (dev->driver_info->flags & FLAG_EEPROM_MAC) { + for (i = 0; i < (ETH_ALEN >> 1); i++) { + ret = asix_read_cmd(dev, AX_CMD_READ_EEPROM, 0x04 + i, + 0, 2, buf + i * 2); + if (ret < 0) + break; + } + } else { + ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, + 0, 0, ETH_ALEN, buf); + } + if (ret < 0) { netdev_dbg(dev->net, "Failed to read MAC address: %d\n", ret); return ret; @@ -872,6 +883,18 @@ static const struct driver_info ax88772_info = { .tx_fixup = asix_tx_fixup, }; +static const struct driver_info ax88772b_info = { + .description = "ASIX AX88772B USB 2.0 Ethernet", + .bind = ax88772_bind, + .status = asix_status, + .link_reset = ax88772_link_reset, + .reset = ax88772_reset, + .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | + FLAG_MULTI_PACKET | FLAG_EEPROM_MAC, + .rx_fixup = asix_rx_fixup, + .tx_fixup = asix_tx_fixup, +}; + static const struct driver_info ax88178_info = { .description = "ASIX AX88178 USB 2.0 Ethernet", .bind = ax88178_bind, @@ -953,7 +976,7 @@ static const struct usb_device_id products [] = { }, { // ASIX AX88772B 10/100 USB_DEVICE (0x0b95, 0x772b), - .driver_info = (unsigned long) &ax88772_info, + .driver_info = (unsigned long) &ax88772b_info, }, { // ASIX AX88772 10/100 USB_DEVICE (0x0b95, 0x7720), diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h index 9bbeabf..8e9516f 100644 --- a/include/linux/usb/usbnet.h +++ b/include/linux/usb/usbnet.h @@ -106,6 +106,7 @@ struct driver_info { */ #define FLAG_MULTI_PACKET 0x2000 #define FLAG_RX_ASSEMBLE 0x4000 /* rx packets may span >1 frames */ +#define FLAG_EEPROM_MAC 0x8000 /* initialize device MAC from eeprom */ /* init device ... can sleep, or cause probe() failure */ int (*bind)(struct usbnet *, struct usb_interface *);
The device comes up with a MAC address of all zeros. We need to read the initial device MAC from EEPROM so it can be set properly later. Signed-off-by: Lucas Stach <dev@lynxeye.de> --- A similar fix was added into U-Boot: http://patchwork.ozlabs.org/patch/179409/ --- drivers/net/usb/asix_devices.c | 29 ++++++++++++++++++++++++++--- include/linux/usb/usbnet.h | 1 + 2 Dateien geändert, 27 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)