linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: reinette chatre <reinette.chatre@intel.com>
To: Helmut Schaa <helmut.schaa@googlemail.com>
Cc: "linville@tuxdriver.com" <linville@tuxdriver.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"ipw3945-devel@lists.sourceforge.net"
	<ipw3945-devel@lists.sourceforge.net>,
	"mabbaswireless@gmail.com" <mabbaswireless@gmail.com>,
	"Winkler, Tomas" <tomas.winkler@intel.com>,
	"Zhu, Yi" <yi.zhu@intel.com>,
	"samuel@sortiz.org" <samuel@sortiz.org>
Subject: Re: [PATCHv2] iwlagn: fix hw-rfkill while the interface is down
Date: Tue, 20 Jan 2009 10:21:05 -0800	[thread overview]
Message-ID: <1232475665.11197.77.camel@rc-desk> (raw)
In-Reply-To: <200901191310.08654.helmut.schaa@gmail.com>

On Mon, 2009-01-19 at 04:10 -0800, Helmut Schaa wrote:
> Currently iwlagn is not able to report hw-killswitch events while the
> interface is down. This has implications on user space tools (like
> NetworkManager) relying on rfkill notifications to bring the interface
> up once the wireless gets enabled through a hw killswitch.
> 
> Thus, enable the device already in iwl_pci_probe instead of iwl_up
> and enable interrups while the interface is down in order to get
> notified about killswitch state changes. The firmware loading is still
> done in iwl_up.
> 
> Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
> ---
> 

<...>


> +               if (!test_bit(STATUS_ALIVE, &priv->status)) {

Is this test necessary? If the intention is to get rfkill state updates
when interface is down (and ucode is thus not loaded, and STATUS_ALIVE
thus not set) then this test is not necessary.

> +                       if (hw_rf_kill)
> +                               set_bit(STATUS_RF_KILL_HW, &priv->status);
> +                       else
> +                               clear_bit(STATUS_RF_KILL_HW, &priv->status);
> +                       queue_work(priv->workqueue, &priv->rf_kill);
>                 }
> 
>                 handled |= CSR_INT_BIT_RF_KILL;
> @@ -2158,7 +2161,8 @@ static void iwl_bg_rf_kill(struct work_struct *work)
>                 IWL_DEBUG(IWL_DL_RF_KILL,
>                           "HW and/or SW RF Kill no longer active, restarting "
>                           "device\n");
> -               if (!test_bit(STATUS_EXIT_PENDING, &priv->status))
> +               if (!test_bit(STATUS_EXIT_PENDING, &priv->status) &&
> +                    test_bit(STATUS_ALIVE, &priv->status))

This ties in with the question above. Above the work is scheduled when
STATUS_ALIVE is not set ... having this test here encourages me to think
that the above test for STATUS_ALIVE is not necessary.

The rest of the patch looks good.

Thank you very much

Reinette




  reply	other threads:[~2009-01-20 18:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-19 12:10 [PATCHv2] iwlagn: fix hw-rfkill while the interface is down Helmut Schaa
2009-01-20 18:21 ` reinette chatre [this message]
2009-01-20 21:25   ` Helmut Schaa
2009-01-20 21:42     ` reinette chatre

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=1232475665.11197.77.camel@rc-desk \
    --to=reinette.chatre@intel.com \
    --cc=helmut.schaa@googlemail.com \
    --cc=ipw3945-devel@lists.sourceforge.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=mabbaswireless@gmail.com \
    --cc=samuel@sortiz.org \
    --cc=tomas.winkler@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 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).