smsc911x driver
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 5 lines · On Fri, 14 Jul 2006 14:11:34 +0000 "Bahadir Balban" <[email protected]>…
Attached is a driver patch for SMSC911x family of ethernet chips, generated against 2.6.18-rc1 sources. There's a similar driver in the tree; this one has been tested by SMSC on all flavors of the chip and claimed to be efficient.
Please submit drivers to [email protected]
Quoted 80 lines
drivers/net/Kconfig +7
-0@@ -865,6 +865,13 @@ config NET_NETX <file:Documentation/networking/net-modules.txt>. The module will be called netx-eth. +config SMSC911X + tristate "SMSC 911x family of embedded ethernet support" + depends on NET_ETHERNET + ---help--- + Say Y here if you want support for SMSC LAN911x family + of ethernet chips. + config DM9000 tristate "DM9000 support" depends on (ARM || MIPS) && NET_ETHERNETdrivers/net/Makefile +1
-0@@ -196,6 +196,7 @@ obj-$(CONFIG_S2IO) += s2io.o obj-$(CONFIG_MYRI10GE) += myri10ge/ obj-$(CONFIG_SMC91X) += smc91x.o obj-$(CONFIG_SMC911X) += smc911x.o +obj-$(CONFIG_SMSC911X) += smsc911x.o obj-$(CONFIG_DM9000) += dm9000.o obj-$(CONFIG_FEC_8XX) += fec_8xx/drivers/net/smsc911x.c new file +44
-0@@ -0,0 +1,2105 @@ +/*************************************************************************** +* +* Copyright (C) 2004-2005 SMSC +* Copyright (C) 2005 ARM +* +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License +* as published by the Free Software Foundation; either version 2 +* of the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. +* +*************************************************************************** +* Rewritten, heavily based on smsc911x simple driver by SMSC. +* Partly uses io macros from smc91x.c by Nicolas Pitre +*/ + +#include <linux/config.h> +#include <linux/delay.h> +#include <linux/errno.h> +#include <linux/etherdevice.h> +#include <linux/init.h> +#include <linux/ioport.h> +#include <linux/kernel.h> +#include <linux/mii.h> +#include <linux/module.h> +#include <linux/netdevice.h> +#include <linux/platform_device.h> +#include <linux/sched.h> +#include <linux/slab.h> +#include <linux/timer.h> +#include <linux/version.h> +#include <asm/bug.h> +#include <asm/bitops.h> +#include <asm/dma.h> +#include <asm/irq.h>
drivers/net/smsc911x.c line 54
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+#include <asm/io.h> +#include "smsc911x.h" + +#define SMSC_CHIPNAME "smsc911x" + +/* Tasklet declarations */ +static unsigned long rx_tasklet_parameter; +static void smsc911x_rx_tasklet (unsigned long data); + +DECLARE_TASKLET (rx_tasklet, smsc911x_rx_tasklet,0 );
* should be static.
* if you want to receive in softirq context use NAPI rather than a tasklet.
Quoted 11 lines
drivers/net/smsc911x.c new file +9
-0+MODULE_LICENSE ("GPL" ); +/* Entry point for starting/opening the interface */ +static int smsc911x_open (struct net_device *dev) +{ +struct smsc911x_data *pdata; +unsigned char *mac_byte; +unsigned int mac_high16; +unsigned int mac_low32; +unsigned int timeout;
drivers/net/smsc911x.c
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+unsigned int intcfg; +int result; + +/* Set interrupt deassertion to 220uS */ + intcfg =22 <<24 ; + timeout =1000 ; + result = -ENODEV ; + pdata =0 ; + +BUG_ON (!dev);
This kind of BUG_ON is useless, you will get whacked when you deref the pointer anyway.
Quoted 35 lines
drivers/net/smsc911x.c new file +35
-0+ pdata =netdev_priv (dev); +BUG_ON (!pdata); + +/* Initialise smsc911x */ + +/* +* dwIntCfg|=INT_CFG_IRQ_POL_; use this to set IRQ_POL bit +* dwIntCfg|=INT_CFG_IRQ_TYPE_; use this to set IRQ_TYPE bit +*/ +if (!smsc911x_lan_initialise (pdata, intcfg)) + goto done; + +SMSC_TRACE ("Testing irq handler using IRQ %d" , dev->irq); + pdata->request_irq_disable =0 ; + pdata->software_irq_signal =0 ; +SMC_regwrite ((SMC_regread (pdata,INT_EN ) |INT_EN_SW_INT_EN_ ), + pdata,INT_EN ); +do { +udelay (10 ); + timeout--; + }while (timeout && (!pdata->software_irq_signal)); + +if (!pdata->software_irq_signal) { +printk ("<1>ISR failed signaling test." ); + result = -ENODEV ; + goto done; + } +SMSC_TRACE ("IRQ handler passed test using IRQ %d" , dev->irq); + +printk ("%s: SMSC9%3x identified at %#08x, IRQ: %d\n" , dev->name, + (u32)((pdata->idrev >>16 ) &0xFFFF ), pdata->base, dev->irq); + +smsc911x_mac_initialise (pdata); + +/* Read mac address from EEPROM */
drivers/net/smsc911x.c
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ mac_high16 =smsc911x_mac_read (pdata,ADDRH ); + mac_low32 =smsc911x_mac_read (pdata,ADDRL ); + +if ((mac_high16 ==0x0000FFFF ) && (mac_low32 ==0xFFFFFFFF )) { +/* Use default MAC addresses if eeprom values are invalid */ + mac_high16 =0x00000070 ; + mac_low32 =0x110F8000 ; + +smsc911x_mac_write (pdata,ADDRH , mac_high16); +smsc911x_mac_write (pdata,ADDRL , mac_low32);
Please use random_ether_addr instead of the default value.
drivers/net/smsc911x.c
Quoted 11 lines
drivers/net/smsc911x.c new file +11
-0+SMSC_TRACE ("MAC Address is set by default to 0x%04X%08X" , + mac_high16, mac_low32); + }else { +SMSC_TRACE ("Mac Address is read from LAN911x as 0x%04X%08X" , + mac_high16, mac_low32); + } + mac_byte = (unsigned char *)&mac_low32; + dev->dev_addr[0 ] = mac_byte[0 ]; + dev->dev_addr[1 ] = mac_byte[1 ]; + dev->dev_addr[2 ] = mac_byte[2 ]; + dev->dev_addr[3 ] = mac_byte[3 ];
What about byte order?
Quoted 70 lines
drivers/net/smsc911x.c new file +70
-0+ + mac_byte = (unsigned char *)&mac_high16; + dev->dev_addr[4 ] = mac_byte[0 ]; + dev->dev_addr[5 ] = mac_byte[1 ]; + +printk ("%s: SMSC9%3x MAC Address: %02x:%02x:%02x:%02x:%02x:%02x\n" , + dev->name, (u32)((pdata->idrev >>16 ) &0xFFFF ), + dev->dev_addr[5 ], dev->dev_addr[4 ], dev->dev_addr[3 ], + dev->dev_addr[2 ], dev->dev_addr[1 ], dev->dev_addr[0 ]); + +netif_carrier_off (dev); +if (!smsc911x_phy_initialise (pdata)) { +SMSC_WARNING ("Failed to initialize PHY" ); + result = -ENODEV ; + goto done; + } + +smsc911x_tx_initialise (pdata); +smsc911x_rx_initialise (pdata); +netif_start_queue (dev); + + result =0 ; + +done: +SMSC_TRACE ("result: %d" , result); +return result; +} + +/* Entry point for stopping the interface */ +static int smsc911x_stop (struct net_device *dev) +{ +unsigned long flags; +unsigned long base; +unsigned long idrev; +struct smsc911x_data *pdata; +struct net_device *pnetdev; + +BUG_ON (!dev); + pdata =netdev_priv (dev); +BUG_ON (!pdata); + + pdata->stop_link_poll =1 ; +del_timer_sync (&pdata->link_poll_timer); + +spin_lock_irqsave (&dev->_xmit_lock, flags); +SMC_regwrite ((SMC_regread (pdata,INT_CFG ) & (~INT_CFG_IRQ_EN_ )), + pdata,INT_CFG ); +netif_stop_queue (pdata->dev); +spin_unlock_irqrestore (&dev->_xmit_lock, flags); + +/* At this point all Rx and Tx activity is stopped */ + pdata->stats.rx_dropped +=SMC_regread (pdata,RX_DROP ); +smsc911x_tx_update_txcounters (pdata); + +/* Preserve important fields */ + base = pdata->base; + idrev = pdata->idrev; + pnetdev = pdata->dev; + +/* Clear all structure */ +memset ((void *)pdata,0 ,sizeof (struct smsc911x_data )); + +/* Reassign important fields */ + pdata->base = base; + pdata->idrev = idrev; + pdata->dev = pnetdev; + +SMSC_TRACE ("<--Simp911x_stop" ); +return 0 ; +}
drivers/net/smsc911x.c
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ +/* Entry point for transmitting a packet */ +static int +smsc911x_hard_start_xmit (struct sk_buff *skb,struct net_device *dev) +{ +struct smsc911x_data *pdata; + +BUG_ON (!skb); +BUG_ON (!dev); +BUG_ON (!netdev_priv (dev));
More bullshit bugon. Why the silly wrapper func?
Quoted 4 lines
drivers/net/smsc911x.c new file +4
-0+ pdata =netdev_priv (dev); +smsc911x_tx_sendskb (pdata, skb); +return 0 ; +}
...
drivers/net/smsc911x.c
Quoted 2 lines
drivers/net/smsc911x.c new file +2
-0+platform_set_drvdata (pdev, netdev); +strcpy (netdev->name,"eth%d" );
Already done by alloc_etherdev...
drivers/net/smsc911x.c
Quoted 12 lines
drivers/net/smsc911x.c new file +12
-0+ retval =register_netdev (netdev); + +if (retval) { +SMSC_WARNING ("Error %i registering device" , retval); + retval = -ENODEV ; + goto out_unmap_io; + }else { +SMSC_TRACE ("Network interface: \"%s\"" ,netdev->name); + } + +if ((retval =smsc911x_init (netdev)) <0 ) + goto out_deregister_netdev;
Please init before registering. Because during registration it is possible that some callback would want to bring up the device automatically, or send a packet.
drivers/net/smsc911x.c
Quoted 6 lines
drivers/net/smsc911x.c new file +6
-0+if (request_irq (netdev->irq, smsc911x_irqhandler, +SA_INTERRUPT ,SMSC_CHIPNAME ,netdev_priv (netdev)) !=0 ) { +SMSC_WARNING ("Unable to claim requested irq: %d" , netdev->irq); + retval = -ENODEV ; + goto out_deregister_netdev; + }
You must have the IRQ in place before registering.
Quoted 27 lines
drivers/net/smsc911x.c new file +16
-0+return 0 ; + +out_deregister_netdev: +unregister_netdev (netdev); +out_unmap_io: +platform_set_drvdata (pdev,NULL ); +iounmap ((void *) base); +out_free_netdev: + rx_tasklet_parameter =0 ; +free_netdev (netdev); +out_release_io: +release_mem_region (res->start, res->end - res->start); +out: +return retval; +} +drivers/net/smsc911x.h new file +5
-0@@ -0,0 +1,526 @@ +#ifndef __SMSC911X_H__ +#define __SMSC911X_H__ + +#define USE_PHY_WORK_AROUND +#define USE_LED1_WORK_AROUND /* 10/100 LED link-state inversion */
drivers/net/smsc911x.h line 15
Quoted 10 lines
drivers/net/smsc911x.h new file +10
-0+ +/* Debugging */ +#define USE_DEBUG 0 +#if USE_DEBUG >=1 +# define SMSC_WARNING(fmt, args...) \ + printk(KERN_EMERG "SMSC_WARNING: %s: " fmt "\n",\ + __FUNCTION__ , ## args) +#else +# define SMSC_WARNING(msg, args...) +#endif /* USE_DEBUG >= 1 */
Consider using pr_debug and the ethtool message level model.
Quoted 7 lines
drivers/net/smsc911x.h new file +7
-0+#if USE_DEBUG >=2 +# define SMSC_TRACE(fmt,args...) \ + printk(KERN_EMERG "SMSC_TRACE: %s: " fmt "\n", \ + __FUNCTION__ , ## args) +#else +# define SMSC_TRACE(msg,args...) +#endif /* USE_DEBUG >= 2 */
Signature · 2 lines
Stephen Hemminger <[email protected]> "And in the Packet there writ down that doome"
SMSC LAN911x and LAN921x vendor driver
Quoted 4 lines
Attached is a driver patch for SMSC911x family of ethernet chips, generated against 2.6.18-rc1 sources. There's a similar driver in the tree; this one has been tested by SMSC on all flavors of the chip and claimed to be efficient.
Updated after feedback from Stephen Hemminger.
Driver updated to also support LAN921x family. Workarounds added for known hardware issues.
--- drivers/net/Kconfig | 7 drivers/net/Makefile | 1 drivers/net/smsc911x.c | 2289 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/net/smsc911x.h | 504 +++++++++++ 4 files changed, 2801 insertions(+), 0 deletions(-)
drivers/net/Kconfig +7 -0
@@ -865,6 +865,13 @@ config NET_NETX <file:Documentation/networking/net-modules.txt>. The module will be called netx-eth. +config SMSC911X + tristate "SMSC 911x family of embedded ethernet support" + depends on NET_ETHERNET + ---help--- + Say Y here if you want support for SMSC LAN911x family + of ethernet chips. + config DM9000 tristate "DM9000 support" depends on (ARM || MIPS) && NET_ETHERNET
drivers/net/Makefile +1 -0
@@ -196,6 +196,7 @@ obj-$(CONFIG_S2IO) += s2io.o obj-$(CONFIG_MYRI10GE) += myri10ge/ obj-$(CONFIG_SMC91X) += smc91x.o obj-$(CONFIG_SMC911X) += smc911x.o +obj-$(CONFIG_SMSC911X) += smsc911x.o obj-$(CONFIG_DM9000) += dm9000.o obj-$(CONFIG_FEC_8XX) += fec_8xx/
drivers/net/smsc911x.c new file +2289 -0
2,289 lines not on this page · raw message
drivers/net/smsc911x.h new file +504 -0
504 lines not on this page · raw message
Signature · 1 line
1.4.0
Re: SMSC LAN911x and LAN921x vendor driver
drivers/net/smsc911x.c line 63
Quoted 6 lines
drivers/net/smsc911x.c new file +5
-0+/* Tasklet declarations */ +static unsigned long rx_tasklet_parameter; +static void smsc911x_rx_tasklet (unsigned long data); + +DECLARE_TASKLET (rx_tasklet, smsc911x_rx_tasklet,0 );
You can make this local with
static DECLARE_TASKLET(rx_tasklet, smsc911x_rx_tasklet, 0);
Re: SMSC LAN911x and LAN921x vendor driver
In reply to Steve Glendinning[...]
Quoted 5 lines
drivers/net/smsc911x.c new file +0
-0
[...]
drivers/net/smsc911x.c line 66
Quoted 8 lines
drivers/net/smsc911x.c new file +8
-0+/* Tasklet declarations */ +static unsigned long rx_tasklet_parameter; +static void smsc911x_rx_tasklet (unsigned long data); + +DECLARE_TASKLET (rx_tasklet, smsc911x_rx_tasklet,0 ); +MODULE_LICENSE ("GPL" ); + +/* MAC */
[lots of forward declarations]
Despite Jeff claiming otherwise, I see no added value in forward declarations. It adds bloat, it pollutes ctags and it is inferior to real kernel docbook documentation.
[...]
Quoted 8 lines
drivers/net/smsc911x.c new file +8
-0+/* Linux network device interface */ +static int smsc911x_open (struct net_device *dev); +static int smsc911x_stop (struct net_device *dev); +static int smsc911x_hard_start_xmit (struct sk_buff *skb, +struct net_device *dev); + +static struct net_device_stats *smsc911x_get_stats (struct net_device *dev); +static void smsc911x_set_multicast_list (struct net_device *dev);
drivers/net/smsc911x.c line 153
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+static irqreturn_t smsc911x_irqhandler (int irq,void *dev_id, +struct pt_regs *regs); + +/* Module interface */ +static int smsc911x_init_module (void ); +static void smsc911x_cleanup_module (void ); + +/* Driver interface */ +static int smsc911x_drv_probe (struct platform_device *pdev); +static int smsc911x_drv_remove (struct platform_device *pdev);
These one are completely useless.
Quoted 6 lines
drivers/net/smsc911x.c new file +6
-0+/* Entry point for starting/opening the interface */ +static int smsc911x_open (struct net_device *dev) +{ +struct smsc911x_data *pdata; +unsigned int mac_high16; +unsigned int mac_low32;
drivers/net/smsc911x.c line 170
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+unsigned int timeout; +unsigned int intcfg; +int result; + +/* Set interrupt deassertion to 220uS */ + intcfg =22 <<24 ; + timeout =1000 ; + result = -ENODEV ; + + pdata =netdev_priv (dev);
It is quite common to set pdata on flight, i.e.:
struct smsc911x_data *pdata = netdev_priv(dev);
Same thing for 'result'.
Quoted 12 lines
drivers/net/smsc911x.c new file +12
-0+ +/* Initialise smsc911x */ + +/* +* dwIntCfg|=INT_CFG_IRQ_POL_; use this to set IRQ_POL bit +* dwIntCfg|=INT_CFG_IRQ_TYPE_; use this to set IRQ_TYPE bit +*/ +if (!smsc911x_lan_initialise (pdata, intcfg)) + goto done; + +SMSC_TRACE ("Testing irq handler using IRQ %d" , dev->irq); + pdata->request_irq_disable =0 ;
drivers/net/smsc911x.c line 192
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ pdata->software_irq_signal =0 ; +smsc911x_reg_write ((smsc911x_reg_read (pdata,INT_EN ) | +INT_EN_SW_INT_EN_ ), pdata,INT_EN ); +do { +udelay (10 ); + timeout--; + }while (timeout && (!pdata->software_irq_signal)); + +if (!pdata->software_irq_signal) { +printk ("<1>ISR failed signaling test." );
Should be printk(KERN_XYZ, ...
It would be nice to display the device name too.
drivers/net/smsc911x.c line 199
Quoted 7 lines
drivers/net/smsc911x.c new file +7
-0+ result = -ENODEV ; + goto done; + } +SMSC_TRACE ("IRQ handler passed test using IRQ %d" , dev->irq); + +printk ("%s: SMSC911x/921x identified at %#08x, IRQ: %d\n" , dev->name, + pdata->base, dev->irq);
printk(KERN_XYZ, ...
[...]
Quoted 4 lines
drivers/net/smsc911x.c new file +4
-0+static int smsc911x_stop (struct net_device *dev) +{ +unsigned long flags; +unsigned long base;
drivers/net/smsc911x.c line 262
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+unsigned long idrev; +struct smsc911x_data *pdata; +struct net_device *pnetdev; + + pdata =netdev_priv (dev); + + pdata->stop_link_poll =1 ; +del_timer_sync (&pdata->link_poll_timer); + +spin_lock_irqsave (&dev->_xmit_lock, flags);
If this is supposed to protect against hard_start_xmit, it is not needed.
drivers/net/smsc911x.c line 265
Quoted 3 lines
drivers/net/smsc911x.c new file +3
-0+smsc911x_reg_write ((smsc911x_reg_read (pdata,INT_CFG ) & + (~INT_CFG_IRQ_EN_ )), pdata,INT_CFG ); +netif_stop_queue (pdata->dev);
Any reason why 'pdata->dev' would be different from 'dev' ? pnetdev seems redundant with dev.
drivers/net/smsc911x.c line 278
Quoted 13 lines
drivers/net/smsc911x.c new file +13
-0+spin_unlock_irqrestore (&dev->_xmit_lock, flags); + +/* At this point all Rx and Tx activity is stopped */ + pdata->stats.rx_dropped +=smsc911x_reg_read (pdata,RX_DROP ); +smsc911x_tx_update_txcounters (pdata); + +/* Preserve important fields */ + base = pdata->base; + idrev = pdata->idrev; + pnetdev = pdata->dev; + +/* Clear all structure */ +memset ((void *)pdata,0 ,sizeof (struct smsc911x_data ));
Useless conversion to void *
Quoted 2 lines
drivers/net/smsc911x.c new file +2
-0+static int smsc911x_hard_start_xmit (struct sk_buff *skb,struct net_device *dev) +{
[...]
drivers/net/smsc911x.c
Quoted 1 line
drivers/net/smsc911x.c new file +1
-0+return 0 ;
s/0/NETDEV_TX_OK/
[...]
Quoted 8 lines
drivers/net/smsc911x.c new file +8
-0+/* Entry point for setting addressing modes */ +static void smsc911x_set_multicast_list (struct net_device *dev) +{ +struct smsc911x_data *pdata; +unsigned long flags; + + pdata =netdev_priv (dev); +
drivers/net/smsc911x.c line 368
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+spin_lock_irqsave (&pdata->phy_lock, flags); + +if (dev->flags &IFF_PROMISC ) { +/* Enabling promiscuous mode */ + pdata->set_bits_mask =MAC_CR_PRMS_ ; + pdata->clear_bits_mask = (MAC_CR_MCPAS_ |MAC_CR_HPFILT_ ); + pdata->hashhi =0 ; + pdata->hashlo =0 ; + goto prepare; + }
I really enjoy 'goto' too but what is wrong with 'if (...) else if (...)' ?
Quoted 21 lines
drivers/net/smsc911x.c new file +21
-0+ +if (dev->flags &IFF_ALLMULTI ) { +/* Enabling all multicast mode */ + pdata->set_bits_mask =MAC_CR_MCPAS_ ; + pdata->clear_bits_mask = (MAC_CR_PRMS_ |MAC_CR_HPFILT_ ); + pdata->hashhi =0 ; + pdata->hashlo =0 ; + goto prepare; + } +if (dev->mc_count >0 ) { +/* Enabling specific multicast addresses */ +unsigned int hash_high =0 ; +unsigned int hash_low =0 ; +unsigned int count =0 ; +struct dev_mc_list *mc_list = dev->mc_list; + + pdata->set_bits_mask =MAC_CR_HPFILT_ ; + pdata->clear_bits_mask = (MAC_CR_PRMS_ |MAC_CR_MCPAS_ ); + +while (mc_list) { + count++;
drivers/net/smsc911x.c line 399
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+if ((mc_list->dmi_addrlen) ==6 ) { +unsigned int mask =0x01 ; +unsigned int bitnum; + bitnum =smsc911x_hash (mc_list->dmi_addr); + mask <<= (bitnum &0x1F ); +if (bitnum &0x20 ) { + hash_high |= mask; + }else { + hash_low |= mask; + }
No brace around single statement please.
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ }else { +SMSC_WARNING ("dmi_addrlen != 6" ); + } + mc_list = mc_list->next; + } +if (count != (unsigned int )dev->mc_count) +SMSC_WARNING ("mc_count != dev->mc_count" ); + + pdata->hashhi = hash_high; + pdata->hashlo = hash_low;
drivers/net/smsc911x.c line 419
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ }else { +/* Enabling local MAC address only */ + pdata->set_bits_mask =0L ; + pdata->clear_bits_mask = + (MAC_CR_PRMS_ |MAC_CR_MCPAS_ |MAC_CR_HPFILT_ ); + pdata->hashhi =0UL ; + pdata->hashlo =0UL ; + } + + prepare:
Kill it :o)
[...]
Quoted 5 lines
drivers/net/smsc911x.c new file +5
-0+static irqreturn_t +smsc911x_irqhandler (int irq,void *dev_id,struct pt_regs *regs) +{ +unsigned int intcfg; +unsigned int intsts;
drivers/net/smsc911x.c line 462
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+int serviced; +unsigned int reserved_bits; +struct smsc911x_data *pdata; + + intcfg =0 ; + intsts =0 ; + serviced =IRQ_NONE ; + reserved_bits =0x00FFCEEE ; + + pdata = (struct smsc911x_data *)dev_id;
Useless cast from void *.
drivers/net/smsc911x.c
Quoted 1 line
drivers/net/smsc911x.c new file +1
-0+BUG_ON (!pdata);
Don't worry, you'll know soon enough if it's NULL.
Quoted 34 lines
drivers/net/smsc911x.c new file +34
-0+ + intcfg =smsc911x_reg_read (pdata,INT_CFG ); + +if (unlikely ((intcfg &0x00001100 ) !=0x00001100 )) { +SMSC_TRACE ("Spurious interrupt, intcfg: 0x%08X" , intcfg); + goto done; + } + +/* this could mean surprise removal */ +if (unlikely (intcfg & reserved_bits)) { +SMSC_WARNING ("SMSC911x irq handler, reserved bits are high." ); + goto done; + } + + intsts =smsc911x_reg_read (pdata,INT_STS ); +if (unlikely (intsts &INT_STS_SW_INT_ )) { +smsc911x_reg_write ((smsc911x_reg_read (pdata,INT_EN ) + & (~INT_EN_SW_INT_EN_ )), pdata,INT_EN ); +smsc911x_reg_write (INT_STS_SW_INT_ , pdata,INT_STS ); + pdata->software_irq_signal =1 ; + serviced =IRQ_HANDLED ; +if (pdata->request_irq_disable) { +smsc911x_reg_write ((smsc911x_reg_read (pdata,INT_CFG ) + & (~INT_CFG_IRQ_EN_ )), pdata, +INT_CFG ); +/* Prevent irqs from being handled */ + intsts =0 ; + } +if (pdata->multicast_update_pending) { +smsc911x_rx_multicast_update (pdata); + } + } + +if (smsc911x_tx_handleirq (pdata, intsts))
drivers/net/smsc911x.c line 507
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ serviced =IRQ_HANDLED ; + +if (likely (smsc911x_rx_handleirq (pdata, intsts))) + serviced =IRQ_HANDLED ; + +if (unlikely (!serviced)) +SMSC_WARNING ("Unserviced irq. intcfg: 0x%08X, intsts: " +"0x%08X, int_en: 0x%08X" , intcfg, intsts, +smsc911x_reg_read (pdata,INT_EN )); + done:
Please label at the start of the line.
Quoted 18 lines
drivers/net/smsc911x.c new file +18
-0+return serviced; +} + +/* Autodetects and initialises external phy for SMSC9115 and SMSC9117 flavors. +* If something goes wrong, returns -ENODEV to revert back to internal phy. */ +static int smsc911x_phy_initialise_external (struct smsc911x_data *pdata) +{ +unsigned int address; +unsigned int hwcfg; +unsigned int phyid1; +unsigned int phyid2; + + hwcfg =smsc911x_reg_read (pdata,HW_CFG ); + +/* External phy is requested, supported, and detected */ +if (hwcfg &HW_CFG_EXT_PHY_DET_ ) { + +/* Attempt to switch to external phy for auto-detecting
drivers/net/smsc911x.c line 535
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+* its address. Assuming tx and rx are stopped because +* smsc911x_phy_initialise is called before +* smsc911x_rx_initialise and tx_initialise. +*/ + +/* Disable phy clocks to the MAC */ + hwcfg &= (~HW_CFG_PHY_CLK_SEL_ ); + hwcfg |=HW_CFG_PHY_CLK_SEL_CLK_DIS_ ; +smsc911x_reg_write (hwcfg, pdata,HW_CFG ); +udelay (10 );/* Enough time for clocks to stop */
I assume that writes are never posted, right ?
[...]
Quoted 14 lines
drivers/net/smsc911x.c new file +14
-0+#ifdef USE_PHY_WORK_AROUND +static int smsc911x_phy_reset (struct smsc911x_data *pdata) +{ +int result =0 ; +unsigned int temp =0 ; +unsigned int lcount =100000 ; + +SMSC_TRACE ("Performing PHY BCR Reset" ); +smsc911x_phy_write (pdata,PHY_BCR ,PHY_BCR_RESET_ ); +do { +udelay (10 ); + temp =smsc911x_phy_read (pdata,PHY_BCR ); + lcount--; + }while ((lcount >0 ) && (temp &PHY_BCR_RESET_ ));
drivers/net/smsc911x.c line 706
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ +if (temp &PHY_BCR_RESET_ ) { +SMSC_WARNING ("PHY reset failed to complete." ); + goto done; + } +/* Extra delay required because the phy may not be completed with +* its reset when PHY_BCR_RESET_ is cleared. Specs say 256 uS is +* enough delay but using 500 here to be safe +*/ +udelay (500 );
If you are in sleepable context, a msleep() may chew less cpu.
[...]
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+/* Fetches a MAC register value. Assumes phy_lock is acquired */ +static unsigned int +smsc911x_mac_read (struct smsc911x_data *pdata,unsigned int offset) +{ +unsigned int result =0xFFFFFFFF ; +volatile unsigned int temp =0 ; + +/* Wait until not busy */ +if (unlikely + (smsc911x_reg_read (pdata,MAC_CSR_CMD ) &MAC_CSR_CMD_CSR_BUSY_ )) {
drivers/net/smsc911x.c line 1338
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+SMSC_WARNING ("smsc911x_mac_read failed, " +"MAC already busy at entry" ); + goto done; + } + +/* Send the MAC cmd */ +smsc911x_reg_write (((offset &0x000000FF ) |MAC_CSR_CMD_CSR_BUSY_ + |MAC_CSR_CMD_R_NOT_W_ ), pdata,MAC_CSR_CMD ); + + temp =smsc911x_reg_read (pdata,BYTE_TEST );/* To flush previous write */
readl() + volatile ?
[...]
Quoted 33 lines
drivers/net/smsc911x.c new file +33
-0+static void smsc911x_rx_processpackets (struct smsc911x_data *pdata) +{ +unsigned int rxstat; +unsigned int *bufptr; + + pdata->rx_congested =0 ; +while ((rxstat =smsc911x_rx_pop_rxstatus (pdata)) !=0 ) { +unsigned int pktlength = ((rxstat &0x3FFF0000 ) >>16 ); +smsc911x_rx_counterrors (pdata, rxstat); + +if (likely ((rxstat &RX_STS_ES_ ) ==0 )) { +struct sk_buff *skb =NULL ; + skb =dev_alloc_skb (pktlength +2 ); +if (likely (skb)) { + skb->data = skb->head; + skb->tail = skb->head; +/* Align IP on 16B boundary */ +skb_reserve (skb,2 ); +skb_put (skb, pktlength -4 ); + +/* Update counters */ + pdata->stats.rx_packets++; + pdata->stats.rx_bytes += (pktlength -4 ); + bufptr = (unsigned int *)skb->head; +smsc911x_rx_readfifo (pdata, bufptr, + (pktlength +2 +3 ) >>2 ); + +smsc911x_rx_handoffskb (pdata, skb); +continue ; + }else { +SMSC_WARNING ("Unable to allocate sk_buff " +"for rx packet, in PIO path" ); + pdata->stats.rx_dropped++;
drivers/net/smsc911x.c line 1678
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ } + } +/* At this point, the packet is to be read out +* of the fifo and discarded */ + pktlength +=2 +3 ; + pktlength >>=2 ; +smsc911x_rx_fastforward (pdata, pktlength); + } + pdata->stats.rx_dropped +=smsc911x_reg_read (pdata,RX_DROP ); +smsc911x_reg_write (INT_STS_RSFL_ , pdata,INT_STS );
dev->last_rx should be updated somewhere.
[...]
drivers/net/smsc911x.c line 1688
Quoted 7 lines
drivers/net/smsc911x.c new file +7
-0+/* Receive tasklet */ +static void smsc911x_rx_tasklet (unsigned long data) +{ +struct net_device *netdev; +struct smsc911x_data *pdata; + + netdev = (struct net_device *)rx_tasklet_parameter;
So this driver will not support more than one adapter ?
[...]
Quoted 8 lines
drivers/net/smsc911x.c new file +8
-0+/* Receiver irq handler */ +static int smsc911x_rx_handleirq (struct smsc911x_data *pdata, +unsigned int intsts) +{ +int result =0 ; + +if (unlikely (intsts &INT_STS_RXE_ )) { +smsc911x_reg_write (INT_STS_RXE_ , pdata,INT_STS );
drivers/net/smsc911x.c line 1714
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ result =1 ; + } +if (!(intsts &INT_STS_RSFL_ )) { +return result; + } + + result =1 ; +smsc911x_reg_write (smsc911x_reg_read (pdata,INT_CFG ) & + (~INT_CFG_IRQ_EN_ ), pdata,INT_CFG ); +tasklet_schedule (&rx_tasklet);
It smells like hand-crafted NAPI. What's wrong with it ?
[...]
Quoted 4 lines
drivers/net/smsc911x.c new file +4
-0+/* Returns hash bit number for given MAC address +* Example: +* 01 00 5E 00 00 01 -> returns bit number 31 */ +static unsigned int smsc911x_hash (char addr[6 ])
drivers/net/smsc911x.c line 1731
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+{ +int i; +int bit; +unsigned int crc; +unsigned int poly; +unsigned int result; +unsigned int data; + + crc =0xFFFFFFFF ; + poly =0xEDB88320 ;
$ grep -i 0xEDB88320 lib/* lib/crc32defs.h:#define CRCPOLY_LE 0xedb88320
Any chance to reuse lib/crc32.c::crc32_le() ?
[...]
Quoted 15 lines
drivers/net/smsc911x.c new file +15
-0+static void smsc911x_rx_multicast_update (struct smsc911x_data *pdata) +{ +unsigned long flags; +unsigned int timeout; +unsigned int mac_cr; + +/* This function is only called for older LAN911x devices +* (revA or revB), where MAC_CR, HASHH and HASHL should not +* be modified during Rx - newer devices immediately update the +* registers */ + +local_irq_save (flags); + +/* Stop Rx */ + mac_cr =smsc911x_mac_read (pdata,MAC_CR );
drivers/net/smsc911x.c line 1774
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ mac_cr &= ~(MAC_CR_RXEN_ ); +smsc911x_mac_write (pdata,MAC_CR , mac_cr); + +/* Poll until Rx has stopped. If a frame is being recieved, this will +* block until the end of this frame. (this may take a long time at +* 10Mbps) */ + timeout =2000 ; +while ((timeout--) + && (!(smsc911x_reg_read (pdata,INT_STS ) &INT_STS_RXSTOP_INT_ ))) { +udelay (1 );
In a completely ideal world the driver would probably race outside of an irq disabled section until it grabs the napi poll handler, thus preserving the nice low latency property of the kernel.
Nevermind :o}
[...]
drivers/net/smsc911x.c line 1879
Quoted 13 lines
drivers/net/smsc911x.c new file +13
-0+static int smsc911x_init (struct net_device *netdev) +{ +int result; +unsigned long idrev; +unsigned int mac_high16; +unsigned int mac_low32; +struct smsc911x_data *pdata; + + idrev =0 ; + pdata =0 ; + result = -ENODEV ; + +BUG_ON (!netdev);
Useless...
drivers/net/smsc911x.c line 1882
Quoted 3 lines
drivers/net/smsc911x.c new file +3
-0+ +SMSC_TRACE ("Driver Parameters:" ); +SMSC_TRACE ("LAN base: 0x%08lX" , netdev->base_addr);
...because you'll see a nice trace here (notwithstanding the fact that the condition can never be satisfied).
drivers/net/smsc911x.c line 1885
Quoted 3 lines
drivers/net/smsc911x.c new file +3
-0+SMSC_TRACE ("IRQ: %d" , netdev->irq); +SMSC_TRACE ("PHY will be autodetected." ); +BUG_ON (!netdev_priv (netdev));
Sic.
[...]
drivers/net/smsc911x.c line 2145
Quoted 7 lines
drivers/net/smsc911x.c new file +7
-0+ether_setup (netdev); + netdev->open = smsc911x_open; + netdev->stop = smsc911x_stop; + netdev->hard_start_xmit = smsc911x_hard_start_xmit; + netdev->get_stats = smsc911x_get_stats; + netdev->set_multicast_list = smsc911x_set_multicast_list; + netdev->flags |=IFF_MULTICAST ;
No ethtool support at all ?
[...]
Quoted 25 lines
drivers/net/smsc911x.c new file +25
-0+static int smsc911x_drv_probe (struct platform_device *pdev) +{ +struct net_device *netdev; +struct resource *res; +unsigned int base; +int res_size; +int retval; + res =platform_get_resource_byname (pdev,IORESOURCE_MEM , +"smsc911x-memory" ); +if (!res) + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); +if (!res) { + retval = -ENODEV ; + goto out; + } + res_size = res->end - res->start; + +if (!request_mem_region (res->start, res_size,SMSC_CHIPNAME )) { + retval = -EBUSY ; + goto out; + } + + netdev =alloc_etherdev (sizeof (struct smsc911x_data )); +if (!netdev) { +printk ("%s: Could not allocate device.\n" ,SMSC_CHIPNAME );
drivers/net/smsc911x.c line 2219
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ retval = -ENOMEM ; + goto out_release_io; + } +SET_MODULE_OWNER (netdev); +SET_NETDEV_DEV (netdev, &pdev->dev); + +/* Global pointer to netdev, used by tasklets */ + rx_tasklet_parameter = (unsigned long )netdev; + netdev->irq =platform_get_irq (pdev,0 ); + netdev->base_addr = (unsigned int )ioremap_nocache (res->start, res_size);
Converting a nice void __iomem * variable to an unsigned int from the start is moderately nice. What about storing it in pdata like in any modern driver ? It would help the incoming jihad for base_addr removal.
drivers/net/smsc911x.c line 2230
Quoted 11 lines
drivers/net/smsc911x.c new file +11
-0+ base = netdev->base_addr; +if (netdev->base_addr ==0 ) { +SMSC_WARNING ("Error smsc911x base address invalid" ); + retval = -ENOMEM ; + goto out_free_netdev; + } + +if ((retval =smsc911x_init (netdev)) <0 ) + goto out_unmap_io; + + if (request_irq(netdev->irq, smsc911x_irqhandler,
retval = request_irq(...)
if (retval < 0) {
...As a general note, the driver could surely propagate more return values instead of relying on local set/test against 0/1.
[...]
Quoted 5 lines
drivers/net/smsc911x.h new file +0
-0
[...]
drivers/net/smsc911x.h line 447
Quoted 6 lines
drivers/net/smsc911x.h new file +6
-0+/* +* Phy register offsets and bit definitions +*/ +#define LAN9118_PHY_ID 0x00C0001C + +#define PHY_BCR 0
Duplicates include/linux/mii.h::MII_BMCR
drivers/net/smsc911x.h line 454
Quoted 7 lines
drivers/net/smsc911x.h new file +7
-0+#define PHY_BCR_RESET_ 0x8000 +#define PHY_BCR_SPEED_SELECT_ 0x2000 +#define PHY_BCR_AUTO_NEG_ENABLE_ 0x1000 +#define PHY_BCR_RESTART_AUTO_NEG_ 0x0200 +#define PHY_BCR_DUPLEX_MODE_ 0x0100 + +#define PHY_BSR 1
Duplicates include/linux/mii.h::MII_BMSR
etc.
Signature · 1 line
Ueimor
Re: SMSC LAN911x and LAN921x vendor driver
Could the tasklet be replaced by using NAPI (dev->poll) routine?
Re: SMSC LAN911x and LAN921x vendor driver
In reply to Francois RomieuHi Francois,
Thanks for your feedback, I have a few questions.
Quoted 4 lines
drivers/net/smsc911x.c
Quoted 4 lines
drivers/net/smsc911x.c new file +4
-0+return serviced; +} + +/* Autodetects and initialises external phy for SMSC9115 and SMSC9117
flavors.
Quoted 1 line
Quoted 1 line
+ * If something goes wrong, returns
-ENODEVto revert back to
internal phy. */
Quoted 1 line
Quoted 1 line
+static int smsc911x_phy_initialise_external(struct
smsc911x_data
*pdata)
Quoted 4 lines
Quoted 4 lines
drivers/net/smsc911x.c new file +4
-0+{ +unsigned int address; +unsigned int hwcfg; +unsigned int phyid1;
Quoted 8 lines
drivers/net/smsc911x.c
Quoted 8 lines
drivers/net/smsc911x.c new file +8
-0+unsigned int phyid2; + + hwcfg =smsc911x_reg_read (pdata,HW_CFG ); + +/* External phy is requested, supported, and detected */ +if (hwcfg &HW_CFG_EXT_PHY_DET_ ) { + +/* Attempt to switch to external phy for
auto-detecting
Quoted 1 line
Quoted 1 line
+ * its address. Assuming tx and rx are
stopped because
Quoted 1 line
Quoted 1 line
+ *
smsc911x_phy_initialiseis called
before
Quoted 1 line
Quoted 1 line
+ *
smsc911x_rx_initialiseand
tx_initialise.
Quoted 7 lines
drivers/net/smsc911x.c
Quoted 7 lines
drivers/net/smsc911x.c new file +7
-0+*/ + +/* Disable phy clocks to the MAC */ + hwcfg &= (~HW_CFG_PHY_CLK_SEL_ ); + hwcfg |=HW_CFG_PHY_CLK_SEL_CLK_DIS_ ; +smsc911x_reg_write (hwcfg, pdata,HW_CFG ); +udelay (10 );/* Enough time
for clocks to stop */
Quoted 3 lines
I assume that writes are never posted, right ?
I don't understand the question, what do you mean?
Quoted 9 lines
drivers/net/smsc911x.c
Quoted 9 lines
drivers/net/smsc911x.c new file +8
-0+static void smsc911x_rx_multicast_update (struct smsc911x_data *pdata) +{ +unsigned long flags; +unsigned int timeout; +unsigned int mac_cr; + +/* This function is only called for older LAN911x devices +* (revA or revB), where MAC_CR, HASHH and HASHL should
not
Quoted 1 line
Quoted 1 line
+ * be modified during Rx - newer devices immediately
update the
Quoted 10 lines
drivers/net/smsc911x.c
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+* registers */ + +local_irq_save (flags); + +/* Stop Rx */ + mac_cr =smsc911x_mac_read (pdata,MAC_CR ); + mac_cr &= ~(MAC_CR_RXEN_ ); +smsc911x_mac_write (pdata,MAC_CR , mac_cr); + +/* Poll until Rx has stopped. If a frame is being
recieved, this will
Quoted 1 line
Quoted 1 line
+ * block until the end of this frame. (this may take a
long time at
Quoted 4 lines
drivers/net/smsc911x.c
Quoted 4 lines
drivers/net/smsc911x.c new file +4
-0+* 10Mbps) */ + timeout =2000 ; + while ((timeout--) + && (!(smsc911x_reg_read(pdata, INT_STS) &
INT_STS_RXSTOP_INT_))) {
Quoted 5 lines
Quoted 1 line
+ udelay(1);
In a completely ideal world the driver would probably race outside of an irq disabled section until it grabs the napi poll handler, thus
preserving
Quoted 3 lines
the nice low latency property of the kernel.
Nevermind :o}
Agreed, I would like to find a nicer way to do this. It's a nasty workaround to a nasty hardware issue :o}
There are two problems. First, on older hardware revisions the multicast hash filters (as well as the promisc flag) cannot be modified while rx is active (bad things might happen). There is an interrupt which can be used to indicate RX has stopped, but on early hardware this is not 100% reliable.
The current solution is the simplest option, and works. A better way could be to use the RX_STOP interrupt, but also schedule a task to run later "just in case"?
Best Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: [email protected]
PATCH 1/1 SMSC LAN911x and LAN921x vendor driver
In reply to Francois RomieuQuoted 9 lines
Quoted 4 lines
Attached is a driver patch for SMSC911x family of ethernet chips, generated against 2.6.18-rc1 sources. There's a similar driver in the tree; this one has been tested by SMSC on all flavors of the chip and claimed to be efficient.
Updated after feedback from Stephen Hemminger.
Driver updated to also support LAN921x family. Workarounds added for known hardware issues.
Many improvements following feedback from Stephen Hemminger and Francois Romieu:
- Tasklet removed, NAPI poll used instead
- Multiple device support
- style fixes & minor improvements
--- drivers/net/Kconfig | 12 drivers/net/Makefile | 1 drivers/net/smsc911x.c | 1931 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/net/smsc911x.h | 442 +++++++++++ 4 files changed, 2386 insertions(+), 0 deletions(-)
drivers/net/Kconfig +12 -0
@@ -865,6 +865,18 @@ config NET_NETX <file:Documentation/networking/net-modules.txt>. The module will be called netx-eth. +config SMSC911X + tristate "SMSC LAN911x/LAN921x families embedded ethernet support" + depends on NET_ETHERNET + select CRC32 + ---help--- + Say Y here if you want support for SMSC LAN911x and LAN921x families + of ethernet controllers. + + To compile this driver as a module, choose M here and read + <file:Documentation/networking/net-modules.txt>. The module + will be called smsc911x. + config DM9000 tristate "DM9000 support" depends on (ARM || MIPS) && NET_ETHERNET
drivers/net/Makefile +1 -0
@@ -196,6 +196,7 @@ obj-$(CONFIG_S2IO) += s2io.o obj-$(CONFIG_MYRI10GE) += myri10ge/ obj-$(CONFIG_SMC91X) += smc91x.o obj-$(CONFIG_SMC911X) += smc911x.o +obj-$(CONFIG_SMSC911X) += smsc911x.o obj-$(CONFIG_DM9000) += dm9000.o obj-$(CONFIG_FEC_8XX) += fec_8xx/
drivers/net/smsc911x.c new file +1931 -0
1,931 lines not on this page · raw message
drivers/net/smsc911x.h new file +442 -0
442 lines not on this page · raw message
Signature · 1 line
1.4.0
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Quoted 15 lines · Steve Glendinning
Quoted 9 lines
Quoted 4 lines
Attached is a driver patch for SMSC911x family of ethernet chips, generated against 2.6.18-rc1 sources. There's a similar driver in the tree; this one has been tested by SMSC on all flavors of the chip and claimed to be efficient.
Updated after feedback from Stephen Hemminger.
Driver updated to also support LAN921x family. Workarounds added for known hardware issues.
Many improvements following feedback from Stephen Hemminger and Francois Romieu:
- Tasklet removed, NAPI poll used instead
- Multiple device support
- style fixes & minor improvements
<snip>
Quoted 7 lines
drivers/net/smsc911x.c new file +7
-0+/* waits for MAC not busy, with timeout. Assumes MacPhyAccessLock has +* already been acquired */ +static int smsc911x_mac_notbusy (struct smsc911x_data *pdata) +{ +int i; + +for (i =0 ; i <40 ; i++) {
drivers/net/smsc911x.c line 106
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+if ((smsc911x_reg_read (pdata,MAC_CSR_CMD ) + &MAC_CSR_CMD_CSR_BUSY_ ) ==0 ) { +return 1 ; + } + } +SMSC_WARNING ("Timed out waiting for MAC not BUSY. " +"MAC_CSR_CMD: 0x%08X" ,smsc911x_reg_read (pdata, +MAC_CSR_CMD )); +return 0 ; +}
How is the length of this timeout controlled? IOW, what prevents it from being too short when the Omegatron 128 running at 10GHz hits the market? Are you relying on the MII clock rate?
<snip>
Quoted 20 lines
drivers/net/smsc911x.c new file +20
-0+/* Gets a phy register, phy_lock must be acquired before calling */ +static unsigned int +smsc911x_phy_read (struct smsc911x_data *pdata,unsigned int index) +{ +unsigned int addr; +unsigned int result =0xFFFF ; +int i; + +/* Confirm MII not busy */ +if (unlikely + ((smsc911x_mac_read (pdata,MII_ACC ) &MII_ACC_MII_BUSY_ ) !=0 )) { +SMSC_WARNING ("MII is busy in smsc911x_phy_read???" ); +return 0 ; + } + +/* Set the address, index & direction (read from PHY) */ + addr = (((pdata->phy_address) &0x1F ) <<11 ) + | ((index &0x1F ) <<6 ); +smsc911x_mac_write (pdata,MII_ACC , addr); +
drivers/net/smsc911x.c line 199
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+/* Wait for read to complete w/ timeout */ +for (i =0 ; i <100 ; i++) { +/* See if MII is finished yet */ +if ((smsc911x_mac_read (pdata,MII_ACC ) + &MII_ACC_MII_BUSY_ ) ==0 ) { + result =smsc911x_mac_read (pdata,MII_DATA ); +return result; + } + } +SMSC_WARNING ("Timed out waiting for MII write to finish" );
Ditto?
<snip>
Quoted 21 lines
drivers/net/smsc911x.c new file +21
-0+/* Sets a phy register, phy_lock must be acquired before calling */ +static void smsc911x_phy_write (struct smsc911x_data *pdata, +unsigned int index,unsigned int val) +{ +unsigned int addr; +int i; + +/* Confirm MII not busy */ +if (unlikely + ((smsc911x_mac_read (pdata,MII_ACC ) &MII_ACC_MII_BUSY_ ) !=0 )) { +SMSC_WARNING ("MII is busy in smsc911x_write_phy???" ); +return ; + } + +/* Put the data to write in the MAC */ +smsc911x_mac_write (pdata,MII_DATA , val); + +/* Set the address, index & direction (write to PHY) */ + addr = (((pdata->phy_address) &0x1F ) <<11 ) | + ((index &0x1F ) <<6 ) |MII_ACC_MII_WRITE_ ; +smsc911x_mac_write (pdata,MII_ACC , addr);
drivers/net/smsc911x.c line 234
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+ +/* Wait for write to complete w/ timeout */ +for (i =0 ; i <100 ; i++) { +/* See if MII is finished yet */ +if ((smsc911x_mac_read (pdata,MII_ACC ) + &MII_ACC_MII_BUSY_ ) ==0 ) +return ; + } +SMSC_WARNING ("Timed out waiting for MII write to finish" ); +}
Ditto?
Quoted 41 lines
drivers/net/smsc911x.c new file +41
-0+/* Autodetects and initialises external phy for SMSC9115 and SMSC9117 flavors. +* If something goes wrong, returns -ENODEV to revert back to internal phy. +* Performed at initialisation only, phy_lock already acquired. */ +static int smsc911x_phy_initialise_external (struct smsc911x_data *pdata) +{ +unsigned int address; +unsigned int hwcfg; +unsigned int phyid1; +unsigned int phyid2; + + hwcfg =smsc911x_reg_read (pdata,HW_CFG ); + +/* External phy is requested, supported, and detected */ +if (hwcfg &HW_CFG_EXT_PHY_DET_ ) { + +/* Attempt to switch to external phy for auto-detecting +* its address. Assuming tx and rx are stopped because +* smsc911x_phy_initialise is called before +* smsc911x_rx_initialise and tx_initialise. +*/ + +/* Disable phy clocks to the MAC */ + hwcfg &= (~HW_CFG_PHY_CLK_SEL_ ); + hwcfg |=HW_CFG_PHY_CLK_SEL_CLK_DIS_ ; +smsc911x_reg_write (hwcfg, pdata,HW_CFG ); +udelay (10 );/* Enough time for clocks to stop */ + +/* Switch to external phy */ + hwcfg |=HW_CFG_EXT_PHY_EN_ ; +smsc911x_reg_write (hwcfg, pdata,HW_CFG ); + +/* Enable phy clocks to the MAC */ + hwcfg &= (~HW_CFG_PHY_CLK_SEL_ ); + hwcfg |=HW_CFG_PHY_CLK_SEL_EXT_PHY_ ; +smsc911x_reg_write (hwcfg, pdata,HW_CFG ); +udelay (10 );/* Enough time for clocks to restart */ + + hwcfg |=HW_CFG_SMI_SEL_ ; +smsc911x_reg_write (hwcfg, pdata,HW_CFG ); + +/* Auto-detect PHY */
drivers/net/smsc911x.c line 286
Quoted 10 lines
drivers/net/smsc911x.c new file +10
-0+for (address =0 ; address <=31 ; address++) { + pdata->phy_address = address; + phyid1 =smsc911x_phy_read (pdata,MII_PHYSID1 ); + phyid2 =smsc911x_phy_read (pdata,MII_PHYSID2 ); +if ((phyid1 !=0xFFFFU ) || (phyid2 !=0xFFFFU )) { +SMSC_TRACE ("Detected PHY at address = " +"0x%02X = %d" , address, address); +break ; + } + }
Does this need the magic "for (addr=1; addr <=32; addr++)" trick that has become idiomatic for PHY discovery in our drivers?
<snip>
Quoted 2 lines
drivers/net/smsc911x.h new file +0
-0
<snip>
Quoted 7 lines
drivers/net/smsc911x.h new file +7
-0+/* SMSC911x registers and bitfields */ +#define RX_DATA_FIFO 0x00 + +#define TX_DATA_FIFO 0x20 +#define TX_CMD_A_ON_COMP_ 0x80000000 +#define TX_CMD_A_BUF_END_ALGN_ 0x03000000 +#define TX_CMD_A_4_BYTE_ALGN_ 0x00000000
drivers/net/smsc911x.h line 139
Quoted 10 lines
drivers/net/smsc911x.h new file +10
-0+#define TX_CMD_A_16_BYTE_ALGN_ 0x01000000 +#define TX_CMD_A_32_BYTE_ALGN_ 0x02000000 +#define TX_CMD_A_DATA_OFFSET_ 0x001F0000 +#define TX_CMD_A_FIRST_SEG_ 0x00002000 +#define TX_CMD_A_LAST_SEG_ 0x00001000 +#define TX_CMD_A_BUF_SIZE_ 0x000007FF +#define TX_CMD_B_PKT_TAG_ 0xFFFF0000 +#define TX_CMD_B_ADD_CRC_DISABLE_ 0x00002000 +#define TX_CMD_B_DISABLE_PADDING_ 0x00001000 +#define TX_CMD_B_PKT_BYTE_LENGTH_ 0x000007FF
Looks like something went haywire w/ your tabbing in this file...?
Your style overall is very nice and easy to read.
Thanks,
John
Signature · 2 lines
John W. Linville [email protected]
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi John,
Thanks for all your feedback.
Quoted 1 line
Quoted 1 line
+/* waits for MAC not busy, with timeout. Assumes MacPhyAccessLock
has
Quoted 7 lines
drivers/net/smsc911x.c
Quoted 7 lines
+ * already been acquired */
drivers/net/smsc911x.c new file +6
-0+static int smsc911x_mac_notbusy (struct smsc911x_data *pdata) +{ +int i; + +for (i =0 ; i <40 ; i++) { + if ((smsc911x_reg_read(pdata,
MAC_CSR_CMD)
Quoted 6 lines
drivers/net/smsc911x.c
Quoted 6 lines
drivers/net/smsc911x.c new file +6
-0+ & MAC_CSR_CMD_CSR_BUSY_) == 0) { +return 1 ; + } + } + SMSC_WARNING("Timed out waiting for MAC not BUSY. " + "MAC_CSR_CMD: 0x%08X",
smsc911x_reg_read(pdata,
Quoted 7 lines
drivers/net/smsc911x.c line 106
Quoted 3 lines
drivers/net/smsc911x.c new file +3
-0+ MAC_CSR_CMD)); +return 0 ; +}How is the length of this timeout controlled? IOW, what prevents it from being too short when the Omegatron 128 running at 10GHz hits the market? Are you relying on the MII clock rate?
The LAN911x and LAN921x devices uses an SRAM-like bus interface with a minimum cycle time of 45ns, so smsc_reg_read() and smsc_reg_write() are guaranteed to take at least 45ns. The MAC operates a little slower, but the operation shouldn't take longer than 225ns (5 read cycles).
The PHY is accessed via slave registers in the MAC (which then relays the command over mii), so its timeout works in the same way.
The timeouts are only there to prevent total lockup if the hardware fails, if the part is working it should take nowhere near 40 iterations.
Quoted 2 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ /* Auto-detect PHY */ + for (address = 0; address <= 31;
address++) {
Quoted 1 line
Quoted 1 line
+
pdata->phy_address=
address;
Quoted 1 line
Quoted 1 line
+ phyid1 =
smsc911x_phy_read(pdata, MII_PHYSID1);
Quoted 1 line
Quoted 1 line
+ phyid2 =
smsc911x_phy_read(pdata, MII_PHYSID2);
Quoted 1 line
Quoted 1 line
+ if ((phyid1 !=
0xFFFFU)
|| (phyid2 != 0xFFFFU)) {
Quoted 8 lines
drivers/net/smsc911x.c line 286
Quoted 5 lines
+ SMSC_TRACE("Detected PHY at address = "
drivers/net/smsc911x.c new file +4
-0+ "0x%02X = %d", address, address); +break ; + } + }Does this need the magic "for (addr=1; addr <=32; addr++)" trick that has become idiomatic for PHY discovery in our drivers?
I don't understand the question - surely 32 is not a valid PHY address?
Quoted 11 lines
Quoted 11 lines
drivers/net/smsc911x.h new file +11
-0+/* SMSC911x registers and bitfields */ +#define RX_DATA_FIFO 0x00 + +#define TX_DATA_FIFO 0x20 +#define TX_CMD_A_ON_COMP_ 0x80000000 +#define TX_CMD_A_BUF_END_ALGN_ 0x03000000 +#define TX_CMD_A_4_BYTE_ALGN_ 0x00000000 +#define TX_CMD_A_16_BYTE_ALGN_ 0x01000000 +#define TX_CMD_A_32_BYTE_ALGN_ 0x02000000 +#define TX_CMD_A_DATA_OFFSET_ 0x001F0000 +#define TX_CMD_A_FIRST_SEG_ 0x00002000
Quoted 8 lines
drivers/net/smsc911x.h line 139
Quoted 6 lines
drivers/net/smsc911x.h new file +6
-0+#define TX_CMD_A_LAST_SEG_ 0x00001000 +#define TX_CMD_A_BUF_SIZE_ 0x000007FF +#define TX_CMD_B_PKT_TAG_ 0xFFFF0000 +#define TX_CMD_B_ADD_CRC_DISABLE_ 0x00002000 +#define TX_CMD_B_DISABLE_PADDING_ 0x00001000 +#define TX_CMD_B_PKT_BYTE_LENGTH_ 0x000007FFLooks like something went haywire w/ your tabbing in this file...?
Its just the "+ " in the patch, once applied it looks quite pretty!
Best Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: [email protected]
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Quoted 4 lines · On Wed, Aug 02, 2006 at 08:23:40PM +0100, [email protected] wrote:
Quoted 2 lines
Does this need the magic "for (addr=1; addr <=32; addr++)" trick that has become idiomatic for PHY discovery in our drivers?
I don't understand the question - surely 32 is not a valid PHY address?
That's why it is magic! :-)
The idea is to probe PHY addr 0 last in the series. Apparently some PHYs don't like seeing addr 0 or somesuch, so you try it last to avoid screwing them up. It may well be folklore and legend at this point. Still, you will find several examples in the various drivers. The sundance driver is one example.
John
Signature · 2 lines
John W. Linville [email protected]
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
In reply to Steve GlendinningQuoted 7 lines · Steve Glendinning
Quoted 7 lines
Quoted 4 lines
Attached is a driver patch for SMSC911x family of ethernet chips, generated against 2.6.18-rc1 sources. There's a similar driver in the tree; this one has been tested by SMSC on all flavors of the chip and claimed to be efficient.
Updated after feedback from Stephen Hemminger.
Quoted 8 lines
Quoted 2 lines
Driver updated to also support LAN921x family. Workarounds added for known hardware issues.
Many improvements following feedback from Stephen Hemminger and Francois Romieu:
- Tasklet removed, NAPI poll used instead
- Multiple device support
- style fixes & minor improvements
Sorry to be coming in late, but I'm curious about why this work is being
submitted as a separate driver, rather than as patches against the driver
from Dustin McIntire that was added a few months ago. Is the intention to go forward with two different drivers for these chips?
Scott
Signature · 4 lines
============================================================================== Scott Murray, [email protected] "Good, bad ... I'm the guy with the gun." - Ash, "Army of Darkness"
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi Scott
Quoted 2 lines
Sorry to be coming in late, but I'm curious about why this work is being submitted as a separate driver, rather than as patches against the
driver
Quoted 1 line
from Dustin McIntire that was added a few months ago. Is the intention
to
Quoted 1 line
go forward with two different drivers for these chips?
I was waiting for someone to ask this!
This driver has been developed by SMSC & ARM, and has several advantages over the already merged smc911x:
- The current driver is arm specific, our smsc911x driver is tested and supported on arm, sh, i386
- smsc911x contains support for the new LAN921x family, as well as LAN911x
- smsc911x contains important workarounds for currently known hardware issues
- It's shorted, and I believe the coding style to be cleaner and easier to follow.
so I'm presenting this as an alternative. Thoughts?
Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: [email protected]
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Quoted 22 lines · On Tue, 1 Aug 2006, [email protected] wrote:
Hi Scott
Quoted 2 lines
Sorry to be coming in late, but I'm curious about why this work is being submitted as a separate driver, rather than as patches against the
driver
Quoted 1 line
from Dustin McIntire that was added a few months ago. Is the intention
to
Quoted 1 line
go forward with two different drivers for these chips?
I was waiting for someone to ask this!
This driver has been developed by SMSC & ARM, and has several advantages over the already merged smc911x:
- The current driver is arm specific, our smsc911x driver is tested and supported on arm, sh, i386
- smsc911x contains support for the new LAN921x family, as well as LAN911x
- smsc911x contains important workarounds for currently known hardware issues
- It's shorted, and I believe the coding style to be cleaner and easier to follow.
Quoted 1 line
so I'm presenting this as an alternative. Thoughts?
I think the main concern that my co-workers here at SOMA Networks and I have is that having two drivers dilutes the community effort. There's
also the fact that smc911x seems to have DMA support, which smsc911x does not. Unless there are plans to enhance smsc911x with DMA support, it would seem to me to make more sense to apply the LAN921x support and hardware workaround changes to smc911x.
Scott
Signature · 4 lines
============================================================================== Scott Murray, [email protected] "Good, bad ... I'm the guy with the gun." - Ash, "Army of Darkness"
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi Scott,
Quoted 6 lines
I think the main concern that my co-workers here at SOMA Networks and I have is that having two drivers dilutes the community effort. There's also the fact that smc911x seems to have DMA support, which smsc911x does not. Unless there are plans to enhance smsc911x with DMA support, it would seem to me to make more sense to apply the LAN921x support and hardware workaround changes to smc911x.
I have kept this driver simple for now, focussing on stable support for multiple platforms. While arm is certainly popular, we also have a large number of customers on sh, x86 and others, and currently there is no in-kernel support (DMA or not) for these people.
Many of our customers use our proprietary drivers (which support DMA on many platforms), and we would like to migrate them over to a standard in-kernel driver - it would make support easier and everyone can benefit from ongoing driver improvements. The DMA code within these proprietary drivers is the next target for submission, although I would like to defer additional complexity until we are all happy with the current PIO-only implementation.
Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: [email protected]
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Quoted 14 lines · On Thu, 3 Aug 2006, [email protected] wrote:
Hi Scott,
Quoted 6 lines
I think the main concern that my co-workers here at SOMA Networks and I have is that having two drivers dilutes the community effort. There's also the fact that smc911x seems to have DMA support, which smsc911x does not. Unless there are plans to enhance smsc911x with DMA support, it would seem to me to make more sense to apply the LAN921x support and hardware workaround changes to smc911x.
I have kept this driver simple for now, focussing on stable support for multiple platforms. While arm is certainly popular, we also have a large number of customers on sh, x86 and others, and currently there is no in-kernel support (DMA or not) for these people.
Quoted 7 lines
Many of our customers use our proprietary drivers (which support DMA on many platforms), and we would like to migrate them over to a standard in-kernel driver - it would make support easier and everyone can benefit from ongoing driver improvements. The DMA code within these proprietary drivers is the next target for submission, although I would like to defer additional complexity until we are all happy with the current PIO-only implementation.
Okay, I guess I'll shut up. We're going to stick with smc911x for now and will probably submit some patches to it in the near future. If smsc911x gets accepted into mainline by Jeff and gains DMA support, we may reconsider.
Scott
Signature · 4 lines
============================================================================== Scott Murray, [email protected] "Good, bad ... I'm the guy with the gun." - Ash, "Army of Darkness"
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
In reply to Steve Glendinning[...]
Quoted 8 lines
/* Writes a packet to the
TX_DATA_FIFO*/
static inline void
smsc911x_tx_writefifo(structsmsc911x_data*pdata, unsigned int *buf,
unsigned int wordcount){ while (wordcount--) { smsc911x_reg_write(*buf++, pdata, TX_DATA_FIFO); }
Curly braces are not needed around single line blocks.
[...]
Quoted 16 lines
/* waits for MAC not busy, with timeout. Assumes MacPhyAccessLock has
* already been acquired */
static intsmsc911x_mac_notbusy(struct smsc911x_data *pdata){ int i; for (i = 0; i < 40; i++) { if ((smsc911x_reg_read(pdata, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY_) == 0) { return 1; } } SMSC_WARNING("Timed out waiting for MAC not BUSY. " "MAC_CSR_CMD: 0x%08X", smsc911x_reg_read(pdata, MAC_CSR_CMD)); return 0;
u32 val;
for (i = 0; i < 40; i++) {
val = smsc911x_reg_read(pdata, MAC_CSR_CMD); if (!(val & MAC_CSR_CMD_CSR_BUSY_))
return 1;
}
SMSC_WARNING("Timed out waiting for MAC not BUSY. "
"MAC_CSR_CMD: 0x%08X", val);
-> no painful line-breaking (s/val/{reg/cmd}/ at will).
Quoted 18 lines
}
/* Fetches a MAC register value. Assumes
phy_lockis acquired */ static unsigned intsmsc911x_mac_read(struct smsc911x_data *pdata, unsigned int offset){ unsigned int result = 0xFFFFFFFF; unsigned int temp; /* Wait until not busy */ if (unlikely (smsc911x_reg_read(pdata, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY_)) { SMSC_WARNING("smsc911x_mac_read failed, " "MAC already busy at entry"); return result; } /* Send the MAC cmd */
Quoted 8 lines
smsc911x_reg_write(((offset & 0x000000FF) | MAC_CSR_CMD_CSR_BUSY_ | MAC_CSR_CMD_R_NOT_W_), pdata, MAC_CSR_CMD); /* Workaround for hardware read-after-write restriction */ temp = smsc911x_reg_read(pdata, BYTE_TEST); /* Wait for the read to happen */ if (unlikely(!smsc911x_mac_notbusy(pdata)))
Double negation (at least :o) ).
[...]
Quoted 20 lines
/* Gets a phy register,
phy_lockmust be acquired before calling */ static unsigned intsmsc911x_phy_read(struct smsc911x_data *pdata, unsigned int index){ unsigned int addr; unsigned int result = 0xFFFF; int i; /* Confirm MII not busy */ if (unlikely ((smsc911x_mac_read(pdata, MII_ACC) & MII_ACC_MII_BUSY_) != 0)) { SMSC_WARNING("MII is busy in smsc911x_phy_read???"); return 0; } /* Set the address, index & direction (read from PHY) */ addr = (((pdata->phy_address) & 0x1F) << 11) | ((index & 0x1F) << 6); smsc911x_mac_write(pdata, MII_ACC, addr);
Quoted 8 lines
/* Wait for read to complete w/ timeout */
for (i = 0; i < 100; i++) {
/* See if MII is finished yet */
if ((smsc911x_mac_read(pdata, MII_ACC)
& MII_ACC_MII_BUSY_) == 0) {
result = smsc911x_mac_read(pdata, MII_DATA);
return result;
} if (!(smsc911x_mac_read(pdata, MII_ACC) & MII_ACC_MII_BUSY_))
return smsc911x_mac_read(pdata, MII_DATA);
Quoted 4 lines
}
SMSC_WARNING("Timed out waiting for MII write to finish");
return result;I'd go with return 0xffff; here and remove 'result' altogether.
Quoted 5 lines
}
/* Sets a phy register,
phy_lockmust be acquired before calling */ static void smsc911x_phy_write(structsmsc911x_data*pdata,
unsigned int index, unsigned int val)
Quoted 7 lines
{
unsigned int addr;
int i;
/* Confirm MII not busy */
if (unlikely
((smsc911x_mac_read(pdata, MII_ACC) & MII_ACC_MII_BUSY_) != 0)) {Factor through a smsc911x_mac_busy(pdata) helper ?
[...]
Quoted 25 lines
static int
smsc911x_phy_initialise_external(struct smsc911x_data *pdata){ unsigned int address; unsigned int hwcfg; unsigned int phyid1; unsigned int phyid2; hwcfg = smsc911x_reg_read(pdata, HW_CFG); /* External phy is requested, supported, and detected */ if (hwcfg & HW_CFG_EXT_PHY_DET_) { /* Attempt to switch to external phy for auto-detecting * its address. Assuming tx and rx are stopped because * smsc911x_phy_initialise is called before * smsc911x_rx_initialise and tx_initialise. */ /* Disable phy clocks to the MAC */ hwcfg &= (~HW_CFG_PHY_CLK_SEL_); hwcfg |= HW_CFG_PHY_CLK_SEL_CLK_DIS_; smsc911x_reg_write(hwcfg, pdata, HW_CFG); udelay(10); /* Enough time for clocks to stop */ /* Switch to external phy */
Quoted 8 lines
hwcfg |= HW_CFG_EXT_PHY_EN_; smsc911x_reg_write(hwcfg, pdata, HW_CFG); /* Enable phy clocks to the MAC */ hwcfg &= (~HW_CFG_PHY_CLK_SEL_); hwcfg |= HW_CFG_PHY_CLK_SEL_EXT_PHY_; smsc911x_reg_write(hwcfg, pdata, HW_CFG); udelay(10); /* Enough time for clocks to restart */
(back to my original question that I should have reworded in a different thread)
Does the platform guarantees that the register write has actually reached the real register when the udelay is issued ?
Quoted 12 lines
hwcfg |= HW_CFG_SMI_SEL_;
smsc911x_reg_write(hwcfg, pdata, HW_CFG);
/* Auto-detect PHY */
for (address = 0; address <= 31; address++) {
pdata->phy_address = address;
phyid1 = smsc911x_phy_read(pdata, MII_PHYSID1);
phyid2 = smsc911x_phy_read(pdata, MII_PHYSID2);
if ((phyid1 != 0xFFFFU) || (phyid2 != 0xFFFFU)) {
SMSC_TRACE("Detected PHY at address = "
"0x%02X = %d", address, address);Quoted 8 lines
break;
}
}
if ((phyid1 == 0xFFFFU) && (phyid2 == 0xFFFFU)) {
SMSC_WARNING("External PHY is not accessable, "
"using internal PHY instead");
/* Revert back to interal phy settings. */s/interal/internal/
[...]
Quoted 14 lines
static int
smsc911x_phy_reset(struct smsc911x_data *pdata){ unsigned int temp; unsigned int lcount = 100000; SMSC_TRACE("Performing PHY BCR Reset"); smsc911x_phy_write(pdata, MII_BMCR, BMCR_RESET); do { udelay(10); temp = smsc911x_phy_read(pdata, MII_BMCR); lcount--; } while ((lcount > 0) && (temp & BMCR_RESET)); if (temp & BMCR_RESET) {
Quoted 8 lines
SMSC_WARNING("PHY reset failed to complete.");
return 0;
}
/* Extra delay required because the phy may not be completed with
* its reset when BMCR_RESET is cleared. Specs say 256 uS is
* enough delay but using 500 here to be safe
*/
udelay(500);It would be nice to turn the udelay() into msleep() but this code is issued from at least one spnlock-protected section.
[...]
Quoted 10 lines
#ifdef
USE_PHY_WORK_AROUND
static intsmsc911x_phy_check_loopbackpkt(struct smsc911x_data *pdata){ unsigned int tries = 0; unsigned int lcount = 0; u32 wrsz; u32 rdsz; u32 bufp; for (tries = 0; tries < 10; tries++) {
Wrong visibility whence an useless initialization of 'lcount'.
[...]
Quoted 75 lines
/* Update link mode if any thing has changed */
static voidsmsc911x_phy_update_linkmode(struct net_device *dev){ struct smsc911x_data *pdata = netdev_priv(dev); unsigned int old_link_speed = pdata->link_speed; unsigned int temp; unsigned long flags; spin_lock_irqsave(&pdata->phy_lock, flags); smsc911x_phy_getlinkmode(pdata); if (old_link_speed != pdata->link_speed) { if (pdata->link_speed != LINK_OFF) { unsigned int phy_reg = 0; switch (pdata->link_speed) { case LINK_SPEED_10HD: SMSC_TRACE("Link is now UP at 10Mbps HD"); break; case LINK_SPEED_10FD: SMSC_TRACE("Link is now UP at 10Mbps FD"); break; case LINK_SPEED_100HD: SMSC_TRACE("Link is now UP at 100Mbps HD"); break; case LINK_SPEED_100FD: SMSC_TRACE("Link is now UP at 100Mbps FD"); break; default: SMSC_WARNING("Link is now UP at unknown link " "speed: 0x%08X", pdata->link_speed); break; } phy_reg = smsc911x_mac_read(pdata, MAC_CR); phy_reg &= ~(MAC_CR_FDPX_ | MAC_CR_RCVOWN_); switch (pdata->link_speed) { case LINK_SPEED_10HD: case LINK_SPEED_100HD: phy_reg |= MAC_CR_RCVOWN_; break; case LINK_SPEED_10FD: case LINK_SPEED_100FD: phy_reg |= MAC_CR_FDPX_; break; default: SMSC_WARNING("Unknown link speed: 0x%08X", pdata->link_speed); break; } smsc911x_mac_write(pdata, MAC_CR, phy_reg); if (pdata->link_settings & LINK_AUTO_NEGOTIATE) { unsigned int linkpartner = 0; unsigned int locallink = 0; locallink = smsc911x_phy_read(pdata, 4); linkpartner = smsc911x_phy_read(pdata, 5); switch (pdata->link_speed) { case LINK_SPEED_10FD: case LINK_SPEED_100FD: if (((locallink & linkpartner) & LPA_PAUSE_CAP) != 0) { /* Enable PAUSE receive and transmit */ smsc911x_mac_write(pdata, FLOW, 0xFFFF0002); temp = smsc911x_reg_read(pdata, AFC_CFG); temp |= 0xF; smsc911x_reg_write(temp, pdata, AFC_CFG); } else if (((locallink &
Quoted 8 lines
(ADVERTISE_PAUSE_CAP |
ADVERTISE_PAUSE_ASYM)) ==
(ADVERTISE_PAUSE_CAP |
ADVERTISE_PAUSE_ASYM)) &&
((linkpartner &
(LPA_PAUSE_CAP |
LPA_PAUSE_ASYM)) ==
LPA_PAUSE_ASYM)) {Wow...
[...]
Quoted 7 lines
/* Increments the Rx error counters */
static voidsmsc911x_rx_counterrors(struct smsc911x_data *pdata, unsigned int rxstat){ int crc_err; crc_err = 0;
Merge declaration/initialization.
[...]
Quoted 13 lines
/* Quickly dumps bad packets */
static voidsmsc911x_rx_fastforward(struct smsc911x_data *pdata, unsigned int count){ if (likely(count >= 4)) { unsigned int timeout = 500; smsc911x_reg_write(RX_DP_CTRL_RX_FFWD_, pdata, RX_DP_CTRL); while (timeout && (smsc911x_reg_read(pdata, RX_DP_CTRL) & RX_DP_CTRL_RX_FFWD_)) { udelay(1); timeout--; } if (unlikely(timeout == 0)) {
Quoted 8 lines
SMSC_WARNING("Timed out waiting for RX FFWD "
"to finish, RX_DP_CTRL: 0x%08X",
smsc911x_reg_read(pdata, RX_DP_CTRL));
}
} else {
while (count) {
volatile unsigned int temp;
temp = smsc911x_reg_read(pdata, RX_DATA_FIFO);Kill the volatile: there is an implicit readl().
Quoted 13 lines
/* NAPI poll function */
static intsmsc911x_poll(struct net_device *dev, int *budget){ struct smsc911x_data *pdata = netdev_priv(dev); int npackets = 0; int quota = min(dev->quota, *budget); while (npackets < quota) { unsigned int pktlength; unsigned int rxstat; rxstat = smsc911x_rx_get_rxstatus(pdata); /* break out of while loop if there are no more packets waiting */
Quoted 8 lines
if (rxstat == 0)
break;
pktlength = ((rxstat & 0x3FFF0000) >> 16);
smsc911x_rx_counterrors(pdata, rxstat);
if (likely((rxstat & RX_STS_ES_) == 0)) {
struct sk_buff *skb = NULL;Useless initialization.
Quoted 1 line
skb = dev_alloc_skb(pktlength + 2);
s/2/NET_IP_ALIGN/
Quoted 5 lines
if (likely(skb)) {
skb->data = skb->head;
skb->tail = skb->head;
/* Align IP on 16B boundary */
skb_reserve(skb, 2);s/2/NET_IP_ALIGN/
[...]
Quoted 57 lines
static int
smsc911x_open(struct net_device *dev){ struct smsc911x_data *pdata = netdev_priv(dev); unsigned int mac_high16; unsigned int mac_low32; unsigned int timeout; unsigned int temp; unsigned long flags; spin_lock_init(&pdata->phy_lock); /* Reset the LAN911x */ smsc911x_reg_write(HW_CFG_SRST_, pdata, HW_CFG); timeout = 10; do { udelay(10); temp = smsc911x_reg_read(pdata, HW_CFG); } while ((--timeout) && (temp & HW_CFG_SRST_)); if (unlikely(temp & HW_CFG_SRST_)) { SMSC_WARNING("Failed to complete reset"); return -ENODEV; } smsc911x_reg_write(0x00050000, pdata, HW_CFG); smsc911x_reg_write(0x006E3740, pdata, AFC_CFG); /* Make sure EEPROM has finished loading before setting GPIO_CFG */ timeout = 50; while ((timeout--) && (smsc911x_reg_read(pdata, E2P_CMD) & E2P_CMD_EPC_BUSY_)) { udelay(10); } if (unlikely(timeout == 0)) { SMSC_WARNING("Timed out waiting for EEPROM " "busy bit to clear\n"); } #if USE_DEBUG >= 1 smsc911x_reg_write(0x00670700, pdata, GPIO_CFG); #else smsc911x_reg_write(0x70070000, pdata, GPIO_CFG);#endif
/* Initialise irqs, but leave all sources disabled */ smsc911x_reg_write(0, pdata, INT_EN); smsc911x_reg_write(0xFFFFFFFF, pdata, INT_STS); /* Set interrupt deassertion to 100uS */ smsc911x_reg_write(((10 << 24) | INT_CFG_IRQ_EN_), pdata, INT_CFG); /* * intcfg |= INT_CFG_IRQ_POL_; use this to set IRQ_POL bit * intcfg |= INT_CFG_IRQ_TYPE_; use this to set IRQ_TYPE bit */ SMSC_TRACE("Testing irq handler using IRQ %d", dev->irq); pdata->request_irq_disable = 0;
Quoted 8 lines
pdata->software_irq_signal = 0;
temp = smsc911x_reg_read(pdata, INT_EN);
temp |= INT_EN_SW_INT_EN_;
smsc911x_reg_write(temp, pdata, INT_EN);
timeout = 1000;
do {
udelay(10);
} while ((--timeout) && (!pdata->software_irq_signal)); while (timeout--) {
smp_rmb();
if (pdata->software_irq_signal)
break;
msleep(1);
}
+ smp_wmb() in the irq handler.Quoted 12 lines
if (!pdata->software_irq_signal) {
printk(KERN_WARNING "%s: ISR failed signaling test (IRQ %d)\n",
dev->name, dev->irq);
return -ENODEV;
}
SMSC_TRACE("IRQ handler passed test using IRQ %d", dev->irq);
printk(KERN_INFO "%s: SMSC911x/921x identified at %#08lx, IRQ: %d\n",
dev->name, (unsigned long)pdata->ioaddr, dev->irq);
spin_lock_irqsave(&pdata->phy_lock, flags);flags useless: ->open() is issued in irq-enabled context.
Quoted 27 lines
/* Read mac address from EEPROM */
mac_high16 = smsc911x_mac_read(pdata, ADDRH);
mac_low32 = smsc911x_mac_read(pdata, ADDRL);
/* Generate random MAC address if eeprom values are invalid */
if ((mac_high16 == 0x0000FFFF) && (mac_low32 == 0xFFFFFFFF)) {
u8 random_mac[6];
random_ether_addr(random_mac);
mac_high16 = (random_mac[5] << 8) | random_mac[4];
mac_low32 = (random_mac[3] << 24) | (random_mac[2] << 16) |
(random_mac[1] << 8) | random_mac[0];
smsc911x_mac_write(pdata, ADDRH, mac_high16);
smsc911x_mac_write(pdata, ADDRL, mac_low32);
SMSC_TRACE("MAC Address is set to random_ether_addr");
} else {
SMSC_TRACE("Mac Address is read from LAN911x EEPROM");
}
dev->dev_addr[0] = (u8)(mac_low32);
dev->dev_addr[1] = (u8)(mac_low32 >> 8);
dev->dev_addr[2] = (u8)(mac_low32 >> 16);
dev->dev_addr[3] = (u8)(mac_low32 >> 24);
dev->dev_addr[4] = (u8)(mac_high16);
dev->dev_addr[5] = (u8)(mac_high16 >> 8);
printk(KERN_INFOQuoted 8 lines
"%s: SMSC911x MAC Address: %02x:%02x:%02x:%02x:%02x:%02x\n",
dev->name, dev->dev_addr[0], dev->dev_addr[1], dev->dev_addr[2],
dev->dev_addr[3], dev->dev_addr[4], dev->dev_addr[5]);
netif_carrier_off(dev);
if (!smsc911x_phy_initialise(dev)) {
SMSC_WARNING("Failed to initialize PHY");
return -ENODEV;return with spinlock held (mantra: use goto).
Any chance you could avoid the spinlock ? It would help turning udelay into sleep (namely smsc911x_phy_reset).
[...]
Quoted 5 lines
#ifdef
CONFIG_NET_POLL_CONTROLLER
voidsmsc911x_poll_controller(struct net_device *dev){ disable_irq(dev->irq); smsc911x_irqhandler(0, (void *)dev, NULL);
No need to cast to (void *)
Quoted 6 lines
enable_irq(dev->irq); }#endif /*
CONFIG_NET_POLL_CONTROLLER*//* Standard ioctls for mii-tool */
intsmsc911x_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
static int
Quoted 4 lines
{
struct smsc911x_data *pdata = netdev_priv(dev);
struct mii_ioctl_data *const data =
(struct mii_ioctl_data *)&ifr->ifr_data;Use if_mii()
Quoted 6 lines
unsigned long flags;
SMSC_TRACE("ioctl cmd 0x%x", cmd);
switch (cmd) {
case SIOCGMIIPHY:
case SIOCDEVPRIVATE:The SIOCDEVPRIVATE can/should be removed.
[...]
Quoted 45 lines
static int
smsc911x_drv_probe(struct platform_device *pdev){ struct net_device *dev; struct smsc911x_data *pdata; struct resource *res; int res_size; int retval; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "smsc911x-memory"); if (!res) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { retval = -ENODEV; goto out; } res_size = res->end - res->start; if (!request_mem_region(res->start, res_size, SMSC_CHIPNAME)) { retval = -EBUSY; goto out; } dev = alloc_etherdev(sizeof(struct smsc911x_data)); if (!dev) { printk(KERN_WARNING "%s: Could not allocate device.\n", SMSC_CHIPNAME); retval = -ENOMEM; goto out_release_io; } SET_MODULE_OWNER(dev); SET_NETDEV_DEV(dev, &pdev->dev); pdata = netdev_priv(dev); memset(pdata, 0, sizeof(struct smsc911x_data)); dev->irq = platform_get_irq(pdev, 0); pdata->ioaddr = ioremap_nocache(res->start, res_size); if (pdata->ioaddr == NULL) { SMSC_WARNING("Error smsc911x base address invalid"); retval = -ENOMEM; goto out_free_netdev; }
Quoted 7 lines
if ((retval = smsc911x_init(dev)) < 0)
goto out_unmap_io;
if (request_irq(dev->irq, smsc911x_irqhandler,
SA_INTERRUPT, SMSC_CHIPNAME, dev) != 0) {
SMSC_WARNING("Unable to claim requested irq: %d", dev->irq);
retval = -ENODEV;Should be retval = request_irq(...)
Quoted 9 lines
goto out_unmap_io;
}
platform_set_drvdata(pdev, dev);
retval = register_netdev(dev);
if (retval) {
SMSC_WARNING("Error %i registering device", retval);
retval = -ENODEV;No need to overwrite retval.
Quoted 9 lines
goto out_unset_drvdata; } else { SMSC_TRACE("Network interface: \"%s\"", dev->name); } return 0;
out_unset_drvdata:platform_set_drvdata(pdev, NULL);
Missing free_irq().
Please number the label: out_0, out_release_io_1, etc... It's easier to check.
Quoted 9 lines
out_unmap_io:iounmap(pdata->ioaddr); out_free_netdev: free_netdev(dev); out_release_io: release_mem_region(res->start, res->end - res->start); out: return retval; }
[...]
Quoted 5 lines
/* Entry point for loading the module */
static int__initsmsc911x_init_module(void){ platform_driver_register(&smsc911x_driver); return 0;
return platform_driver_register(...);
Quoted 2 lines
---
/dev/null
++b/drivers/net/smsc911x.h
|...]
Quoted 11 lines
static inline u32
smsc911x_reg_read(struct smsc911x_data *pdata, u32 reg){ u32 reg_val; unsigned long flags; /* these two 16-bit reads must be performed consecutively, so must * not be interrupted by our own ISR (which would start another * read operation) */ local_irq_save(flags); reg_val = ((readw((u16 *)(((unsigned int)pdata->ioaddr) + reg)) & 0xFFFF) |
pdata->ioaddr is void * now. Please simplify.
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
Hi Francois,
Thanks again for all your feedback. I have implemented most of your suggestions,
Quoted 5 lines
Quoted 5 lines
/* Enable phy clocks to the MAC */ hwcfg &= (~HW_CFG_PHY_CLK_SEL_); hwcfg |= HW_CFG_PHY_CLK_SEL_EXT_PHY_; smsc911x_reg_write(hwcfg, pdata, HW_CFG); udelay(10); /* Enough time
for clocks to restart */
Quoted 5 lines
(back to my original question that I should have reworded in a different thread)
Does the platform guarantees that the register write has actually
reached
Quoted 1 line
the real register when the udelay is issued ?
I think so, but maybe you can help me check. The LAN911x device is always directly connected to a simple SRAM-like host bus, and smsc911x_reg_write is implemented using readl. Does this implicitly guarantee it to be volatile?
Quoted 3 lines
Quoted 3 lines
if (!pdata->software_irq_signal) {
printk(KERN_WARNING "%s: ISR failed
signaling test (IRQ %d)\n",
Quoted 4 lines
Quoted 4 lines
dev->name, dev->irq); return -ENODEV; } SMSC_TRACE("IRQ handler passed test using IRQ %d",
dev->irq);
Quoted 2 lines
Quoted 2 lines
printk(KERN_INFO "%s: SMSC911x/921x identified at %#08lx,
IRQ: %d\n",
Quoted 1 line
Quoted 1 line
dev->name, (unsigned long)pdata->ioaddr,
dev->irq);
Quoted 4 lines
Quoted 2 lines
spin_lock_irqsave(&pdata->phy_lock, flags);flags useless: ->open() is issued in irq-enabled context.
How do you mean? I thought an irq-enabled context meant i DO have to disable irqs?
Quoted 8 lines
Quoted 6 lines
unsigned long flags; SMSC_TRACE("ioctl cmd 0x%x", cmd); switch (cmd) { case SIOCGMIIPHY: case SIOCDEVPRIVATE:The SIOCDEVPRIVATE can/should be removed.
I have removed these, they were only in as a quick fix because mii-tool here sends SIOCDEVPRIVATE instead of SIOCGMIIPHY. I fixed my copy of mii-tool instead :o)
Best Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: [email protected]
Re: [PATCH] SMSC LAN911x and LAN921x vendor driver
[email protected] <[email protected]> :
Quoted 1 line
Mezigues :
[...]
Quoted 8 lines
Quoted 1 line
Does the platform guarantees that the register write has actually
reached
Quoted 1 line
the real register when the udelay is issued ?
I think so, but maybe you can help me check. The LAN911x device is always directly connected to a simple SRAM-like host bus, and
smsc911x_reg_writeis implemented using readl. Does this implicitly guarantee it to be volatile?
(s/readl/writel/)
It's probably safe if it's non-cached SRAM like but I strongly suggest to read Documentation/DocBook/deviceiobook.tmpl. It explains better than me.
[...]
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
spin_lock_irqsave(&pdata->phy_lock, flags);flags useless: ->open() is issued in irq-enabled context.
How do you mean? I thought an irq-enabled context meant i DO have to disable irqs?
Yes but you can disable unconditionally and later enable unconditionnally because you know that the irq are _always_ enabled before the lock (in ->open()).
'flags' saves the state. If the state is constant, you can either:
- s/spin_{lock_irqsave/unlock_irqrestore}/spin_{lock/unlock}_irq/ (irq always on before the lock)
or:
- s/spin_{lock_irqsave/unlock_irqrestore}/spin_{lock/unlock}/ (irq always off before the lock)
Signature · 1 line
Ueimor
PATCH 1/1 SMSC LAN911x and LAN921x vendor driver
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Attached is a driver patch for SMSC911x family of ethernet chips, generated against 2.6.18-rc1 sources. There's a similar driver in the tree; this one has been tested by SMSC on all flavors of the chip and claimed to be efficient.
Updated after feedback from Stephen Hemminger.
Improvements following feedback from Stephen Hemminger and Francois Romieu
Updated after additional feedback from netdev list:
- added ethtool support
- simplified phy code using generic mii library
--- drivers/net/Kconfig | 13 drivers/net/Makefile | 1 drivers/net/smsc911x.c | 2035 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/net/smsc911x.h | 445 ++++++++++ 4 files changed, 2494 insertions(+), 0 deletions(-)
drivers/net/Kconfig +13 -0
@@ -865,6 +865,19 @@ config NET_NETX <file:Documentation/networking/net-modules.txt>. The module will be called netx-eth. +config SMSC911X + tristate "SMSC LAN911x/LAN921x families embedded ethernet support" + depends on NET_ETHERNET + select CRC32 + select MII + ---help--- + Say Y here if you want support for SMSC LAN911x and LAN921x families + of ethernet controllers. + + To compile this driver as a module, choose M here and read + <file:Documentation/networking/net-modules.txt>. The module + will be called smsc911x. + config DM9000 tristate "DM9000 support" depends on (ARM || MIPS) && NET_ETHERNET
drivers/net/Makefile +1 -0
@@ -196,6 +196,7 @@ obj-$(CONFIG_S2IO) += s2io.o obj-$(CONFIG_MYRI10GE) += myri10ge/ obj-$(CONFIG_SMC91X) += smc91x.o obj-$(CONFIG_SMC911X) += smc911x.o +obj-$(CONFIG_SMSC911X) += smsc911x.o obj-$(CONFIG_DM9000) += dm9000.o obj-$(CONFIG_FEC_8XX) += fec_8xx/
drivers/net/smsc911x.c new file +2035 -0
2,035 lines not on this page · raw message
drivers/net/smsc911x.h new file +445 -0
445 lines not on this page · raw message
Signature · 1 line
1.4.0