net: add dm9620 net usb driver
Subsystems (3)
PATCH 1/1 net: add dm9620 net usb driver
DM9620 is an USB2.0 network adapter rather than DM9601 USB1.1. This driver processed the RX data with 4 bytes header, TX 2 bytes header, make the control bit exactly right in PHY write operation, and optional IFF_ALLMULTI bit for RX control.
Tested good for many platforms, include X86 desktop and ARM embedded.
--- drivers/net/usb/Kconfig | 13 + drivers/net/usb/Makefile | 1 + drivers/net/usb/dm9620.c | 798 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 812 insertions(+), 0 deletions(-) create mode 100644 drivers/net/usb/dm9620.c
drivers/net/usb/Kconfig +13 -0
@@ -272,6 +272,19 @@ config USB_NET_DM9601 This option adds support for Davicom DM9601 based USB 1.1 10/100 Ethernet adapters. +config USB_NET_DM9620 + tristate "Davicom DM9620/21 based USB 2.0 10/100 ethernet devices" + depends on USB_USBNET + select CRC32 + help + This option adds support for Davicom DM9620 based USB 2.0 + 10/100 Ethernet adapters. + + This driver creates an interface named "ethX", where X depends on + what other networking devices you have in use. + + To compile this driver as a module, choose M here. + config USB_NET_SMSC75XX tristate "SMSC LAN75XX based USB 2.0 gigabit ethernet devices" depends on USB_USBNET
drivers/net/usb/Makefile +1 -0
@@ -14,6 +14,7 @@ obj-$(CONFIG_USB_NET_AX88179_178A) += ax88179_178a.o obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o obj-$(CONFIG_USB_NET_CDC_EEM) += cdc_eem.o obj-$(CONFIG_USB_NET_DM9601) += dm9601.o +obj-$(CONFIG_USB_NET_DM9620) += dm9620.o obj-$(CONFIG_USB_NET_SMSC75XX) += smsc75xx.o obj-$(CONFIG_USB_NET_SMSC95XX) += smsc95xx.o obj-$(CONFIG_USB_NET_GL620A) += gl620a.o
drivers/net/usb/dm9620.c new file +798 -0
798 lines not on this page · raw message
Signature · 1 line
1.7.1
Hi Joseph,
had a fast look...
[ ... ]
Quoted 12 lines
drivers/net/usb/dm9620.c new file +12
-0+static int dm9620_set_eeprom (struct net_device *net, +struct ethtool_eeprom *eeprom,u8 *data) +{ +struct usbnet *dev =netdev_priv (net); +int offset = eeprom->offset; +int len = eeprom->len; +int done; + +if (eeprom->magic !=MD96XX_EEPROM_MAGIC ) { +netdev_err (dev->net,"EEPROM: magic value mismatch, magic = 0x%x\n" , + eeprom->magic); +return -EINVAL ;
drivers/net/usb/dm9620.c line 290
Quoted 10 lines
drivers/net/usb/dm9620.c new file +10
-0+ } + +while (len >0 ) { +if (len &1 || offset &1 ) { +int which = offset &1 ; +u8 tmp[2 ]; +dm_read_eeprom_word (dev, offset /2 , tmp); + tmp[which] = *data; +dm_write_eeprom_word (dev, offset /2 , tmp); +mdelay (10 );
Please don't use mdelay, use msleep or possiblymsleep_interruptable
Quoted 11 lines
drivers/net/usb/dm9620.c new file +11
-0+ done =1 ; + }else { +dm_write_eeprom_word (dev, offset /2 , data); + done =2 ; + } + data += done; + offset += done; + len -= done; + } +return 0 ; +}
[ ... ]
drivers/net/usb/dm9620.c line 500
Quoted 9 lines
drivers/net/usb/dm9620.c new file +9
-0+static int dm9620_bind (struct usbnet *dev,struct usb_interface *intf) +{ +int ret; +u8 mac[ETH_ALEN ], id; +u16 value; + + ret =usbnet_get_endpoints (dev, intf); +if (ret) + goto out;
maybe here is better
if (ret) return ret;
Quoted 9 lines
drivers/net/usb/dm9620.c new file +9
-0+ + dev->net->netdev_ops = &dm9620_netdev_ops; + dev->net->ethtool_ops = &dm9620_ethtool_ops; + dev->net->hard_header_len +=DM_TX_OVERHEAD ; + dev->hard_mtu = dev->net->mtu + dev->net->hard_header_len; + +/* ftp fail fixed */ + dev->rx_urb_size = dev->net->mtu +ETH_HLEN +DM_RX_OVERHEAD +1 ; +
drivers/net/usb/dm9620.c line 519
Quoted 10 lines
drivers/net/usb/dm9620.c new file +10
-0+ dev->mii.dev = dev->net; + dev->mii.mdio_read = dm9620_mdio_read; + dev->mii.mdio_write = dm9620_mdio_write; + dev->mii.phy_id_mask =0x1f ; + dev->mii.reg_num_mask =0x1f ; + dev->mii.phy_id =DM9620_PHY_ID ; + +/* reset */ +dm_write_reg (dev,DM_NET_CTRL ,1 ); +udelay (20 );
from Documentation/timers/timers-howto.txt:
SLEEPING FOR "A FEW" USECS ( < ~10us? ):
* Use udelay
use udelay if you want to sleep for less than 10us, otherwise use usleep_range
drivers/net/usb/dm9620.c line 526
Quoted 7 lines
drivers/net/usb/dm9620.c new file +7
-0+ +/* read MAC */ +if (dm_read (dev,DM_PHY_ADDR ,ETH_ALEN , mac) <0 ) { +netdev_err (dev->net,"Error reading MAC address\n" ); + ret = -ENODEV ; + goto out; + }
you can get read of the 'out' label if in all the goto you use you just 'return -ENODEV;' instead of 'goto out;'
Quoted 5 lines
drivers/net/usb/dm9620.c new file +5
-0+ +/* Overwrite the auto-generated address only with good ones */ +if (is_valid_ether_addr (mac)) +memcpy (dev->net->dev_addr, mac,ETH_ALEN ); +else {
drivers/net/usb/dm9620.c line 541
Quoted 10 lines
drivers/net/usb/dm9620.c new file +10
-0+netdev_warn (dev->net, +"dm9620: No valid MAC address in EEPROM, using %pM\n" , + dev->net->dev_addr); +__dm9620_set_mac_address (dev); + } + +if (dm_read_reg (dev,DM_CHIP_ID , &id) <0 ) { +netdev_err (dev->net,"Error reading chip ID\n" ); + ret = -ENODEV ; + goto out;
same here
Quoted 24 lines
drivers/net/usb/dm9620.c new file +24
-0+ } + +dm_read (dev,DM_PID ,2 , &value); + +/* Add for check Product dm9620a/21a */ +if (value ==0x1269 || value ==0x0269 ) +dm_write_reg (dev,DM_MODE_CTRL ,DM_MODE9620 |DM_MODE_CDC_DES ); +else +dm_write_reg (dev,DM_MODE_CTRL ,DM_MODE9620 ); + +dm_write_reg (dev,DM_RX_CRC_CTRL ,DM_RX_RCSEN ); + +/* power up phy */ +dm_write_reg (dev,DM_GPR_CTRL ,1 ); +dm_write_reg (dev,DM_GPR_DATA ,0 ); + +/* receive broadcast packets */ +dm9620_set_multicast (dev->net); + +dm9620_mdio_write (dev->net, dev->mii.phy_id,MII_BMCR ,BMCR_RESET ); + +/* TX Pause Packet Enable */ +dm_write_reg (dev,DM_FCR ,DM_FCR_TXPEN |DM_FCR_BKPM |DM_FCR_FLCE ); +
drivers/net/usb/dm9620.c line 575
Quoted 10 lines
drivers/net/usb/dm9620.c new file +10
-0+/* ADVERTISE_PAUSE_CAP */ +dm9620_mdio_write (dev->net, dev->mii.phy_id,MII_ADVERTISE , +ADVERTISE_ALL |ADVERTISE_CSMA |ADVERTISE_PAUSE_CAP ); + +dm_write_reg (dev,DM_USB_CTRL ,DM_USB_EP3ACK ); + +mii_nway_restart (&dev->mii); + +out: +return ret;
yeah... you wouldn't need this anymore
drivers/net/usb/dm9620.c line 578
Quoted 3 lines
drivers/net/usb/dm9620.c new file +3
-0+} + +void dm9620_unbind (struct usbnet *dev,struct usb_interface *intf)
Should this be static?
Quoted 7 lines
drivers/net/usb/dm9620.c new file +7
-0+{ +netdev_warn (dev->net,"[dm962] Linux Driver = V2.6 - unbind\n" ); +} + +static int dm9620_rx_fixup (struct usbnet *dev,struct sk_buff *skb) +{ +u8 status;
drivers/net/usb/dm9620.c line 595
Quoted 10 lines
drivers/net/usb/dm9620.c new file +10
-0+int len; + +/* format: +b0: rx_flag +b1: rx status +b2: packet length (incl crc) low +b3: packet length (incl crc) high +b4..n-4: packet data +bn-3..bn: ethernet crc +*/
Allow me this nitpick please check the coding style for comments in Documentation/CodyngStyle:
For files in net/ and drivers/net/ the preferred style for long (multi-line) comments is a little different.
/* The preferred comment style for files in net/ and
* drivers/net
* looks like this.
*
* It is nearly the same as the generally preferred
* comment style,
* but there is no initial almost-blank line. */
Quoted 25 lines
drivers/net/usb/dm9620.c new file +25
-0+if (unlikely (skb->len <DM_RX_OVERHEAD )) { +dev_err (&dev->udev->dev,"unexpected tiny rx frame\n" ); +return 0 ; + } + + status = skb->data[1 ]; + len = (skb->data[2 ] | (skb->data[3 ] <<8 )) -4 ; + +if (unlikely (status &0xbf )) { +if (status &0x01 ) + dev->net->stats.rx_fifo_errors++; +if (status &0x02 ) + dev->net->stats.rx_crc_errors++; +if (status &0x04 ) + dev->net->stats.rx_frame_errors++; +if (status &0x20 ) + dev->net->stats.rx_missed_errors++; +if (status &0x90 ) + dev->net->stats.rx_length_errors++; +return 0 ; + } + +skb_pull (skb,4 ); +skb_trim (skb, len); +
drivers/net/usb/dm9620.c line 631
Quoted 10 lines
drivers/net/usb/dm9620.c new file +10
-0+return 1 ; +} + +static bool davicom_bulkout_fix (int len,unsigned fullp) +{ + len = ((len+1 )/2 )*2 ; + len +=2 ; +if ((len % fullp) ==0 ) +return true; +return false;
personally here I like more the form
return !(len % fullp) ? true : false;
But this is more a nitpick, you don't really need to change
drivers/net/usb/dm9620.c line 638
Quoted 7 lines
drivers/net/usb/dm9620.c new file +7
-0+} + +static int dm9620_tx_oddadd_len (int len) +{ +if (len &1 ) +return 1 ; +return 0 ;
(len & 1) is the same as (len == 1), personally I find it a bit ugly, I would rather write it differently:
return (len == 1) ? 1 : 0;
or if you like the '&'
return len & 1;
Quoted 15 lines
drivers/net/usb/dm9620.c new file +15
-0+} + +static const struct usb_device_id products[] = { + { +USB_DEVICE (0x0a46 ,0x9620 ),/* dm9620 */ + .driver_info = (unsigned long )&dm9620_info, + }, + { +USB_DEVICE (0x0a46 ,0x9621 ),/* dm9621 */ + .driver_info = (unsigned long )&dm9620_info, + }, + { +USB_DEVICE (0x0a46 ,0x9622 ),/* dm9622 */ + .driver_info = (unsigned long )&dm9620_info, + },
drivers/net/usb/dm9620.c line 780
Quoted 10 lines
drivers/net/usb/dm9620.c new file +10
-0+ { + USB_DEVICE(0x0a46, 0x0269), /* dm9620a */ + .driver_info = (unsigned long )&dm9620_info, + }, + { +USB_DEVICE (0x0a46 ,0x1269 ),/* dm9621a */ + .driver_info = (unsigned long)&dm9620_info, + }, + {}, +};
It would be cool if in the code we could have some logical defines for all the hexadecimal values you are using all around the code ;)
Andi