at1700: fix double free_irq
Subsystems (3)
PATCH 1/1 at1700: fix double free_irq
free_irq() is called both in net_close() and cleanup_card(). Since it is requested in at1700_probe1(), leave free_irq() only in cleanup_card() for balance.
--- drivers/net/at1700.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
drivers/net/at1700.c +1 -3
@@ -811,10 +811,8 @@ static int net_close(struct net_device *dev)/* No statistic counters on the chip to update. */ /* Disable the IRQ on boards of fmv18x where it is feasible. */ -if (lp->jumpered) { +if (lp->jumpered)outb (0x00 , ioaddr +IOCONFIG1 ); -free_irq (dev->irq, dev); - }/* Power-down the chip. Green, green, green! */ outb (0x00 , ioaddr +CONFIG_1 );
Signature · 1 line
1.7.0.4
Quoted 4 lines · Kulikov Vasiliy
free_irq()is called both innet_close()andcleanup_card(). Since it is requested inat1700_probe1(), leavefree_irq()only incleanup_card()for balance.
Are you sure? I would think that we should make the free_irq() in cleanup_card() conditional instead.
drivers/net/at1700.c static int net_close(struct net_device *dev) · line 813
Quoted 13 lines
Signed-off-by Kulikov Vasiliy <[email protected]>--- drivers/net/at1700.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)drivers/net/at1700.c +0
-0@@ -811,10 +811,8 @@ static int net_close(struct net_device *dev)/* No statistic counters on the chip to update. */ /* Disable the IRQ on boards of fmv18x where it is feasible. */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
It seems like this comment should be updated?
drivers/net/at1700.c static int net_close(struct net_device *dev) · line 817
Quoted 5 lines
drivers/net/at1700.c +1
-3-if (lp->jumpered) { +if (lp->jumpered)outb (0x00 , ioaddr +IOCONFIG1 ); -free_irq (dev->irq, dev); - }
regards,
dan carpenter
Quoted 8 lines · Dan Carpenter
Quoted 4 lines · Kulikov Vasiliy
free_irq()is called both innet_close()andcleanup_card(). Since it is requested inat1700_probe1(), leavefree_irq()only incleanup_card()for balance.Are you sure? I would think that we should make the
free_irq()incleanup_card()conditional instead.
See balanced functions: net_open() & net_close(), at1700_probe1() & cleanup_card(). request_irq() is in probe, so it must not be freed on 'ifconfig down'. E.g.
modprobe at1700 <= request_irq()
ifconfig eth0 up
ifconfig eth0 down <= first free_irq()
ifconfig eth0 up <= no request_irq() here!
ifconfig eth0 down <= second free_irq()
rmmod at1700 <= third free_irq()
Quoted 13 lines
Quoted 13 lines
Signed-off-by Kulikov Vasiliy <[email protected]>--- drivers/net/at1700.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)drivers/net/at1700.c +0
-0@@ -811,10 +811,8 @@ static int net_close(struct net_device *dev)/* No statistic counters on the chip to update. */
Quoted 4 lines
drivers/net/at1700.c static int net_close(struct net_device *dev) · line 813
Quoted 1 line
drivers/net/at1700.c +0
-0/* Disable the IRQ on boards of fmv18x where it is feasible. */ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^It seems like this comment should be updated?
Maybe yes, but I don't know what these damn IO requests mean. Sure, it's better to request IRQ in xxx_open(), but as it is already done in probe() I leave it here.
If it is a bug then I do nothing with it, but if it is not then I'll create a bug.
Quoted 9 lines
drivers/net/at1700.c static int net_close(struct net_device *dev) · line 817
Quoted 6 lines
drivers/net/at1700.c +1
-3-if (lp->jumpered) { +if (lp->jumpered)outb (0x00 , ioaddr +IOCONFIG1 ); -free_irq (dev->irq, dev); - }regards,
dan carpenter
Quoted 12 lines · Kulikov Vasiliy
Quoted 4 lines
Quoted 1 line
/* Disable the IRQ on boards of fmv18x where it is feasible. */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^It seems like this comment should be updated?
Maybe yes, but I don't know what these damn IO requests mean. Sure, it's better to request IRQ in
xxx_open(), but as it is already done inprobe()I leave it here.If it is a bug then I do nothing with it, but if it is not then I'll create a bug.
Yeah. I see what you mean. You are probably right. It should be easy to test if anyone had the hardware. But this driver is 17 years old so I doubt anyone does. :P
regards,
dan carpenter
From: Kulikov Vasiliy <[email protected]>
Date: Fri, 9 Jul 2010 16:31:26 +0400
Quoted 5 lines
free_irq()is called both innet_close()andcleanup_card(). Since it is requested inat1700_probe1(), leavefree_irq()only incleanup_card()for balance.Signed-off-by Kulikov Vasiliy <[email protected]>
Applied.