Ayaz Abdulla wrote:
This patch adds support for configuration of various parameters. This
includes module parameters and ethtool commands.
+
+ if (netif_running(dev)) {
+ nv_start_rx(dev);
+ nv_start_tx(dev);
+ nv_enable_irq(dev);
+ }
+
Why no spin_lock() calls? Otherwise start_xmit could be running
concurrently, or the irq handler could run if the interrupt is shared.
+ if (netif_running(dev)) {
+ nv_disable_irq(dev);
+ spin_lock_bh(&dev->xmit_lock);
+ spin_lock(&np->lock);
+ /* stop engines */
+ nv_stop_rx(dev);
+ nv_stop_tx(dev);
+ nv_txrx_reset(dev);
+ /* drain queues */
+ nv_drain_rx(dev);
+ nv_drain_tx(dev);
+ /* delete queues */
+ free_rings(dev);
+ }
[snip]
+
+ if (netif_running(dev)) {
[snip]
+
+ /* restart engines */
+ nv_start_rx(dev);
+ nv_start_tx(dev);
+ spin_unlock(&np->lock);
+ spin_unlock_bh(&dev->xmit_lock);
+ nv_enable_irq(dev);
+ }
Dito: We might miss the spin_unlock.
+static int nv_set_pauseparam(struct net_device *dev, struct
ethtool_pauseparam* pause)
+{
+ struct fe_priv *np = netdev_priv(dev);
+ int adv, bmcr;
+
+ if ((!np->autoneg && np->duplex == 0) ||
+ (np->autoneg && !pause->autoneg && np->duplex == 0)) {
+ printk(KERN_INFO "%s: can not set pause settings when forced link is in half duplex.\n",
+ dev->name);
printk's due to invalid ethtool commands. Really the right thing (tm) to do?
Jeff?
+ nv_disable_hw_interrupts(dev, NVREG_IRQ_TIMER);
If the timer is disabled, does it disable all periodic interrupts?
So far there were always a few interrupts per second, even if the nic
was totally idle.
If this really disables all periodic interrupts, then the LINK_TIMEOUT
code must be replaced with a proper timer, not juse a check within the
irq handler.
+ if (dma_64bit) {
+ if (pci_set_dma_mask(pci_dev, 0x0000007fffffffffULL)) {
+ printk(KERN_INFO "forcedeth: 64-bit DMA failed,
using 32-bit addressing for device %s.\n",
+ pci_name(pci_dev));
} else {
- dev->features |= NETIF_F_HIGHDMA;
- printk(KERN_INFO "forcedeth: using HIGHDMA\n");
+ if (pci_set_consistent_dma_mask(pci_dev,
0x0000007fffffffffULL)) {
+ printk(KERN_INFO "forcedeth: 64-bit DMA
(consistent) failed for device %s.\n",
+ pci_name(pci_dev));
+ goto out_relreg;
+ } else {
+ dev->features |= NETIF_F_HIGHDMA;
+ printk(KERN_INFO "forcedeth: using
HIGHDMA\n");
+ }
The consistend dma mask is independant from the NETIF_F_HIGHDMA setting.
We should avoid to move bugs around.
+ if
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector,
&nv_nic_irq_rx, SA_SHIRQ, dev->name, dev) != 0) {
+ printk(KERN_INFO "forcedeth:
request_irq failed for rx %d\n", ret);
+ pci_disable_msix(np->pci_dev);
+ np->msi_flags &=
~NV_MSI_X_ENABLED;
+ return 1;
+ }
+ /* Request irq for tx handling */
+ if
(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_TX].vector,
&nv_nic_irq_tx, SA_SHIRQ, dev->name, dev) != 0) {
+ printk(KERN_INFO "forcedeth:
request_irq failed for tx %d\n", ret);
+ pci_disable_msix(np->pci_dev);
+ np->msi_flags &=
~NV_MSI_X_ENABLED;
+ return 1;
+ }
Dito: this can leak interrupts.
--
Manfred
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
