Patchwork [net-next,#2,28/39] natsemi: stop using net_device.{base_addr, irq}.

login
register
mail settings
Submitter françois romieu
Date April 6, 2012, 10:06 a.m.
Message ID <d13055fe45a281c97ec85454ca8e1c2bc9932166.1333704409.git.romieu@fr.zoreil.com>
Download mbox | patch
Permalink /patch/151144/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

françois romieu - April 6, 2012, 10:06 a.m.
It's useless to check mem_start on a newly allocated device.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Tim Hockin <thockin@hockin.org>
---
 drivers/net/ethernet/natsemi/natsemi.c |   67 ++++++++++++++++++-------------
 1 files changed, 39 insertions(+), 28 deletions(-)
David Miller - April 10, 2012, 11:30 p.m.
From: Tim Hockin <thockin@hockin.org>
Date: Tue, 10 Apr 2012 15:53:53 -0700

> First, because I am not so involved any more, I can't say with
> certainty that these fields of struct net_device are not needed.

It's the whole basis of this patch set, and described neatly
in his "00/39" posting.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tim Hockin - April 10, 2012, 11:39 p.m.
OK, so that addresses that concern. :)  Thanks.

Overall nothing in the CL stands out as bad, except for the IO vs MMIO
BAR thing, which I can not answer with certainty any more.

On Tue, Apr 10, 2012 at 4:30 PM, David Miller <davem@davemloft.net> wrote:
> From: Tim Hockin <thockin@hockin.org>
> Date: Tue, 10 Apr 2012 15:53:53 -0700
>
>> First, because I am not so involved any more, I can't say with
>> certainty that these fields of struct net_device are not needed.
>
> It's the whole basis of this patch set, and described neatly
> in his "00/39" posting.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 11, 2012, 12:02 a.m.
From: Tim Hockin <thockin@hockin.org>
Date: Tue, 10 Apr 2012 16:39:31 -0700

> OK, so that addresses that concern. :)  Thanks.
> 
> Overall nothing in the CL stands out as bad, except for the IO vs MMIO
> BAR thing, which I can not answer with certainty any more.

If you capacity to review patches to the driver for anything other
than trivial changes is close to zero, which it appears to be, you
might consider whether you should be listed in MAINTAINERS for it any
long.  And also, therefore, whether this driver should be marked
as Maintained.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tim Hockin - April 11, 2012, 12:19 a.m.
I agree.  The patches to this driver have been very few and far
between, but even those I am largely unable to review any more.

I'm more than willing to hand it off to someone who can do a better job of it

Tim


On Tue, Apr 10, 2012 at 5:02 PM, David Miller <davem@davemloft.net> wrote:
> From: Tim Hockin <thockin@hockin.org>
> Date: Tue, 10 Apr 2012 16:39:31 -0700
>
>> OK, so that addresses that concern. :)  Thanks.
>>
>> Overall nothing in the CL stands out as bad, except for the IO vs MMIO
>> BAR thing, which I can not answer with certainty any more.
>
> If you capacity to review patches to the driver for anything other
> than trivial changes is close to zero, which it appears to be, you
> might consider whether you should be listed in MAINTAINERS for it any
> long.  And also, therefore, whether this driver should be marked
> as Maintained.
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 11, 2012, 12:42 a.m.
From: Tim Hockin <thockin@hockin.org>
Date: Tue, 10 Apr 2012 17:19:42 -0700

> I agree.  The patches to this driver have been very few and far
> between, but even those I am largely unable to review any more.
> 
> I'm more than willing to hand it off to someone who can do a better job of it

My inclination was the mark the driver Orphan'd, which represents
the current state of affairs quite accurately.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tim Hockin - April 11, 2012, 12:59 a.m.
On Tue, Apr 10, 2012 at 5:42 PM, David Miller <davem@davemloft.net> wrote:
> From: Tim Hockin <thockin@hockin.org>
> Date: Tue, 10 Apr 2012 17:19:42 -0700
>
>> I agree.  The patches to this driver have been very few and far
>> between, but even those I am largely unable to review any more.
>>
>> I'm more than willing to hand it off to someone who can do a better job of it
>
> My inclination was the mark the driver Orphan'd, which represents
> the current state of affairs quite accurately.

I'm OK with that. As much as I don't like being a deadbeat, it's a
more accurate snapshot of this driver today.

Tim
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 11, 2012, 1:11 a.m.
From: Tim Hockin <thockin@hockin.org>
Date: Tue, 10 Apr 2012 17:59:40 -0700

> On Tue, Apr 10, 2012 at 5:42 PM, David Miller <davem@davemloft.net> wrote:
>> From: Tim Hockin <thockin@hockin.org>
>> Date: Tue, 10 Apr 2012 17:19:42 -0700
>>
>>> I agree.  The patches to this driver have been very few and far
>>> between, but even those I am largely unable to review any more.
>>>
>>> I'm more than willing to hand it off to someone who can do a better job of it
>>
>> My inclination was the mark the driver Orphan'd, which represents
>> the current state of affairs quite accurately.
> 
> I'm OK with that. As much as I don't like being a deadbeat, it's a
> more accurate snapshot of this driver today.

Ok, I'll push that MAINTAINERS change, thanks Tim.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tim Hockin - April 11, 2012, 1:16 a.m.
On Tue, Apr 10, 2012 at 6:11 PM, David Miller <davem@davemloft.net> wrote:
> From: Tim Hockin <thockin@hockin.org>
> Date: Tue, 10 Apr 2012 17:59:40 -0700
>
>> On Tue, Apr 10, 2012 at 5:42 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Tim Hockin <thockin@hockin.org>
>>> Date: Tue, 10 Apr 2012 17:19:42 -0700
>>>
>>>> I agree.  The patches to this driver have been very few and far
>>>> between, but even those I am largely unable to review any more.
>>>>
>>>> I'm more than willing to hand it off to someone who can do a better job of it
>>>
>>> My inclination was the mark the driver Orphan'd, which represents
>>> the current state of affairs quite accurately.
>>
>> I'm OK with that. As much as I don't like being a deadbeat, it's a
>> more accurate snapshot of this driver today.
>
> Ok, I'll push that MAINTAINERS change, thanks Tim.

I wish there were a way to say "I know a fair amount about this
device" without claiming an "M" line (send patches here).

Tim
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/natsemi/natsemi.c b/drivers/net/ethernet/natsemi/natsemi.c
index d38e48d..5b61d12 100644
--- a/drivers/net/ethernet/natsemi/natsemi.c
+++ b/drivers/net/ethernet/natsemi/natsemi.c
@@ -547,6 +547,7 @@  struct netdev_private {
 	struct sk_buff *tx_skbuff[TX_RING_SIZE];
 	dma_addr_t tx_dma[TX_RING_SIZE];
 	struct net_device *dev;
+	void __iomem *ioaddr;
 	struct napi_struct napi;
 	/* Media monitoring timer */
 	struct timer_list timer;
@@ -699,7 +700,9 @@  static ssize_t natsemi_set_dspcfg_workaround(struct device *dev,
 
 static inline void __iomem *ns_ioaddr(struct net_device *dev)
 {
-	return (void __iomem *) dev->base_addr;
+	struct netdev_private *np = netdev_priv(dev);
+
+	return np->ioaddr;
 }
 
 static inline void natsemi_irq_enable(struct net_device *dev)
@@ -863,10 +866,9 @@  static int __devinit natsemi_probe1 (struct pci_dev *pdev,
 	/* Store MAC Address in perm_addr */
 	memcpy(dev->perm_addr, dev->dev_addr, ETH_ALEN);
 
-	dev->base_addr = (unsigned long __force) ioaddr;
-	dev->irq = irq;
-
 	np = netdev_priv(dev);
+	np->ioaddr = ioaddr;
+
 	netif_napi_add(dev, &np->napi, natsemi_poll, 64);
 	np->dev = dev;
 
@@ -914,9 +916,6 @@  static int __devinit natsemi_probe1 (struct pci_dev *pdev,
 	}
 
 	option = find_cnt < MAX_UNITS ? options[find_cnt] : 0;
-	if (dev->mem_start)
-		option = dev->mem_start;
-
 	/* The lower four bits are the media type. */
 	if (option) {
 		if (option & 0x200)
@@ -1532,20 +1531,21 @@  static int netdev_open(struct net_device *dev)
 {
 	struct netdev_private *np = netdev_priv(dev);
 	void __iomem * ioaddr = ns_ioaddr(dev);
+	const int irq = np->pci_dev->irq;
 	int i;
 
 	/* Reset the chip, just in case. */
 	natsemi_reset(dev);
 
-	i = request_irq(dev->irq, intr_handler, IRQF_SHARED, dev->name, dev);
+	i = request_irq(irq, intr_handler, IRQF_SHARED, dev->name, dev);
 	if (i) return i;
 
 	if (netif_msg_ifup(np))
 		printk(KERN_DEBUG "%s: netdev_open() irq %d.\n",
-			dev->name, dev->irq);
+			dev->name, irq);
 	i = alloc_ring(dev);
 	if (i < 0) {
-		free_irq(dev->irq, dev);
+		free_irq(irq, dev);
 		return i;
 	}
 	napi_enable(&np->napi);
@@ -1794,6 +1794,7 @@  static void netdev_timer(unsigned long data)
 	struct netdev_private *np = netdev_priv(dev);
 	void __iomem * ioaddr = ns_ioaddr(dev);
 	int next_tick = NATSEMI_TIMER_FREQ;
+	const int irq = np->pci_dev->irq;
 
 	if (netif_msg_timer(np)) {
 		/* DO NOT read the IntrStatus register,
@@ -1817,14 +1818,14 @@  static void netdev_timer(unsigned long data)
 				if (netif_msg_drv(np))
 					printk(KERN_NOTICE "%s: possible phy reset: "
 						"re-initializing\n", dev->name);
-				disable_irq(dev->irq);
+				disable_irq(irq);
 				spin_lock_irq(&np->lock);
 				natsemi_stop_rxtx(dev);
 				dump_ring(dev);
 				reinit_ring(dev);
 				init_registers(dev);
 				spin_unlock_irq(&np->lock);
-				enable_irq(dev->irq);
+				enable_irq(irq);
 			} else {
 				/* hurry back */
 				next_tick = HZ;
@@ -1841,10 +1842,10 @@  static void netdev_timer(unsigned long data)
 		spin_unlock_irq(&np->lock);
 	}
 	if (np->oom) {
-		disable_irq(dev->irq);
+		disable_irq(irq);
 		np->oom = 0;
 		refill_rx(dev);
-		enable_irq(dev->irq);
+		enable_irq(irq);
 		if (!np->oom) {
 			writel(RxOn, ioaddr + ChipCmd);
 		} else {
@@ -1885,8 +1886,9 @@  static void ns_tx_timeout(struct net_device *dev)
 {
 	struct netdev_private *np = netdev_priv(dev);
 	void __iomem * ioaddr = ns_ioaddr(dev);
+	const int irq = np->pci_dev->irq;
 
-	disable_irq(dev->irq);
+	disable_irq(irq);
 	spin_lock_irq(&np->lock);
 	if (!np->hands_off) {
 		if (netif_msg_tx_err(np))
@@ -1905,7 +1907,7 @@  static void ns_tx_timeout(struct net_device *dev)
 			dev->name);
 	}
 	spin_unlock_irq(&np->lock);
-	enable_irq(dev->irq);
+	enable_irq(irq);
 
 	dev->trans_start = jiffies; /* prevent tx timeout */
 	dev->stats.tx_errors++;
@@ -2470,9 +2472,12 @@  static struct net_device_stats *get_stats(struct net_device *dev)
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void natsemi_poll_controller(struct net_device *dev)
 {
-	disable_irq(dev->irq);
-	intr_handler(dev->irq, dev);
-	enable_irq(dev->irq);
+	struct netdev_private *np = netdev_priv(dev);
+	const int irq = np->pci_dev->irq;
+
+	disable_irq(irq);
+	intr_handler(irq, dev);
+	enable_irq(irq);
 }
 #endif
 
@@ -2523,8 +2528,9 @@  static int natsemi_change_mtu(struct net_device *dev, int new_mtu)
 	if (netif_running(dev)) {
 		struct netdev_private *np = netdev_priv(dev);
 		void __iomem * ioaddr = ns_ioaddr(dev);
+		const int irq = np->pci_dev->irq;
 
-		disable_irq(dev->irq);
+		disable_irq(irq);
 		spin_lock(&np->lock);
 		/* stop engines */
 		natsemi_stop_rxtx(dev);
@@ -2537,7 +2543,7 @@  static int natsemi_change_mtu(struct net_device *dev, int new_mtu)
 		/* restart engines */
 		writel(RxOn | TxOn, ioaddr + ChipCmd);
 		spin_unlock(&np->lock);
-		enable_irq(dev->irq);
+		enable_irq(irq);
 	}
 	return 0;
 }
@@ -3135,6 +3141,7 @@  static int netdev_close(struct net_device *dev)
 {
 	void __iomem * ioaddr = ns_ioaddr(dev);
 	struct netdev_private *np = netdev_priv(dev);
+	const int irq = np->pci_dev->irq;
 
 	if (netif_msg_ifdown(np))
 		printk(KERN_DEBUG
@@ -3156,14 +3163,14 @@  static int netdev_close(struct net_device *dev)
 	 */
 
 	del_timer_sync(&np->timer);
-	disable_irq(dev->irq);
+	disable_irq(irq);
 	spin_lock_irq(&np->lock);
 	natsemi_irq_disable(dev);
 	np->hands_off = 1;
 	spin_unlock_irq(&np->lock);
-	enable_irq(dev->irq);
+	enable_irq(irq);
 
-	free_irq(dev->irq, dev);
+	free_irq(irq, dev);
 
 	/* Interrupt disabled, interrupt handler released,
 	 * queue stopped, timer deleted, rtnl_lock held
@@ -3256,9 +3263,11 @@  static int natsemi_suspend (struct pci_dev *pdev, pm_message_t state)
 
 	rtnl_lock();
 	if (netif_running (dev)) {
+		const int irq = np->pci_dev->irq;
+
 		del_timer_sync(&np->timer);
 
-		disable_irq(dev->irq);
+		disable_irq(irq);
 		spin_lock_irq(&np->lock);
 
 		natsemi_irq_disable(dev);
@@ -3267,7 +3276,7 @@  static int natsemi_suspend (struct pci_dev *pdev, pm_message_t state)
 		netif_stop_queue(dev);
 
 		spin_unlock_irq(&np->lock);
-		enable_irq(dev->irq);
+		enable_irq(irq);
 
 		napi_disable(&np->napi);
 
@@ -3307,6 +3316,8 @@  static int natsemi_resume (struct pci_dev *pdev)
 	if (netif_device_present(dev))
 		goto out;
 	if (netif_running(dev)) {
+		const int irq = np->pci_dev->irq;
+
 		BUG_ON(!np->hands_off);
 		ret = pci_enable_device(pdev);
 		if (ret < 0) {
@@ -3320,13 +3331,13 @@  static int natsemi_resume (struct pci_dev *pdev)
 
 		natsemi_reset(dev);
 		init_ring(dev);
-		disable_irq(dev->irq);
+		disable_irq(irq);
 		spin_lock_irq(&np->lock);
 		np->hands_off = 0;
 		init_registers(dev);
 		netif_device_attach(dev);
 		spin_unlock_irq(&np->lock);
-		enable_irq(dev->irq);
+		enable_irq(irq);
 
 		mod_timer(&np->timer, round_jiffies(jiffies + 1*HZ));
 	}