Discussion:
Loss of p80211_allow_ioctls in Linux drivers/staging/wlan-ng
Karl Relton
2009-02-09 21:00:02 UTC
Permalink
Greg/Solomon

The version of prism2_usb in the Linux kernel 2.6.28 and onwards (in
drivers/staging/wlan-ng) does not seem to have the call to
p80211_allow_ioctls.

This function was added to the linux-wlan-ng version of the driver back
in March 2008 (see
http://lists.linux-wlan.com/pipermail/linux-wlan-devel/2008-March/003819.html) and was needed to prevent a race between scripts launched by udev (as a result of the SET_NETDEV_DEV call) and the card being properly declared ready (by the prism2sta_ifstate call at line 141 of the new prism2usb.c).

Was this deliberately taken out? I would imagine we still need it, since
I would still expect to do some work in userland triggered off udev as
the card is coming up.

Regards
Karl
Richard Kennedy
2009-02-17 12:05:49 UTC
Permalink
Post by Karl Relton
Greg/Solomon
The version of prism2_usb in the Linux kernel 2.6.28 and onwards (in
drivers/staging/wlan-ng) does not seem to have the call to
p80211_allow_ioctls.
This function was added to the linux-wlan-ng version of the driver back
in March 2008 (see
http://lists.linux-wlan.com/pipermail/linux-wlan-devel/2008-March/003819.html) and was needed to prevent a race between scripts launched by udev (as a result of the SET_NETDEV_DEV call) and the card being properly declared ready (by the prism2sta_ifstate call at line 141 of the new prism2usb.c).
Was this deliberately taken out? I would imagine we still need it, since
I would still expect to do some work in userland triggered off udev as
the card is coming up.
Regards
Karl
Hi Karl,

I think this just didn't get ported across to the kernel version as the
staging driver was based on a older code drop. I've forward ported this
patch to the current git kernel 2.6.29-rc5. It passes basic testing in
that I can connect to the network :)

So far I haven't tried to load the firmware using the staging driver, as
I'm not sure what to do about the user space utility prism2_dl. I would
prefer to get the driver to do the work preparing the firmware & use the
kernel firmware loading function.

But I'm not sure what the kernel developers would say to adding all that
code into the driver.
Any thoughts to which direction you prefer?

I'll post the patch in a separate email.

regards
Richard
Karl Relton
2009-02-18 21:26:20 UTC
Permalink
Post by Richard Kennedy
So far I haven't tried to load the firmware using the staging driver, as
I'm not sure what to do about the user space utility prism2_dl. I would
prefer to get the driver to do the work preparing the firmware & use the
kernel firmware loading function.
But I'm not sure what the kernel developers would say to adding all that
code into the driver.
Any thoughts to which direction you prefer?
Regarding firmware loading, presumably for the moment prism2_dl
userspace program still works?

For the future, my vote would be use the kernel firmware loading
function to load prism2_ru.hex (from the standard kernel location for
firmware) so that a default install would no longer need prism2dl.

I'm not sure it is as much code as we might think. Since this staging
driver is just for USB, the stuff in the userspace shared.prism2 script
actually just boils down to 'prism2dl -r' (all the rest being
redundant). We would need to move code from prism2dl into the kernel,
but only the bits that support the '-r' function.
Post by Richard Kennedy
From my recollection, when we last discussed the prospect of
kernel-mechanism firmware loading it was in the more generic case
covering any card, which Solomon pointed out would be much harder.

Karl
Greg KH
2009-02-18 02:21:17 UTC
Permalink
Add a mutex to block ioctls before the card is fully initialised and
only allow one ioctl at a time.
This stops udev trying to load the firmware before to card is fully up.
patch ported from wlan-ng-devel
missing from the staging version,
http://lists.linux-wlan.com/pipermail/linux-wlan-devel/2009-February/003890.html
Karl sent me a patch a few days before you that also added this
functionality.

Unfortunatly, both of your patches don't apply to the latest version of
the wlan-ng driver as it has had a lot of work done on it recently.

Could one of you grab the linux-next tree and make a patch up against
the version in it so that I can apply this?

thanks,

greg k-h
Richard Kennedy
2009-02-18 18:14:34 UTC
Permalink
Post by Greg KH
Add a mutex to block ioctls before the card is fully initialised and
only allow one ioctl at a time.
This stops udev trying to load the firmware before to card is fully up.
patch ported from wlan-ng-devel
missing from the staging version,
http://lists.linux-wlan.com/pipermail/linux-wlan-devel/2009-February/003890.html
Karl sent me a patch a few days before you that also added this
functionality.
Unfortunatly, both of your patches don't apply to the latest version of
the wlan-ng driver as it has had a lot of work done on it recently.
Could one of you grab the linux-next tree and make a patch up against
the version in it so that I can apply this?
thanks,
greg k-h
Sorry about that, I'd forgotten that wlan-ng was included in linux-next.

I've updated the patch and am just testing it & will post it shortly.

BTW thanks to Moritz Muehlenhoff for the clean-ups.

regards
Richard
Richard Kennedy
2009-02-20 12:09:12 UTC
Permalink
Add a mutex to block ioctls before the card is fully initialised and
only allow one ioctl at a time.
This stops udev trying to load the firmware before to card is fully up.

patch ported from wlan-ng-devel

Karl Relton <***@ntlworld.com> spotted that this was
missing from the staging version,
http://lists.linux-wlan.com/pipermail/linux-wlan-devel/2009-February/003890.html

Signed-off-by: Richard Kennedy <***@rsk.demon.co.uk>
---
Hi Greg,

patch against linux-next-20090219.
booted & working on my laptop.

sorry about the delay, it's taken me a while to get linux-next running
on my laptop.

regards
Richard


diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
index 50fa8d6..b2a606a 100644
--- a/drivers/staging/wlan-ng/p80211netdev.c
+++ b/drivers/staging/wlan-ng/p80211netdev.c
@@ -567,6 +567,8 @@ static int p80211knetdev_do_ioctl(netdevice_t *dev, struct ifreq *ifr, int cmd)

pr_debug("rx'd ioctl, cmd=%d, len=%d\n", cmd, req->len);

+ mutex_lock(&wlandev->ioctl_lock);
+
#ifdef SIOCETHTOOL
if (cmd == SIOCETHTOOL) {
result =
@@ -607,6 +609,8 @@ static int p80211knetdev_do_ioctl(netdevice_t *dev, struct ifreq *ifr, int cmd)
result = -ENOMEM;
}
bail:
+ mutex_unlock(&wlandev->ioctl_lock);
+
return result; /* If allocate,copyfrom or copyto fails, return errno */
}

@@ -758,6 +762,11 @@ int wlan_setup(wlandevice_t *wlandev)
dev->open = p80211knetdev_open;
dev->stop = p80211knetdev_stop;

+ 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);
+
#if (WIRELESS_EXT < 21)
dev->get_wireless_stats = p80211wext_get_wireless_stats;
#endif
@@ -1098,3 +1107,8 @@ static void p80211knetdev_tx_timeout(netdevice_t *netdev)
netif_wake_queue(wlandev->netdev);
}
}
+
+void p80211_allow_ioctls(wlandevice_t *wlandev)
+{
+ mutex_unlock(&wlandev->ioctl_lock);
+}
diff --git a/drivers/staging/wlan-ng/p80211netdev.h b/drivers/staging/wlan-ng/p80211netdev.h
index 42e3b92..b96090d 100644
--- a/drivers/staging/wlan-ng/p80211netdev.h
+++ b/drivers/staging/wlan-ng/p80211netdev.h
@@ -227,6 +227,8 @@ typedef struct wlandevice {
u8 spy_number;
char spy_address[IW_MAX_SPY][ETH_ALEN];
struct iw_quality spy_stat[IW_MAX_SPY];
+
+ struct mutex ioctl_lock;
} wlandevice_t;

/* WEP stuff */
@@ -242,5 +244,5 @@ int register_wlandev(wlandevice_t *wlandev);
int unregister_wlandev(wlandevice_t *wlandev);
void p80211netdev_rx(wlandevice_t *wlandev, struct sk_buff *skb);
void p80211netdev_hwremoved(wlandevice_t *wlandev);
-
+void p80211_allow_ioctls(wlandevice_t *wlandev);
#endif
diff --git a/drivers/staging/wlan-ng/prism2usb.c b/drivers/staging/wlan-ng/prism2usb.c
index 252312e..d8a1298 100644
--- a/drivers/staging/wlan-ng/prism2usb.c
+++ b/drivers/staging/wlan-ng/prism2usb.c
@@ -170,6 +170,7 @@ failed:
wlandev = NULL;

done:
+ p80211_allow_ioctls(wlandev);
usb_set_intfdata(interface, wlandev);
return result;
}

Loading...