netdev
[Top] [All Lists]

Re: [PATCH] Road Runner HIPPI driver (rrunner)

To: Stephen Hemminger <shemminger@xxxxxxxx>
Subject: Re: [PATCH] Road Runner HIPPI driver (rrunner)
From: Jeff Garzik <jgarzik@xxxxxxxxx>
Date: Fri, 12 Sep 2003 18:06:55 -0400
Cc: Jes Sorensen <jes@xxxxxxxxxxxxxxxxxx>, netdev@xxxxxxxxxxx
In-reply-to: <20030912144208.2886e2b9.shemminger@osdl.org>
Organization: none
References: <20030912144208.2886e2b9.shemminger@osdl.org>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20021213 Debian/1.2.1-2.bunk
Stephen Hemminger wrote:
@@ -209,9 +208,9 @@ static int __devinit rr_init_one(struct dev->base_addr = 0;
- ret = register_netdev(dev);
- if (ret)
+ if (register_netdev(dev)) goto out;
+
return 0;
out:

We lose the register_netdev() return value, whereas the driver got it right before.



static void __devexit rr_remove_one (struct pci_dev *pdev)
{
- struct net_device *dev = pci_get_drvdata(pdev);
- struct rr_private *rr = (struct rr_private *)dev->priv;
+ struct net_device *dev;
+ struct rr_private *rr;
- if (dev) {
- if (!(readl(&rr->regs->HostCtrl) & NIC_HALTED)){
- printk(KERN_ERR "%s: trying to unload running NIC\n",
- dev->name);
- writel(HALT_NIC, &rr->regs->HostCtrl);
- }
-
- pci_free_consistent(pdev, EVT_RING_SIZE, rr->evt_ring,
- rr->evt_ring_dma);
- pci_free_consistent(pdev, RX_TOTAL_SIZE, rr->rx_ring,
- rr->rx_ring_dma);
- pci_free_consistent(pdev, TX_TOTAL_SIZE, rr->tx_ring,
- rr->tx_ring_dma);
- unregister_netdev(dev);
- iounmap(rr->regs);
- free_netdev(dev);
- pci_release_regions(pdev);
- pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
+ if (!(dev = pci_get_drvdata(pdev)))
+ return;

this is a fairly ugly construct. It looks better as the driver had it, as an initializer. Don't combine C statements when you don't need to.



@@ -1201,8 +1205,8 @@ static int rr_open(struct net_device *de
readl(&regs->HostCtrl);
spin_unlock_irqrestore(&rrpriv->lock, flags);
- if (request_irq(dev->irq, rr_interrupt, SA_SHIRQ, rrpriv->name, dev))
- {
+ if (request_irq(dev->irq, rr_interrupt, SA_SHIRQ, + "RoadRunner serial HIPPI", dev)) {
printk(KERN_WARNING "%s: Requested IRQ %d is busy\n",
dev->name, dev->irq);
ecode = -EAGAIN;

If Jes doesn't mind, I would prefer that we pass in the interface name (dev->name I assume?) here.


        Jeff




<Prev in Thread] Current Thread [Next in Thread>