linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Larry Finger <Larry.Finger@lwfinger.net>
Cc: John W Linville <linville@tuxdriver.com>,
	kay.sievers@vrfy.org, torvalds@linux-foundation.org,
	linux-wireless@vger.kernel.org
Subject: Re: [RFC] rtlwifi: Convert to asynchronous firmware load
Date: Thu, 19 Jan 2012 18:43:50 +0100	[thread overview]
Message-ID: <1326995030.3631.7.camel@jlt3.sipsolutions.net> (raw)
In-Reply-To: <4f18547e.Wk+PvRcReqCTbnIk%Larry.Finger@lwfinger.net> (sfid-20120119_183607_855723_7E85CDF5)

On Thu, 2012-01-19 at 11:35 -0600, Larry Finger wrote:

> This patch converts all rtlwifi-based drivers to use the asynchronous firmware
> loading mechanism. A completion queue entry is inititialized and the firmware
> load is started with the device initialization. The firmware-loaded callback routine
> issues a complete() call. The routine that uploads firmware to the device calls
> wait_for_completion_interruptible_timeout() with a 5 sec timeout.
> 
> The current patch is working, but as this is my first usage of the completion
> mechanism, I would like an expert to look it over. In particular, is it
> sufficient and prudent to issue a complete() call when the driver is unloaded
> so as to not leave a queue entry when the underlying struct is destroyed?

I don't think this makes a lot of sense. You're already registering with
mac80211, and then you wait inside start -- but that would be holding
the RTNL etc.

The way we do this in iwlwifi (and some others) makes more sense I
think: we only register with mac80211 after we actually load firmware,
i.e. in your rtl_fw_cb() you'd continue with ieee80211_register_hw() etc
instead of doing it in pci_probe(). If you never get firmware, you
obviously don't do that -- you unbind instead by calling
device_release_driver().

Doing that not only saves you the complexity of the completion but also
the potential deadlock (until timeout) when the tool that uploads the
firmware needs to acquire the rtnl for some reason and ifup is waiting
for the firmware.

johannes


      reply	other threads:[~2012-01-19 17:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-19 17:35 [RFC] rtlwifi: Convert to asynchronous firmware load Larry Finger
2012-01-19 17:43 ` Johannes Berg [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1326995030.3631.7.camel@jlt3.sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=Larry.Finger@lwfinger.net \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).