Review comments (many) follow.
Still needs work.
Overall: t1_write_reg_4 and friends should be converted to native
readl/writel functions. Don't use wrappers for low-level I/O functions.
diff -puN /dev/null drivers/net/chelsio/ch_ethtool.h
--- /dev/null 2004-08-10 19:55:00.000000000 -0600
+++ 25-akpm/drivers/net/chelsio/ch_ethtool.h 2005-03-24 21:17:43.000000000
-0700
+enum {
+ ETHTOOL_SETREG,
+ ETHTOOL_GETREG,
It's not a good idea to add "black hole" interfaces such as low level
register read/write interfaces, generally.
+ ETHTOOL_SETTPI,
+ ETHTOOL_GETTPI,
+ ETHTOOL_DEVUP,
+ ETHTOOL_GETMTUTAB,
+ ETHTOOL_SETMTUTAB,
+ ETHTOOL_GETMTU,
+ ETHTOOL_SET_PM,
+ ETHTOOL_GET_PM,
+ ETHTOOL_GET_TCAM,
+ ETHTOOL_SET_TCAM,
+ ETHTOOL_GET_TCB,
+ ETHTOOL_READ_TCAM_WORD,
+};
Please don't use the public ETHTOOL_ namespace for these values.
+struct ethtool_reg {
+ uint32_t cmd;
+ uint32_t addr;
+ uint32_t val;
+};
+
+struct ethtool_mtus {
+ uint32_t cmd;
+ uint16_t mtus[NMTUS];
+};
+
+struct ethtool_pm {
+ uint32_t cmd;
+ uint32_t tx_pg_sz;
+ uint32_t tx_num_pg;
+ uint32_t rx_pg_sz;
+ uint32_t rx_num_pg;
+ uint32_t pm_total;
+};
some of these struct members are never used
+struct ethtool_tcam {
+ uint32_t cmd;
+ uint32_t tcam_size;
+ uint32_t nservers;
+ uint32_t nroutes;
+};
+
+struct ethtool_tcb {
+ uint32_t cmd;
+ uint32_t tcb_index;
+ uint32_t tcb_data[TCB_WORDS];
+};
+
+struct ethtool_tcam_word {
+ uint32_t cmd;
+ uint32_t addr;
+ uint32_t buf[3];
+};
1) don't use "ethtool_" namespace for these NIC-specific structs
2) unless this header is shared with userspace, you should be using
u8/u16/u32 types.
diff -puN /dev/null drivers/net/chelsio/common.h
--- /dev/null 2004-08-10 19:55:00.000000000 -0600
+++ 25-akpm/drivers/net/chelsio/common.h 2005-03-24 21:17:43.000000000
-0700
+#define DIMOF(x) (sizeof(x)/sizeof(x[0]))
delete, and use standard ARRAY_SIZE() macro
+#define NMTUS 8
+#define MAX_NPORTS 4
+#define TCB_SIZE 128
enum?
+enum {
+ CHBT_BOARD_7500,
+ CHBT_BOARD_8000,
+ CHBT_BOARD_CHT101,
+ CHBT_BOARD_CHT110,
+ CHBT_BOARD_CHT210,
+ CHBT_BOARD_CHT204,
+ CHBT_BOARD_N110,
+ CHBT_BOARD_N210,
+ CHBT_BOARD_COUGAR,
+ CHBT_BOARD_6800,
+ CHBT_BOARD_SIMUL
+};
+
+enum {
+ CHBT_TERM_FPGA,
+ CHBT_TERM_T1,
+ CHBT_TERM_T2,
+ CHBT_TERM_T3
+};
+
+enum {
+ CHBT_MAC_CHELSIO_A,
+ CHBT_MAC_IXF1010,
+ CHBT_MAC_PM3393,
+ CHBT_MAC_VSC7321,
+ CHBT_MAC_DUMMY
+};
+
+enum {
+ CHBT_PHY_88E1041,
+ CHBT_PHY_88E1111,
+ CHBT_PHY_88X2010,
+ CHBT_PHY_XPAK,
+ CHBT_PHY_MY3126,
+ CHBT_PHY_DUMMY
+};
+
+enum {
+ PAUSE_RX = 1,
+ PAUSE_TX = 2,
+ PAUSE_AUTONEG = 4
+};
I believe these should be bits, in the "(1 << n)" notation?
+/* Revisions of T1 chip */
+#define TERM_T1A 0
+#define TERM_T1B 1
+#define TERM_T2 3
why not enums for these, too?
+struct tp_params {
+ unsigned int pm_size;
+ unsigned int cm_size;
+ unsigned int pm_rx_base;
+ unsigned int pm_tx_base;
+ unsigned int pm_rx_pg_size;
+ unsigned int pm_tx_pg_size;
+ unsigned int pm_rx_num_pgs;
+ unsigned int pm_tx_num_pgs;
+ unsigned int use_5tuple_mode;
+};
where is this ever used?
+struct sge_params {
+ unsigned int cmdQ_size[2];
+ unsigned int freelQ_size[2];
+ unsigned int large_buf_capacity;
+ unsigned int rx_coalesce_usecs;
+ unsigned int last_rx_coalesce_raw;
+ unsigned int default_rx_coalesce_usecs;
+ unsigned int sample_interval_usecs;
+ unsigned int coalesce_enable;
+ unsigned int polling;
+};
+
+struct mc5_params {
+ unsigned int mode; /* selects MC5 width */
+ unsigned int nservers; /* size of server region */
+ unsigned int nroutes; /* size of routing region */
+};
+
+/* Default MC5 region sizes */
+#define DEFAULT_SERVER_REGION_LEN 256
+#define DEFAULT_RT_REGION_LEN 1024
enum?
+struct pci_params {
+ unsigned short speed;
+ unsigned char width;
+ unsigned char is_pcix;
+};
please don't use "pci_" namespace.
Also, typically you want a boolean variable defined as a bit flag in an
'unsigned long' variable, or at least a bitfield "unsigned foo : 1;".
+struct adapter_params {
+ struct sge_params sge;
+ struct mc5_params mc5;
+ struct tp_params tp;
+ struct pci_params pci;
+
+ const struct board_info *brd_info;
+
+ unsigned short mtus[NMTUS];
+ unsigned int nports; /* # of ethernet ports */
+ unsigned int stats_update_period;
+ unsigned short chip_revision;
+ unsigned char chip_version;
+ unsigned char is_asic;
+};
ditto the above comment about booleans, for 'is_asic'
+struct pci_err_cnt {
+ unsigned int master_parity_err;
+ unsigned int sig_target_abort;
+ unsigned int rcv_target_abort;
+ unsigned int rcv_master_abort;
+ unsigned int sig_sys_err;
+ unsigned int det_parity_err;
+ unsigned int pio_parity_err;
+ unsigned int wf_parity_err;
+ unsigned int rf_parity_err;
+ unsigned int cf_parity_err;
+};
* counters should typically be unsigned long
* don't use "pci_" namespace
+struct link_config {
+ unsigned int supported; /* link capabilities */
+ unsigned int advertising; /* advertised capabilities */
+ unsigned short requested_speed; /* speed user has requested */
+ unsigned short speed; /* actual link speed */
+ unsigned char requested_duplex; /* duplex user has requested */
+ unsigned char duplex; /* actual link duplex */
+ unsigned char requested_fc; /* flow control user has requested */
+ unsigned char fc; /* actual link flow control */
+ unsigned char autoneg; /* autonegotiating? */
+};
+
+#define SPEED_INVALID 0xffff
+#define DUPLEX_INVALID 0xff
+
+struct mdio_ops;
+struct gmac;
+struct gphy;
+
+struct board_info {
+ unsigned char board;
+ unsigned char port_number;
+ unsigned long caps;
+ unsigned char chip_term;
+ unsigned char chip_mac;
+ unsigned char chip_phy;
+ unsigned int clock_core;
+ unsigned int clock_mc3;
+ unsigned int clock_mc4;
+ unsigned int espi_nports;
+ unsigned int clock_cspi;
+ unsigned int clock_elmer0;
+ unsigned char mdio_mdien;
+ unsigned char mdio_mdiinv;
+ unsigned char mdio_mdc;
+ unsigned char mdio_phybaseaddr;
+ struct gmac *gmac;
+ struct gphy *gphy;
+ struct mdio_ops *mdio_ops;
can this be made 'const' ?
+ const char *desc;
+};
+
+#include "osdep.h"
+
+#ifndef PCI_VENDOR_ID_CHELSIO
+#define PCI_VENDOR_ID_CHELSIO 0x1425
+#endif
remove this, and patch include/linux/pci_ids.h
+int t1_tpi_write(adapter_t *adapter, u32 addr, u32 value);
+int t1_tpi_read(adapter_t *adapter, u32 addr, u32 *value);
+
+void t1_interrupts_enable(adapter_t *adapter);
+void t1_interrupts_disable(adapter_t *adapter);
+void t1_interrupts_clear(adapter_t *adapter);
+int elmer0_ext_intr_handler(adapter_t *adapter);
+int t1_slow_intr_handler(adapter_t *adapter);
+
+int t1_link_start(struct cphy *phy, struct cmac *mac, struct link_config *lc);
+const struct board_info *t1_get_board_info(unsigned int board_id);
+const struct board_info *t1_get_board_info_from_ids(unsigned int devid,
+ unsigned short ssid);
+int t1_seeprom_read(adapter_t *adapter, u32 addr, u32 *data);
+int t1_get_board_rev(adapter_t *adapter, const struct board_info *bi,
+ struct adapter_params *p);
+int t1_init_hw_modules(adapter_t *adapter);
+int t1_init_sw_modules(adapter_t *adapter, const struct board_info *bi);
+void t1_free_sw_modules(adapter_t *adapter);
+void t1_fatal_err(adapter_t *adapter);
+#endif
please use the 'extern' prefix for these functions.
also, for all headers, please append a comment after the ending #endif,
indicating what symbol the #endif closes. example:
#endif /* CHELSIO_COMMON_H */
diff -puN /dev/null drivers/net/chelsio/cpl5_cmd.h
--- /dev/null 2004-08-10 19:55:00.000000000 -0600
+++ 25-akpm/drivers/net/chelsio/cpl5_cmd.h 2005-03-24 21:17:43.000000000
-0700
+/*
+ * We want this header's alignment to be no more stringent than 2-byte aligned.
+ * All fields are u8 or u16 except for the length. However that field is not
+ * used so we break it into 2 16-bit parts to easily meet our alignment needs.
+ */
Use
struct foo {
...
} __attribute__ ((packed));
to eliminate the compiler's ABI struct member padding and alignment.
+struct cpl_tx_pkt {
+ __u8 opcode;
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+ __u8 iff:4;
+ __u8 ip_csum_dis:1;
+ __u8 l4_csum_dis:1;
+ __u8 vlan_valid:1;
+ __u8 rsvd:1;
+#else
+ __u8 rsvd:1;
+ __u8 vlan_valid:1;
+ __u8 l4_csum_dis:1;
+ __u8 ip_csum_dis:1;
+ __u8 iff:4;
+#endif
+ __u16 vlan;
+ __u16 len_hi;
+ __u16 len_lo;
+};
+
+struct cpl_tx_pkt_lso {
+ __u8 opcode;
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+ __u8 iff:4;
+ __u8 ip_csum_dis:1;
+ __u8 l4_csum_dis:1;
+ __u8 vlan_valid:1;
+ __u8 rsvd:1;
+#else
+ __u8 rsvd:1;
+ __u8 vlan_valid:1;
+ __u8 l4_csum_dis:1;
+ __u8 ip_csum_dis:1;
+ __u8 iff:4;
+#endif
+ __u16 vlan;
+ __u32 len;
+
+ __u32 rsvd2;
+ __u8 rsvd3;
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+ __u8 tcp_hdr_words:4;
+ __u8 ip_hdr_words:4;
+#else
+ __u8 ip_hdr_words:4;
+ __u8 tcp_hdr_words:4;
+#endif
+ __u16 eth_type_mss;
+};
+
+struct cpl_rx_pkt {
+ __u8 opcode;
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+ __u8 iff:4;
+ __u8 csum_valid:1;
+ __u8 bad_pkt:1;
+ __u8 vlan_valid:1;
+ __u8 rsvd:1;
+#else
+ __u8 rsvd:1;
+ __u8 vlan_valid:1;
+ __u8 bad_pkt:1;
+ __u8 csum_valid:1;
+ __u8 iff:4;
+#endif
+ __u16 csum;
+ __u16 vlan;
+ __u16 len;
+};
two general comments:
1) prefer u8/u16/u32 kernel types to __u8/__u16/__u32 compiler types.
2) using bitflags in the code, rather than bitfields, are generally
easier to maintain. the choice is yours, but I encourage doing it like:
struct foo {
u32 bar;
};
...
u32 tmp = le32_to_cpu(dma_structure->foo);
u32 tmp1 = tmp & 0xf; /* get lower 4 bits */
u32 tmp2 = tmp & (1 << 30); /* get bit 30 */
etc.
+#endif
diff -puN /dev/null drivers/net/chelsio/cxgb2.c
--- /dev/null 2004-08-10 19:55:00.000000000 -0600
+++ 25-akpm/drivers/net/chelsio/cxgb2.c 2005-03-24 21:17:43.000000000 -0700
+static inline void cancel_mac_stats_update(struct adapter *ap)
+{
+ cancel_delayed_work(&ap->stats_update_task);
+}
+
+#if BITS_PER_LONG == 64 && !defined(CONFIG_X86_64)
+# define FMT64 "l"
+#else
+# define FMT64 "ll"
+#endif
+
+# define DRV_TYPE ""
delete this, it is unused
+# define MODULE_DESC "Chelsio Network Driver"
1) don't use the MODULE_xxx namespace for your own purposes
2) just include this string inline. don't define a macro for it at all.
+static char driver_name[] = DRV_NAME;
delete this, and just use DRV_NAME inline. The compiler should merge
references to duplicate read-only strings.
+static char driver_version[] = "2.1.0";
ditto the DRV_NAME issue.
+#define PCI_DMA_64BIT ~0ULL
+#define PCI_DMA_32BIT 0xffffffffULL
delete, and use DMA_{32,64}BIT_MASK in include/linux/dma-mapping.h.
+#define MAX_CMDQ_ENTRIES 16384
+#define MAX_CMDQ1_ENTRIES 1024
+#define MAX_RX_BUFFERS 16384
+#define MAX_RX_JUMBO_BUFFERS 16384
+#define MAX_TX_BUFFERS_HIGH 16384U
+#define MAX_TX_BUFFERS_LOW 1536U
+#define MIN_FL_ENTRIES 32
+
+#define PORT_MASK ((1 << MAX_NPORTS) - 1)
enum?
+#define DFLT_MSG_ENABLE (NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK | \
+ NETIF_MSG_TIMER | NETIF_MSG_IFDOWN | NETIF_MSG_IFUP |\
+ NETIF_MSG_RX_ERR | NETIF_MSG_TX_ERR)
+
+/*
+ * The EEPROM is actually bigger but only the first few bytes are used so we
+ * only report those.
+ */
+#define EEPROM_SIZE 32
+
+MODULE_DESCRIPTION(MODULE_DESC);
+MODULE_AUTHOR("Chelsio Communications");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(pci, t1_pci_tbl);
+
+static int dflt_msg_enable = DFLT_MSG_ENABLE;
+
+MODULE_PARM(dflt_msg_enable, "i");
+MODULE_PARM_DESC(dflt_msg_enable, "Chelsio T1 message enable bitmap");
+
+
+static const char pci_speed[][4] = {
+ "33", "66", "100", "133"
+};
+
+/*
+ * Setup MAC to receive the types of packets we want.
+ */
+static void t1_set_rxmode(struct net_device *dev)
+{
+ struct adapter *adapter = dev->priv;
+ struct cmac *mac = adapter->port[dev->if_port].mac;
+ struct t1_rx_mode rm;
+
+ rm.dev = dev;
+ rm.idx = 0;
+ rm.list = dev->mc_list;
+ mac->ops->set_rx_mode(mac, &rm);
+}
delete mac->ops->set_rx_mode() hook, and call pm3393_set_rx_mode() directly
+static void link_report(struct port_info *p)
+{
+ if (!netif_carrier_ok(p->dev))
+ printk(KERN_INFO "%s: link is down\n", p->dev->name);
+ else {
+ const char *s = "10 Mbps";
+
+ switch (p->link_config.speed) {
+ case SPEED_10000: s = "10 Gbps"; break;
+ case SPEED_1000: s = "1000 Mbps"; break;
+ case SPEED_100: s = "100 Mbps"; break;
+ }
+
+ printk(KERN_INFO "%s: link is up at %s, %s duplex\n",
+ p->dev->name, s,
+ p->link_config.duplex == DUPLEX_FULL ? "full" : "half");
+ }
+}
Consider using a printk() message as close as possible to the one in
drivers/net/mii.c:
printk(KERN_INFO "%s: link up, %sMbps, %s-duplex, lpa
0x%04X\n",
+void t1_link_changed(struct adapter *adapter, int port_id, int link_stat,
+ int speed, int duplex, int pause)
+{
+ struct port_info *p = &adapter->port[port_id];
+
+ if (link_stat != netif_carrier_ok(p->dev)) {
+ if (link_stat)
+ netif_carrier_on(p->dev);
+ else
+ netif_carrier_off(p->dev);
+ link_report(p);
+
+ }
+}
+
+static void link_start(struct port_info *p)
+{
+ struct cmac *mac = p->mac;
+
+ mac->ops->reset(mac);
+ if (mac->ops->macaddress_set)
+ mac->ops->macaddress_set(mac, p->dev->dev_addr);
+ t1_set_rxmode(p->dev);
+ t1_link_start(p->phy, mac, &p->link_config);
+ mac->ops->enable(mac, MAC_DIRECTION_RX | MAC_DIRECTION_TX);
+}
delete hooks, call MAC functions directly
+static void enable_hw_csum(struct adapter *adapter)
+{
+ if (adapter->flags & TSO_CAPABLE)
+ t1_tp_set_ip_checksum_offload(adapter->tp, 1); /* for TSO only
*/
+ if (adapter->flags & UDP_CSUM_CAPABLE)
+ t1_tp_set_udp_checksum_offload(adapter->tp, 1);
+ t1_tp_set_tcp_checksum_offload(adapter->tp, 1);
+}
+
+/*
+ * Things to do upon first use of a card.
+ * This must run with the rtnl lock held.
+ */
+static int cxgb_up(struct adapter *adapter)
+{
+ int err = 0;
+
+ if (!(adapter->flags & FULL_INIT_DONE)) {
+ err = t1_init_hw_modules(adapter);
+ if (err)
+ goto out_err;
+
+ enable_hw_csum(adapter);
+ adapter->flags |= FULL_INIT_DONE;
+ }
+
+ t1_interrupts_clear(adapter);
+
+ if ((err = request_irq(adapter->pdev->irq, &t1_interrupt, SA_SHIRQ,
+ adapter->name, adapter)))
+ goto out_err;
+
+ t1_sge_start(adapter->sge);
+ t1_interrupts_enable(adapter);
+
+ err = 0;
+ out_err:
+ return err;
leak on error. You should free the resources allocated on
t1_init_hw_modules().
+ * Release resources when all the ports have been stopped.
+ */
+static void cxgb_down(struct adapter *adapter)
+{
+ t1_sge_stop(adapter->sge);
+ t1_interrupts_disable(adapter);
+ free_irq(adapter->pdev->irq, adapter);
+}
+
+static int cxgb_open(struct net_device *dev)
+{
+ int err;
+ struct adapter *adapter = dev->priv;
+ int other_ports = adapter->open_device_map & PORT_MASK;
+
+ if (!adapter->open_device_map && (err = cxgb_up(adapter)) < 0)
+ return err;
+
+ __set_bit(dev->if_port, &adapter->open_device_map);
do you need atomicity for adapter->open_device_map ?
If not, just open-code the bitsetting:
adapter->open_device_map |= (1 << n)
+ link_start(&adapter->port[dev->if_port]);
+ netif_start_queue(dev);
+ if (!other_ports && adapter->params.stats_update_period)
+ schedule_mac_stats_update(adapter,
+ adapter->params.stats_update_period);
+ return 0;
+}
+
+static int cxgb_close(struct net_device *dev)
+{
+ struct adapter *adapter = dev->priv;
+ struct port_info *p = &adapter->port[dev->if_port];
+ struct cmac *mac = p->mac;
+
+ netif_stop_queue(dev);
+ mac->ops->disable(mac, MAC_DIRECTION_TX | MAC_DIRECTION_RX);
+ netif_carrier_off(dev);
delete hook
+ clear_bit(dev->if_port, &adapter->open_device_map);
+ if (adapter->params.stats_update_period &&
+ !(adapter->open_device_map & PORT_MASK)) {
+ /* Stop statistics accumulation. */
+ smp_mb__after_clear_bit();
+ spin_lock(&adapter->work_lock); /* sync with update task */
+ spin_unlock(&adapter->work_lock);
+ cancel_mac_stats_update(adapter);
reconsider. instead, you should flush the workqueue.
and also, the atomicity of adapter->open_device_map isn't needed.
+ if (!adapter->open_device_map)
+ cxgb_down(adapter);
+ return 0;
+}
+
+static struct net_device_stats *t1_get_stats(struct net_device *dev)
+{
+ struct adapter *adapter = dev->priv;
+ struct port_info *p = &adapter->port[dev->if_port];
+ struct net_device_stats *ns = &p->netstats;
+ const struct cmac_statistics *pstats;
+
+ /* Do a full update of the MAC stats */
+ pstats = p->mac->ops->statistics_update(p->mac,
+ MAC_STATS_UPDATE_FULL);
+
+ ns->tx_packets = pstats->TxUnicastFramesOK +
+ pstats->TxMulticastFramesOK + pstats->TxBroadcastFramesOK;
+
+ ns->rx_packets = pstats->RxUnicastFramesOK +
+ pstats->RxMulticastFramesOK + pstats->RxBroadcastFramesOK;
+
+ ns->tx_bytes = pstats->TxOctetsOK;
+ ns->rx_bytes = pstats->RxOctetsOK;
+
+ ns->tx_errors = pstats->TxLateCollisions + pstats->TxLengthErrors +
+ pstats->TxUnderrun + pstats->TxFramesAbortedDueToXSCollisions;
+ ns->rx_errors = pstats->RxDataErrors + pstats->RxJabberErrors +
+ pstats->RxFCSErrors + pstats->RxAlignErrors +
+ pstats->RxSequenceErrors + pstats->RxFrameTooLongErrors +
+ pstats->RxSymbolErrors + pstats->RxRuntErrors;
+
+ ns->multicast = pstats->RxMulticastFramesOK;
+ ns->collisions = pstats->TxTotalCollisions;
+
+ /* detailed rx_errors */
+ ns->rx_length_errors = pstats->RxFrameTooLongErrors +
+ pstats->RxJabberErrors;
+ ns->rx_over_errors = 0;
+ ns->rx_crc_errors = pstats->RxFCSErrors;
+ ns->rx_frame_errors = pstats->RxAlignErrors;
+ ns->rx_fifo_errors = 0;
+ ns->rx_missed_errors = 0;
+
+ /* detailed tx_errors */
+ ns->tx_aborted_errors = pstats->TxFramesAbortedDueToXSCollisions;
+ ns->tx_carrier_errors = 0;
+ ns->tx_fifo_errors = pstats->TxUnderrun;
+ ns->tx_heartbeat_errors = 0;
+ ns->tx_window_errors = pstats->TxLateCollisions;
+ return ns;
+}
+
+static u32 get_msglevel(struct net_device *dev)
+{
+ struct adapter *adapter = dev->priv;
+
+ return adapter->msg_enable;
+}
+
+static void set_msglevel(struct net_device *dev, u32 val)
+{
+ struct adapter *adapter = dev->priv;
+
+ adapter->msg_enable = val;
+}
+
+static char stats_strings[][ETH_GSTRING_LEN] = {
+ "TxOctetsOK",
+ "TxOctetsBad",
+ "TxUnicastFramesOK",
+ "TxMulticastFramesOK",
+ "TxBroadcastFramesOK",
+ "TxPauseFrames",
+ "TxFramesWithDeferredXmissions",
+ "TxLateCollisions",
+ "TxTotalCollisions",
+ "TxFramesAbortedDueToXSCollisions",
+ "TxUnderrun",
+ "TxLengthErrors",
+ "TxInternalMACXmitError",
+ "TxFramesWithExcessiveDeferral",
+ "TxFCSErrors",
+
+ "RxOctetsOK",
+ "RxOctetsBad",
+ "RxUnicastFramesOK",
+ "RxMulticastFramesOK",
+ "RxBroadcastFramesOK",
+ "RxPauseFrames",
+ "RxFCSErrors",
+ "RxAlignErrors",
+ "RxSymbolErrors",
+ "RxDataErrors",
+ "RxSequenceErrors",
+ "RxRuntErrors",
+ "RxJabberErrors",
+ "RxInternalMACRcvError",
+ "RxInRangeLengthErrors",
+ "RxOutOfRangeLengthField",
+ "RxFrameTooLongErrors"
+};
+
+static void get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
+{
+ struct adapter *adapter = dev->priv;
+
+ strcpy(info->driver, driver_name);
+ strcpy(info->version, driver_version);
+ strcpy(info->fw_version, "N/A");
+ strcpy(info->bus_info, pci_name(adapter->pdev));
+}
+
+static int get_stats_count(struct net_device *dev)
+{
+ return ARRAY_SIZE(stats_strings);
+}
+
+static void get_strings(struct net_device *dev, u32 stringset, u8 *data)
+{
+ if (stringset == ETH_SS_STATS)
+ memcpy(data, stats_strings, sizeof(stats_strings));
+}
+
+static void get_stats(struct net_device *dev, struct ethtool_stats *stats,
+ u64 *data)
+{
+ struct adapter *adapter = dev->priv;
+ struct cmac *mac = adapter->port[dev->if_port].mac;
+ const struct cmac_statistics *s;
+
+ s = mac->ops->statistics_update(mac, MAC_STATS_UPDATE_FULL);
delete hook
+ *data++ = s->TxOctetsOK;
+ *data++ = s->TxOctetsBad;
+ *data++ = s->TxUnicastFramesOK;
+ *data++ = s->TxMulticastFramesOK;
+ *data++ = s->TxBroadcastFramesOK;
+ *data++ = s->TxPauseFrames;
+ *data++ = s->TxFramesWithDeferredXmissions;
+ *data++ = s->TxLateCollisions;
+ *data++ = s->TxTotalCollisions;
+ *data++ = s->TxFramesAbortedDueToXSCollisions;
+ *data++ = s->TxUnderrun;
+ *data++ = s->TxLengthErrors;
+ *data++ = s->TxInternalMACXmitError;
+ *data++ = s->TxFramesWithExcessiveDeferral;
+ *data++ = s->TxFCSErrors;
+
+ *data++ = s->RxOctetsOK;
+ *data++ = s->RxOctetsBad;
+ *data++ = s->RxUnicastFramesOK;
+ *data++ = s->RxMulticastFramesOK;
+ *data++ = s->RxBroadcastFramesOK;
+ *data++ = s->RxPauseFrames;
+ *data++ = s->RxFCSErrors;
+ *data++ = s->RxAlignErrors;
+ *data++ = s->RxSymbolErrors;
+ *data++ = s->RxDataErrors;
+ *data++ = s->RxSequenceErrors;
+ *data++ = s->RxRuntErrors;
+ *data++ = s->RxJabberErrors;
+ *data++ = s->RxInternalMACRcvError;
+ *data++ = s->RxInRangeLengthErrors;
+ *data++ = s->RxOutOfRangeLengthField;
+ *data++ = s->RxFrameTooLongErrors;
other drivers insert a programmer sanity check, to ensure that the
number of stat values output in this function matches the number
returned by the ->get_stats_count() hook.
+static int get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ struct adapter *adapter = dev->priv;
+ struct port_info *p = &adapter->port[dev->if_port];
+
+ cmd->supported = p->link_config.supported;
+ cmd->advertising = p->link_config.advertising;
+
+ if (netif_carrier_ok(dev)) {
+ cmd->speed = p->link_config.speed;
+ cmd->duplex = p->link_config.duplex;
+ } else {
+ cmd->speed = -1;
+ cmd->duplex = -1;
+ }
I'm concerned about this -1 value, which userland isn't really expecting.
It appears e1000 is doing this too :(
+ cmd->port = (cmd->supported & SUPPORTED_TP) ? PORT_TP : PORT_FIBRE;
+ cmd->phy_address = p->phy->addr;
+ cmd->transceiver = XCVR_EXTERNAL;
+ cmd->autoneg = p->link_config.autoneg;
+ cmd->maxtxpkt = 0;
+ cmd->maxrxpkt = 0;
+ return 0;
+}
+
+static int speed_duplex_to_caps(int speed, int duplex)
+{
+ int cap = 0;
+
+ switch (speed) {
+ case SPEED_10:
+ if (duplex == DUPLEX_FULL)
+ cap = SUPPORTED_10baseT_Full;
+ else
+ cap = SUPPORTED_10baseT_Half;
+ break;
+ case SPEED_100:
+ if (duplex == DUPLEX_FULL)
+ cap = SUPPORTED_100baseT_Full;
+ else
+ cap = SUPPORTED_100baseT_Half;
+ break;
+ case SPEED_1000:
+ if (duplex == DUPLEX_FULL)
+ cap = SUPPORTED_1000baseT_Full;
+ else
+ cap = SUPPORTED_1000baseT_Half;
+ break;
+ case SPEED_10000:
+ if (duplex == DUPLEX_FULL)
+ cap = SUPPORTED_10000baseT_Full;
+ }
+ return cap;
+}
might be nice to make this a generic function.
but leave that until after the driver is merged.
+static int set_coalesce(struct net_device *dev, struct ethtool_coalesce *c)
+{
+ struct adapter *adapter = dev->priv;
+
+ unsigned int sge_coalesce_usecs = 0;
+
+ sge_coalesce_usecs = adapter->params.sge.last_rx_coalesce_raw;
+ sge_coalesce_usecs /= board_info(adapter)->clock_core / 1000000;
do you _ever_ use clock_core value, when you are not dividing it by some
constant?
It seems like the variable should be scaled once, rather than performing
a divide each time you use it.
+ if ( (adapter->params.sge.coalesce_enable && !c->use_adaptive_rx_coalesce)
&&
+ (c->rx_coalesce_usecs == sge_coalesce_usecs) ) {
+ adapter->params.sge.rx_coalesce_usecs =
+ adapter->params.sge.default_rx_coalesce_usecs;
+ } else {
+ adapter->params.sge.rx_coalesce_usecs = c->rx_coalesce_usecs;
+ }
style: singleton C statements shouldn't be enclosed in braces {}
+ adapter->params.sge.last_rx_coalesce_raw =
adapter->params.sge.rx_coalesce_usecs;
+ adapter->params.sge.last_rx_coalesce_raw *=
(board_info(adapter)->clock_core / 1000000);
+ adapter->params.sge.sample_interval_usecs = c->rate_sample_interval;
+ adapter->params.sge.coalesce_enable = c->use_adaptive_rx_coalesce;
+ t1_sge_set_coalesce_params(adapter->sge, &adapter->params.sge);
+ return 0;
+}
+
+static int ethtool_ioctl(struct net_device *dev, void *useraddr)
+{
+ u32 cmd;
+ struct adapter *adapter = dev->priv;
+
+ if (copy_from_user(&cmd, useraddr, sizeof(cmd)))
+ return -EFAULT;
+
+ switch (cmd) {
+ case ETHTOOL_SETREG: {
+ struct ethtool_reg edata;
+
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
+ if (copy_from_user(&edata, useraddr, sizeof(edata)))
+ return -EFAULT;
+ if ((edata.addr & 3) != 0 || edata.addr >= adapter->mmio_len)
+ return -EINVAL;
+ if (edata.addr == A_ESPI_MISC_CONTROL)
+ t1_espi_set_misc_ctrl(adapter, edata.val);
+ else {
+ if (edata.addr == 0x950)
+ t1_sge_set_ptimeout(adapter, edata.val);
+ else
+ writel(edata.val, adapter->regs + edata.addr);
+ }
+ break;
+ }
+ case ETHTOOL_GETREG: {
+ struct ethtool_reg edata;
+
+ if (copy_from_user(&edata, useraddr, sizeof(edata)))
+ return -EFAULT;
+ if ((edata.addr & 3) != 0 || edata.addr >= adapter->mmio_len)
+ return -EINVAL;
+ if (edata.addr >= 0x900 && edata.addr <= 0x93c)
+ edata.val = t1_espi_get_mon(adapter, edata.addr, 1);
+ else {
+ if (edata.addr == 0x950)
+ edata.val = t1_sge_get_ptimeout(adapter);
+ else
+ edata.val = readl(adapter->regs + edata.addr);
+ }
+ if (copy_to_user(useraddr, &edata, sizeof(edata)))
+ return -EFAULT;
+ break;
+ }
+ case ETHTOOL_SETTPI: {
+ struct ethtool_reg edata;
+
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
+ if (copy_from_user(&edata, useraddr, sizeof(edata)))
+ return -EFAULT;
+ if ((edata.addr & 3) != 0)
+ return -EINVAL;
+ t1_tpi_write(adapter, edata.addr, edata.val);
+ break;
+ }
+ case ETHTOOL_GETTPI: {
+ struct ethtool_reg edata;
+
+ if (copy_from_user(&edata, useraddr, sizeof(edata)))
+ return -EFAULT;
+ if ((edata.addr & 3) != 0)
+ return -EINVAL;
+ t1_tpi_read(adapter, edata.addr, &edata.val);
+ if (copy_to_user(useraddr, &edata, sizeof(edata)))
+ return -EFAULT;
+ break;
+ }
+ default:
+ return -EOPNOTSUPP;
+ }
+ return 0;
+}
eliminate get/set register ioctls. don't use ethtool namespace.
+static int t1_ioctl(struct net_device *dev, struct ifreq *req, int cmd)
+{
+ struct adapter *adapter = dev->priv;
+ struct mii_ioctl_data *data = (struct mii_ioctl_data *)&req->ifr_data;
+
+ switch (cmd) {
+ case SIOCGMIIPHY:
+ data->phy_id = adapter->port[dev->if_port].phy->addr;
+ /* FALLTHRU */
+ case SIOCGMIIREG: {
+ struct cphy *phy = adapter->port[dev->if_port].phy;
+ u32 val;
+
+ if (!phy->mdio_read) return -EOPNOTSUPP;
one C statement per line
+ phy->mdio_read(adapter, data->phy_id, 0, data->reg_num & 0x1f,
+ &val);
+ data->val_out = val;
+ break;
+ }
+ case SIOCSMIIREG: {
+ struct cphy *phy = adapter->port[dev->if_port].phy;
+
+ if (!capable(CAP_NET_ADMIN)) return -EPERM;
+ if (!phy->mdio_write) return -EOPNOTSUPP;
one C statement per line
+ phy->mdio_write(adapter, data->phy_id, 0, data->reg_num & 0x1f,
+ data->val_in);
+ break;
+ }
+
+ case SIOCCHETHTOOL:
+ return ethtool_ioctl(dev, (void *)req->ifr_data);
don't obfuscate. Use SIOCDEVPRIVATE.
+ default:
+ return -EOPNOTSUPP;
+ }
+ return 0;
+}
+
+static int t1_change_mtu(struct net_device *dev, int new_mtu)
+{
+ int ret;
+ struct adapter *adapter = dev->priv;
+ struct cmac *mac = adapter->port[dev->if_port].mac;
+
+ if (!mac->ops->set_mtu)
+ return -EOPNOTSUPP;
delete hook
+ if (new_mtu < 68)
+ return -EINVAL;
+ if ((ret = mac->ops->set_mtu(mac, new_mtu)))
+ return ret;
ditto
+ dev->mtu = new_mtu;
+ return 0;
+}
+
+static int t1_set_mac_addr(struct net_device *dev, void *p)
+{
+ struct adapter *adapter = dev->priv;
+ struct cmac *mac = adapter->port[dev->if_port].mac;
+ struct sockaddr *addr = p;
+
+ if (!mac->ops->macaddress_set)
+ return -EOPNOTSUPP;
+
+ memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
+ mac->ops->macaddress_set(mac, dev->dev_addr);
+ return 0;
ditto
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+static void vlan_rx_register(struct net_device *dev,
+ struct vlan_group *grp)
+{
+ struct adapter *adapter = dev->priv;
+
+ spin_lock_irq(&adapter->async_lock);
+ adapter->vlan_grp = grp;
+ t1_set_vlan_accel(adapter, grp != NULL);
+ spin_unlock_irq(&adapter->async_lock);
+}
+
+static void vlan_rx_kill_vid(struct net_device *dev, unsigned short vid)
+{
+ struct adapter *adapter = dev->priv;
+
+ spin_lock_irq(&adapter->async_lock);
+ if (adapter->vlan_grp)
+ adapter->vlan_grp->vlan_devices[vid] = NULL;
+ spin_unlock_irq(&adapter->async_lock);
+}
+#endif
+
+#ifdef CONFIG_NET_POLL_CONTROLLER
+static void t1_netpoll(struct net_device *dev)
+{
+ struct adapter *adapter = dev->priv;
+
+ t1_interrupt(adapter->pdev->irq, adapter, NULL);
+}
+#endif
missing disable/enable_irq() calls
+/*
+ * Periodic accumulation of MAC statistics. This is used only if the MAC
+ * does not have any other way to prevent stats counter overflow.
+ */
counters will always overflow.
The stats reader is responsible to collecting stats at a rate rapid
enough to compensate for this.
+static void ext_intr_task(void *data)
+{
+ u32 enable;
+ struct adapter *adapter = data;
+
+ elmer0_ext_intr_handler(adapter);
+
+ /* Now reenable external interrupts */
+ t1_write_reg_4(adapter, A_PL_CAUSE, F_PL_INTR_EXT);
+ enable = t1_read_reg_4(adapter, A_PL_ENABLE);
+ t1_write_reg_4(adapter, A_PL_ENABLE, enable | F_PL_INTR_EXT);
+ adapter->slow_intr_mask |= F_PL_INTR_EXT;
+}
+/*
+ * Interrupt-context handler for elmer0 external interrupts.
+ */
+void t1_elmer0_ext_intr(struct adapter *adapter)
+{
+ u32 enable = t1_read_reg_4(adapter, A_PL_ENABLE);
+
+ /*
+ * Schedule a task to handle external interrupts as we require
+ * a process context. We disable EXT interrupts in the interim
+ * and let the task reenable them when it's done.
+ */
+ adapter->slow_intr_mask &= ~F_PL_INTR_EXT;
+ t1_write_reg_4(adapter, A_PL_ENABLE, enable & ~F_PL_INTR_EXT);
+ schedule_work(&adapter->ext_intr_handler_task);
+}
the above two functions have a tiny race window
+static int __devinit init_one(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ static int version_printed;
+
+ int i, err, pci_using_dac = 0;
+ unsigned long mmio_start, mmio_len;
+ const struct board_info *bi;
+ struct adapter *adapter = NULL;
+ struct port_info *pi;
+
+ if (!version_printed) {
+ printk(KERN_INFO "%s - version %s\n", driver_string,
+ driver_version);
+ ++version_printed;
+ }
+
+ err = pci_enable_device(pdev);
+ if (err)
+ return err;
+
+ if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM)) {
+ CH_ERR("%s: cannot find PCI device memory base address\n",
+ pci_name(pdev));
+ err = -ENODEV;
+ goto out_disable_pdev;
+ }
+
+ if (!pci_set_dma_mask(pdev, PCI_DMA_64BIT)) {
+ pci_using_dac = 1;
+ if (pci_set_consistent_dma_mask(pdev, PCI_DMA_64BIT)) {
+ CH_ERR("%s: unable to obtain 64-bit DMA for"
+ "consistent allocations\n", pci_name(pdev));
+ err = -ENODEV;
+ goto out_disable_pdev;
+ }
+ } else if ((err = pci_set_dma_mask(pdev, PCI_DMA_32BIT)) != 0) {
+ CH_ERR("%s: no usable DMA configuration\n", pci_name(pdev));
+ goto out_disable_pdev;
+ }
missing call to pci_set_consistent_dma_mask()
see drivers/net/tg3.c for an example.
+ err = pci_request_regions(pdev, driver_name);
+ if (err) {
+ CH_ERR("%s: cannot obtain PCI resources\n", pci_name(pdev));
+ goto out_disable_pdev;
+ }
+
+ pci_set_master(pdev);
+
+ mmio_start = pci_resource_start(pdev, 0);
+ mmio_len = pci_resource_len(pdev, 0);
+ bi = t1_get_board_info(ent->driver_data);
+
+ for (i = 0; i < bi->port_number; ++i) {
+ struct net_device *netdev;
+
+ netdev = alloc_etherdev(adapter ? 0 : sizeof(*adapter));
+ if (!netdev) {
+ err = -ENOMEM;
+ goto out_free_dev;
+ }
+
+ SET_MODULE_OWNER(netdev);
+ SET_NETDEV_DEV(netdev, &pdev->dev);
+
+ if (!adapter) {
+ adapter = netdev->priv;
+ adapter->pdev = pdev;
+ adapter->port[0].dev = netdev; /* so we don't leak it
*/
+
+ adapter->regs = ioremap(mmio_start, mmio_len);
+ if (!adapter->regs) {
+ CH_ERR("%s: cannot map device registers\n",
+ pci_name(pdev));
+ err = -ENOMEM;
+ goto out_free_dev;
+ }
+
+ if (t1_get_board_rev(adapter, bi, &adapter->params)) {
+ err = -ENODEV; /* Can't handle this chip rev
*/
+ goto out_free_dev;
+ }
+
+ adapter->name = pci_name(pdev);
+ adapter->msg_enable = dflt_msg_enable;
+ adapter->mmio_len = mmio_len;
+
+ init_MUTEX(&adapter->mib_mutex);
+ spin_lock_init(&adapter->tpi_lock);
+ spin_lock_init(&adapter->work_lock);
+ spin_lock_init(&adapter->async_lock);
+
+ INIT_WORK(&adapter->ext_intr_handler_task,
+ ext_intr_task, adapter);
+ INIT_WORK(&adapter->stats_update_task, mac_stats_task,
+ adapter);
+
+ pci_set_drvdata(pdev, netdev);
+
+ }
+
+ pi = &adapter->port[i];
+ pi->dev = netdev;
+ netif_carrier_off(netdev);
+ netdev->irq = pdev->irq;
+ netdev->if_port = i;
Why are you setting this? Does the value actually affect any operations?
+ netdev->mem_start = mmio_start;
+ netdev->mem_end = mmio_start + mmio_len - 1;
don't set this at all
+ netdev->priv = adapter;
+ netdev->features |= NETIF_F_SG | NETIF_F_IP_CSUM;
+ adapter->flags |= RX_CSUM_ENABLED | TCP_CSUM_CAPABLE;
+ if (pci_using_dac)
+ netdev->features |= NETIF_F_HIGHDMA;
+ if (vlan_tso_capable(adapter)) {
+ adapter->flags |= UDP_CSUM_CAPABLE;
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+ adapter->flags |= VLAN_ACCEL_CAPABLE;
+ netdev->features |=
+ NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
+ netdev->vlan_rx_register = vlan_rx_register;
+ netdev->vlan_rx_kill_vid = vlan_rx_kill_vid;
+#endif
+ adapter->flags |= TSO_CAPABLE;
+ netdev->features |= NETIF_F_TSO;
+ }
+
+ netdev->open = cxgb_open;
+ netdev->stop = cxgb_close;
+ netdev->hard_start_xmit = t1_start_xmit;
+ netdev->hard_header_len += (adapter->flags & TSO_CAPABLE) ?
+ sizeof(struct cpl_tx_pkt_lso) :
+ sizeof(struct cpl_tx_pkt);
it's a bit unusual to be messing with hard_header_len.
probably unwise.
+ netdev->get_stats = t1_get_stats;
+ netdev->set_multicast_list = t1_set_rxmode;
+ netdev->do_ioctl = t1_ioctl;
+ netdev->change_mtu = t1_change_mtu;
+ netdev->set_mac_address = t1_set_mac_addr;
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ netdev->poll_controller = t1_netpoll;
+#endif
+ netdev->weight = 64;
+
+ SET_ETHTOOL_OPS(netdev, &t1_ethtool_ops);
+ }
+
+ if (t1_init_sw_modules(adapter, bi) < 0) {
+ err = -ENODEV;
+ goto out_free_dev;
+ }
+
+ /*
+ * The card is now ready to go. If any errors occur during device
+ * registration we do not fail the whole card but rather proceed only
+ * with the ports we manage to register successfully. However we must
+ * register at least one net device.
+ */
+ for (i = 0; i < bi->port_number; ++i) {
+ err = register_netdev(adapter->port[i].dev);
+ if (err)
+ CH_WARN("%s: cannot register net device %s, skipping\n",
+ pci_name(pdev), adapter->port[i].dev->name);
+ else {
+ /*
+ * Change the name we use for messages to the name of
+ * the first successfully registered interface.
+ */
+ if (!adapter->registered_device_map)
+ adapter->name = adapter->port[i].dev->name;
+
+ __set_bit(i, &adapter->registered_device_map);
+ }
+ }
+ if (!adapter->registered_device_map) {
+ CH_ERR("%s: could not register any net devices\n",
+ pci_name(pdev));
+ goto out_release_adapter_res;
+ }
+
+ printk(KERN_INFO "%s: %s (rev %d), %s %dMHz/%d-bit\n", adapter->name,
+ bi->desc, adapter->params.chip_revision,
+ adapter->params.pci.is_pcix ? "PCIX" : "PCI",
+ adapter->params.pci.speed, adapter->params.pci.width);
+ return 0;
+
+ out_release_adapter_res:
+ t1_free_sw_modules(adapter);
+ out_free_dev:
+ if (adapter) {
+ if (adapter->regs)
+ iounmap(adapter->regs);
+ for (i = bi->port_number - 1; i >= 0; --i)
+ if (adapter->port[i].dev)
+ free_netdev(adapter->port[i].dev);
+ }
+ pci_release_regions(pdev);
+ out_disable_pdev:
+ pci_disable_device(pdev);
+ pci_set_drvdata(pdev, NULL);
+ return err;
+}
+
+static inline void t1_sw_reset(struct pci_dev *pdev)
+{
+ pci_write_config_dword(pdev, A_PCICFG_PM_CSR, 3);
+ pci_write_config_dword(pdev, A_PCICFG_PM_CSR, 0);
+}
there should be standard functions for D3->D0 transition, IIRC
+static void __devexit remove_one(struct pci_dev *pdev)
+{
+ struct net_device *dev = pci_get_drvdata(pdev);
+
+ if (dev) {
+ int i;
+ struct adapter *adapter = dev->priv;
+
+ for_each_port(adapter, i)
+ if (test_bit(i, &adapter->registered_device_map))
+ unregister_netdev(adapter->port[i].dev);
+
+ t1_free_sw_modules(adapter);
+ iounmap(adapter->regs);
+ while (--i >= 0)
+ if (adapter->port[i].dev)
+ free_netdev(adapter->port[i].dev);
+ pci_release_regions(pdev);
+ pci_disable_device(pdev);
+ pci_set_drvdata(pdev, NULL);
+ t1_sw_reset(pdev);
+ }
+}
+
+static struct pci_driver driver = {
+ .name = driver_name,
+ .id_table = t1_pci_tbl,
+ .probe = init_one,
+ .remove = __devexit_p(remove_one),
+};
+
+static int __init t1_init_module(void)
+{
+ return pci_module_init(&driver);
+}
+
+static void __exit t1_cleanup_module(void)
+{
+ pci_unregister_driver(&driver);
+}
+
+module_init(t1_init_module);
+module_exit(t1_cleanup_module);
+
diff -puN /dev/null drivers/net/chelsio/cxgb2.h
--- /dev/null 2004-08-10 19:55:00.000000000 -0600
+++ 25-akpm/drivers/net/chelsio/cxgb2.h 2005-03-24 21:17:43.000000000 -0700
+/* This belongs in if_ether.h */
+#define ETH_P_CPL5 0xf
What is CPL5?
Might as well patch if_ether.h.
+struct cmac;
+struct cphy;
+
+struct port_info {
+ struct net_device *dev;
+ struct cmac *mac;
+ struct cphy *phy;
+ struct link_config link_config;
+ struct net_device_stats netstats;
+};
+
+struct cxgbdev;
+struct t1_sge;
+struct pemc3;
+struct pemc4;
+struct pemc5;
+struct peulp;
+struct petp;
+struct pecspi;
+struct peespi;
+struct work_struct;
+struct vlan_group;
+
+enum { /* adapter flags */
+ FULL_INIT_DONE = 0x1,
+ USING_MSI = 0x2,
+ TSO_CAPABLE = 0x4,
+ TCP_CSUM_CAPABLE = 0x8,
+ UDP_CSUM_CAPABLE = 0x10,
+ VLAN_ACCEL_CAPABLE = 0x20,
+ RX_CSUM_ENABLED = 0x40,
+};
use bit style '1 << n' ?
+struct adapter {
+ u8 *regs;
+ struct pci_dev *pdev;
+ unsigned long registered_device_map;
+ unsigned long open_device_map;
+ unsigned int flags;
unsigned long is generally better for bitflag variables
+ const char *name;
+ int msg_enable;
+ u32 mmio_len;
+
+ struct work_struct ext_intr_handler_task;
+ struct adapter_params params;
+
+ struct vlan_group *vlan_grp;
+
+ /* Terminator modules. */
+ struct sge *sge;
+ struct pemc3 *mc3;
+ struct pemc4 *mc4;
+ struct pemc5 *mc5;
+ struct petp *tp;
+ struct pecspi *cspi;
+ struct peespi *espi;
+ struct peulp *ulp;
+
+ struct port_info port[MAX_NPORTS];
+ struct work_struct stats_update_task;
+ struct timer_list stats_update_timer;
+
+ struct semaphore mib_mutex;
+ spinlock_t tpi_lock;
+ spinlock_t work_lock;
+
+ spinlock_t async_lock ____cacheline_aligned; /* guards async operations
*/
can you explain this? You really shouldn't mess with attributes on
spinlocks. That's highly arch-specific.
I gave up reviewing at this point. That's plenty of changes to tackle,
particularly the removal of all the t1_write_reg_4() wrappers.
Jeff
|