* [PATCH 3/5 V2] p54usb: Load firmware from work queue and not from probe routine
@ 2012-03-09 4:28 Larry Finger
2012-03-09 21:45 ` Christian Lamparter
0 siblings, 1 reply; 6+ messages in thread
From: Larry Finger @ 2012-03-09 4:28 UTC (permalink / raw)
To: linville; +Cc: Larry Finger, linux-wireless, chunkeey
Drivers that load firmware from their probe routine have problems with the
latest versions of udev as they get timeouts while waiting for user
space to start. The problem is fixed by loading the firmware and starting
mac80211 from a work queue. By using this method, most of the
original code is preserved.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
V2 - Convert from delayed to ordinary work queue
John,
Again material for 3.5
Larry
---
drivers/net/wireless/p54/p54usb.c | 120 +++++++++++++++++-------------------
drivers/net/wireless/p54/p54usb.h | 1 +
2 files changed, 58 insertions(+), 63 deletions(-)
Index: wireless-testing-new/drivers/net/wireless/p54/p54usb.c
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/p54/p54usb.c
+++ wireless-testing-new/drivers/net/wireless/p54/p54usb.c
@@ -836,9 +836,33 @@ fail:
return err;
}
-static int p54u_load_firmware(struct ieee80211_hw *dev)
+static int p54u_open(struct ieee80211_hw *dev)
{
struct p54u_priv *priv = dev->priv;
+ int err;
+
+ err = p54u_init_urbs(dev);
+ if (err)
+ return err;
+
+ priv->common.open = p54u_init_urbs;
+
+ return 0;
+}
+
+static void p54u_stop(struct ieee80211_hw *dev)
+{
+ /* TODO: figure out how to reliably stop the 3887 and net2280 so
+ the hardware is still usable next time we want to start it.
+ until then, we just stop listening to the hardware.. */
+ p54u_free_urbs(dev);
+}
+
+static void p54u_load_firmware(struct work_struct *work)
+{
+ struct p54u_priv *priv = container_of(work,
+ struct p54u_priv, firmware_load);
+ struct ieee80211_hw *dev = usb_get_intfdata(priv->intf);
int err, i;
BUILD_BUG_ON(ARRAY_SIZE(p54u_fwlist) != __NUM_P54U_HWTYPES);
@@ -847,59 +871,53 @@ static int p54u_load_firmware(struct iee
if (p54u_fwlist[i].type == priv->hw_type)
break;
- if (i == __NUM_P54U_HWTYPES)
- return -EOPNOTSUPP;
+ if (i == __NUM_P54U_HWTYPES) {
+ dev_err(&priv->udev->dev, "Device not supported\n");
+ return;
+ }
err = request_firmware(&priv->fw, p54u_fwlist[i].fw, &priv->udev->dev);
if (err) {
- dev_err(&priv->udev->dev, "(p54usb) cannot load firmware %s "
- "(%d)!\n", p54u_fwlist[i].fw, err);
-
err = request_firmware(&priv->fw, p54u_fwlist[i].fw_legacy,
&priv->udev->dev);
- if (err)
- return err;
+
+ if (err) {
+ dev_err(&priv->udev->dev,
+ "(p54usb) cannot load firmware %s or %s(%d)!\n",
+ p54u_fwlist[i].fw, p54u_fwlist[i].fw_legacy,
+ err);
+ return;
+ }
}
err = p54_parse_firmware(dev, priv->fw);
- if (err)
- goto out;
+ if (err) {
+ dev_err(&priv->udev->dev, "Error parsing firmware\n");
+ return;
+ }
if (priv->common.fw_interface != p54u_fwlist[i].intf) {
dev_err(&priv->udev->dev, "wrong firmware, please get "
"a firmware for \"%s\" and try again.\n",
p54u_fwlist[i].hw);
- err = -EINVAL;
+ return;
}
-
-out:
- if (err)
- release_firmware(priv->fw);
-
- return err;
-}
-
-static int p54u_open(struct ieee80211_hw *dev)
-{
- struct p54u_priv *priv = dev->priv;
- int err;
-
- err = p54u_init_urbs(dev);
+ err = priv->upload_fw(dev);
if (err) {
- return err;
+ dev_err(&priv->udev->dev, "Error uploading firmware\n");
+ return;
}
- priv->common.open = p54u_init_urbs;
-
- return 0;
-}
+ p54u_open(dev);
+ err = p54_read_eeprom(dev);
+ if (err)
+ dev_err(&priv->udev->dev, "Error reading eeprom\n");
+ p54u_stop(dev);
+ if (err)
+ return;
-static void p54u_stop(struct ieee80211_hw *dev)
-{
- /* TODO: figure out how to reliably stop the 3887 and net2280 so
- the hardware is still usable next time we want to start it.
- until then, we just stop listening to the hardware.. */
- p54u_free_urbs(dev);
+ /* firmware available - start operations */
+ err = p54_register_common(dev, &priv->udev->dev);
}
static int __devinit p54u_probe(struct usb_interface *intf,
@@ -969,34 +987,10 @@ static int __devinit p54u_probe(struct u
priv->common.tx = p54u_tx_net2280;
priv->upload_fw = p54u_upload_firmware_net2280;
}
- err = p54u_load_firmware(dev);
- if (err)
- goto err_free_dev;
-
- err = priv->upload_fw(dev);
- if (err)
- goto err_free_fw;
-
- p54u_open(dev);
- err = p54_read_eeprom(dev);
- p54u_stop(dev);
- if (err)
- goto err_free_fw;
-
- err = p54_register_common(dev, &udev->dev);
- if (err)
- goto err_free_fw;
-
+ /* setup and start work to load firmware */
+ INIT_WORK(&priv->firmware_load, p54u_load_firmware);
+ schedule_work(&priv->firmware_load);
return 0;
-
-err_free_fw:
- release_firmware(priv->fw);
-
-err_free_dev:
- p54_free_common(dev);
- usb_set_intfdata(intf, NULL);
- usb_put_dev(udev);
- return err;
}
static void __devexit p54u_disconnect(struct usb_interface *intf)
@@ -1007,9 +1001,10 @@ static void __devexit p54u_disconnect(st
if (!dev)
return;
+ priv = dev->priv;
+ cancel_work_sync(&priv->firmware_load);
p54_unregister_common(dev);
- priv = dev->priv;
usb_put_dev(interface_to_usbdev(intf));
release_firmware(priv->fw);
p54_free_common(dev);
Index: wireless-testing-new/drivers/net/wireless/p54/p54usb.h
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/p54/p54usb.h
+++ wireless-testing-new/drivers/net/wireless/p54/p54usb.h
@@ -143,6 +143,7 @@ struct p54u_priv {
struct sk_buff_head rx_queue;
struct usb_anchor submitted;
const struct firmware *fw;
+ struct work_struct firmware_load;
};
#endif /* P54USB_H */
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/5 V2] p54usb: Load firmware from work queue and not from probe routine 2012-03-09 4:28 [PATCH 3/5 V2] p54usb: Load firmware from work queue and not from probe routine Larry Finger @ 2012-03-09 21:45 ` Christian Lamparter 2012-03-09 23:45 ` Larry Finger 0 siblings, 1 reply; 6+ messages in thread From: Christian Lamparter @ 2012-03-09 21:45 UTC (permalink / raw) To: Larry Finger; +Cc: linville, linux-wireless On Friday, March 09, 2012 05:28:57 AM Larry Finger wrote: > Drivers that load firmware from their probe routine have problems with the > latest versions of udev as they get timeouts while waiting for user > space to start. The problem is fixed by loading the firmware and starting > mac80211 from a work queue. By using this method, most of the > original code is preserved. > > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> > --- Well, I thought this over and I think unless we change the Kconfig and make the backend modules [p54pci, p54usb and p54spi] module-only options, we have to go with request_firmware_nowait. You see, if the p54* modules are compiled into the very bzImage: The instant workqueue option wouldn't work because the device might be initialized before the filesystem is. A combo approach [delayed workqueue, when no userspacehelper is available and a direct call to request_firmware (when it is availabe)] would work too, but then we would be reimplementing request_firmware_nowait ... Regards, Chr ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/5 V2] p54usb: Load firmware from work queue and not from probe routine 2012-03-09 21:45 ` Christian Lamparter @ 2012-03-09 23:45 ` Larry Finger 2012-03-10 0:21 ` Christian Lamparter 0 siblings, 1 reply; 6+ messages in thread From: Larry Finger @ 2012-03-09 23:45 UTC (permalink / raw) To: Christian Lamparter; +Cc: linville, linux-wireless On 03/09/2012 03:45 PM, Christian Lamparter wrote: > On Friday, March 09, 2012 05:28:57 AM Larry Finger wrote: >> Drivers that load firmware from their probe routine have problems with the >> latest versions of udev as they get timeouts while waiting for user >> space to start. The problem is fixed by loading the firmware and starting >> mac80211 from a work queue. By using this method, most of the >> original code is preserved. >> >> Signed-off-by: Larry Finger<Larry.Finger@lwfinger.net> >> --- > Well, I thought this over and I think unless we change the Kconfig > and make the backend modules [p54pci, p54usb and p54spi] > module-only options, we have to go with request_firmware_nowait. > > You see, if the p54* modules are compiled into the very bzImage: > The instant workqueue option wouldn't work because the device > might be initialized before the filesystem is. A combo approach > [delayed workqueue, when no userspacehelper is available and a > direct call to request_firmware (when it is availabe)] would > work too, but then we would be reimplementing > request_firmware_nowait ... Christian, Your point is well taken. I will rewrite this one. John was holding it for 3.5 anyway. Do we still want to try for the legacy firmware if the primary is not available? Larry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/5 V2] p54usb: Load firmware from work queue and not from probe routine 2012-03-09 23:45 ` Larry Finger @ 2012-03-10 0:21 ` Christian Lamparter 2012-03-16 21:51 ` Christian Lamparter 0 siblings, 1 reply; 6+ messages in thread From: Christian Lamparter @ 2012-03-10 0:21 UTC (permalink / raw) To: Larry Finger; +Cc: linville, linux-wireless On Saturday 10 March 2012 00:45:22 Larry Finger wrote: > On 03/09/2012 03:45 PM, Christian Lamparter wrote: > > On Friday, March 09, 2012 05:28:57 AM Larry Finger wrote: > >> Drivers that load firmware from their probe routine have problems with the > >> latest versions of udev as they get timeouts while waiting for user > >> space to start. The problem is fixed by loading the firmware and starting > >> mac80211 from a work queue. By using this method, most of the > >> original code is preserved. > >> > >> Signed-off-by: Larry Finger<Larry.Finger@lwfinger.net> > >> --- > > Well, I thought this over and I think unless we change the Kconfig > > and make the backend modules [p54pci, p54usb and p54spi] > > module-only options, we have to go with request_firmware_nowait. > > > > You see, if the p54* modules are compiled into the very bzImage: > > The instant workqueue option wouldn't work because the device > > might be initialized before the filesystem is. A combo approach > > [delayed workqueue, when no userspacehelper is available and a > > direct call to request_firmware (when it is availabe)] would > > work too, but then we would be reimplementing > > request_firmware_nowait ... > > Christian, > > Your point is well taken. I will rewrite this one. John was > holding it for 3.5 anyway. Thanks, I really appreciate your help. In the meantime. I'll try to talk some sense into the pcmcia and firmware_class people [or they talk some sense into me ;)]. > Do we still want to try for the legacy firmware if the primary > is not available? It's not like we drop support for legacy firmware, I just want to get rid of the confusing name [The first generation usb devices uses a isl3886 chip too [behind a net2280 pci<->usb bridge]. So people tried to use the usb firmwares with pcmcia/pci cards - needless to say that didn't work]. Of course, this wasn't the only reason why I hated the idea of having them in the first place, so while I lost the argument back then... At least I managed to keep the legacy firmware names from being listed by modinfo. So officially we "never" really claimed we know them :-D. Regards, Christan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/5 V2] p54usb: Load firmware from work queue and not from probe routine 2012-03-10 0:21 ` Christian Lamparter @ 2012-03-16 21:51 ` Christian Lamparter 2012-03-17 2:17 ` Larry Finger 0 siblings, 1 reply; 6+ messages in thread From: Christian Lamparter @ 2012-03-16 21:51 UTC (permalink / raw) To: Larry Finger; +Cc: linville, linux-wireless On Saturday 10 March 2012 01:21:53 Christian Lamparter wrote: > On Saturday 10 March 2012 00:45:22 Larry Finger wrote: > > On 03/09/2012 03:45 PM, Christian Lamparter wrote: > > > On Friday, March 09, 2012 05:28:57 AM Larry Finger wrote: > > >> Drivers that load firmware from their probe routine have problems with the > > >> latest versions of udev as they get timeouts while waiting for user > > >> space to start. The problem is fixed by loading the firmware and starting > > >> mac80211 from a work queue. By using this method, most of the > > >> original code is preserved. > > >> > > >> Signed-off-by: Larry Finger<Larry.Finger@lwfinger.net> > > >> --- > > > Well, I thought this over and I think unless we change the Kconfig > > > and make the backend modules [p54pci, p54usb and p54spi] > > > module-only options, we have to go with request_firmware_nowait. > > > > > > You see, if the p54* modules are compiled into the very bzImage: > > > The instant workqueue option wouldn't work because the device > > > might be initialized before the filesystem is. A combo approach > > > [delayed workqueue, when no userspacehelper is available and a > > > direct call to request_firmware (when it is availabe)] would > > > work too, but then we would be reimplementing > > > request_firmware_nowait ... > > > > Christian, > > > > Your point is well taken. I will rewrite this one. John was > > holding it for 3.5 anyway. > Thanks, I really appreciate your help. In the meantime. I'll try > to talk some sense into the pcmcia and firmware_class people [or > they talk some sense into me ;)]. Just a heads up: [No, I haven't forgotten about this] While my patches certainly caused some "noise" @ lkml. So far there hasn't been any progress about request_firmware_nowait. I'll keep at it but it could take a while longer... In the meantime: I think we could get atleast fix p54pci and friends. Larry, what do you say? And John: Would you accept the patches? even though they need the listed pcmcia and firmware_class fixes in order to work [without those, on resume you get WARNINGs and an you need to reload the driver/replug the device] Regards, Chr ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/5 V2] p54usb: Load firmware from work queue and not from probe routine 2012-03-16 21:51 ` Christian Lamparter @ 2012-03-17 2:17 ` Larry Finger 0 siblings, 0 replies; 6+ messages in thread From: Larry Finger @ 2012-03-17 2:17 UTC (permalink / raw) To: Christian Lamparter; +Cc: linville, linux-wireless [-- Attachment #1: Type: text/plain, Size: 625 bytes --] On 03/16/2012 04:51 PM, Christian Lamparter wrote: > Just a heads up: [No, I haven't forgotten about this] > > While my patches certainly caused some "noise" @ lkml. So far > there hasn't been any progress about request_firmware_nowait. > I'll keep at it but it could take a while longer... > In the meantime: I think we could get atleast fix p54pci and friends. > Larry, what do you say? Attached is a patch for p54usb that uses request_firmware_nowait(). It works fine here, but I have not tested suspend/resume as my box does not support that functionality. I will submit this one more formally as an RFC/RFT. Larry [-- Attachment #2: p54usb_async_firmware --] [-- Type: text/plain, Size: 6282 bytes --] Drivers that load firmware from their probe routine have problems with the latest versions of udev as they get timeouts while waiting for user space to start. The problem is fixed by using request_firmware_nowait() and delaying the start of mac80211 until the firmware is loaded. To prevent the possibility of the driver being unloaded while the firmware loading callback is still active, a completion queue entry is used. Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> --- p54usb.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++----------------- p54usb.h | 3 + 2 files changed, 115 insertions(+), 41 deletions(-) --- Index: wireless-testing-new/drivers/net/wireless/p54/p54usb.c =================================================================== --- wireless-testing-new.orig/drivers/net/wireless/p54/p54usb.c +++ wireless-testing-new/drivers/net/wireless/p54/p54usb.c @@ -836,45 +836,133 @@ fail: return err; } -static int p54u_load_firmware(struct ieee80211_hw *dev) +static int p54_find_type(struct p54u_priv *priv) { - struct p54u_priv *priv = dev->priv; - int err, i; - - BUILD_BUG_ON(ARRAY_SIZE(p54u_fwlist) != __NUM_P54U_HWTYPES); + int i; for (i = 0; i < __NUM_P54U_HWTYPES; i++) if (p54u_fwlist[i].type == priv->hw_type) break; - if (i == __NUM_P54U_HWTYPES) return -EOPNOTSUPP; - err = request_firmware(&priv->fw, p54u_fwlist[i].fw, &priv->udev->dev); - if (err) { - dev_err(&priv->udev->dev, "(p54usb) cannot load firmware %s " - "(%d)!\n", p54u_fwlist[i].fw, err); + return i; +} - err = request_firmware(&priv->fw, p54u_fwlist[i].fw_legacy, - &priv->udev->dev); - if (err) - return err; - } +static int p54u_open(struct ieee80211_hw *dev); +static void p54u_stop(struct ieee80211_hw *dev); + +static void p54_start_ops(struct usb_interface *intf) +{ + struct usb_device *udev = interface_to_usbdev(intf); + struct ieee80211_hw *dev = usb_get_intfdata(intf); + struct p54u_priv *priv = dev->priv; + int i, err; err = p54_parse_firmware(dev, priv->fw); if (err) - goto out; + goto err_free_dev; + i = p54_find_type(priv); + if (i < 0) + goto err_free_dev; if (priv->common.fw_interface != p54u_fwlist[i].intf) { dev_err(&priv->udev->dev, "wrong firmware, please get " "a firmware for \"%s\" and try again.\n", p54u_fwlist[i].hw); - err = -EINVAL; + goto err_free_dev; + } + err = priv->upload_fw(dev); + if (err) + goto err_free_dev; + + p54u_open(dev); + err = p54_read_eeprom(dev); + p54u_stop(dev); + if (err) + goto err_free_dev; + + err = p54_register_common(dev, &udev->dev); + if (err) + goto err_free_common; + return; + +err_free_common: + p54_free_common(dev); +err_free_dev: + release_firmware(priv->fw); + usb_set_intfdata(intf, NULL); + usb_put_dev(udev); +} + +static void p54u_load_firmware_cb2(const struct firmware *firmware, + void *context) +{ + struct usb_interface *intf = context; + struct ieee80211_hw *dev = usb_get_intfdata(intf); + struct p54u_priv *priv = dev->priv; + + complete(&priv->fw_loaded); + if (!firmware) { + dev_err(&priv->udev->dev, "Firmware not found\n"); + return; } + priv->fw = firmware; + p54_start_ops(intf); +} + +static void p54u_load_firmware_cb(const struct firmware *firmware, + void *context) +{ + struct usb_interface *intf = context; + struct ieee80211_hw *dev = usb_get_intfdata(intf); + struct p54u_priv *priv = dev->priv; + struct usb_device *udev = interface_to_usbdev(intf); + struct device *device = &udev->dev; + int i, err; + + if (!firmware) { + i = p54_find_type(priv); + if (i < 0) + return; + /* Primary firmware not found - try for legacy variety */ + dev_info(&priv->udev->dev, "Loading firmware file %s\n", + p54u_fwlist[i].fw_legacy); + err = request_firmware_nowait(THIS_MODULE, 1, + p54u_fwlist[i].fw_legacy, + device, GFP_KERNEL, intf, + p54u_load_firmware_cb2); + if (err) + return; + } + complete(&priv->fw_loaded); + priv->fw = firmware; + p54_start_ops(intf); +} -out: +static int p54u_load_firmware(struct ieee80211_hw *dev, + struct usb_interface *intf) +{ + struct usb_device *udev = interface_to_usbdev(intf); + struct p54u_priv *priv = dev->priv; + struct device *device = &udev->dev; + int err, i; + + BUILD_BUG_ON(ARRAY_SIZE(p54u_fwlist) != __NUM_P54U_HWTYPES); + + init_completion(&priv->fw_loaded); + i = p54_find_type(priv); + if (i < 0) + return i; + + dev_info(&priv->udev->dev, "Loading firmware file %s\n", + p54u_fwlist[i].fw); + err = request_firmware_nowait(THIS_MODULE, 1, p54u_fwlist[i].fw, + device, GFP_KERNEL, intf, + p54u_load_firmware_cb); if (err) - release_firmware(priv->fw); + dev_err(&priv->udev->dev, "(p54usb) cannot load firmware %s " + "(%d)!\n", p54u_fwlist[i].fw, err); return err; } @@ -969,33 +1057,7 @@ static int __devinit p54u_probe(struct u priv->common.tx = p54u_tx_net2280; priv->upload_fw = p54u_upload_firmware_net2280; } - err = p54u_load_firmware(dev); - if (err) - goto err_free_dev; - - err = priv->upload_fw(dev); - if (err) - goto err_free_fw; - - p54u_open(dev); - err = p54_read_eeprom(dev); - p54u_stop(dev); - if (err) - goto err_free_fw; - - err = p54_register_common(dev, &udev->dev); - if (err) - goto err_free_fw; - - return 0; - -err_free_fw: - release_firmware(priv->fw); - -err_free_dev: - p54_free_common(dev); - usb_set_intfdata(intf, NULL); - usb_put_dev(udev); + err = p54u_load_firmware(dev, intf); return err; } @@ -1007,9 +1069,10 @@ static void __devexit p54u_disconnect(st if (!dev) return; + priv = dev->priv; + wait_for_completion(&priv->fw_loaded); p54_unregister_common(dev); - priv = dev->priv; usb_put_dev(interface_to_usbdev(intf)); release_firmware(priv->fw); p54_free_common(dev); Index: wireless-testing-new/drivers/net/wireless/p54/p54usb.h =================================================================== --- wireless-testing-new.orig/drivers/net/wireless/p54/p54usb.h +++ wireless-testing-new/drivers/net/wireless/p54/p54usb.h @@ -143,6 +143,9 @@ struct p54u_priv { struct sk_buff_head rx_queue; struct usb_anchor submitted; const struct firmware *fw; + + /* asynchronous firmware callback */ + struct completion fw_loaded; }; #endif /* P54USB_H */ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-17 2:17 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-09 4:28 [PATCH 3/5 V2] p54usb: Load firmware from work queue and not from probe routine Larry Finger 2012-03-09 21:45 ` Christian Lamparter 2012-03-09 23:45 ` Larry Finger 2012-03-10 0:21 ` Christian Lamparter 2012-03-16 21:51 ` Christian Lamparter 2012-03-17 2:17 ` Larry Finger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).