Manfred Spraul wrote:
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.
You are correct about irq handler for shared interrupt. I call
netif_carrier_off() so the xmit routine should not be called.
+ 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.
I don't understand this comment...what do you mean by we will 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.
This code first enables the periodic interrupt and then disables it. It
is in the context of diagnostic test.
In general, the periodic timer interrupt is always enabled when we care
about the link state. The nic only has one periodic timer interrupt.
+ 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.
Yes, I will fix it.
+ 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.
Yes, I will fix it.
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may
contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact the
sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
-
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
