[PATCH] skge: request IRQ on activating the interface

[PATCH] skge: request IRQ on activating the interface

[Michal Schmidt]

skge requests IRQ in its probe function. This causes a problem in
the following real-life scenario with two different NICs in the machine:

1. modprobe skge
   The card is detected as eth0 and requests IRQ 17. Directory
   /proc/irq/17/eth0 is created.
2. There is an udev rule which says this interface should be called
   eth1, so udev renames eth0 -> eth1.
3. modprobe 8139too
   The Realtek card is detected as eth0. It will be using IRQ 17 too.
4. ip link set eth0 up
   Now 8139too requests IRQ 17.

The result is:
WARNING: at fs/proc/generic.c:590 proc_register ...
proc_dir_entry '17/eth0' already registered
...

And "ls /proc/irq/17" shows two subdirectories, both called eth0.

Fix it by requesting the IRQ in skge when the interface is activated.
This works, because interfaces can be renamed only while they are down.

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---

 drivers/net/skge.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/net/skge.c b/drivers/net/skge.c
index 62e852e..7e90f27 100644
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -105,6 +105,7 @@ static void yukon_init(struct skge_hw *hw, int port);
 static void genesis_mac_init(struct skge_hw *hw, int port);
 static void genesis_link_up(struct skge_port *skge);
 static void skge_set_multicast(struct net_device *dev);
+static irqreturn_t skge_intr(int irq, void *dev_id);
 
 /* Avoid conditionals by using array */
 static const int txqaddr[] = { Q_XA1, Q_XA2 };
@@ -2572,18 +2573,26 @@ static int skge_up(struct net_device *dev)
 	if (netif_msg_ifup(skge))
 		printk(KERN_INFO PFX "%s: enabling interface\n", dev->name);
 
+	err = request_irq(dev->irq, skge_intr, IRQF_SHARED, dev->name, hw);
+	if (err) {
+		dev_err(&hw->pdev->dev, "%s: cannot assign irq %d\n",
+			dev->name, dev->irq);
+		return err;
+	}
+
 	if (dev->mtu > RX_BUF_SIZE)
 		skge->rx_buf_size = dev->mtu + ETH_HLEN;
 	else
 		skge->rx_buf_size = RX_BUF_SIZE;
 
-
 	rx_size = skge->rx_ring.count * sizeof(struct skge_rx_desc);
 	tx_size = skge->tx_ring.count * sizeof(struct skge_tx_desc);
 	skge->mem_size = tx_size + rx_size;
 	skge->mem = pci_alloc_consistent(hw->pdev, skge->mem_size, &skge->dma);
-	if (!skge->mem)
-		return -ENOMEM;
+	if (!skge->mem) {
+		err = -ENOMEM;
+		goto free_irq;
+	}
 
 	BUG_ON(skge->dma & 7);
 
@@ -2646,6 +2655,8 @@ static int skge_up(struct net_device *dev)
  free_pci_mem:
 	pci_free_consistent(hw->pdev, skge->mem_size, skge->mem, skge->dma);
 	skge->mem = NULL;
+ free_irq:
+	free_irq(dev->irq, hw);
 
 	return err;
 }
@@ -2733,6 +2744,7 @@ static int skge_down(struct net_device *dev)
 	kfree(skge->tx_ring.start);
 	pci_free_consistent(hw->pdev, skge->mem_size, skge->mem, skge->dma);
 	skge->mem = NULL;
+	free_irq(dev->irq, hw);
 	return 0;
 }
 
@@ -3974,12 +3986,6 @@ static int __devinit skge_probe(struct pci_dev *pdev,
 		goto err_out_free_netdev;
 	}
 
-	err = request_irq(pdev->irq, skge_intr, IRQF_SHARED, dev->name, hw);
-	if (err) {
-		dev_err(&pdev->dev, "%s: cannot assign irq %d\n",
-		       dev->name, pdev->irq);
-		goto err_out_unregister;
-	}
 	skge_show_addr(dev);
 
 	if (hw->ports > 1 && (dev1 = skge_devinit(hw, 1, using_dac))) {
@@ -3996,8 +4002,6 @@ static int __devinit skge_probe(struct pci_dev *pdev,
 
 	return 0;
 
-err_out_unregister:
-	unregister_netdev(dev);
 err_out_free_netdev:
 	free_netdev(dev);
 err_out_led_off:
@@ -4041,7 +4045,6 @@ static void __devexit skge_remove(struct pci_dev *pdev)
 	skge_write16(hw, B0_LED, LED_STAT_OFF);
 	skge_write8(hw, B0_CTST, CS_RST_SET);
 
-	free_irq(pdev->irq, hw);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
 	if (dev1)

Re: [PATCH] skge: request IRQ on activating the interface

[Stephen Hemminger]

On Tue, 22 Sep 2009 14:01:31 +0200
Michal Schmidt <mschmidt@redhat.com> wrote:

> skge requests IRQ in its probe function. This causes a problem in
> the following real-life scenario with two different NICs in the machine:
> 
> 1. modprobe skge
>    The card is detected as eth0 and requests IRQ 17. Directory
>    /proc/irq/17/eth0 is created.
> 2. There is an udev rule which says this interface should be called
>    eth1, so udev renames eth0 -> eth1.
> 3. modprobe 8139too
>    The Realtek card is detected as eth0. It will be using IRQ 17 too.
> 4. ip link set eth0 up
>    Now 8139too requests IRQ 17.
> 
> The result is:
> WARNING: at fs/proc/generic.c:590 proc_register ...
> proc_dir_entry '17/eth0' already registered
> ...
> 
> And "ls /proc/irq/17" shows two subdirectories, both called eth0.
> 
> Fix it by requesting the IRQ in skge when the interface is activated.
> This works, because interfaces can be renamed only while they are down.
> 
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>

No. because two port cards have a single IRQ for both ports.
The choice of ethX in irq name was done because irqbalance looks for this.
Probably better to change skge/sky2 and other devices with same issue
to use skge-N ... for request_irq, and teach irqbalance how to do deal
with it.

[PATCH] skge: use unique IRQ name

[Michal Schmidt]

Most network drivers request their IRQ when the interface is activated.
skge does it in ->probe() instead, because it can work with two-port
cards where the two net_devices use the same IRQ. This works fine most
of the time, except in some situations when the interface gets renamed.
Consider this example:

1. modprobe skge
   The card is detected as eth0 and requests IRQ 17. Directory
   /proc/irq/17/eth0 is created.
2. There is an udev rule which says this interface should be called
   eth1, so udev renames eth0 -> eth1.
3. modprobe 8139too
   The Realtek card is detected as eth0. It will be using IRQ 17 too.
4. ip link set eth0 up
   Now 8139too requests IRQ 17.

The result is:
WARNING: at fs/proc/generic.c:590 proc_register ...
proc_dir_entry '17/eth0' already registered
...
And "ls /proc/irq/17" shows two subdirectories, both called eth0.

Fix it by using a unique name for skge's IRQ, based on the PCI address.
The naming from the example then looks like this:
$ grep skge /proc/interrupts
 17:        169   IO-APIC-fasteoi   skge@0000:00:0a.0, eth0

irqbalance daemon will have to be taught to recognize "skge@" as an
Ethernet interrupt. This will be a one-liner addition in classify.c. I
will send a patch to irqbalance if this change is accepted.

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>

Index: kernel/drivers/net/skge.c
===================================================================
--- kernel.orig/drivers/net/skge.c
+++ kernel/drivers/net/skge.c
@@ -3895,6 +3895,7 @@ static int __devinit skge_probe(struct p
 	struct net_device *dev, *dev1;
 	struct skge_hw *hw;
 	int err, using_dac = 0;
+	size_t irq_name_len;
 
 	err = pci_enable_device(pdev);
 	if (err) {
@@ -3935,11 +3936,13 @@ static int __devinit skge_probe(struct p
 #endif
 
 	err = -ENOMEM;
-	hw = kzalloc(sizeof(*hw), GFP_KERNEL);
+	irq_name_len = strlen(DRV_NAME) + strlen(dev_name(&pdev->dev)) + 2;
+	hw = kzalloc(sizeof(*hw) + irq_name_len, GFP_KERNEL);
 	if (!hw) {
 		dev_err(&pdev->dev, "cannot allocate hardware struct\n");
 		goto err_out_free_regions;
 	}
+	sprintf(hw->irq_name, DRV_NAME "@%s", dev_name(&pdev->dev));
 
 	hw->pdev = pdev;
 	spin_lock_init(&hw->hw_lock);
@@ -3974,7 +3977,7 @@ static int __devinit skge_probe(struct p
 		goto err_out_free_netdev;
 	}
 
-	err = request_irq(pdev->irq, skge_intr, IRQF_SHARED, dev->name, hw);
+	err = request_irq(pdev->irq, skge_intr, IRQF_SHARED, hw->irq_name, hw);
 	if (err) {
 		dev_err(&pdev->dev, "%s: cannot assign irq %d\n",
 		       dev->name, pdev->irq);
Index: kernel/drivers/net/skge.h
===================================================================
--- kernel.orig/drivers/net/skge.h
+++ kernel/drivers/net/skge.h
@@ -2423,6 +2423,8 @@ struct skge_hw {
 	u16		     phy_addr;
 	spinlock_t	     phy_lock;
 	struct tasklet_struct phy_task;
+
+	char		     irq_name[0]; /* name for /proc/interrupts */
 };
 
 enum pause_control {

Re: [PATCH] skge: use unique IRQ name

[Stephen Hemminger]

On Thu, 1 Oct 2009 12:27:20 +0200
Michal Schmidt <mschmidt@redhat.com> wrote:

> Most network drivers request their IRQ when the interface is activated.
> skge does it in ->probe() instead, because it can work with two-port
> cards where the two net_devices use the same IRQ. This works fine most
> of the time, except in some situations when the interface gets renamed.
> Consider this example:
> 
> 1. modprobe skge
>    The card is detected as eth0 and requests IRQ 17. Directory
>    /proc/irq/17/eth0 is created.
> 2. There is an udev rule which says this interface should be called
>    eth1, so udev renames eth0 -> eth1.
> 3. modprobe 8139too
>    The Realtek card is detected as eth0. It will be using IRQ 17 too.
> 4. ip link set eth0 up
>    Now 8139too requests IRQ 17.
> 
> The result is:
> WARNING: at fs/proc/generic.c:590 proc_register ...
> proc_dir_entry '17/eth0' already registered
> ...
> And "ls /proc/irq/17" shows two subdirectories, both called eth0.
> 
> Fix it by using a unique name for skge's IRQ, based on the PCI address.
> The naming from the example then looks like this:
> $ grep skge /proc/interrupts
>  17:        169   IO-APIC-fasteoi   skge@0000:00:0a.0, eth0
> 
> irqbalance daemon will have to be taught to recognize "skge@" as an
> Ethernet interrupt. This will be a one-liner addition in classify.c. I
> will send a patch to irqbalance if this change is accepted.
> 
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> 
> Index: kernel/drivers/net/skge.c
> ===================================================================
> --- kernel.orig/drivers/net/skge.c
> +++ kernel/drivers/net/skge.c
> @@ -3895,6 +3895,7 @@ static int __devinit skge_probe(struct p
>  	struct net_device *dev, *dev1;
>  	struct skge_hw *hw;
>  	int err, using_dac = 0;
> +	size_t irq_name_len;
>  
>  	err = pci_enable_device(pdev);
>  	if (err) {
> @@ -3935,11 +3936,13 @@ static int __devinit skge_probe(struct p
>  #endif
>  
>  	err = -ENOMEM;
> -	hw = kzalloc(sizeof(*hw), GFP_KERNEL);
> +	irq_name_len = strlen(DRV_NAME) + strlen(dev_name(&pdev->dev)) + 2;
> +	hw = kzalloc(sizeof(*hw) + irq_name_len, GFP_KERNEL);
>  	if (!hw) {
>  		dev_err(&pdev->dev, "cannot allocate hardware struct\n");
>  		goto err_out_free_regions;
>  	}
> +	sprintf(hw->irq_name, DRV_NAME "@%s", dev_name(&pdev->dev));

I like this with one small change. Please use:
         skge@pci:0000:00:02.0
This makes the driver follow same format as existing DRM graphics drivers.
Michal could you follow up with additional patches for:
   1. sky2 driver has same issue
   2. irqbalance has a list of special drivers that needs to be updated

Re: [PATCH] skge: use unique IRQ name

[Stephen Hemminger]

(revised to use pci:)

From: Michal Schmidt <mschmidt@redhat.com>

Most network drivers request their IRQ when the interface is activated.
skge does it in ->probe() instead, because it can work with two-port
cards where the two net_devices use the same IRQ. This works fine most
of the time, except in some situations when the interface gets renamed.
Consider this example:

1. modprobe skge
   The card is detected as eth0 and requests IRQ 17. Directory
   /proc/irq/17/eth0 is created.
2. There is an udev rule which says this interface should be called
   eth1, so udev renames eth0 -> eth1.
3. modprobe 8139too
   The Realtek card is detected as eth0. It will be using IRQ 17 too.
4. ip link set eth0 up
   Now 8139too requests IRQ 17.

The result is:
WARNING: at fs/proc/generic.c:590 proc_register ...
proc_dir_entry '17/eth0' already registered
...
And "ls /proc/irq/17" shows two subdirectories, both called eth0.

Fix it by using a unique name for skge's IRQ, based on the PCI address.
The naming from the example then looks like this:
$ grep skge /proc/interrupts
 17:        169   IO-APIC-fasteoi   skge@pci:0000:00:0a.0, eth0

irqbalance daemon will have to be taught to recognize "skge" as an
Ethernet interrupt. This will be a one-liner addition in classify.c. I
will send a patch to irqbalance if this change is accepted.

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/skge.c	2009-10-01 09:36:21.893095582 -0700
+++ b/drivers/net/skge.c	2009-10-01 09:46:22.064555679 -0700
@@ -3895,6 +3895,7 @@ static int __devinit skge_probe(struct p
 	struct net_device *dev, *dev1;
 	struct skge_hw *hw;
 	int err, using_dac = 0;
+	size_t irq_name_len;
 
 	err = pci_enable_device(pdev);
 	if (err) {
@@ -3935,11 +3936,14 @@ static int __devinit skge_probe(struct p
 #endif
 
 	err = -ENOMEM;
-	hw = kzalloc(sizeof(*hw), GFP_KERNEL);
+	/* space for skge@pci:0000:04:00.0 */
+	irq_name_len = strlen(DRV_NAME) + strlen(dev_name(&pdev->dev)) + 6;
+	hw = kzalloc(sizeof(*hw) + irq_name_len, GFP_KERNEL);
 	if (!hw) {
 		dev_err(&pdev->dev, "cannot allocate hardware struct\n");
 		goto err_out_free_regions;
 	}
+	sprintf(hw->irq_name, DRV_NAME "@pci:%s", pci_name(pdev));
 
 	hw->pdev = pdev;
 	spin_lock_init(&hw->hw_lock);
@@ -3974,7 +3978,7 @@ static int __devinit skge_probe(struct p
 		goto err_out_free_netdev;
 	}
 
-	err = request_irq(pdev->irq, skge_intr, IRQF_SHARED, dev->name, hw);
+	err = request_irq(pdev->irq, skge_intr, IRQF_SHARED, hw->irq_name, hw);
 	if (err) {
 		dev_err(&pdev->dev, "%s: cannot assign irq %d\n",
 		       dev->name, pdev->irq);
--- a/drivers/net/skge.h	2009-10-01 09:34:51.036505545 -0700
+++ b/drivers/net/skge.h	2009-10-01 09:47:38.096558002 -0700
@@ -2423,6 +2423,8 @@ struct skge_hw {
 	u16		     phy_addr;
 	spinlock_t	     phy_lock;
 	struct tasklet_struct phy_task;
+
+	char		     irq_name[0]; /* skge@pci:000:04:00.0 */
 };
 
 enum pause_control {

Re: [PATCH] skge: use unique IRQ name

[Michal Schmidt]

Dne Thu, 1 Oct 2009 09:50:32 -0700 Stephen Hemminger napsal:
>  	err = -ENOMEM;
> -	hw = kzalloc(sizeof(*hw), GFP_KERNEL);
> +	/* space for skge@pci:0000:04:00.0 */
> +	irq_name_len = strlen(DRV_NAME) +
> strlen(dev_name(&pdev->dev)) + 6;

You replaced "dev_name(&pdev->dev)" with "pci_name(pdev)" below.
That's nice, so we should replace it here too for consistency.

> +	hw = kzalloc(sizeof(*hw) + irq_name_len, GFP_KERNEL);
>  	if (!hw) {
>  		dev_err(&pdev->dev, "cannot allocate hardware
> struct\n"); goto err_out_free_regions;
>  	}
> +	sprintf(hw->irq_name, DRV_NAME "@pci:%s", pci_name(pdev));

Michal

[PATCH v3] skge: use unique IRQ name

[Stephen Hemminger]

From: Michal Schmidt <mschmidt@redhat.com>

Most network drivers request their IRQ when the interface is activated.
skge does it in ->probe() instead, because it can work with two-port
cards where the two net_devices use the same IRQ. This works fine most
of the time, except in some situations when the interface gets renamed.
Consider this example:

1. modprobe skge
   The card is detected as eth0 and requests IRQ 17. Directory
   /proc/irq/17/eth0 is created.
2. There is an udev rule which says this interface should be called
   eth1, so udev renames eth0 -> eth1.
3. modprobe 8139too
   The Realtek card is detected as eth0. It will be using IRQ 17 too.
4. ip link set eth0 up
   Now 8139too requests IRQ 17.

The result is:
WARNING: at fs/proc/generic.c:590 proc_register ...
proc_dir_entry '17/eth0' already registered
...
And "ls /proc/irq/17" shows two subdirectories, both called eth0.

Fix it by using a unique name for skge's IRQ, based on the PCI address.
The naming from the example then looks like this:
$ grep skge /proc/interrupts
 17:        169   IO-APIC-fasteoi   skge@0000:00:0a.0, eth0

irqbalance daemon will have to be taught to recognize "skge@" as an
Ethernet interrupt. This will be a one-liner addition in classify.c. I
will send a patch to irqbalance if this change is accepted.

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>

---
 Changes:
   v2 use pci: in irq name
   v3 simplify calculation of string length

--- a/drivers/net/skge.c	2009-10-01 11:09:19.057676199 -0700
+++ b/drivers/net/skge.c	2009-10-01 11:10:45.786382284 -0700
@@ -3935,11 +3935,14 @@ static int __devinit skge_probe(struct p
 #endif
 
 	err = -ENOMEM;
-	hw = kzalloc(sizeof(*hw), GFP_KERNEL);
+	/* space for skge@pci:0000:04:00.0 */
+	hw = kzalloc(sizeof(*hw) + strlen(DRV_NAME "@pci:" )
+		     + strlen(pci_name(pdev)) + 1, GFP_KERNEL);
 	if (!hw) {
 		dev_err(&pdev->dev, "cannot allocate hardware struct\n");
 		goto err_out_free_regions;
 	}
+	sprintf(hw->irq_name, DRV_NAME "@pci:%s", pci_name(pdev));
 
 	hw->pdev = pdev;
 	spin_lock_init(&hw->hw_lock);
@@ -3974,7 +3977,7 @@ static int __devinit skge_probe(struct p
 		goto err_out_free_netdev;
 	}
 
-	err = request_irq(pdev->irq, skge_intr, IRQF_SHARED, dev->name, hw);
+	err = request_irq(pdev->irq, skge_intr, IRQF_SHARED, hw->irq_name, hw);
 	if (err) {
 		dev_err(&pdev->dev, "%s: cannot assign irq %d\n",
 		       dev->name, pdev->irq);
--- a/drivers/net/skge.h	2009-10-01 11:09:19.067695070 -0700
+++ b/drivers/net/skge.h	2009-10-01 11:09:34.147674089 -0700
@@ -2423,6 +2423,8 @@ struct skge_hw {
 	u16		     phy_addr;
 	spinlock_t	     phy_lock;
 	struct tasklet_struct phy_task;
+
+	char		     irq_name[0]; /* skge@pci:000:04:00.0 */
 };
 
 enum pause_control {

Re: [PATCH v3] skge: use unique IRQ name

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 1 Oct 2009 11:13:23 -0700

> $ grep skge /proc/interrupts
>  17:        169   IO-APIC-fasteoi   skge@0000:00:0a.0, eth0

Please fix this example output to match the code :-)

> +	sprintf(hw->irq_name, DRV_NAME "@pci:%s", pci_name(pdev));

Thanks!

Re: [PATCH v3] skge: use unique IRQ name

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Thu, 01 Oct 2009 13:31:55 -0700 (PDT)

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 1 Oct 2009 11:13:23 -0700
> 
>> $ grep skge /proc/interrupts
>>  17:        169   IO-APIC-fasteoi   skge@0000:00:0a.0, eth0
> 
> Please fix this example output to match the code :-)
> 
>> +	sprintf(hw->irq_name, DRV_NAME "@pci:%s", pci_name(pdev));
> 
> Thanks!

Nevermind, I took care of this when applying your patch.

[PATCH] sky2: irqname based on pci address

[Stephen Hemminger]

This is based on Michal Schmidt fix for skge.

Most network drivers request their IRQ when the interface is activated.
sky2 does it in ->probe() instead, because it can work with two-port
cards where the two net_devices use the same IRQ. This works fine most
of the time, except in some situations when the interface gets renamed.
Consider this example:

1. modprobe sky2
   The card is detected as eth0 and requests IRQ 17. Directory
   /proc/irq/17/eth0 is created.
2. There is an udev rule which says this interface should be called
   eth1, so udev renames eth0 -> eth1.
3. modprobe 8139too
   The Realtek card is detected as eth0. It will be using IRQ 17 too.
4. ip link set eth0 up
   Now 8139too requests IRQ 17.

The result is:
WARNING: at fs/proc/generic.c:590 proc_register ...
proc_dir_entry '17/eth0' already registered

The fix is for sky2 to name the irq based on the pci device, as is done
by some other devices DRM, infiniband, ...  ie. sky2@pci:0000:00:00

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/sky2.c	2009-10-01 09:51:30.604556725 -0700
+++ b/drivers/net/sky2.c	2009-10-01 09:56:38.893342161 -0700
@@ -4487,13 +4487,16 @@ static int __devinit sky2_probe(struct p
 	wol_default = device_may_wakeup(&pdev->dev) ? WAKE_MAGIC : 0;
 
 	err = -ENOMEM;
-	hw = kzalloc(sizeof(*hw), GFP_KERNEL);
+
+	hw = kzalloc(sizeof(*hw) + strlen(DRV_NAME "@pci:")
+		     + strlen(pci_name(pdev)) + 1, GFP_KERNEL);
 	if (!hw) {
 		dev_err(&pdev->dev, "cannot allocate hardware struct\n");
 		goto err_out_free_regions;
 	}
 
 	hw->pdev = pdev;
+	sprintf(hw->irq_name, DRV_NAME "@pci:%s", pci_name(pdev));
 
 	hw->regs = ioremap_nocache(pci_resource_start(pdev, 0), 0x4000);
 	if (!hw->regs) {
@@ -4539,7 +4542,7 @@ static int __devinit sky2_probe(struct p
 
 	err = request_irq(pdev->irq, sky2_intr,
 			  (hw->flags & SKY2_HW_USE_MSI) ? 0 : IRQF_SHARED,
-			  dev->name, hw);
+			  hw->irq_name, hw);
 	if (err) {
 		dev_err(&pdev->dev, "cannot assign irq %d\n", pdev->irq);
 		goto err_out_unregister;
--- a/drivers/net/sky2.h	2009-10-01 09:51:17.553559116 -0700
+++ b/drivers/net/sky2.h	2009-10-01 09:51:42.069510492 -0700
@@ -2085,6 +2085,8 @@ struct sky2_hw {
 	struct timer_list    watchdog_timer;
 	struct work_struct   restart_work;
 	wait_queue_head_t    msi_wait;
+
+	char		     irq_name[0];
 };
 
 static inline int sky2_is_copper(const struct sky2_hw *hw)

Re: [PATCH] sky2: irqname based on pci address

[Stephen Hemminger]

On Thu, 1 Oct 2009 10:11:46 -0700
Stephen Hemminger <shemminger@vyatta.com> wrote:

> This is based on Michal Schmidt fix for skge.
> 
> Most network drivers request their IRQ when the interface is activated.
> sky2 does it in ->probe() instead, because it can work with two-port
> cards where the two net_devices use the same IRQ. This works fine most
> of the time, except in some situations when the interface gets renamed.
> Consider this example:
> 
> 1. modprobe sky2
>    The card is detected as eth0 and requests IRQ 17. Directory
>    /proc/irq/17/eth0 is created.
> 2. There is an udev rule which says this interface should be called
>    eth1, so udev renames eth0 -> eth1.
> 3. modprobe 8139too
>    The Realtek card is detected as eth0. It will be using IRQ 17 too.
> 4. ip link set eth0 up
>    Now 8139too requests IRQ 17.

One other note, the issue is less of a problem for most usage of sky2
because the drive is used mostly on systems that support MSI interrupts
which can never be shared.

Re: [PATCH] sky2: irqname based on pci address

[Michal Schmidt]

Dne Thu, 1 Oct 2009 10:11:46 -0700 Stephen Hemminger napsal(a):
> This is based on Michal Schmidt fix for skge.
> 
> Most network drivers request their IRQ when the interface is
> activated. sky2 does it in ->probe() instead, because it can work
> with two-port cards where the two net_devices use the same IRQ. This
> works fine most of the time, except in some situations when the
> interface gets renamed. Consider this example:
> 
> 1. modprobe sky2
>    The card is detected as eth0 and requests IRQ 17. Directory
>    /proc/irq/17/eth0 is created.
> 2. There is an udev rule which says this interface should be called
>    eth1, so udev renames eth0 -> eth1.
> 3. modprobe 8139too
>    The Realtek card is detected as eth0. It will be using IRQ 17 too.
> 4. ip link set eth0 up
>    Now 8139too requests IRQ 17.
> 
> The result is:
> WARNING: at fs/proc/generic.c:590 proc_register ...
> proc_dir_entry '17/eth0' already registered
> 
> The fix is for sky2 to name the irq based on the pci device, as is
> done by some other devices DRM, infiniband, ...  ie.
> sky2@pci:0000:00:00
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Reviewed-by: Michal Schmidt <mschmidt@redhat.com>

Re: [PATCH] sky2: irqname based on pci address

[David Miller]

From: Michal Schmidt <mschmidt@redhat.com>
Date: Thu, 1 Oct 2009 20:03:55 +0200

> Dne Thu, 1 Oct 2009 10:11:46 -0700 Stephen Hemminger napsal(a):
>> This is based on Michal Schmidt fix for skge.
>> 
>> Most network drivers request their IRQ when the interface is
>> activated. sky2 does it in ->probe() instead, because it can work
>> with two-port cards where the two net_devices use the same IRQ. This
>> works fine most of the time, except in some situations when the
>> interface gets renamed. Consider this example:
 ...
>> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> Reviewed-by: Michal Schmidt <mschmidt@redhat.com>

Applied.