[lwlan-devel] timing issues with udev and firmware loading

[Richard Kennedy]

On Mon, 2008-03-24 at 11:10 +0000, Karl Relton wrote:
> On Wed, 2008-03-12 at 22:41 +0100, Tormod Volden wrote:
> > Sorry for not following up on this thread for a while. On Debian with
> > v1854, firmware loading works at boot, also without any "sleep". After
> > an unplug/replug, the firmware is not loaded, only if I also unload
> > the modules or if I add a "sleep 2" to wlan-udev.sh. So I guess the
> > "reason" it works at boot is that the modules are not already loaded.
> > 
> > 
> On Thu, 2008-03-13 at 17:46 +0000, Richard Kennedy wrote:
> > I checked and the firmware _does_ get loaded properly at boot time, but
> > it fails on unload/reload! I guess it's just luck that udev is kind of
> > late to startup so the usb card is settled before it starts loading the
> > firmware. 
> > 
> 
> I've done some of my own tests to confirm your results. Basically
> wlan-udev.sh is being called at different points in the overall
> sequence, depending on whether it is a 'cold' or 'warm' start. I'm not
> sure why there is the difference - perhaps something in udev. If it is
> something in udev, then on a 'cold' start it is working in our favour
> 'by chance' rather by design.

Yes, the usb discovery happens really early (at least on my laptop) and
I think it's just luck that the probe routine has finished before udev
starts trying to load the firmware.


> Since the 'correct' fix is to move firmware loading into the driver,
> which is non-trivial, perhaps a work-around would be put a loop test in
> wlan-udev.sh to wait for the card to become ready.
> 
> I'll do some more tests to think on that idea.
> 
> Karl
> 

What about this patch? It adds a mutex to protect the ioctl calls and
holds it locked until the init routine has finished.

It works for me, but it's only had minimal testing. 

But it works for both the cold boot and reload cases and we don't have
to add any delays into the scripts. It should just work, and hold up any
user space processes until prism2sta_probe_usb has finished. 
It also means that you can't run 2 ioctls at the same time, which I
think is a good thing :) 

BTW -- it will only work on usb devices -- I haven't fixed up the others
to release the ioctl mutex so it will probably lock up on them!!

Richard

diff --git a/src/include/wlan/p80211netdev.h
b/src/include/wlan/p80211netdev.h
index 3e23102..b6e683d 100644
--- a/src/include/wlan/p80211netdev.h
+++ b/src/include/wlan/p80211netdev.h
@@ -257,6 +257,7 @@ typedef struct wlandevice
 
 #endif
 
+	struct mutex ioctl_lock;
 } wlandevice_t;
 
 /* WEP stuff */
@@ -281,6 +282,8 @@ void	p80211netdev_hwremoved(wlandevice_t *wlandev);
 void    p80211_suspend(wlandevice_t *wlandev);
 void    p80211_resume(wlandevice_t *wlandev);
 
+void p80211_allow_ioctls(wlandevice_t *wlandev);
+
 /*================================================================*/
 /* Function Definitions */
 
diff --git a/src/p80211/p80211mod.c b/src/p80211/p80211mod.c
index 130a0de..f356a66 100644
--- a/src/p80211/p80211mod.c
+++ b/src/p80211/p80211mod.c
@@ -216,3 +216,4 @@ EXPORT_SYMBOL(p80211skb_rxmeta_attach);
 EXPORT_SYMBOL(p80211wext_event_associated);
 
 EXPORT_SYMBOL(wlan_wext_write);
+EXPORT_SYMBOL(p80211_allow_ioctls);
diff --git a/src/p80211/p80211netdev.c b/src/p80211/p80211netdev.c
index 9c40038..78f93b0 100644
--- a/src/p80211/p80211netdev.c
+++ b/src/p80211/p80211netdev.c
@@ -702,6 +702,7 @@ static int p80211knetdev_do_ioctl(netdevice_t *dev,
struct ifreq *ifr, int cmd)
 
 	WLAN_LOG_DEBUG(2, "rx'd ioctl, cmd=%d, len=%d\n", cmd, req->len);
 
+	mutex_lock(&wlandev->ioctl_lock);
 #if WIRELESS_EXT < 13
 	/* Is this a wireless extensions ioctl? */
 	if ((cmd >= SIOCIWFIRST) && (cmd <= SIOCIWLAST)) {
@@ -751,6 +752,8 @@ static int p80211knetdev_do_ioctl(netdevice_t *dev,
struct ifreq *ifr, int cmd)
 		result = -ENOMEM;
 	}
 bail:
+	mutex_unlock(&wlandev->ioctl_lock);
+
 	DBFEXIT;
 
 	return result; /* If allocate,copyfrom or copyto fails, return errno
*/
@@ -920,6 +923,11 @@ int wlan_setup(wlandevice_t *wlandev)
 		dev->priv = wlandev;
 		dev->hard_start_xmit =	p80211knetdev_hard_start_xmit;
 		dev->get_stats =	p80211knetdev_get_stats;
+
+		mutex_init(&wlandev->ioctl_lock);
+		/* block ioctls until fully initialised. Don't forget to call
allow_ioctls at some point!*/
+		mutex_lock(&wlandev->ioctl_lock);
+		
 #ifdef HAVE_PRIVATE_IOCTL
 		dev->do_ioctl = 	p80211knetdev_do_ioctl;
 #endif
@@ -1519,3 +1527,7 @@ static void p80211knetdev_tx_timeout( netdevice_t
*netdev)
 
 	DBFEXIT;
 }
+
+void p80211_allow_ioctls(wlandevice_t *wlandev) {
+	mutex_unlock(&wlandev->ioctl_lock);
+}
diff --git a/src/prism2/driver/prism2_usb.c
b/src/prism2/driver/prism2_usb.c
index b250250..fe051ed 100644
--- a/src/prism2/driver/prism2_usb.c
+++ b/src/prism2/driver/prism2_usb.c
@@ -180,6 +180,7 @@ static int prism2sta_probe_usb(
  done:
 	DBFEXIT;
 
+	p80211_allow_ioctls(wlandev);
 #if (LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0))
 	return wlandev;
 #else