Netdev List
 help / color / mirror / Atom feed
* Question about the email about "[PATCH 2/3] IPV6: unify 3 similar code path in ndisc_recv_ns()"
From: Ren Junyuan @ 2007-11-08  7:04 UTC (permalink / raw)
  To: netdev, davem

Dear David S. Miller :

Happy to write to you.
In the email  <20040206.181006.79694138.yoshfuji@linux-ipv6.org >
<http://oss.sgi.com/archives/netdev/2004-02/msg00111.html>>,
yoshfuji unified 3  similar code patch in ndisc_recv_ns(). I have a 
question
about the "flags sent with  
NA in reply to this NS" .  In the kernel 2.4.21,
it is as follows:
 ndisc_send_na(dev, neigh, saddr,
                  &msg->target,
                      idev->cnf.forwarding, 1, 0, inc);
in you patch it is as follows:
ndisc_send_na(dev, neigh, saddr, &msg->target,
                idev->cnf.forwarding,
                1, (ifp != NULL && inc), inc);
Can you tell me why you modified the flag O(override)?
Thanks a lot!

-- 
Regards
Ren Junyuan
 --------------------------------------------------
 Ren Junyuan
 Development Dept.I
 Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
 8/F., Civil Defense Building, No.189 Guangzhou Road,
 Nanjing, 210029, China 
 TEL: +86+25-86630566-851
 COINS: 79955-851
 FAX: +86+25-83317685
 MAIL: renjy@cn.fujitsu.com



^ permalink raw reply

* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: Adrian Bunk @ 2007-11-08  3:30 UTC (permalink / raw)
  To: David Brownell, David Miller
  Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <20071107225232.GB26163@stusta.de>

On Wed, Nov 07, 2007 at 11:52:32PM +0100, Adrian Bunk wrote:
> On Wed, Nov 07, 2007 at 02:34:52PM -0800, David Brownell wrote:
> > > > But on the other hand, it seems that only the ASIX code will work
> > > > right; the DM9601 and MCS7830 Kconfig is different/wrong.
> > > 
> > > I'm not seeing the problem.
> > > 
> > > Which configuration will be handled wrongly?
> > 
> > Notice how only the ASIX kconfig depended on NET_ETHERNET...
> > since MII depends on NET_ETHERNET, and (last I knew) the
> > reverse dependencies didn't capture the complete dependency
> > tree, selecting only MII would leave out some stuff.
> 
> Except for one s390 net driver (I'll check why it's doing this) the 
> NET_ETHERNET option does not influence what code is being generated - 
> it's just a Kconfig-internal option allowing to disable a huge bunch
> of drivers at once.

Damn, I shouldn't have only grep'ed under drivers/.

@davem:

Please look at net/ipv4/arp.c:arp_process()

Am I right that CONFIG_NET_ETHERNET=n and CONFIG_NETDEV_1000=y or 
CONFIG_NETDEV_10000=y will not be handled correctly there?

And the best solution is to nuke all #ifdef's in this function and make 
the code unconditionally available?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply

* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: Adrian Bunk @ 2007-11-08  3:23 UTC (permalink / raw)
  To: David Brownell; +Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <200711071853.48460.david-b@pacbell.net>

On Wed, Nov 07, 2007 at 06:53:48PM -0800, David Brownell wrote:
> On Wednesday 07 November 2007, Adrian Bunk wrote:
> > On Wed, Nov 07, 2007 at 02:34:52PM -0800, David Brownell wrote:
> > > > > But on the other hand, it seems that only the ASIX code will work
> > > > > right; the DM9601 and MCS7830 Kconfig is different/wrong.
> > > > 
> > > > I'm not seeing the problem.
> > > > 
> > > > Which configuration will be handled wrongly?
> > > 
> > > Notice how only the ASIX kconfig depended on NET_ETHERNET...
> > > since MII depends on NET_ETHERNET, and (last I knew) the
> > > reverse dependencies didn't capture the complete dependency
> > > tree, selecting only MII would leave out some stuff.
> > 
> > Except for one s390 net driver (I'll check why it's doing this) the 
> > NET_ETHERNET option does not influence what code is being generated - 
> > it's just a Kconfig-internal option allowing to disable a huge bunch
> > of drivers at once.
> 
> Drivers like ... AX88xxx, DM9601, and MCS7830!!  Except as
> it turns out, only the first one behaves as intended.
> 
> You can tell it's a problem by the way it's inconsistent,
> regardless of the details of the problem.  :)

I'm all for cleanups that make things consistent.  :)

As long as we can agree that there's a difference between a problem like 
a compile or runtime error and an opportunity for making things 
consistent.

> - Dave

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

^ permalink raw reply

* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: David Brownell @ 2007-11-08  2:53 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <20071107225232.GB26163@stusta.de>

On Wednesday 07 November 2007, Adrian Bunk wrote:
> On Wed, Nov 07, 2007 at 02:34:52PM -0800, David Brownell wrote:
> > > > But on the other hand, it seems that only the ASIX code will work
> > > > right; the DM9601 and MCS7830 Kconfig is different/wrong.
> > > 
> > > I'm not seeing the problem.
> > > 
> > > Which configuration will be handled wrongly?
> > 
> > Notice how only the ASIX kconfig depended on NET_ETHERNET...
> > since MII depends on NET_ETHERNET, and (last I knew) the
> > reverse dependencies didn't capture the complete dependency
> > tree, selecting only MII would leave out some stuff.
> 
> Except for one s390 net driver (I'll check why it's doing this) the 
> NET_ETHERNET option does not influence what code is being generated - 
> it's just a Kconfig-internal option allowing to disable a huge bunch
> of drivers at once.

Drivers like ... AX88xxx, DM9601, and MCS7830!!  Except as
it turns out, only the first one behaves as intended.

You can tell it's a problem by the way it's inconsistent,
regardless of the details of the problem.  :)

- Dave

^ permalink raw reply

* Please pull 'fixes-davem' branch of wireless-2.6 (this time for real!)
From: John W. Linville @ 2007-11-08  2:11 UTC (permalink / raw)
  To: David Miller
  Cc: jeff-o2qLIJkoznsdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20071107.163200.25509588.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On Wed, Nov 07, 2007 at 04:32:00PM -0800, David Miller wrote:
> From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> Date: Wed, 7 Nov 2007 13:51:54 -0500
> 
> > Hold-off on this one for now if -- clearly Johannes and I need to
> > brush-up on our Kconfig skills... :-(
> > 
> > I'll post a new pull request soon.
> 
> Ok.

Dave,

These fixes are additive on top of the previous request archived here:

	http://marc.info/?l=linux-wireless&m=119439453721827&w=2

I've been sending ssb patches to you, but I'm not sure if that is the
right place to send them.  If you don't want them, can you suggest where
else I should send them?

Anyway, let me know if there are any problems!

Thanks,

John

---

The entire series (i.e. both from yesterday and today) is available
here:

	http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/fixes-davem

---

The following changes since commit d81bd04a737c102481c0ee7b46443796e14b6b5c:
  John W. Linville (1):
        mac80211: make "decrypt failed" messages conditional upon MAC80211_DEBUG

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git fixes-davem

Johannes Berg (1):
      mac80211: fix MAC80211_RCSIMPLE Kconfig

Michael Buesch (1):
      ssb: Fix PCMCIA-host lowlevel bus access

 drivers/ssb/main.c   |    1 +
 drivers/ssb/pcmcia.c |   56 +++++++++++++++++++++++++-------------------------
 net/mac80211/Kconfig |    4 +-
 3 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c
index fc1d589..85a2054 100644
--- a/drivers/ssb/main.c
+++ b/drivers/ssb/main.c
@@ -440,6 +440,7 @@ static int ssb_devices_register(struct ssb_bus *bus)
 			break;
 		case SSB_BUSTYPE_PCMCIA:
 #ifdef CONFIG_SSB_PCMCIAHOST
+			sdev->irq = bus->host_pcmcia->irq.AssignedIRQ;
 			dev->parent = &bus->host_pcmcia->dev;
 #endif
 			break;
diff --git a/drivers/ssb/pcmcia.c b/drivers/ssb/pcmcia.c
index b6abee8..bb44a76 100644
--- a/drivers/ssb/pcmcia.c
+++ b/drivers/ssb/pcmcia.c
@@ -63,17 +63,17 @@ int ssb_pcmcia_switch_coreidx(struct ssb_bus *bus,
 		err = pcmcia_access_configuration_register(pdev, &reg);
 		if (err != CS_SUCCESS)
 			goto error;
-		read_addr |= (reg.Value & 0xF) << 12;
+		read_addr |= ((u32)(reg.Value & 0x0F)) << 12;
 		reg.Offset = 0x30;
 		err = pcmcia_access_configuration_register(pdev, &reg);
 		if (err != CS_SUCCESS)
 			goto error;
-		read_addr |= reg.Value << 16;
+		read_addr |= ((u32)reg.Value) << 16;
 		reg.Offset = 0x32;
 		err = pcmcia_access_configuration_register(pdev, &reg);
 		if (err != CS_SUCCESS)
 			goto error;
-		read_addr |= reg.Value << 24;
+		read_addr |= ((u32)reg.Value) << 24;
 
 		cur_core = (read_addr - SSB_ENUM_BASE) / SSB_CORE_SIZE;
 		if (cur_core == coreidx)
@@ -152,28 +152,29 @@ error:
 	goto out_unlock;
 }
 
-/* These are the main device register access functions.
- * do_select_core is inline to have the likely hotpath inline.
- * All unlikely codepaths are out-of-line. */
-static inline int do_select_core(struct ssb_bus *bus,
-				 struct ssb_device *dev,
-				 u16 *offset)
+static int select_core_and_segment(struct ssb_device *dev,
+				   u16 *offset)
 {
+	struct ssb_bus *bus = dev->bus;
 	int err;
-	u8 need_seg = (*offset >= 0x800) ? 1 : 0;
+	u8 need_segment;
+
+	if (*offset >= 0x800) {
+		*offset -= 0x800;
+		need_segment = 1;
+	} else
+		need_segment = 0;
 
 	if (unlikely(dev != bus->mapped_device)) {
 		err = ssb_pcmcia_switch_core(bus, dev);
 		if (unlikely(err))
 			return err;
 	}
-	if (unlikely(need_seg != bus->mapped_pcmcia_seg)) {
-		err = ssb_pcmcia_switch_segment(bus, need_seg);
+	if (unlikely(need_segment != bus->mapped_pcmcia_seg)) {
+		err = ssb_pcmcia_switch_segment(bus, need_segment);
 		if (unlikely(err))
 			return err;
 	}
-	if (need_seg == 1)
-		*offset -= 0x800;
 
 	return 0;
 }
@@ -181,32 +182,31 @@ static inline int do_select_core(struct ssb_bus *bus,
 static u16 ssb_pcmcia_read16(struct ssb_device *dev, u16 offset)
 {
 	struct ssb_bus *bus = dev->bus;
-	u16 x;
 
-	if (unlikely(do_select_core(bus, dev, &offset)))
+	if (unlikely(select_core_and_segment(dev, &offset)))
 		return 0xFFFF;
-	x = readw(bus->mmio + offset);
 
-	return x;
+	return readw(bus->mmio + offset);
 }
 
 static u32 ssb_pcmcia_read32(struct ssb_device *dev, u16 offset)
 {
 	struct ssb_bus *bus = dev->bus;
-	u32 x;
+	u32 lo, hi;
 
-	if (unlikely(do_select_core(bus, dev, &offset)))
+	if (unlikely(select_core_and_segment(dev, &offset)))
 		return 0xFFFFFFFF;
-	x = readl(bus->mmio + offset);
+	lo = readw(bus->mmio + offset);
+	hi = readw(bus->mmio + offset + 2);
 
-	return x;
+	return (lo | (hi << 16));
 }
 
 static void ssb_pcmcia_write16(struct ssb_device *dev, u16 offset, u16 value)
 {
 	struct ssb_bus *bus = dev->bus;
 
-	if (unlikely(do_select_core(bus, dev, &offset)))
+	if (unlikely(select_core_and_segment(dev, &offset)))
 		return;
 	writew(value, bus->mmio + offset);
 }
@@ -215,12 +215,12 @@ static void ssb_pcmcia_write32(struct ssb_device *dev, u16 offset, u32 value)
 {
 	struct ssb_bus *bus = dev->bus;
 
-	if (unlikely(do_select_core(bus, dev, &offset)))
+	if (unlikely(select_core_and_segment(dev, &offset)))
 		return;
-	readw(bus->mmio + offset);
-	writew(value >> 16, bus->mmio + offset + 2);
-	readw(bus->mmio + offset);
-	writew(value, bus->mmio + offset);
+	writeb((value & 0xFF000000) >> 24, bus->mmio + offset + 3);
+	writeb((value & 0x00FF0000) >> 16, bus->mmio + offset + 2);
+	writeb((value & 0x0000FF00) >> 8, bus->mmio + offset + 1);
+	writeb((value & 0x000000FF) >> 0, bus->mmio + offset + 0);
 }
 
 /* Not "static", as it's used in main.c */
diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index 32c8c08..ce176e6 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -14,9 +14,9 @@ config MAC80211
 	networking stack.
 
 config MAC80211_RCSIMPLE
-	bool "'simple' rate control algorithm"
+	bool "'simple' rate control algorithm" if EMBEDDED
 	default y
-	depends on MAC80211 && EMBEDDED
+	depends on MAC80211
 	help
 	  This option allows you to turn off the 'simple' rate
 	  control algorithm in mac80211. If you do turn it off,
-- 
John W. Linville
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org

^ permalink raw reply related

* Please pull 'fixes-jgarzik' branch of wireless-2.6 (this time for real!)
From: John W. Linville @ 2007-11-08  1:34 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20071107192419.GA15784-oTNwCEtKUwI/11+TDStg7g@public.gmane.org>

On Wed, Nov 07, 2007 at 02:24:19PM -0500, Jeff Garzik wrote:
> On Wed, Nov 07, 2007 at 02:13:29PM -0500, John W. Linville wrote:
> > Jeff,
> > 
> > If you haven't already pulled this then please hold-off.  I'll post
> > a new request soon.
> 
> Haven't pulled yet...

Jeff,

These fixes are additive on top of the previous request in this thread.
That request is archived here:

	http://marc.info/?l=linux-wireless&m=119438263704232&w=2

Also, note that the first three from Michael Buesch in the list below
were already sent to you directed for 2.6.25, but I think they belong in
2.6.24 instead.  I cherry-picked them, so I'm fairly certain git will be
smart enough to drop them from your 2.6.25 branch when you rebase.

Let me know if there are any problems!

Thanks,

John

---

The entire series (i.e. both from yesterday and today) is available
here:

	http://www.kernel.org/pub//linux/kernel/people/linville/wireless-2.6/fixes-jgarzik/

---

The following changes since commit 33a463d0c82cad08a64526c217f6d835a51dfc1c:
  Michael Buesch (1):
        b43: pcmcia-host initialization bugfixes

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git fixes-jgarzik

Michael Buesch (6):
      b43: Fix rfkill callback deadlock
      b43: debugfs SHM read buffer overrun fix
      b43: Rewrite and fix rfkill init
      b43: properly request pcmcia IRQ
      b43legacy: Fix sparse warning
      b43: Fix kconfig dependencies for rfkill and leds

Stefano Brivio (4):
      b43legacy: fix possible buffer overrun in debugfs
      b43legacy: add me as maintainer and fix URLs
      b43: fix shared IRQ race condition
      b43legacy: fix shared IRQ race condition

 MAINTAINERS                              |   10 ++-
 drivers/net/wireless/b43/Kconfig         |   10 ++-
 drivers/net/wireless/b43/debugfs.c       |    2 +-
 drivers/net/wireless/b43/main.c          |   19 +++---
 drivers/net/wireless/b43/pcmcia.c        |    8 ++
 drivers/net/wireless/b43/rfkill.c        |  115 +++++++++++++----------------
 drivers/net/wireless/b43/rfkill.h        |   14 +---
 drivers/net/wireless/b43legacy/debugfs.c |    2 +-
 drivers/net/wireless/b43legacy/main.c    |   21 +++---
 9 files changed, 100 insertions(+), 101 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1c7c229..6a97027 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -787,23 +787,25 @@ B43 WIRELESS DRIVER
 P:	Michael Buesch
 M:	mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org
 P:	Stefano Brivio
-M:	st3-sGOZH3hwPm2sTnJN9+BGXg@public.gmane.org
+M:	stefano.brivio-hl5o88x/ua9eoWH0uzbU5w@public.gmane.org
 L:	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
-W:	http://bcm43xx.berlios.de/
+W:	http://linuxwireless.org/en/users/Drivers/b43
 S:	Maintained
 
 B43LEGACY WIRELESS DRIVER
 P:	Larry Finger
 M:	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org
+P:	Stefano Brivio
+M:	stefano.brivio-hl5o88x/ua9eoWH0uzbU5w@public.gmane.org
 L:	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
-W:	http://bcm43xx.berlios.de/
+W:	http://linuxwireless.org/en/users/Drivers/b43
 S:	Maintained
 
 BCM43XX WIRELESS DRIVER (SOFTMAC BASED VERSION)
 P:	Larry Finger
 M:	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org
 P:	Stefano Brivio
-M:	st3-sGOZH3hwPm2sTnJN9+BGXg@public.gmane.org
+M:	stefano.brivio-hl5o88x/ua9eoWH0uzbU5w@public.gmane.org
 L:	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 W:	http://bcm43xx.berlios.de/
 S:	Maintained
diff --git a/drivers/net/wireless/b43/Kconfig b/drivers/net/wireless/b43/Kconfig
index e3c573e..fdbc351 100644
--- a/drivers/net/wireless/b43/Kconfig
+++ b/drivers/net/wireless/b43/Kconfig
@@ -61,16 +61,18 @@ config B43_PCMCIA
 
 	  If unsure, say N.
 
-# LED support
+# This config option automatically enables b43 LEDS support,
+# if it's possible.
 config B43_LEDS
 	bool
-	depends on B43 && MAC80211_LEDS
+	depends on B43 && MAC80211_LEDS && (LEDS_CLASS = y || LEDS_CLASS = B43)
 	default y
 
-# RFKILL support
+# This config option automatically enables b43 RFKILL support,
+# if it's possible.
 config B43_RFKILL
 	bool
-	depends on B43 && RFKILL && RFKILL_INPUT && INPUT_POLLDEV
+	depends on B43 && (RFKILL = y || RFKILL = B43) && RFKILL_INPUT && (INPUT_POLLDEV = y || INPUT_POLLDEV = B43)
 	default y
 
 config B43_DEBUG
diff --git a/drivers/net/wireless/b43/debugfs.c b/drivers/net/wireless/b43/debugfs.c
index 734e70e..ef0075d 100644
--- a/drivers/net/wireless/b43/debugfs.c
+++ b/drivers/net/wireless/b43/debugfs.c
@@ -128,7 +128,7 @@ static ssize_t shm_read_file(struct b43_wldev *dev,
 	__le16 *le16buf = (__le16 *)buf;
 
 	for (i = 0; i < 0x1000; i++) {
-		if (bufsize <= 0)
+		if (bufsize < sizeof(tmp))
 			break;
 		tmp = b43_shm_read16(dev, B43_SHM_SHARED, 2 * i);
 		le16buf[i] = cpu_to_le16(tmp);
diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index 5058e60..2b17c1d 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -2985,6 +2985,16 @@ static void b43_wireless_core_stop(struct b43_wldev *dev)
 
 	if (b43_status(dev) < B43_STAT_STARTED)
 		return;
+
+	/* Disable and sync interrupts. We must do this before than
+	 * setting the status to INITIALIZED, as the interrupt handler
+	 * won't care about IRQs then. */
+	spin_lock_irqsave(&wl->irq_lock, flags);
+	dev->irq_savedstate = b43_interrupt_disable(dev, B43_IRQ_ALL);
+	b43_read32(dev, B43_MMIO_GEN_IRQ_MASK);	/* flush */
+	spin_unlock_irqrestore(&wl->irq_lock, flags);
+	b43_synchronize_irq(dev);
+
 	b43_set_status(dev, B43_STAT_INITIALIZED);
 
 	mutex_unlock(&wl->mutex);
@@ -2995,13 +3005,6 @@ static void b43_wireless_core_stop(struct b43_wldev *dev)
 
 	ieee80211_stop_queues(wl->hw);	//FIXME this could cause a deadlock, as mac80211 seems buggy.
 
-	/* Disable and sync interrupts. */
-	spin_lock_irqsave(&wl->irq_lock, flags);
-	dev->irq_savedstate = b43_interrupt_disable(dev, B43_IRQ_ALL);
-	b43_read32(dev, B43_MMIO_GEN_IRQ_MASK);	/* flush */
-	spin_unlock_irqrestore(&wl->irq_lock, flags);
-	b43_synchronize_irq(dev);
-
 	b43_mac_suspend(dev);
 	free_irq(dev->dev->irq, dev);
 	b43dbg(wl, "Wireless interface stopped\n");
@@ -3661,7 +3664,6 @@ static int b43_setup_modes(struct b43_wldev *dev,
 
 static void b43_wireless_core_detach(struct b43_wldev *dev)
 {
-	b43_rfkill_free(dev);
 	/* We release firmware that late to not be required to re-request
 	 * is all the time when we reinit the core. */
 	b43_release_firmware(dev);
@@ -3747,7 +3749,6 @@ static int b43_wireless_core_attach(struct b43_wldev *dev)
 	if (!wl->current_dev)
 		wl->current_dev = dev;
 	INIT_WORK(&dev->restart_work, b43_chip_reset);
-	b43_rfkill_alloc(dev);
 
 	b43_radio_turn_off(dev, 1);
 	b43_switch_analog(dev, 0);
diff --git a/drivers/net/wireless/b43/pcmcia.c b/drivers/net/wireless/b43/pcmcia.c
index 4b6648f..b79a6bd 100644
--- a/drivers/net/wireless/b43/pcmcia.c
+++ b/drivers/net/wireless/b43/pcmcia.c
@@ -112,6 +112,14 @@ static int __devinit b43_pcmcia_probe(struct pcmcia_device *dev)
 	if (res != CS_SUCCESS)
 		goto err_disable;
 
+	dev->irq.Attributes = IRQ_TYPE_DYNAMIC_SHARING | IRQ_FIRST_SHARED;
+	dev->irq.IRQInfo1 = IRQ_LEVEL_ID | IRQ_SHARE_ID;
+	dev->irq.Handler = NULL; /* The handler is registered later. */
+	dev->irq.Instance = NULL;
+	res = pcmcia_request_irq(dev, &dev->irq);
+	if (res != CS_SUCCESS)
+		goto err_disable;
+
 	res = pcmcia_request_configuration(dev, &dev->conf);
 	if (res != CS_SUCCESS)
 		goto err_disable;
diff --git a/drivers/net/wireless/b43/rfkill.c b/drivers/net/wireless/b43/rfkill.c
index 800e0a6..9b1f905 100644
--- a/drivers/net/wireless/b43/rfkill.c
+++ b/drivers/net/wireless/b43/rfkill.c
@@ -47,32 +47,35 @@ static void b43_rfkill_poll(struct input_polled_dev *poll_dev)
 	struct b43_wldev *dev = poll_dev->private;
 	struct b43_wl *wl = dev->wl;
 	bool enabled;
+	bool report_change = 0;
 
 	mutex_lock(&wl->mutex);
 	B43_WARN_ON(b43_status(dev) < B43_STAT_INITIALIZED);
 	enabled = b43_is_hw_radio_enabled(dev);
 	if (unlikely(enabled != dev->radio_hw_enable)) {
 		dev->radio_hw_enable = enabled;
+		report_change = 1;
 		b43info(wl, "Radio hardware status changed to %s\n",
 			enabled ? "ENABLED" : "DISABLED");
-		mutex_unlock(&wl->mutex);
+	}
+	mutex_unlock(&wl->mutex);
+
+	if (unlikely(report_change))
 		input_report_key(poll_dev->input, KEY_WLAN, enabled);
-	} else
-		mutex_unlock(&wl->mutex);
 }
 
-/* Called when the RFKILL toggled in software.
- * This is called without locking. */
+/* Called when the RFKILL toggled in software. */
 static int b43_rfkill_soft_toggle(void *data, enum rfkill_state state)
 {
 	struct b43_wldev *dev = data;
 	struct b43_wl *wl = dev->wl;
 	int err = 0;
 
-	mutex_lock(&wl->mutex);
-	if (b43_status(dev) < B43_STAT_INITIALIZED)
-		goto out_unlock;
+	if (!wl->rfkill.registered)
+		return 0;
 
+	mutex_lock(&wl->mutex);
+	B43_WARN_ON(b43_status(dev) < B43_STAT_INITIALIZED);
 	switch (state) {
 	case RFKILL_STATE_ON:
 		if (!dev->radio_hw_enable) {
@@ -89,7 +92,6 @@ static int b43_rfkill_soft_toggle(void *data, enum rfkill_state state)
 			b43_radio_turn_off(dev, 0);
 		break;
 	}
-
 out_unlock:
 	mutex_unlock(&wl->mutex);
 
@@ -98,11 +100,11 @@ out_unlock:
 
 char * b43_rfkill_led_name(struct b43_wldev *dev)
 {
-	struct b43_wl *wl = dev->wl;
+	struct b43_rfkill *rfk = &(dev->wl->rfkill);
 
-	if (!wl->rfkill.rfkill)
+	if (!rfk->registered)
 		return NULL;
-	return rfkill_get_led_name(wl->rfkill.rfkill);
+	return rfkill_get_led_name(rfk->rfkill);
 }
 
 void b43_rfkill_init(struct b43_wldev *dev)
@@ -111,53 +113,13 @@ void b43_rfkill_init(struct b43_wldev *dev)
 	struct b43_rfkill *rfk = &(wl->rfkill);
 	int err;
 
-	if (rfk->rfkill) {
-		err = rfkill_register(rfk->rfkill);
-		if (err) {
-			b43warn(wl, "Failed to register RF-kill button\n");
-			goto err_free_rfk;
-		}
-	}
-	if (rfk->poll_dev) {
-		err = input_register_polled_device(rfk->poll_dev);
-		if (err) {
-			b43warn(wl, "Failed to register RF-kill polldev\n");
-			goto err_free_polldev;
-		}
-	}
-
-	return;
-err_free_rfk:
-	rfkill_free(rfk->rfkill);
-	rfk->rfkill = NULL;
-err_free_polldev:
-	input_free_polled_device(rfk->poll_dev);
-	rfk->poll_dev = NULL;
-}
-
-void b43_rfkill_exit(struct b43_wldev *dev)
-{
-	struct b43_rfkill *rfk = &(dev->wl->rfkill);
-
-	if (rfk->poll_dev)
-		input_unregister_polled_device(rfk->poll_dev);
-	if (rfk->rfkill)
-		rfkill_unregister(rfk->rfkill);
-}
-
-void b43_rfkill_alloc(struct b43_wldev *dev)
-{
-	struct b43_wl *wl = dev->wl;
-	struct b43_rfkill *rfk = &(wl->rfkill);
+	rfk->registered = 0;
 
+	rfk->rfkill = rfkill_allocate(dev->dev->dev, RFKILL_TYPE_WLAN);
+	if (!rfk->rfkill)
+		goto out_error;
 	snprintf(rfk->name, sizeof(rfk->name),
 		 "b43-%s", wiphy_name(wl->hw->wiphy));
-
-	rfk->rfkill = rfkill_allocate(dev->dev->dev, RFKILL_TYPE_WLAN);
-	if (!rfk->rfkill) {
-		b43warn(wl, "Failed to allocate RF-kill button\n");
-		return;
-	}
 	rfk->rfkill->name = rfk->name;
 	rfk->rfkill->state = RFKILL_STATE_ON;
 	rfk->rfkill->data = dev;
@@ -165,18 +127,45 @@ void b43_rfkill_alloc(struct b43_wldev *dev)
 	rfk->rfkill->user_claim_unsupported = 1;
 
 	rfk->poll_dev = input_allocate_polled_device();
-	if (rfk->poll_dev) {
-		rfk->poll_dev->private = dev;
-		rfk->poll_dev->poll = b43_rfkill_poll;
-		rfk->poll_dev->poll_interval = 1000; /* msecs */
-	} else
-		b43warn(wl, "Failed to allocate RF-kill polldev\n");
+	if (!rfk->poll_dev)
+		goto err_free_rfk;
+	rfk->poll_dev->private = dev;
+	rfk->poll_dev->poll = b43_rfkill_poll;
+	rfk->poll_dev->poll_interval = 1000; /* msecs */
+
+	err = rfkill_register(rfk->rfkill);
+	if (err)
+		goto err_free_polldev;
+	err = input_register_polled_device(rfk->poll_dev);
+	if (err)
+		goto err_unreg_rfk;
+
+	rfk->registered = 1;
+
+	return;
+err_unreg_rfk:
+	rfkill_unregister(rfk->rfkill);
+err_free_polldev:
+	input_free_polled_device(rfk->poll_dev);
+	rfk->poll_dev = NULL;
+err_free_rfk:
+	rfkill_free(rfk->rfkill);
+	rfk->rfkill = NULL;
+out_error:
+	rfk->registered = 0;
+	b43warn(wl, "RF-kill button init failed\n");
 }
 
-void b43_rfkill_free(struct b43_wldev *dev)
+void b43_rfkill_exit(struct b43_wldev *dev)
 {
 	struct b43_rfkill *rfk = &(dev->wl->rfkill);
 
+	if (!rfk->registered)
+		return;
+	rfk->registered = 0;
+
+	input_unregister_polled_device(rfk->poll_dev);
+	rfkill_unregister(rfk->rfkill);
 	input_free_polled_device(rfk->poll_dev);
 	rfk->poll_dev = NULL;
 	rfkill_free(rfk->rfkill);
diff --git a/drivers/net/wireless/b43/rfkill.h b/drivers/net/wireless/b43/rfkill.h
index 29544e8..adacf93 100644
--- a/drivers/net/wireless/b43/rfkill.h
+++ b/drivers/net/wireless/b43/rfkill.h
@@ -15,14 +15,14 @@ struct b43_rfkill {
 	struct rfkill *rfkill;
 	/* The poll device for the RFKILL input button */
 	struct input_polled_dev *poll_dev;
+	/* Did initialization succeed? Used for freeing. */
+	bool registered;
 	/* The unique name of this rfkill switch */
-	char name[32];
+	char name[sizeof("b43-phy4294967295")];
 };
 
-/* All the init functions return void, because we are not interested
+/* The init function returns void, because we are not interested
  * in failing the b43 init process when rfkill init failed. */
-void b43_rfkill_alloc(struct b43_wldev *dev);
-void b43_rfkill_free(struct b43_wldev *dev);
 void b43_rfkill_init(struct b43_wldev *dev);
 void b43_rfkill_exit(struct b43_wldev *dev);
 
@@ -36,12 +36,6 @@ struct b43_rfkill {
 	/* empty */
 };
 
-static inline void b43_rfkill_alloc(struct b43_wldev *dev)
-{
-}
-static inline void b43_rfkill_free(struct b43_wldev *dev)
-{
-}
 static inline void b43_rfkill_init(struct b43_wldev *dev)
 {
 }
diff --git a/drivers/net/wireless/b43legacy/debugfs.c b/drivers/net/wireless/b43legacy/debugfs.c
index eefa6fb..619b453 100644
--- a/drivers/net/wireless/b43legacy/debugfs.c
+++ b/drivers/net/wireless/b43legacy/debugfs.c
@@ -124,7 +124,7 @@ static ssize_t shm_read_file(struct b43legacy_wldev *dev, char *buf, size_t bufs
 	__le16 *le16buf = (__le16 *)buf;
 
 	for (i = 0; i < 0x1000; i++) {
-		if (bufsize <= 0)
+		if (bufsize < sizeof(tmp))
 			break;
 		tmp = b43legacy_shm_read16(dev, B43legacy_SHM_SHARED, 2 * i);
 		le16buf[i] = cpu_to_le16(tmp);
diff --git a/drivers/net/wireless/b43legacy/main.c b/drivers/net/wireless/b43legacy/main.c
index f0e56df..3bde1e9 100644
--- a/drivers/net/wireless/b43legacy/main.c
+++ b/drivers/net/wireless/b43legacy/main.c
@@ -2781,6 +2781,17 @@ static void b43legacy_wireless_core_stop(struct b43legacy_wldev *dev)
 
 	if (b43legacy_status(dev) < B43legacy_STAT_STARTED)
 		return;
+
+	/* Disable and sync interrupts. We must do this before than
+	 * setting the status to INITIALIZED, as the interrupt handler
+	 * won't care about IRQs then. */
+	spin_lock_irqsave(&wl->irq_lock, flags);
+	dev->irq_savedstate = b43legacy_interrupt_disable(dev,
+							  B43legacy_IRQ_ALL);
+	b43legacy_read32(dev, B43legacy_MMIO_GEN_IRQ_MASK); /* flush */
+	spin_unlock_irqrestore(&wl->irq_lock, flags);
+	b43legacy_synchronize_irq(dev);
+
 	b43legacy_set_status(dev, B43legacy_STAT_INITIALIZED);
 
 	mutex_unlock(&wl->mutex);
@@ -2791,14 +2802,6 @@ static void b43legacy_wireless_core_stop(struct b43legacy_wldev *dev)
 
 	ieee80211_stop_queues(wl->hw); /* FIXME this could cause a deadlock */
 
-	/* Disable and sync interrupts. */
-	spin_lock_irqsave(&wl->irq_lock, flags);
-	dev->irq_savedstate = b43legacy_interrupt_disable(dev,
-							  B43legacy_IRQ_ALL);
-	b43legacy_read32(dev, B43legacy_MMIO_GEN_IRQ_MASK); /* flush */
-	spin_unlock_irqrestore(&wl->irq_lock, flags);
-	b43legacy_synchronize_irq(dev);
-
 	b43legacy_mac_suspend(dev);
 	free_irq(dev->dev->irq, dev);
 	b43legacydbg(wl, "Wireless interface stopped\n");
@@ -3332,7 +3335,7 @@ out_mutex_unlock:
 	return err;
 }
 
-void b43legacy_stop(struct ieee80211_hw *hw)
+static void b43legacy_stop(struct ieee80211_hw *hw)
 {
 	struct b43legacy_wl *wl = hw_to_b43legacy_wl(hw);
 	struct b43legacy_wldev *dev = wl->current_dev;
-- 
John W. Linville
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org

^ permalink raw reply related

* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: Adrian Bunk @ 2007-11-07 22:52 UTC (permalink / raw)
  To: David Brownell; +Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <200711071434.52607.david-b@pacbell.net>

On Wed, Nov 07, 2007 at 02:34:52PM -0800, David Brownell wrote:
> > > But on the other hand, it seems that only the ASIX code will work
> > > right; the DM9601 and MCS7830 Kconfig is different/wrong.
> > 
> > I'm not seeing the problem.
> > 
> > Which configuration will be handled wrongly?
> 
> Notice how only the ASIX kconfig depended on NET_ETHERNET...
> since MII depends on NET_ETHERNET, and (last I knew) the
> reverse dependencies didn't capture the complete dependency
> tree, selecting only MII would leave out some stuff.

Except for one s390 net driver (I'll check why it's doing this) the 
NET_ETHERNET option does not influence what code is being generated - 
it's just a Kconfig-internal option allowing to disable a huge bunch
of drivers at once.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply

* Re: [PATCH 2/2]: e1000: avoid lockup durig error recovery
From: Stephen Hemminger @ 2007-11-08  1:15 UTC (permalink / raw)
  To: Kok, Auke
  Cc: Jeff Garzik, e1000-devel, NetDev, Brandeburg,  Jesse,
	john.ronciak, jeffrey.t.kirsher, Linas Vepstas, wenxiong
In-Reply-To: <47323FFE.2060008@intel.com>

On Wed, 07 Nov 2007 14:45:18 -0800
"Kok, Auke" <auke-jan.h.kok@intel.com> wrote:

> [adding netdev, jeff G to the Cc]
> 
> Linas Vepstas wrote:
> > On Wed, Nov 07, 2007 at 01:50:17PM -0800, Kok, Auke wrote:
> >> Linas Vepstas wrote:
> >>> If a PCI bus error is encountered during device open, the
> >>> error recovery routines will attempt to close the device.
> >>> If napi has not yet been enabled, the napi disable in the
> >>> close will hang. 
> >>>
> >>> Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
> >>>
> >>> ----
> >>> The "elegence" of this solution is arguable: one could
> >>> say its "better" to perform this check in e1000_down().
> >>> However, doing so will disrupt a commonly used path,
> >>> whereas here, the hack is in the infrequently used
> >>> error path, and thus less intrusive. 
> >>>

Same problem might be possible in suspend/resume.
The problem with testing napi_enabled is that state can get changed
by racing irq.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply

* RE: [PATCH 01/05] ipv6: RFC4214 Support (2)
From: Templin, Fred L @ 2007-11-08  1:14 UTC (permalink / raw)
  To: Templin, Fred L, netdev
In-Reply-To: <39C363776A4E8C4A94691D2BD9D1C9A1029EDC04@XCH-NW-7V2.nw.nos.boeing.com>

I apologize for this, but it appears that my mailer is still
inserting line breaks where it shouldn't; please disregard.

Can anyone advise on how to configure Microsoft Office so
that the line breaks do not occur?

Thanks - Fred
fred.l.templin@boeing.com

> -----Original Message-----
> From: Templin, Fred L
> Sent: Wednesday, November 07, 2007 5:09 PM
> To: netdev@vger.kernel.org
> Subject: [PATCH 01/05] ipv6: RFC4214 Support (2)
> 
> 
> From: Fred L. Templin <fred.l.templin@boeing.com>
> 
> This is experimental support for the Intra-Site Automatic
> Tunnel Addressing Protocol (ISATAP) per RFC4214. It uses
> the SIT module, and is configured using the unmodified
> "ip" utility with device names beginning with: "isatap".
> 
> The following diffs are specific to the Linux 2.6.23
> kernel distribution.
> 
> Signed-off-by: Fred L. Templin <fred.l.templin@boeing.com>
> 
> ---
> 
> --- linux-2.6.23/include/linux/if.h.orig	2007-10-29
> 09:22:26.000000000 -0700
> +++ linux-2.6.23/include/linux/if.h	2007-11-07 13:48:08.000000000
> -0800
> @@ -61,6 +61,7 @@
>  #define IFF_MASTER_ALB	0x10		/* bonding master,
balance-alb.
> */
>  #define IFF_BONDING	0x20		/* bonding master or slave
> */
>  #define IFF_SLAVE_NEEDARP 0x40		/* need ARPs for
validation
> */
> +#define IFF_ISATAP	0x80		/* ISATAP interface (RFC4214)
> */
> 
>  #define IF_GET_IFACE	0x0001		/* for querying only */
>  #define IF_GET_PROTO	0x0002
> -
> 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

^ permalink raw reply

* Re: [PATCH 01/05] ipv6: RFC4214 Support (2)
From: Stephen Hemminger @ 2007-11-08  1:12 UTC (permalink / raw)
  To: Templin, Fred L; +Cc: netdev
In-Reply-To: <39C363776A4E8C4A94691D2BD9D1C9A1029EDC04@XCH-NW-7V2.nw.nos.boeing.com>

On Wed, 7 Nov 2007 17:08:50 -0800
"Templin, Fred L" <Fred.L.Templin@boeing.com> wrote:

> 
> From: Fred L. Templin <fred.l.templin@boeing.com>
>  
> This is experimental support for the Intra-Site Automatic
> Tunnel Addressing Protocol (ISATAP) per RFC4214. It uses
> the SIT module, and is configured using the unmodified
> "ip" utility with device names beginning with: "isatap".
> 
> The following diffs are specific to the Linux 2.6.23
> kernel distribution.

Please submit all patches against the latest kernel (2.6.24-rc2 now).
Otherwise, it gets hard to do integration.

> ---
> 
> --- linux-2.6.23/include/linux/if.h.orig	2007-10-29
> 09:22:26.000000000 -0700
> +++ linux-2.6.23/include/linux/if.h	2007-11-07 13:48:08.000000000
> -0800
> @@ -61,6 +61,7 @@
>  #define IFF_MASTER_ALB	0x10		/* bonding master, balance-alb.
> */
>  #define IFF_BONDING	0x20		/* bonding master or slave
> */
>  #define IFF_SLAVE_NEEDARP 0x40		/* need ARPs for validation
> */
> +#define IFF_ISATAP	0x80		/* ISATAP interface (RFC4214)
> */

Your mail client is wrapping the patch.
-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply

* [PATCH 01/05] ipv6: RFC4214 Support (2)
From: Templin, Fred L @ 2007-11-08  1:08 UTC (permalink / raw)
  To: netdev
In-Reply-To: <39C363776A4E8C4A94691D2BD9D1C9A1029EDBE2@XCH-NW-7V2.nw.nos.boeing.com>


From: Fred L. Templin <fred.l.templin@boeing.com>
 
This is experimental support for the Intra-Site Automatic
Tunnel Addressing Protocol (ISATAP) per RFC4214. It uses
the SIT module, and is configured using the unmodified
"ip" utility with device names beginning with: "isatap".

The following diffs are specific to the Linux 2.6.23
kernel distribution.

Signed-off-by: Fred L. Templin <fred.l.templin@boeing.com>

---

--- linux-2.6.23/include/linux/if.h.orig	2007-10-29
09:22:26.000000000 -0700
+++ linux-2.6.23/include/linux/if.h	2007-11-07 13:48:08.000000000
-0800
@@ -61,6 +61,7 @@
 #define IFF_MASTER_ALB	0x10		/* bonding master, balance-alb.
*/
 #define IFF_BONDING	0x20		/* bonding master or slave
*/
 #define IFF_SLAVE_NEEDARP 0x40		/* need ARPs for validation
*/
+#define IFF_ISATAP	0x80		/* ISATAP interface (RFC4214)
*/
 
 #define IF_GET_IFACE	0x0001		/* for querying only */
 #define IF_GET_PROTO	0x0002

^ permalink raw reply

* Re: [PATCH 13/24] [IPSEC]: Move x->outer_mode->output out of locked section
From: Herbert Xu @ 2007-11-08  0:39 UTC (permalink / raw)
  To: Ingo Oeser; +Cc: David S. Miller, netdev
In-Reply-To: <200711071717.43429.netdev@axxeo.de>

On Wed, Nov 07, 2007 at 05:17:42PM +0100, Ingo Oeser wrote:
> Hi Herbert,
> 
> Herbert Xu schrieb:
> > diff --git a/net/ipv6/xfrm6_mode_ro.c b/net/ipv6/xfrm6_mode_ro.c
> > index a7bc8c6..4a01cb3 100644
> > --- a/net/ipv6/xfrm6_mode_ro.c
> > +++ b/net/ipv6/xfrm6_mode_ro.c
> > @@ -53,7 +54,9 @@ static int xfrm6_ro_output(struct xfrm_state *x, struct sk_buff *skb)
> >  	__skb_pull(skb, hdr_len);
> >  	memmove(ipv6_hdr(skb), iph, hdr_len);
> >  
> > +	spin_lock_bh(&x->lock);
> >  	x->lastused = get_seconds();
> > +	spin_unlock_bh(&x->lock);
> >  
> >  	return 0;
> >  }
> 
> Can you move the retrieval of the seconds outside the spinlock?

You certainly could.  Whether it's worth it I won't speculate :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: Please pull 'fixes-davem' branch of wireless-2.6
From: David Miller @ 2007-11-08  0:32 UTC (permalink / raw)
  To: linville; +Cc: jeff, netdev, linux-wireless
In-Reply-To: <20071107185147.GA21097@tuxdriver.com>

From: "John W. Linville" <linville@tuxdriver.com>
Date: Wed, 7 Nov 2007 13:51:54 -0500

> Hold-off on this one for now if -- clearly Johannes and I need to
> brush-up on our Kconfig skills... :-(
> 
> I'll post a new pull request soon.

Ok.

^ permalink raw reply

* Re: [PATCH] NET: Remove unneeded type cast in skb_truesize_check()
From: David Miller @ 2007-11-08  0:15 UTC (permalink / raw)
  To: chuck.lever; +Cc: netdev
In-Reply-To: <4731D5A0.2090101@oracle.com>

From: Chuck Lever <chuck.lever@oracle.com>
Date: Wed, 07 Nov 2007 10:11:28 -0500

> The (int) type cast in that assertion is nothing more than a comment. 
> The compiler promotes that side of the comparison to match the type on 
> the right side, and the explicit type cast is entirely ignored.  So in 
> fact, my patch removed nothing but a mistaken assumption.  The assertion 
> behaves the same after my patch as it did before.

Then please post a patch that makes the assertion work
instead of removing it, by casting both branches of the
comparison to be signed.

^ permalink raw reply

* Re: [PATCH 2/2]: e1000: avoid lockup durig error recovery
From: Linas Vepstas @ 2007-11-07 23:21 UTC (permalink / raw)
  To: Kok, Auke
  Cc: Jeff Garzik, e1000-devel, NetDev, Brandeburg,  Jesse,
	john.ronciak, jeffrey.t.kirsher, 'Stephen Hemminger',
	wenxiong
In-Reply-To: <47323FFE.2060008@intel.com>

On Wed, Nov 07, 2007 at 02:45:18PM -0800, Kok, Auke wrote:
> [adding netdev, jeff G to the Cc]
> 
> Linas Vepstas wrote:
> > On Wed, Nov 07, 2007 at 01:50:17PM -0800, Kok, Auke wrote:
> >> Linas Vepstas wrote:
> >>> If a PCI bus error is encountered during device open, the
> >>> error recovery routines will attempt to close the device.
> >>> If napi has not yet been enabled, the napi disable in the
> >>> close will hang. 
> >>>
> >>> Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
> >>>
> >>> ----
> >>> The "elegence" of this solution is arguable: one could
> >>> say its "better" to perform this check in e1000_down().
> >>> However, doing so will disrupt a commonly used path,
> >>> whereas here, the hack is in the infrequently used
> >>> error path, and thus less intrusive. 
> >>>
> >>>  drivers/net/e1000/e1000_main.c |    9 ++++++++-
> >>>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>>
> >> I think this is OK, but it's quite awful looking if you ask me.
> > 
> > Yeah, ... 
> > 
> > There are several alternatives: below are two. If you
> > find one to be more appealing.. could you use it? Consider them
> > to be "signed-off-by"; I have not actually compiled or tested
> > either of them.
> 
> I'm not a particular fan of putting extra state tracking in the driver for
> something we could extract from the napi subsystem already.
> 
> Jeff, Stephen, can't we have a generic napi_enabled() inline in netdevice.h that
> tests for NAPI_STATE_SCHED ?

Like this?

 include/linux/netdevice.h |   12 ++++++++++++
 1 file changed, 12 insertions(+)

Index: linux-2.6.23-rc8-mm1/include/linux/netdevice.h
===================================================================
--- linux-2.6.23-rc8-mm1.orig/include/linux/netdevice.h	2007-09-26 15:07:05.000000000 -0500
+++ linux-2.6.23-rc8-mm1/include/linux/netdevice.h	2007-11-07 17:14:50.000000000 -0600
@@ -384,6 +384,18 @@ static inline void napi_enable(struct na
 	clear_bit(NAPI_STATE_SCHED, &n->state);
 }
 
+/**
+ *	napi_enabled_p - return non-zero if napi enabled
+ *	@n: napi context
+ * 
+ * Mnemonic: _p stands for "predicate", returning a yes/no
+ * answer to the question.
+ */
+static inline int napi_enabled_p(struct napi_struct *n)
+{
+	return !test_bit(NAPI_STATE_SCHED, &n->state);
+}
+
 /*
  *	The DEVICE structure.
  *	Actually, this whole structure is a big mistake.  It mixes I/O


> I wonder if there isn't something in the PCI error recovery missing the point and
> we can solve this problem better for all drivers somehow.

Well, there's also scsi, which doesn't use napi :-)
For the most part, error recovery is a fairly cut-n-paste
set of steps. However, I don't quite have enough confidence
to say "yea verily, all network adapters will use these 
same steps."

--linas

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

^ permalink raw reply

* Re: net: skge breakage on 2.6.24-rc1
From: Heikki Orsila @ 2007-11-07 23:13 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Rafael J. Wysocki, linux-kernel, Stephen Hemminger, netdev
In-Reply-To: <20071107150621.f2da7f69.akpm@linux-foundation.org>

On Wed, Nov 07, 2007 at 03:06:21PM -0800, Andrew Morton wrote:
> > On Wed, 7 Nov 2007 23:50:30 +0100 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > On Wednesday, 7 of November 2007, Heikki Orsila wrote:
> > > On Wed, Nov 07, 2007 at 11:46:21PM +0200, Heikki Orsila wrote:
> > > > After some bisecting, I found that net skge driver broke on
> > > > 
> > > > commit 7fb7ac241162dc51ec0f7644d4a97b2855213c32
> 
> 
> Thanks for doing the bisection.  It's a good idea to cc the author of the
> offending patch after having done this.

Sorry, I just forgot this time. Just after the bug report I did mail 
the author in private (out of the list).

-- 
Heikki Orsila			Barbie's law:
heikki.orsila@iki.fi		"Math is hard, let's go shopping!"
http://www.iki.fi/shd

^ permalink raw reply

* Re: net: skge breakage on 2.6.24-rc1
From: Andrew Morton @ 2007-11-07 23:06 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: shdl, linux-kernel, Stephen Hemminger, netdev
In-Reply-To: <200711072350.30645.rjw@sisk.pl>

> On Wed, 7 Nov 2007 23:50:30 +0100 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> On Wednesday, 7 of November 2007, Heikki Orsila wrote:
> > On Wed, Nov 07, 2007 at 11:46:21PM +0200, Heikki Orsila wrote:
> > > After some bisecting, I found that net skge driver broke on
> > > 
> > > commit 7fb7ac241162dc51ec0f7644d4a97b2855213c32


Thanks for doing the bisection.  It's a good idea to cc the author of the
offending patch after having done this.

> > > My network card is:
> > > 
> > > 0000:00:0e.0 Ethernet controller: 3Com Corporation 3c940 10/100/1000Base-T [Marvell] (rev 12)
> > > 
> > > Linux cheradenine 2.6.24-rc1-dirty #15 Wed Nov 7 23:39:28 EET 2007 x86_64 GNU/Linux
> > 
> > Sorry, forgot to say what the problem is. Doing
> > 
> > 	nc host port < /dev/zero
> > 
> > on a sending machine (not skge) to an skge machine that is receiving:
> > 
> > 	nc -l -p port >/dev/null
> > 
> > with ~60 MiB/s speed, causes the interface go malfunct. A slow 
> > transfer doesn't cause a problem.
> > 
> > Also, after some fiddling, I noticed that not changing the register 
> > write order on patch:
> > 
> > +       skge_write32(hw, RB_ADDR(q, RB_END), end);
> >         skge_write32(hw, RB_ADDR(q, RB_WP), start);
> >         skge_write32(hw, RB_ADDR(q, RB_RP), start);
> > -       skge_write32(hw, RB_ADDR(q, RB_END), end);
> > 
> > fixes the visible effect.. Possibly not the root cause of the problem, 
> > but changing the order back fixes networking here.
> 
> Please put all of the relevant information into the bugzilla entry at
> http://bugzilla.kernel.org/show_bug.cgi?id=9321

Please cc netdev@vger.kernel.org on networking-related discussions.

^ permalink raw reply

* Re: [PATCH 2/2]: e1000: avoid lockup durig error recovery
From: Kok, Auke @ 2007-11-07 22:45 UTC (permalink / raw)
  To: Linas Vepstas
  Cc: wenxiong, e1000-devel, Brandeburg, Jesse, john.ronciak,
	jeffrey.t.kirsher, Jeff Garzik, NetDev,
	'Stephen Hemminger'
In-Reply-To: <20071107222446.GM4239@austin.ibm.com>

[adding netdev, jeff G to the Cc]

Linas Vepstas wrote:
> On Wed, Nov 07, 2007 at 01:50:17PM -0800, Kok, Auke wrote:
>> Linas Vepstas wrote:
>>> If a PCI bus error is encountered during device open, the
>>> error recovery routines will attempt to close the device.
>>> If napi has not yet been enabled, the napi disable in the
>>> close will hang. 
>>>
>>> Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
>>>
>>> ----
>>> The "elegence" of this solution is arguable: one could
>>> say its "better" to perform this check in e1000_down().
>>> However, doing so will disrupt a commonly used path,
>>> whereas here, the hack is in the infrequently used
>>> error path, and thus less intrusive. 
>>>
>>>  drivers/net/e1000/e1000_main.c |    9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> Index: linux-2.6.23-rc8-mm1/drivers/net/e1000/e1000_main.c
>>> ===================================================================
>>> --- linux-2.6.23-rc8-mm1.orig/drivers/net/e1000/e1000_main.c	2007-11-07 15:04:45.000000000 -0600
>>> +++ linux-2.6.23-rc8-mm1/drivers/net/e1000/e1000_main.c	2007-11-07 15:04:52.000000000 -0600
>>> @@ -5268,8 +5268,15 @@ static pci_ers_result_t e1000_io_error_d
>>>  
>>>  	netif_device_detach(netdev);
>>>  
>>> -	if (netif_running(netdev))
>>> +	if (netif_running(netdev)) {
>>> +#ifdef CONFIG_E1000_NAPI
>>> +		/* e1000_down will spinlock in napi_disable() if we
>>> +		 * catch an error before napi_enable() was called. */
>>> +		if (test_bit(NAPI_STATE_SCHED, &adapter->napi.state))
>>> +			napi_enable(&adapter->napi);
>>> +#endif
>>>  		e1000_down(adapter);
>>> +	}
>>>  	pci_disable_device(pdev);
>>>  
>>>  	/* Request a slot slot reset. */
>> I think this is OK, but it's quite awful looking if you ask me.
> 
> Yeah, ... 
> 
> There are several alternatives: below are two. If you
> find one to be more appealing.. could you use it? Consider them
> to be "signed-off-by"; I have not actually compiled or tested
> either of them.

I'm not a particular fan of putting extra state tracking in the driver for
something we could extract from the napi subsystem already.

Jeff, Stephen, can't we have a generic napi_enabled() inline in netdevice.h that
tests for NAPI_STATE_SCHED ?

I wonder if there isn't something in the PCI error recovery missing the point and
we can solve this problem better for all drivers somehow.

Auke



> 
> 
>  drivers/net/e1000/e1000_main.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6.23-rc8-mm1/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.23-rc8-mm1.orig/drivers/net/e1000/e1000_main.c	2007-11-07 16:11:36.000000000 -0600
> +++ linux-2.6.23-rc8-mm1/drivers/net/e1000/e1000_main.c	2007-11-07 16:15:17.000000000 -0600
> @@ -633,7 +633,11 @@ e1000_down(struct e1000_adapter *adapter
>  	set_bit(__E1000_DOWN, &adapter->flags);
>  
>  #ifdef CONFIG_E1000_NAPI
> -	napi_disable(&adapter->napi);
> +	/* napi_disable() will spinlock if we are in the
> +	 * pci error recovery path, and caught a pci
> +	 * error before napi_enable() was called. */
> +	if (!test_bit(NAPI_STATE_SCHED, &adapter->napi.state))
> +		napi_disable(&adapter->napi);
>  #endif
>  	e1000_irq_disable(adapter);
>  
> and here's another:
> 
>  drivers/net/e1000/e1000.h      |    1 +
>  drivers/net/e1000/e1000_main.c |    7 ++++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6.23-rc8-mm1/drivers/net/e1000/e1000.h
> ===================================================================
> --- linux-2.6.23-rc8-mm1.orig/drivers/net/e1000/e1000.h	2007-09-26 15:06:56.000000000 -0500
> +++ linux-2.6.23-rc8-mm1/drivers/net/e1000/e1000.h	2007-11-07 16:19:16.000000000 -0600
> @@ -301,6 +301,7 @@ struct e1000_adapter {
>  	struct e1000_rx_ring *rx_ring;      /* One per active queue */
>  #ifdef CONFIG_E1000_NAPI
>  	struct napi_struct napi;
> +	int napi_enabled;
>  	struct net_device *polling_netdev;  /* One per active queue */
>  #endif
>  	int num_tx_queues;
> Index: linux-2.6.23-rc8-mm1/drivers/net/e1000/e1000_main.c
> ===================================================================
> --- linux-2.6.23-rc8-mm1.orig/drivers/net/e1000/e1000_main.c	2007-11-07 16:16:11.000000000 -0600
> +++ linux-2.6.23-rc8-mm1/drivers/net/e1000/e1000_main.c	2007-11-07 16:22:30.000000000 -0600
> @@ -545,6 +545,7 @@ int e1000_up(struct e1000_adapter *adapt
>  
>  #ifdef CONFIG_E1000_NAPI
>  	napi_enable(&adapter->napi);
> +	adapter->napi_enabled = 1;
>  #endif
>  	e1000_irq_enable(adapter);
>  
> @@ -633,7 +634,10 @@ e1000_down(struct e1000_adapter *adapter
>  	set_bit(__E1000_DOWN, &adapter->flags);
>  
>  #ifdef CONFIG_E1000_NAPI
> -	napi_disable(&adapter->napi);
> +	if (adapter->napi_enabled) {
> +		napi_disable(&adapter->napi);
> +		adapter->napi_enabled = 0;
> +	}
>  #endif
>  	e1000_irq_disable(adapter);
>  
> @@ -1437,6 +1441,7 @@ e1000_open(struct net_device *netdev)
>  
>  #ifdef CONFIG_E1000_NAPI
>  	napi_enable(&adapter->napi);
> +	adapter->napi_enabled = 1;
>  #endif
>  
>  	e1000_irq_enable(adapter);

^ permalink raw reply

* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: David Brownell @ 2007-11-07 22:34 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <20071102195517.GW30287@stusta.de>

> > But on the other hand, it seems that only the ASIX code will work
> > right; the DM9601 and MCS7830 Kconfig is different/wrong.
> 
> I'm not seeing the problem.
> 
> Which configuration will be handled wrongly?

Notice how only the ASIX kconfig depended on NET_ETHERNET...
since MII depends on NET_ETHERNET, and (last I knew) the
reverse dependencies didn't capture the complete dependency
tree, selecting only MII would leave out some stuff.



^ permalink raw reply

* Re: [2.6 patch] usbnet.c: check for the right MII variable
From: David Brownell @ 2007-11-07 22:31 UTC (permalink / raw)
  To: David Miller
  Cc: Adrian Bunk, Toralf Förster, jgarzik, linux-kernel, netdev
In-Reply-To: <20071102154615.GS30287@stusta.de>

On Friday 02 November 2007, Adrian Bunk wrote:
> <--  snip  -->
> 
> 
> This patch fixes the following compile error with CONFIG_MII=m, 
> CONFIG_USB_USBNET=y, CONFIG_USB_USBNET_MII=n:

This is the patch I liked better, if there was going to be
one going upstream without additional test from me ... sorry,
the patch you appear to have accepted was the worst of the
three patches in flight.

> 
> <--  snip  -->
> 
> ...
>   LD      .tmp_vmlinux1
> drivers/built-in.o: In function `usbnet_set_settings':
> (.text+0xf1876): undefined reference to `mii_ethtool_sset'
> drivers/built-in.o: In function `usbnet_get_settings':
> (.text+0xf1836): undefined reference to `mii_ethtool_gset'
> drivers/built-in.o: In function `usbnet_get_link':
> (.text+0xf18d6): undefined reference to `mii_link_ok'
> drivers/built-in.o: In function `usbnet_nway_reset':
> (.text+0xf18f6): undefined reference to `mii_nway_restart'
> make: *** [.tmp_vmlinux1] Error 1
> 
> <--  snip  -->
> 
> This bug was introduced by commit 18ee91fa9815fa3bb4e51cdcb8229bd0a0f11a70
> and reported by Toralf Förster.
> 
> Signed-off-by: Adrian Bunk <bunk@kernel.org>
> 
> ---
> 
> BTW:
> The Kconfig part of this patch is not really required, but testing for
>   #if defined(CONFIG_USB_USBNET_MII) || defined(CONFIG_USB_USBNET_MII_MODULE)
> would look needlessly ugly.
> 
>  drivers/net/usb/Kconfig  |    5 ++---
>  drivers/net/usb/usbnet.c |    7 +++----
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> a421e4910eb30b140a315e274632e87c7a218df6 
> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
> index 5a96d74..9261371 100644
> --- a/drivers/net/usb/Kconfig
> +++ b/drivers/net/usb/Kconfig
> @@ -94,12 +94,11 @@ config USB_RTL8150
>  	  module will be called rtl8150.
>  
>  config USB_USBNET_MII
> -	tristate
> -	default n
> +	bool
>  
>  config USB_USBNET
>  	tristate "Multi-purpose USB Networking Framework"
> -	select MII if USB_USBNET_MII != n
> +	select MII if USB_USBNET_MII
>  	---help---
>  	  This driver supports several kinds of network links over USB,
>  	  with "minidrivers" built around a common network driver core
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index acd5f1c..7393ab0 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -683,8 +683,7 @@ done_nopm:
>   * they'll probably want to use this base set.
>   */
>  
> -#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
> -#define HAVE_MII
> +#ifdef CONFIG_USB_USBNET_MII
>  
>  int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
>  {
> @@ -744,7 +743,7 @@ int usbnet_nway_reset(struct net_device *net)
>  }
>  EXPORT_SYMBOL_GPL(usbnet_nway_reset);
>  
> -#endif	/* HAVE_MII */
> +#endif	/*  CONFIG_USB_USBNET_MII  */
>  
>  void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
>  {
> @@ -776,7 +775,7 @@ EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
>  
>  /* drivers may override default ethtool_ops in their bind() routine */
>  static struct ethtool_ops usbnet_ethtool_ops = {
> -#ifdef	HAVE_MII
> +#ifdef CONFIG_USB_USBNET_MII
>  	.get_settings		= usbnet_get_settings,
>  	.set_settings		= usbnet_set_settings,
>  	.get_link		= usbnet_get_link,
> 



^ permalink raw reply

* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: David Brownell @ 2007-11-07 22:15 UTC (permalink / raw)
  To: David Miller; +Cc: randy.dunlap, toralf.foerster, linux-kernel, netdev, greg
In-Reply-To: <20071107.002007.00442348.davem@davemloft.net>

On Wednesday 07 November 2007, David Miller wrote:
> David, I hate to say this and point you out like this, but you are a
> real cancer for bug fixes to USB things in the kernel,

You didn't hate it enough to find a way to deal with your issue
that doesn't involve namecalling or other flamage, I'll note.

I know, I know ... you wouldn't want anyone to get the mistaken
impression that LKML is one of those easy-to-get-along-on mailing
lists.  Namecalling helps prevent anyone from ever thinking that!


> If I had a nickel for every patch from someone else you grinded into
> the ground and stalled I'd truly be a millionare.

Hyperbole helps too...

Twenty million patches, surely from some large fraction of a
million kernel patch submitters ... what a crazy dream!
(An enviable one, maybe; but way below the last statistics
on the subject which I read.)

As for "grind into the ground and stall" ... clearly that's
not how I'd describe things.


> You absolutely stifle development progress.  I thought my OHCI
> deadlock patch was an isolated case (and nothing is still applied,
> which is just awesome, my original patch was posted more than a month
> ago),

I'm not sure why the patch I signed off on didn't merge either;
that was specificially related to the OHCI part of that problem.

Hmm, digging through my mail I see several other patches doing
the same thing were also floating around.  None of them had any
improvement over what I had signed off on.  Maybe there was just
too much confusion for Greg to want to merge the one which I'd
already signed off on...


It hasn't been removed from my merge queue, since it's not yet
shown up from kernel.org ... which means that it'd be one of
the patches to be resubmitted before we get much deeper into
this particular RC series.  In fact, I just resent it (with a
few minor tweaks, essentially comment updates).


Of course, *YOU* are the roadblock on the more generic side of
that problem.  I don't recall hearing back from you about the
minor/obvious update to the HUB+EHCI patch adding a msleep()
to bypass the hardware race you reported.

That is, other than your refusal to even try letting the three
or more clock domains finish synchronizing, before releasing
that lock and then starting something that relied on that synch
having finished...


>       but you're doing the same exact thing to Adrian here too.

Hmm, and there I was prepared to sign off on Adrian's last patch.
True, I hadn't yet done so.  But there were several workable fixes
in hand, plus a trivial workaround ... I just didn't make time to
try that one out over the weekend.

I'm glad you grabbed the patch I would have signed off on, if I'd
had time to verify it.


> You want to see things fixed your way.  But you can get away with the
> if, and only if, you can spend every day working on your own version
> of fixes when you don't like the submitters version.

You know, that's hardly a standard kernel-wide policy for how
maintainers work ... except maybe ones in "supported" areas,
and not always even then.

Though I certainly know why it could seem attactive to want that
treatment for patches you submit. I know many of us would just love
to get that level of response/support for everything we submit.
I don't always get it; in some areas, it *never* happens.

In fact, it's pretty routine to have patches wait for a while
before they get merged, or even sometimes reviewed ... where
"a while" will sometimes cover many (annoying) months.


Also, some maintainers demand many iterations of patches, without
doing *any* fixup themselves.  It's as if ... hmm ... maybe there
were only so many hours per day going into that such stuff?  Or as
if people had different priorities?  Bizarre notions, I know!!


>       But unlike me
> you don't have that luxury so you have to give patch submitters a
> larger level of freedom and, plainly, just "let go".

Same goes for maintainers too, you know.  If you're still
wondering about a patch, a private "ping" is common practice;
far more so than public vilification.

In fact, it's best if you never use a public vilification step.
Certainly not until after conventional measures ("ping!") fail
repeatedly, and bad faith has been clearly shown.  (Neither of
which apply in this case.)



^ permalink raw reply

* Re: [PATCH] PCI: export pci_restore_msi_state()
From: Linas Vepstas @ 2007-11-07 22:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: linux-pci, netdev, mcarlson, wenxiong, mchan, Kok, Auke
In-Reply-To: <20071107214359.GJ4239@austin.ibm.com>

Hi,

On Wed, Nov 07, 2007 at 03:43:59PM -0600, Linas Vepstas wrote:
> 
> PCI error recovery usually involves the PCI adapter being reset.
> If the device is using MSI, the reset will cause the MSI state 
> to be lost; the device driver needs to restore the MSI state.
> 
> The pci_restore_msi_state() routine is currently protected
> by CONFIG_PM; remove this, and also export the symbol, so
> that it can be used in a modle.
> 
> Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

The long time delay has managed to muddle notes & recollection 
of prior discussions. This patch should have had a 

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>

on it; its the same patch that was submitted a long time ago.

During the discussions of 21 Oct, it was proposed that there
should be an arch hook for pci_restore_msi_state(), so that
it would be treated at the same level as msi setup and teardown.
I'd volunteered to write that patch. 

When I sat down to do it, however, I realized that I did not
actually *need* it. And so I wondered: why am I writing un-needed,
but theoretically proper, code? So I punted, and I didn't.
Does that make sense?

That's also why this patch is just a resubmission of the old
patch. The original thread is here:

http://www.mail-archive.com/netdev@vger.kernel.org/msg51296.html

--linas




^ permalink raw reply

* [PATCH 2/2] qla3xxx: bugfix: Fix bad logical operation in link state machine.
From: Ron Mercer @ 2007-11-07 21:59 UTC (permalink / raw)
  To: jeff; +Cc: netdev, Ron Mercer
In-Reply-To: <20071107215608.GB7090@linux-7mw0.qlogic.org>

Luckily, this wasn't reported or reproduced. The logical operation for
setting duplex had wrong grouping.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qla3xxx.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/qla3xxx.c b/drivers/net/qla3xxx.c
index 4f0fd41..a579111 100644
--- a/drivers/net/qla3xxx.c
+++ b/drivers/net/qla3xxx.c
@@ -1456,16 +1456,11 @@ static void ql_phy_start_neg_ex(struct ql3_adapter *qdev)
 			   PHYAddr[qdev->mac_index]);
 	reg &= ~PHY_GIG_ALL_PARAMS;
 
-	if(portConfiguration &
-	   PORT_CONFIG_FULL_DUPLEX_ENABLED &
-	   PORT_CONFIG_1000MB_SPEED) {
-		reg |= PHY_GIG_ADV_1000F;
-	}
-
-	if(portConfiguration &
-	   PORT_CONFIG_HALF_DUPLEX_ENABLED &
-	   PORT_CONFIG_1000MB_SPEED) {
-		reg |= PHY_GIG_ADV_1000H;
+	if(portConfiguration & PORT_CONFIG_1000MB_SPEED) {
+		if(portConfiguration & PORT_CONFIG_FULL_DUPLEX_ENABLED) 
+			reg |= PHY_GIG_ADV_1000F;
+		else 
+			reg |= PHY_GIG_ADV_1000H;
 	}
 
 	ql_mii_write_reg_ex(qdev, PHY_GIG_CONTROL, reg,
-- 
1.5.0.rc4.16.g9e258


^ permalink raw reply related

* [PATCH 1/2] qla3xxx:  bugfix: Move link state machine into a worker thread
From: Ron Mercer @ 2007-11-07 21:59 UTC (permalink / raw)
  To: jeff; +Cc: netdev, Ron Mercer
In-Reply-To: <20071107215608.GB7090@linux-7mw0.qlogic.org>

The link state machine requires access to some resources that
are shared with the iSCSI function on the chip.  (See iSCSI
driver at drivers/scsi/qla4xxx)  If the interface is being
up/downed at a rapid pace this driver may need to sleep
waiting to get access to the common resources. For this we
are moving the state machine to run as a work thread.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qla3xxx.c |   27 +++++++++++++--------------
 drivers/net/qla3xxx.h |    1 +
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/qla3xxx.c b/drivers/net/qla3xxx.c
index 30adf72..4f0fd41 100644
--- a/drivers/net/qla3xxx.c
+++ b/drivers/net/qla3xxx.c
@@ -1645,8 +1645,11 @@ static int ql_finish_auto_neg(struct ql3_adapter *qdev)
 	return 0;
 }
 
-static void ql_link_state_machine(struct ql3_adapter *qdev)
+static void ql_link_state_machine_work(struct work_struct *work)
 {
+	struct ql3_adapter *qdev =
+		container_of(work, struct ql3_adapter, link_state_work.work);
+
 	u32 curr_link_state;
 	unsigned long hw_flags;
 
@@ -1661,6 +1664,10 @@ static void ql_link_state_machine(struct ql3_adapter *qdev)
 			       "state.\n", qdev->ndev->name);
 
 		spin_unlock_irqrestore(&qdev->hw_lock, hw_flags);
+
+		/* Restart timer on 2 second interval. */
+		mod_timer(&qdev->adapter_timer, jiffies + HZ * 1);\
+
 		return;
 	}
 
@@ -1705,6 +1712,9 @@ static void ql_link_state_machine(struct ql3_adapter *qdev)
 		break;
 	}
 	spin_unlock_irqrestore(&qdev->hw_lock, hw_flags);
+
+	/* Restart timer on 2 second interval. */
+	mod_timer(&qdev->adapter_timer, jiffies + HZ * 1);
 }
 
 /*
@@ -3941,19 +3951,7 @@ static void ql_get_board_info(struct ql3_adapter *qdev)
 static void ql3xxx_timer(unsigned long ptr)
 {
 	struct ql3_adapter *qdev = (struct ql3_adapter *)ptr;
-
-	if (test_bit(QL_RESET_ACTIVE,&qdev->flags)) {
-		printk(KERN_DEBUG PFX
-		       "%s: Reset in progress.\n",
-		       qdev->ndev->name);
-		goto end;
-	}
-
-	ql_link_state_machine(qdev);
-
-	/* Restart timer on 2 second interval. */
-end:
-	mod_timer(&qdev->adapter_timer, jiffies + HZ * 1);
+	queue_delayed_work(qdev->workqueue, &qdev->link_state_work, 0);
 }
 
 static int __devinit ql3xxx_probe(struct pci_dev *pdev,
@@ -4103,6 +4101,7 @@ static int __devinit ql3xxx_probe(struct pci_dev *pdev,
 	qdev->workqueue = create_singlethread_workqueue(ndev->name);
 	INIT_DELAYED_WORK(&qdev->reset_work, ql_reset_work);
 	INIT_DELAYED_WORK(&qdev->tx_timeout_work, ql_tx_timeout_work);
+	INIT_DELAYED_WORK(&qdev->link_state_work, ql_link_state_machine_work);
 
 	init_timer(&qdev->adapter_timer);
 	qdev->adapter_timer.function = ql3xxx_timer;
diff --git a/drivers/net/qla3xxx.h b/drivers/net/qla3xxx.h
index fbcb0b9..d0ffb30 100644
--- a/drivers/net/qla3xxx.h
+++ b/drivers/net/qla3xxx.h
@@ -1286,6 +1286,7 @@ struct ql3_adapter {
 	struct workqueue_struct *workqueue;
 	struct delayed_work reset_work;
 	struct delayed_work tx_timeout_work;
+	struct delayed_work link_state_work;
 	u32 max_frame_size;
 	u32 device_id;
 	u16 phyType;
-- 
1.5.0.rc4.16.g9e258


^ permalink raw reply related

* [PATCH 0/2] qla3xxx: bugfixes to link management.
From: Ron Mercer @ 2007-11-07 21:56 UTC (permalink / raw)
  To: jeff; +Cc: netdev

Hi Jeff,

The attached two patches fix a couple of problems in the link management.
Built and tested on today's netdev/upstream.
Regards,
Ron Mercer

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox