From: Samuel Ortiz <sameo@linux.intel.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: John Linville <linville@tuxdriver.com>,
linux-wireless@vger.kernel.org, Zhu Yi <yi.zhu@intel.com>,
Samuel Ortiz <samuel.ortiz@intel.com>
Subject: Re: [PATCH 01/12][2.6.31 and w-t] iwmc3200wifi: fix sdio initialisation race
Date: Mon, 15 Jun 2009 19:52:26 +0200 [thread overview]
Message-ID: <20090615175225.GF4094@sortiz.org> (raw)
In-Reply-To: <1245080647.23912.3.camel@johannes.local>
On Mon, Jun 15, 2009 at 05:44:07PM +0200, Johannes Berg wrote:
> On Mon, 2009-06-15 at 17:39 +0200, Samuel Ortiz wrote:
> > From: Samuel Ortiz <samuel.ortiz@intel.com>
> >
> > When modprobing this driver, the netdev interface is ready before the SDIO
> > function is correctly set.
> > This can lead to userspace calling ndo_open() before the SDIO bus is ready.
> > We fix that by returning an error from ndo_open() when the
> > IWM_STATUS_BUS_READY bit is not set yet.
>
> Can't you wait for it to be set instead?
I thought about it, but I didnt want to add one more completion/wait_queue
pointer to our already crowded iwm structures.
But if you or John insist on it, I can definitely do it.
Cheers,
Samuel.
> johannes
>
> > Signed-off-by: Samuel Ortiz <samuel.ortiz@intel.com>
> > ---
> > drivers/net/wireless/iwmc3200wifi/iwm.h | 11 ++++++-----
> > drivers/net/wireless/iwmc3200wifi/main.c | 6 +++++-
> > drivers/net/wireless/iwmc3200wifi/netdev.c | 10 ++++++----
> > drivers/net/wireless/iwmc3200wifi/sdio.c | 4 ++++
> > 4 files changed, 21 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/wireless/iwmc3200wifi/iwm.h b/drivers/net/wireless/iwmc3200wifi/iwm.h
> > index 635c16e..3cc4e91 100644
> > --- a/drivers/net/wireless/iwmc3200wifi/iwm.h
> > +++ b/drivers/net/wireless/iwmc3200wifi/iwm.h
> > @@ -180,11 +180,12 @@ struct iwm_key {
> >
> > #define IWM_SCAN_ID_MAX 0xff
> >
> > -#define IWM_STATUS_READY 0
> > -#define IWM_STATUS_SCANNING 1
> > -#define IWM_STATUS_SCAN_ABORTING 2
> > -#define IWM_STATUS_ASSOCIATING 3
> > -#define IWM_STATUS_ASSOCIATED 4
> > +#define IWM_STATUS_BUS_READY 0
> > +#define IWM_STATUS_READY 1
> > +#define IWM_STATUS_SCANNING 2
> > +#define IWM_STATUS_SCAN_ABORTING 3
> > +#define IWM_STATUS_ASSOCIATING 4
> > +#define IWM_STATUS_ASSOCIATED 5
> >
> > #define IWM_RADIO_RFKILL_OFF 0
> > #define IWM_RADIO_RFKILL_HW 1
> > diff --git a/drivers/net/wireless/iwmc3200wifi/main.c b/drivers/net/wireless/iwmc3200wifi/main.c
> > index 6a2640f..09ed247 100644
> > --- a/drivers/net/wireless/iwmc3200wifi/main.c
> > +++ b/drivers/net/wireless/iwmc3200wifi/main.c
> > @@ -227,11 +227,15 @@ int iwm_priv_init(struct iwm_priv *iwm)
> > void iwm_reset(struct iwm_priv *iwm)
> > {
> > struct iwm_notif *notif, *next;
> > + unsigned long status = 0;
> >
> > if (test_bit(IWM_STATUS_READY, &iwm->status))
> > iwm_target_reset(iwm);
> >
> > - iwm->status = 0;
> > + if (test_bit(IWM_STATUS_BUS_READY, &iwm->status))
> > + set_bit(IWM_STATUS_BUS_READY, &status);
> > +
> > + iwm->status = status;
> > iwm->scan_id = 1;
> >
> > list_for_each_entry_safe(notif, next, &iwm->pending_notif, pending) {
> > diff --git a/drivers/net/wireless/iwmc3200wifi/netdev.c b/drivers/net/wireless/iwmc3200wifi/netdev.c
> > index 68e2c3b..5b7aa34 100644
> > --- a/drivers/net/wireless/iwmc3200wifi/netdev.c
> > +++ b/drivers/net/wireless/iwmc3200wifi/netdev.c
> > @@ -54,9 +54,10 @@
> > static int iwm_open(struct net_device *ndev)
> > {
> > struct iwm_priv *iwm = ndev_to_iwm(ndev);
> > - int ret = 0;
> > + int ret = -ENODEV;
> >
> > - if (!test_bit(IWM_RADIO_RFKILL_SW, &iwm->radio))
> > + if (!test_bit(IWM_RADIO_RFKILL_SW, &iwm->radio)
> > + && test_bit(IWM_STATUS_BUS_READY, &iwm->status))
> > ret = iwm_up(iwm);
> >
> > return ret;
> > @@ -65,9 +66,10 @@ static int iwm_open(struct net_device *ndev)
> > static int iwm_stop(struct net_device *ndev)
> > {
> > struct iwm_priv *iwm = ndev_to_iwm(ndev);
> > - int ret = 0;
> > + int ret = -ENODEV;
> >
> > - if (!test_bit(IWM_RADIO_RFKILL_SW, &iwm->radio))
> > + if (!test_bit(IWM_RADIO_RFKILL_SW, &iwm->radio)
> > + && test_bit(IWM_STATUS_BUS_READY, &iwm->status))
> > ret = iwm_down(iwm);
> >
> > return ret;
> > diff --git a/drivers/net/wireless/iwmc3200wifi/sdio.c b/drivers/net/wireless/iwmc3200wifi/sdio.c
> > index b54da67..97e7da3 100644
> > --- a/drivers/net/wireless/iwmc3200wifi/sdio.c
> > +++ b/drivers/net/wireless/iwmc3200wifi/sdio.c
> > @@ -454,6 +454,9 @@ static int iwm_sdio_probe(struct sdio_func *func,
> >
> > INIT_WORK(&hw->isr_worker, iwm_sdio_isr_worker);
> >
> > + /* The SDIO bus is set and ready to be enabled */
> > + set_bit(IWM_STATUS_BUS_READY, &iwm->status);
> > +
> > dev_info(dev, "IWM SDIO probe\n");
> >
> > return 0;
> > @@ -476,6 +479,7 @@ static void iwm_sdio_remove(struct sdio_func *func)
> > destroy_workqueue(hw->isr_wq);
> >
> > sdio_set_drvdata(func, NULL);
> > + clear_bit(IWM_STATUS_BUS_READY, &iwm->status);
> >
> > dev_info(dev, "IWM SDIO remove\n");
> >
--
Intel Open Source Technology Centre
http://oss.intel.com/
next prev parent reply other threads:[~2009-06-15 17:50 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-06-15 15:39 [PATCH 00/12] iwmc3200wifi updates Samuel Ortiz
2009-06-15 15:39 ` [PATCH 01/12][2.6.31 and w-t] iwmc3200wifi: fix sdio initialisation race Samuel Ortiz
2009-06-15 15:44 ` Johannes Berg
2009-06-15 17:52 ` Samuel Ortiz [this message]
2009-06-15 15:39 ` [PATCH 02/12][2.6.31 and w-t] iwmc3200wifi: check for iwm_priv_init error Samuel Ortiz
2009-06-15 15:39 ` [PATCH 03/12][2.6.31 and w-t] iwmc3200wifi: add iwm_if_add and iwm_if_remove Samuel Ortiz
2009-06-15 15:39 ` [PATCH 04/12][2.6.31 and w-t] iwmc3200wifi: fix potential kernel oops on module removal Samuel Ortiz
2009-06-15 15:39 ` [PATCH 05/12][2.6.31 and w-t] iwmc3200wifi: add a mutex to protect iwm_reset_worker Samuel Ortiz
2009-06-15 15:39 ` [PATCH 06/12][w-t] iwmc3200wifi: invalidate keys when changing the BSSID Samuel Ortiz
2009-06-15 15:39 ` [PATCH 07/12][w-t] iwmc3200wifi: handling wifi_if_ntfy responses Samuel Ortiz
2009-06-15 15:39 ` [PATCH 08/12][w-t] iwmc3200wifi: cfg80211 key hooks implemetation Samuel Ortiz
2009-06-15 15:39 ` [PATCH 09/12][w-t] iwmc3200wifi: change coexist periodic calibration flag Samuel Ortiz
2009-06-15 15:39 ` [PATCH 10/12][w-t] iwmc3200wifi: cache keys when interface is down Samuel Ortiz
2009-06-15 15:39 ` [PATCH 11/12][w-t] iwmc3200wifi: remove select LIB80211 from Kconfig Samuel Ortiz
2009-06-15 15:39 ` [PATCH 12/12][w-t] iwmc3200wifi: rfkill cleanup Samuel Ortiz
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=20090615175225.GF4094@sortiz.org \
--to=sameo@linux.intel.com \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=samuel.ortiz@intel.com \
--cc=yi.zhu@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.