Re: [PATCH v2 8/8] p54: convert to sysdata API

2016-08-10 Thread Luis R. Rodriguez
On Fri, Jun 17, 2016 at 08:35:03PM +0200, Luis R. Rodriguez wrote:
> On Thu, Jun 16, 2016 at 05:09:30PM -1000, Linus Torvalds wrote:
> > On Thu, Jun 16, 2016 at 3:36 PM, Luis R. Rodriguez  
> > wrote:
> > >
> > > Reason this could not wait is folks seem to want to keep extending the 
> > > API,
> > > which is another reason for this, do we want to put an end to an 
> > > unflexible
> > > API now or should we wait ?
> > 
> > So I absolutely abhor "changes for changes sake".
> > 
> > If the existing code works for existing drivers, let them keep it.
> > 
> > And if a new interface is truly more flexible, then it should be able
> > to implement the old interface with no changes, so that drivers
> > shouldn't need to be changed/upgraded.
> 
> Are we OK to leave only the usermode helper support in place for the
> 2 drivers I've noted that explicitly require the usermode helper in their
> firmware request [0] ?
> 
> If so then I can do what you recommend below.
> 
> > Then, drivers that actually _want_ new features, or that can take
> > advantage of new interfaces to actually make things *simpler*, can
> > choose to make those changes. But those changes should have real
> > advantages.

As I noted above -- this still needs to be decided.

We only have 2 more drivers using the usermode helper explicitly now. Other
than this we have the old CONFIG_FW_LOADER_USER_HELPER_FALLBACK -- which
explicitly forced the usermode helper on every call. This later option is no
longer widely used by distributions, and distros these days just enable
CONFIG_FW_LOADER_USER_HELPER, with this only 2 drivers are using the usermode
helper. This still should mean we should not break users of
CONFIG_FW_LOADER_USER_HELPER_FALLBACK, as stupid as it may have been.

My preferred approach is as follows (and this is what I'll follow unless
I hear otherwise):

Obviously let's not break CONFIG_FW_LOADER_USER_HELPER_FALLBACK -- even if it
logically means long term most drivers if not all will convert to the new API,
there is no need for a full swing change. Lets leave drivers as-is, given most
distros are sensible and only enabling CONFIG_FW_LOADER_USER_HELPER.  Given
most distros disable CONFIG_FW_LOADER_USER_HELPER_FALLBACK, and we have now
verified only 2 explicit user mode helper scallers exists this means we have
mostly put away most of the sore of the user mode helper. With Daniel Wagner's
change to change the firmware API to use the new swait stuff it further pushes
some other user mode helper crap away [0]. Then, only drivers *that care and
want to change* to the new API will do so but we put a stop gap measure so that
new features only go through the new API. This means we mark
CONFIG_FW_LOADER_USER_HELPER_FALLBACK as deprecated.

We can strive to mark CONFIG_FW_LOADER_USER_HELPER as deprecated but to be fair
this requires more work:

Despite my best efforts to call out for valid new uses of the user mode helper
the only valid use I've heard so far over CONFIG_FW_LOADER_USER_HELPER was for
huge files for remoteproc as explained by Bjorn Andersson given the
deterministic aspect issue of ensuring a file is ready and also that they
cannot use initramfs to stuff the firmware [1]. As recently discussed in that
same thread with Bjorn though we can easily just add this upstream either as
a simple file sentinel or better yet -- a simple event from userspace [2] to
be used either to indicate the rootfs is ready or if we wanted farther
granularity per enum kernel_read_file_id (READING_FIRMWARE, READING_MODULE,
READING_KEXEC_IMAGE, READING_KEXEC_INITRAMFS, READING_POLICY), and that would
seem to put everyone's concerns over direct file loading at ease, including
Bjorn's [1]. This needs to be implemented. When that is done -- unless we hear
otherwise over requirements for the usermode helper -- we can finally mark
CONFIG_FW_LOADER_USER_HELPER as deprecated.

[0] https://lkml.kernel.org/r/1470313636-670-1-git-send-email-w...@monom.org
[1] https://lkml.kernel.org/r/20160803173955.GD13516@tuxbot
[2] https://lkml.kernel.org/r/20160803184058.gs3...@wotan.suse.de

> Sure agreed.
> 
> > Having to have a callback, 
> 
> This is because devm is not used, as such the consumer is needed prior to
> freeing. I can give adding devm support a shot if folks seem to be generally 
> agree its the right approach. I do expect this will mean tons of code
> deletions and helping with bugs.

Regarding this -- Dmitry recenlty noted devm only works if used on the probe
path, and as we now determined, we don't want firmware loading on probe [3], 
unless
async probe is used, so this would make a devm solution here not ideal for
freeing the firmware. Even if you use async probe -- that seems very special
use case and adding devm support for the firmware API just for that seems silly.

As such the current devised solution in the sysdata API is called for, given
if you indicated that keep = false, you are explicitly telling the firmware
API that your firmware will certainly not b

Re: [PATCH v2 8/8] p54: convert to sysdata API

2016-08-10 Thread Luis R. Rodriguez
On Wed, Aug 10, 2016 at 08:17:08PM +0100, Mark Brown wrote:
> On Wed, Aug 10, 2016 at 09:04:38PM +0200, Arend Van Spriel wrote:
> 
> > So why would drivers want the devm solution anyway. Once firmware is
> > loaded in the device it can be freed or do you expect device drivers to
> > keep the firmware/sysdata for suspend/resume scenario as some do because
> > of firmware cache behaviour. Does the "rootfs ready" event cover
> > suspend/resume?
> 
> There are classes of devices that spend a large proportion of their time
> powered off and are only powered up and have firmware loaded when they
> are actively in use.  DSPs used in audio systems are one example of
> this, I'd not be surprised to learn that similar things are done with
> video.  It's too expensive to keep the device powered up and you may be
> swapping between firmwares depending on use case anyway for a lot of
> these devices.

devm use and probe is orthogonal to this use case and suspend/resume.
In any case the proposed new API simply allows the driver caller to
either free the firmware after the consumer callback or to keep it.
It does this without devm, so allows this feature without the API
being used on probe. The point about devm was that it would only
beneficial to us if most users for the API were on probe. Clearly
that's not the case and in fact unless async probe is used that use
is likely buggy as it would delay boot further.

If having a local cache of firmware *beyond* the suspend / resume
case is an optimization that can help we should look into that, but
my preference would be to peg this onto the API through an optional
setting in the request passed.

  Luis


Re: [PATCH v2 8/8] p54: convert to sysdata API

2016-08-10 Thread Luis R. Rodriguez
On Wed, Aug 10, 2016 at 09:04:38PM +0200, Arend Van Spriel wrote:
> On 10-8-2016 20:32, Luis R. Rodriguez wrote:
> > On Fri, Jun 17, 2016 at 08:35:03PM +0200, Luis R. Rodriguez wrote:
> >> On Thu, Jun 16, 2016 at 05:09:30PM -1000, Linus Torvalds wrote:
> >>> On Thu, Jun 16, 2016 at 3:36 PM, Luis R. Rodriguez  
> >>> wrote:
> 
>  Reason this could not wait is folks seem to want to keep extending the 
>  API,
>  which is another reason for this, do we want to put an end to an 
>  unflexible
>  API now or should we wait ?
> 
> [big snip]
> 
> > 
> > Regarding this -- Dmitry recenlty noted devm only works if used on the probe
> > path, and as we now determined, we don't want firmware loading on probe 
> > [3], unless
> > async probe is used, so this would make a devm solution here not ideal for
> > freeing the firmware. Even if you use async probe -- that seems very special
> > use case and adding devm support for the firmware API just for that seems 
> > silly.
> 
> So why would drivers want the devm solution anyway.

It depends on the use case, some may want to keep the firmware around,
some may not, but by default the new API ssumes you will not (keep = false)
and we free it. My point above was that using devm will not typically
be the most fruitful solution to free the firmware givne that there are only
a few drivers that should need it upon probe, and drivers using firmware
APIs on probe shoudld be using async probe anyway to delay avoid delaying
boot as processing the firmware can take time.

> Once firmware is
> loaded in the device it can be freed or do you expect device drivers to
> keep the firmware/sysdata for suspend/resume scenario as some do because
> of firmware cache behaviour.

You would think! Upon careful inspection there are tons of odd things drivers
do, some modify the firmware and therefore have their own reasons to keep
it. Whatever the reasons are -- I recall seeing a few well justified uses.

> Does the "rootfs ready" event cover
> suspend/resume?

The "rootfs ready" is orthogonal to suspend/resume case given the firmware_class
cache firmware feature.

The firmware API as-is upstream already has a cache firmware solution added
a long time ago, as reflected by a resent patch set (before this one) I
updated documentation for this given its not clearly well known and people
keep adding their own caching solutions, the firmware API requests firmware
prior to suspend so that upon resume the firmware is present, precisely to
avoid race issues. I will note that this feature is only for non-usermode helper
firmware API, upon suspend we *kill* any pending user mode helper requests
given that this can delay suspend, as such drivers using or relying (only
2) on the user mode helper for firmware have no solution built in for
the cache stuff and I can't say I care given there seems to be no valid
modern use case given as a requirement for it yet. In fact there was a bug
in the firmware_class code that *allows* the cache call to request the
usermode helper, obviously that's buggy if we are trying to kill the
usermode helper upon suspend...  so a pending patch fixes that. That's been
a long standing bug and surprised no one ever picked up on it.

> > As such the current devised solution in the sysdata API is called for, given
> > if you indicated that keep = false, you are explicitly telling the firmware
> > API that your firmware will certainly not be needed after the callback is
> > called.
> > 
> > So for the sync case, a new callback is needed, and that explains the extra
> > bit of code if someone conerts from the old API to the new one.
> > 
> > [3] https://lkml.kernel.org/r/20160803161821.GB32965@dtor-ws
> > 
> >>> or a magical "sysdata_desc" descriptor,
> >>
> >> This is one way to make a flexible and extensible API without affecting 
> >> drivers
> >> with further collateral evolutions as it gets extended. Its no different 
> >> than
> >> the "flags" lesson learned from writing system calls, for instance.
> >>
> >> Descriptor seemed, fitting, let me know if you have any other preference.
> > 
> > I haven't heard otherwise so will be sticking to that.
> 
> How about sysdata_req{,uest}_params?

Thanks will go with that.

> >>> and having a new name ("sysdata") that is less descriptive than the old 
> >>> one
> >>> ("firmware")
> >>
> >> Well no, the APIs are used in *a lot* of cases for things that are not 
> >> firmware
> >> already, and let's recall I originally started working on this to replace 
> >> CRDA
> >> from userspace to be able to just fetch the signed regulatory database file
> >> from the kernel. Calling it firmware simply makes no sense anymore.
> > 
> > So help me bike shed. This seems to be sticking point and I frankly don't
> > care what we call it. I've asked others for name suggestions and here are
> > a few suggestions:
> > 
> >  o driver_data
> >  o dsd: device specific data
> >  o xfw - eXtensible firmware API
> >  o bbl - binary blob loader
> > 

Re: [PATCH v2 8/8] p54: convert to sysdata API

2016-08-10 Thread Mark Brown
On Wed, Aug 10, 2016 at 09:04:38PM +0200, Arend Van Spriel wrote:

> So why would drivers want the devm solution anyway. Once firmware is
> loaded in the device it can be freed or do you expect device drivers to
> keep the firmware/sysdata for suspend/resume scenario as some do because
> of firmware cache behaviour. Does the "rootfs ready" event cover
> suspend/resume?

There are classes of devices that spend a large proportion of their time
powered off and are only powered up and have firmware loaded when they
are actively in use.  DSPs used in audio systems are one example of
this, I'd not be surprised to learn that similar things are done with
video.  It's too expensive to keep the device powered up and you may be
swapping between firmwares depending on use case anyway for a lot of
these devices.


signature.asc
Description: PGP signature


Re: [PATCH v2 8/8] p54: convert to sysdata API

2016-08-10 Thread Arend Van Spriel
On 10-8-2016 20:32, Luis R. Rodriguez wrote:
> On Fri, Jun 17, 2016 at 08:35:03PM +0200, Luis R. Rodriguez wrote:
>> On Thu, Jun 16, 2016 at 05:09:30PM -1000, Linus Torvalds wrote:
>>> On Thu, Jun 16, 2016 at 3:36 PM, Luis R. Rodriguez  
>>> wrote:

 Reason this could not wait is folks seem to want to keep extending the API,
 which is another reason for this, do we want to put an end to an unflexible
 API now or should we wait ?

[big snip]

> 
> Regarding this -- Dmitry recenlty noted devm only works if used on the probe
> path, and as we now determined, we don't want firmware loading on probe [3], 
> unless
> async probe is used, so this would make a devm solution here not ideal for
> freeing the firmware. Even if you use async probe -- that seems very special
> use case and adding devm support for the firmware API just for that seems 
> silly.

So why would drivers want the devm solution anyway. Once firmware is
loaded in the device it can be freed or do you expect device drivers to
keep the firmware/sysdata for suspend/resume scenario as some do because
of firmware cache behaviour. Does the "rootfs ready" event cover
suspend/resume?

> As such the current devised solution in the sysdata API is called for, given
> if you indicated that keep = false, you are explicitly telling the firmware
> API that your firmware will certainly not be needed after the callback is
> called.
> 
> So for the sync case, a new callback is needed, and that explains the extra
> bit of code if someone conerts from the old API to the new one.
> 
> [3] https://lkml.kernel.org/r/20160803161821.GB32965@dtor-ws
> 
>>> or a magical "sysdata_desc" descriptor,
>>
>> This is one way to make a flexible and extensible API without affecting 
>> drivers
>> with further collateral evolutions as it gets extended. Its no different than
>> the "flags" lesson learned from writing system calls, for instance.
>>
>> Descriptor seemed, fitting, let me know if you have any other preference.
> 
> I haven't heard otherwise so will be sticking to that.

How about sysdata_req{,uest}_params?

>>> and having a new name ("sysdata") that is less descriptive than the old one
>>> ("firmware")
>>
>> Well no, the APIs are used in *a lot* of cases for things that are not 
>> firmware
>> already, and let's recall I originally started working on this to replace 
>> CRDA
>> from userspace to be able to just fetch the signed regulatory database file
>> from the kernel. Calling it firmware simply makes no sense anymore.
> 
> So help me bike shed. This seems to be sticking point and I frankly don't
> care what we call it. I've asked others for name suggestions and here are
> a few suggestions:
> 
>  o driver_data
>  o dsd: device specific data
>  o xfw - eXtensible firmware API
>  o bbl - binary blob loader
> 
> Can someone just pick something? I really, really do not care.
> 
> If I don't hear anything concrete I will go with driver_data.

bit of skin crawling here, but not enough to care.

>>> are all in my opinion making the example patch be a
>>> step _backwards_ rather than an improvement. It does not look like a
>>> simpler or more natural interface for a driver.
>>
>> Hope the above explains the current state. Feedback on desirable changes
>> welcomed.
>>
>> [0] 
>> https://lkml.kernel.org/r/1466117661-22075-5-git-send-email-mcg...@kernel.org
> 
> All this said, this series is still justified, the extra code only comes in
> place when porting the sync requests due to the callback stuff described above
> and the inability to use devm there. As far as I can tell, just the bike
> shedding is left.
> 
> As it stands then, unless I hear back, I'll roll Daniel Wagner's changes into
> my series to be applied first, then rename sysdata driver_data, rebase all 
> this
> and shoot it out again.
> 
> Only a few drivers will be converted over as demos. The SmPL grammar can be 
> used
> by those who do want a change due to the few features added. Long term we'll
> add more features to the new API:
> 
>  o the whole ihex conversion is crap, we should do this within the API and
>this can just be specified as a descriptor preference, then drivers
>don't have to deal with the ihex crap themselves.
> 
>  o firmware singing - this lets us kill CRDA as a requirement

Strongly suspect a typo here :-p

Regards,
Arend


Re: [PATCH v2 8/8] p54: convert to sysdata API

2016-06-17 Thread Luis R. Rodriguez
On Thu, Jun 16, 2016 at 05:09:30PM -1000, Linus Torvalds wrote:
> On Thu, Jun 16, 2016 at 3:36 PM, Luis R. Rodriguez  wrote:
> >
> > Reason this could not wait is folks seem to want to keep extending the API,
> > which is another reason for this, do we want to put an end to an unflexible
> > API now or should we wait ?
> 
> So I absolutely abhor "changes for changes sake".
> 
> If the existing code works for existing drivers, let them keep it.
> 
> And if a new interface is truly more flexible, then it should be able
> to implement the old interface with no changes, so that drivers
> shouldn't need to be changed/upgraded.

Are we OK to leave only the usermode helper support in place for the
2 drivers I've noted that explicitly require the usermode helper in their
firmware request [0] ?

If so then I can do what you recommend below.

> Then, drivers that actually _want_ new features, or that can take
> advantage of new interfaces to actually make things *simpler*, can
> choose to make those changes. But those changes should have real
> advantages.

Sure agreed.

> Having to have a callback, 

This is because devm is not used, as such the consumer is needed prior to
freeing. I can give adding devm support a shot if folks seem to be generally 
agree its the right approach. I do expect this will mean tons of code
deletions and helping with bugs.

> or a magical "sysdata_desc" descriptor,

This is one way to make a flexible and extensible API without affecting drivers
with further collateral evolutions as it gets extended. Its no different than
the "flags" lesson learned from writing system calls, for instance.

Descriptor seemed, fitting, let me know if you have any other preference.

> and having a new name ("sysdata") that is less descriptive than the old one
> ("firmware")

Well no, the APIs are used in *a lot* of cases for things that are not firmware
already, and let's recall I originally started working on this to replace CRDA
from userspace to be able to just fetch the signed regulatory database file
from the kernel. Calling it firmware simply makes no sense anymore.

> are all in my opinion making the example patch be a
> step _backwards_ rather than an improvement. It does not look like a
> simpler or more natural interface for a driver.

Hope the above explains the current state. Feedback on desirable changes
welcomed.

[0] 
https://lkml.kernel.org/r/1466117661-22075-5-git-send-email-mcg...@kernel.org

  Luis


Re: [PATCH v2 8/8] p54: convert to sysdata API

2016-06-16 Thread Linus Torvalds
On Thu, Jun 16, 2016 at 3:36 PM, Luis R. Rodriguez  wrote:
>
> Reason this could not wait is folks seem to want to keep extending the API,
> which is another reason for this, do we want to put an end to an unflexible
> API now or should we wait ?

So I absolutely abhor "changes for changes sake".

If the existing code works for existing drivers, let them keep it.

And if a new interface is truly more flexible, then it should be able
to implement the old interface with no changes, so that drivers
shouldn't need to be changed/upgraded.

Then, drivers that actually _want_ new features, or that can take
advantage of new interfaces to actually make things *simpler*, can
choose to make those changes. But those changes should have real
advantages.

Having to have a callback, or a magical "sysdata_desc" descriptor, and
having a new name ("sysdata") that is less descriptive than the old
one ("firmware") are all in my opinion making the example patch be a
step _backwards_ rather than an improvement. It does not look like a
simpler or more natural interface for a driver.

   Linus


Re: [PATCH v2 8/8] p54: convert to sysdata API

2016-06-16 Thread Luis R. Rodriguez
On Thu, Jun 16, 2016 at 02:21:02PM -1000, Linus Torvalds wrote:
> So what is the advantage of this, since it needs to add more lines of code
> than it removes?
>
> It doesn't seem to be a simplification. Quite the reverse.
> 
> Your diffstat of the whole automated conversion did that too.

A lot of the diff stat of automated changes is space changes which consists of
not wanted new lines in one case caused by Coccinelle.  To get an idea by
comparison one would have to cleanup the output. Typically its on par,
sometimes you save, sometimes a bit more code due to the syn case needing a
callback. In this case its more given the non-keep cases which need a callback,
and also some changes around an #ifdef to make code cleaner.

> Why would we ever want to apply a patch like this?

Well, it depends, in terms of API it helps us extend it without having to do
more collateral evolutions. The p54 driver happens to have both sync and async
calls, and also has a few "KEEP" cases, the SmPL grammar does not deal with the
keep cases -- this change to p54 was just a demo, but what I'd much prefer is
to deal with the keep cases by folding the fw into devm to also avoid the
free'ing in drivers just as with the non-keep cases. If that's desirable it
needs discussion given its a significant change. The grammar still produces
more changes than before for the sync cases given a callback is needed, its
just that simple.

Reason this could not wait is folks seem to want to keep extending the API,
which is another reason for this, do we want to put an end to an unflexible
API now or should we wait ?

If we want to wait for devm integration -- that's cool with me, its however
better to release often and release early. So the point of this particular
series is to show where development is at on the front of a new flexible API 
that
also avoids the usermode helper. I posted this also *now* given I saw the old
API being extended further. So another option is to evaluate a merge now, and
evolve the devm integration later.

Alternatively, since its only 2 drivers that explicitly require the usermode
helper, another option is to just compartamentalize the usermode helper
explicit callers with a its own API now and save the others with a strategy
devised by the current sysdata API, without requiring any changes at all.
This is a significant change though, so still requires discussion. As I
see it this is worth considering just because we get the same end result
if we transform drivers to sysdata or we just compartamentalize the usermode
helper now to the 2 callers.

Ideas, patches, feedback, rants welcomed.

  Luis


[PATCH v2 8/8] p54: convert to sysdata API

2016-06-16 Thread Luis R. Rodriguez
The Coccinelle sysdata patches were used to help with
this transition. The changes have been carefully manually
vetted for. With the conversion we modify the cases that do
not need the firmware to be kept so that the sysdata API
can release it for us. Using the new sysdata API also means
we can get rid of our own completions.

Generated-by: Coccinelle SmPL
Signed-off-by: Luis R. Rodriguez 
---
 drivers/net/wireless/intersil/p54/eeprom.c |  2 +-
 drivers/net/wireless/intersil/p54/fwio.c   |  5 +-
 drivers/net/wireless/intersil/p54/led.c|  2 +-
 drivers/net/wireless/intersil/p54/main.c   |  2 +-
 drivers/net/wireless/intersil/p54/p54.h|  3 +-
 drivers/net/wireless/intersil/p54/p54pci.c | 26 ++
 drivers/net/wireless/intersil/p54/p54pci.h |  4 +-
 drivers/net/wireless/intersil/p54/p54spi.c | 81 +++---
 drivers/net/wireless/intersil/p54/p54spi.h |  2 +-
 drivers/net/wireless/intersil/p54/p54usb.c | 18 +++
 drivers/net/wireless/intersil/p54/p54usb.h |  4 +-
 drivers/net/wireless/intersil/p54/txrx.c   |  2 +-
 12 files changed, 90 insertions(+), 61 deletions(-)

diff --git a/drivers/net/wireless/intersil/p54/eeprom.c 
b/drivers/net/wireless/intersil/p54/eeprom.c
index d4c73d39336f..9d048fa1f07f 100644
--- a/drivers/net/wireless/intersil/p54/eeprom.c
+++ b/drivers/net/wireless/intersil/p54/eeprom.c
@@ -16,7 +16,7 @@
  * published by the Free Software Foundation.
  */
 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/net/wireless/intersil/p54/fwio.c 
b/drivers/net/wireless/intersil/p54/fwio.c
index 257a9eadd595..6bc8463ccf5e 100644
--- a/drivers/net/wireless/intersil/p54/fwio.c
+++ b/drivers/net/wireless/intersil/p54/fwio.c
@@ -17,7 +17,7 @@
  */
 
 #include 
-#include 
+#include 
 #include 
 #include 
 
@@ -27,7 +27,8 @@
 #include "eeprom.h"
 #include "lmac.h"
 
-int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw)
+int p54_parse_firmware(struct ieee80211_hw *dev,
+  const struct sysdata_file *fw)
 {
struct p54_common *priv = dev->priv;
struct exp_if *exp_if;
diff --git a/drivers/net/wireless/intersil/p54/led.c 
b/drivers/net/wireless/intersil/p54/led.c
index 9a8fedd3c0f5..b9f18082b1ff 100644
--- a/drivers/net/wireless/intersil/p54/led.c
+++ b/drivers/net/wireless/intersil/p54/led.c
@@ -16,7 +16,7 @@
  * published by the Free Software Foundation.
  */
 
-#include 
+#include 
 #include 
 
 #include 
diff --git a/drivers/net/wireless/intersil/p54/main.c 
b/drivers/net/wireless/intersil/p54/main.c
index d5a3bf91a03e..f000e39f677a 100644
--- a/drivers/net/wireless/intersil/p54/main.c
+++ b/drivers/net/wireless/intersil/p54/main.c
@@ -17,7 +17,7 @@
  */
 
 #include 
-#include 
+#include 
 #include 
 #include 
 
diff --git a/drivers/net/wireless/intersil/p54/p54.h 
b/drivers/net/wireless/intersil/p54/p54.h
index 529939e611cd..1747119921d6 100644
--- a/drivers/net/wireless/intersil/p54/p54.h
+++ b/drivers/net/wireless/intersil/p54/p54.h
@@ -268,7 +268,8 @@ struct p54_common {
 /* interfaces for the drivers */
 int p54_rx(struct ieee80211_hw *dev, struct sk_buff *skb);
 void p54_free_skb(struct ieee80211_hw *dev, struct sk_buff *skb);
-int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw);
+int p54_parse_firmware(struct ieee80211_hw *dev,
+  const struct sysdata_file *fw);
 int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len);
 int p54_read_eeprom(struct ieee80211_hw *dev);
 
diff --git a/drivers/net/wireless/intersil/p54/p54pci.c 
b/drivers/net/wireless/intersil/p54/p54pci.c
index 27a49068d32d..e65497de5454 100644
--- a/drivers/net/wireless/intersil/p54/p54pci.c
+++ b/drivers/net/wireless/intersil/p54/p54pci.c
@@ -15,7 +15,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -490,7 +490,7 @@ static int p54p_open(struct ieee80211_hw *dev)
return 0;
 }
 
-static void p54p_firmware_step2(const struct firmware *fw,
+static void p54p_firmware_step2(const struct sysdata_file *fw,
void *context)
 {
struct p54p_priv *priv = context;
@@ -520,8 +520,6 @@ static void p54p_firmware_step2(const struct firmware *fw,
 
 out:
 
-   complete(&priv->fw_loaded);
-
if (err) {
struct device *parent = pdev->dev.parent;
 
@@ -542,6 +540,17 @@ out:
pci_dev_put(pdev);
 }
 
+static int p54p_load_firmware(struct p54p_priv *priv)
+{
+   const struct sysdata_file_desc sysdata_desc = {
+   SYSDATA_KEEP_ASYNC(p54p_firmware_step2, priv),
+   };
+
+   return sysdata_file_request_async("isl3886pci", &sysdata_desc,
+ &priv->pdev->dev,
+ &priv->fw_async_cookie);
+}
+
 static int p54p_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
 {
@@ -595,7 +604,6 @@ static int p54p_probe(struct pci_dev *pdev,
priv