Olof Johansson wrote:
On Mon, Jan 29, 2007 at 10:22:33AM -0800, Stephen Hemminger wrote:
Basic initalization, setup comments.

Thanks, fixes have been incorporated and will be reposted. Most of them
was obviously just my lack of diligence. See however the two below.

+static irqreturn_t pasemi_mac_tx_intr(int irq, void *data)
+{
+       struct net_device *dev = data;
+       struct pasemi_mac *mac = netdev_priv(dev);
+       unsigned int reg;
+
+       pasemi_mac_clean_tx(mac);
+
+       reg = PAS_IOB_DMA_TXCH_RESET_PINTC | PAS_IOB_DMA_TXCH_RESET_SINTC;
+       if (*mac->tx_status & PAS_STATUS_TIMER)
+               reg |= PAS_IOB_DMA_TXCH_RESET_TINTC;
+
+       pci_write_config_dword(mac->iob_pdev, 
PAS_IOB_DMA_TXCH_RESET(mac->dma_txch),
+                              reg);
+
+       return IRQ_HANDLED;
+}
To do shared IRQ's properly you need to check to see if
this is your device IRQ or not. Maybe reading config value?

Right now it's guaranteed that the interrupts will not be shared. They're
fixed for the on-chip devices, and no other driver should be binding to
the same channels (and thus irqs).

If it changes in the future, the driver would need other rework as well.

Nonetheless, it is far more sane to check for work, and return if no work. Who knows if the hardware will signal an interrupt early or late.


+static struct pci_driver pasemi_mac_driver = {
+   .name = "pasemi_mac",
+   .id_table = pasemi_mac_pci_tbl,
+   .probe = pasemi_mac_probe,
Don't you need a remove routine?

No hotplug support at this time, so I didn't see any use in providing one.

module remove.

        Jeff



-
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

Reply via email to