> +
> +struct work_struct;
> +struct dentry;
Why do you need these extra forward declarations?
...
> +
> +struct sge_rspq { /* state for an SGE response queue */
> + unsigned int credits; /* # of pending response credits */
> + unsigned int size; /* capacity of response queue */
> + unsigned int cidx; /* consumer index */
> + unsigned int gen; /* current generation bit */
> + unsigned int polling; /* is the queue serviced through NAPI? */
> + unsigned int holdoff_tmr; /* interrupt holdoff timer in 100ns */
> + unsigned int next_holdoff; /* holdoff time for next interrupt */
> + struct rsp_desc *desc; /* address of HW response ring */
> + dma_addr_t phys_addr; /* physical address of the ring */
> + unsigned int cntxt_id; /* SGE context id for the response q */
> + spinlock_t lock; /* guards response processing */
> + struct sk_buff *rx_head; /* offload packet receive queue head */
> + struct sk_buff *rx_tail; /* offload packet receive queue tail */
Why not use sk_buff_head?
...
> + /*
> + * Dummy netdevices are needed when using multiple receive queues with
> + * NAPI as each netdevice can service only one queue.
> + */
> + struct net_device *dummy_netdev[SGE_QSETS - 1];
>
Rather than abusing, NAPI with multiple receive queue's why not do it
by either:
1) changing NAPI to have multiple receive contexts
2) or use your own tasklet and processing.
> + struct dentry *debugfs_root;
> +
> + struct semaphore mdio_lock;
> + spinlock_t stats_lock;
> + spinlock_t work_lock;
> +};
> +
> +#define MDIO_LOCK(adapter) down(&(adapter)->mdio_lock)
> +#define MDIO_UNLOCK(adapter) up(&(adapter)->mdio_lock)
Please don't wrap locks
> +static inline u32 t3_read_reg(adapter_t *adapter, u32 reg_addr)
> +{
> + u32 val = readl(adapter->regs + reg_addr);
> +
> + CH_DBG(adapter, MMIO, "read register 0x%x value 0x%x\n", reg_addr,
> + val);
> + return val;
> +}
> +
> +static inline void t3_write_reg(adapter_t *adapter, u32 reg_addr, u32 val)
> +{
> + CH_DBG(adapter, MMIO, "setting register 0x%x to 0x%x\n", reg_addr,
> + val);
> + writel(val, adapter->regs + reg_addr);
> +}
This kind of wrapper makes sense.
> +static inline int t3_os_pci_save_state(adapter_t *adapter)
> +{
> + return pci_save_state(adapter->pdev);
> +}
Please don't add bogus OS independent wrappers. It makes it harder for later
maintenance.
> +static inline int t3_os_pci_restore_state(adapter_t *adapter)
> +{
> + return pci_restore_state(adapter->pdev);
> +}
> +
> +static inline void t3_os_pci_write_config_4(adapter_t *adapter, int reg,
> + u32 val)
> +{
> + pci_write_config_dword(adapter->pdev, reg, val);
> +}
> +
> +static inline void t3_os_pci_read_config_4(adapter_t *adapter, int reg,
> + u32 *val)
> +{
> + pci_read_config_dword(adapter->pdev, reg, val);
> +}
> +
> +static inline void t3_os_pci_write_config_2(adapter_t *adapter, int reg,
> + u16 val)
> +{
> + pci_write_config_word(adapter->pdev, reg, val);
> +}
> +
> +static inline void t3_os_pci_read_config_2(adapter_t *adapter, int reg,
> + u16 *val)
> +{
> + pci_read_config_word(adapter->pdev, reg, val);
> +}
> +
> +static inline int t3_os_find_pci_capability(adapter_t *adapter, int cap)
> +{
> + return pci_find_capability(adapter->pdev, cap);
> +}
> +
> +static inline const char *adapter_name(adapter_t *adapter)
> +{
> + return adapter->name;
> +}
> +
> +static inline const char *port_name(adapter_t *adapter, unsigned int port_idx)
> +{
> + return adapter->port[port_idx].dev->name;
> +}
Why not do this inline.
> +static inline void t3_os_set_hw_addr(adapter_t *adapter, int port_idx,
> + u8 hw_addr[])
> +{
> + memcpy(adapter->port[port_idx].dev->dev_addr, hw_addr, ETH_ALEN);
> +#ifdef ETHTOOL_GPERMADDR
> + memcpy(adapter->port[port_idx].dev->perm_addr, hw_addr, ETH_ALEN);
> +#endif
> +}
Another bogus wrapper.
> +/*
> + * We use the spare atalk_ptr to map a net device to its SGE queue set.
> + * This is a macro so it can be used as l-value.
> + */
> +#define dev2qset(netdev) ((netdev)->atalk_ptr)
That looks a bad idea.
> +
> +#define OFFLOAD_DEVMAP_BIT 15
> +
> +#define tdev2adap(d) container_of(d, struct adapter, tdev)
> +
> +static inline int offload_running(adapter_t *adapter)
> +{
> + return test_bit(OFFLOAD_DEVMAP_BIT, &adapter->open_device_map);
> +}
> +
> +int t3_offload_tx(struct t3cdev *tdev, struct sk_buff *skb);
What kind of offload? You remember TOE was rejected.
> +#define promisc_rx_mode(rm) ((rm)->dev->flags & IFF_PROMISC)
> +#define allmulti_rx_mode(rm) ((rm)->dev->flags & IFF_ALLMULTI)
Yet another high maintenance wrapper
> +struct sg_ent { /* SGE scatter/gather entry */
> + u32 len[2];
> + u64 addr[2];
> +};
Shouldn't 2 be NPORTS or somthing.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
