[PATCH 1/1] iwlwifi: fix error path of iwl_rfkill_init

[PATCH 1/1] iwlwifi: fix error path of iwl_rfkill_init

[Tomas Winkler]

This patch cleans rfkill error path. The problem was result of removing
the input device

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/net/wireless/iwlwifi/iwl-rfkill.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-rfkill.c b/drivers/net/wireless/iwlwifi/iwl-rfkill.c
index aa9f31e..eebaf43 100644
--- a/drivers/net/wireless/iwlwifi/iwl-rfkill.c
+++ b/drivers/net/wireless/iwlwifi/iwl-rfkill.c
@@ -101,17 +101,13 @@ int iwl_rfkill_init(struct iwl_priv *priv)
 	ret = rfkill_register(priv->rfkill_mngr.rfkill);
 	if (ret) {
 		IWL_ERROR("Unable to register rfkill: %d\n", ret);
-		goto unregister_rfkill;
+		goto free_rfkill;
 	}
 
 	IWL_DEBUG_RF_KILL("RFKILL initialization complete.\n");
 	return ret;
 
-unregister_rfkill:
-	rfkill_unregister(priv->rfkill_mngr.rfkill);
-	priv->rfkill_mngr.rfkill = NULL;
-
-freed_rfkill:
+free_rfkill:
 	if (priv->rfkill_mngr.rfkill != NULL)
 		rfkill_free(priv->rfkill_mngr.rfkill);
 	priv->rfkill_mngr.rfkill = NULL;
-- 
1.5.4.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[PATCH 1/2] mac80211: add bar capability

[Tomas Winkler]

From: Ron Rindjunsky <ron.rindjunsky@intel.com>

This patch adds block ack request capability

Signed-off-by: Ester Kummer <ester.kummer@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
---
 include/linux/ieee80211.h  |    4 ++++
 include/net/mac80211.h     |    3 +++
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/main.c        |   21 +++++++++++++++++++--
 net/mac80211/mlme.c        |   31 +++++++++++++++++++++++++++++++
 5 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index cffd6d0..aa603c3 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -658,6 +658,10 @@ struct ieee80211_bar {
 	__le16 start_seq_num;
 } __attribute__((packed));
 
+/* 802.11 BAR control masks */
+#define IEEE80211_BAR_CTRL_ACK_POLICY_NORMAL     0x0000
+#define IEEE80211_BAR_CTRL_CBMTID_COMPRESSED_BA  0x0004
+
 /**
  * struct ieee80211_ht_cap - HT capabilities
  *
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 3a204ac..0a5de3e 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -235,6 +235,8 @@ struct ieee80211_bss_conf {
  * @IEEE80211_TX_STAT_ACK: Frame was acknowledged
  * @IEEE80211_TX_STAT_AMPDU: The frame was aggregated, so status
  * 	is for the whole aggregation.
+ * @IEEE80211_TX_STAT_AMPDU_NO_BACK: no block ack was returned,
+ * 	so consider using block ack request (BAR).
  */
 enum mac80211_tx_control_flags {
 	IEEE80211_TX_CTL_REQ_TX_STATUS		= BIT(0),
@@ -260,6 +262,7 @@ enum mac80211_tx_control_flags {
 	IEEE80211_TX_STAT_TX_FILTERED		= BIT(20),
 	IEEE80211_TX_STAT_ACK			= BIT(21),
 	IEEE80211_TX_STAT_AMPDU			= BIT(22),
+	IEEE80211_TX_STAT_AMPDU_NO_BACK		= BIT(23),
 };
 
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index fcdbf09..775d5e8 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -903,6 +903,7 @@ void ieee80211_send_addba_request(struct net_device *dev, const u8 *da,
 				  u16 agg_size, u16 timeout);
 void ieee80211_send_delba(struct net_device *dev, const u8 *da, u16 tid,
 				u16 initiator, u16 reason_code);
+void ieee80211_send_bar(struct net_device *dev, u8 *ra, u16 tid, u16 ssn);
 
 void ieee80211_sta_stop_rx_ba_session(struct net_device *dev, u8 *da,
 				u16 tid, u16 initiator, u16 reason);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index f18cfd7..074f71a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1404,14 +1404,15 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 	struct ieee80211_local *local = hw_to_local(hw);
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	u16 frag, type;
+	__le16 fc;
 	struct ieee80211_tx_status_rtap_hdr *rthdr;
 	struct ieee80211_sub_if_data *sdata;
 	struct net_device *prev_dev = NULL;
+	struct sta_info *sta;
 
 	rcu_read_lock();
 
 	if (info->status.excessive_retries) {
-		struct sta_info *sta;
 		sta = sta_info_get(local, hdr->addr1);
 		if (sta) {
 			if (test_sta_flags(sta, WLAN_STA_PS)) {
@@ -1426,8 +1427,24 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 		}
 	}
 
+	fc = hdr->frame_control;
+
+	if ((info->flags & IEEE80211_TX_STAT_AMPDU_NO_BACK) &&
+	    (ieee80211_is_data_qos(fc))) {
+		u16 tid, ssn;
+		u8 *qc;
+		sta = sta_info_get(local, hdr->addr1);
+		if (sta) {
+			qc = ieee80211_get_qos_ctl(hdr);
+			tid = qc[0] & 0xf;
+			ssn = ((le16_to_cpu(hdr->seq_ctrl) + 0x10)
+						& IEEE80211_SCTL_SEQ);
+			ieee80211_send_bar(sta->sdata->dev, hdr->addr1,
+					   tid, ssn);
+		}
+	}
+
 	if (info->flags & IEEE80211_TX_STAT_TX_FILTERED) {
-		struct sta_info *sta;
 		sta = sta_info_get(local, hdr->addr1);
 		if (sta) {
 			ieee80211_handle_filtered_frame(local, sta, skb);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 0d50d52..283e55f 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1538,6 +1538,37 @@ void ieee80211_send_delba(struct net_device *dev, const u8 *da, u16 tid,
 	ieee80211_sta_tx(dev, skb, 0);
 }
 
+void ieee80211_send_bar(struct net_device *dev, u8 *ra, u16 tid, u16 ssn)
+{
+	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+	struct sk_buff *skb;
+	struct ieee80211_bar *bar;
+	u16 bar_control = 0;
+
+	skb = dev_alloc_skb(sizeof(*bar) + local->hw.extra_tx_headroom);
+	if (!skb) {
+		printk(KERN_ERR "%s: failed to allocate buffer for "
+			"bar frame\n", dev->name);
+		return;
+	}
+	skb_reserve(skb, local->hw.extra_tx_headroom);
+	bar = (struct ieee80211_bar *)skb_put(skb, sizeof(*bar));
+	memset(bar, 0, sizeof(*bar));
+	bar->frame_control = IEEE80211_FC(IEEE80211_FTYPE_CTL,
+					IEEE80211_STYPE_BACK_REQ);
+	memcpy(bar->ra, ra, ETH_ALEN);
+	memcpy(bar->ta, dev->dev_addr, ETH_ALEN);
+	bar_control |= (u16)IEEE80211_BAR_CTRL_ACK_POLICY_NORMAL;
+	bar_control |= (u16)IEEE80211_BAR_CTRL_CBMTID_COMPRESSED_BA;
+	bar_control |= (u16)(tid << 12);
+	bar->control = cpu_to_le16(bar_control);
+	bar->start_seq_num = cpu_to_le16(ssn);
+
+	/* must be sent through voice AC */
+	skb->priority = 3;
+	ieee80211_sta_tx(dev, skb, 0);
+}
+
 void ieee80211_sta_stop_rx_ba_session(struct net_device *dev, u8 *ra, u16 tid,
 					u16 initiator, u16 reason)
 {
-- 
1.5.4.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[PATCH 1/1] mac80211: fix warning: unused variable ifsta

[Tomas Winkler]

This patch fixes warning unused variable ifsta
when compiling without CONFIG_MAC80211_VERBOSE_DEBUG

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 net/mac80211/mlme.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 4fc843e..283e55f 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -366,8 +366,10 @@ static u32 ieee80211_handle_protect_preamb(struct ieee80211_sub_if_data *sdata,
 					   bool use_short_preamble)
 {
 	struct ieee80211_bss_conf *bss_conf = &sdata->bss_conf;
+#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
 	struct ieee80211_if_sta *ifsta = &sdata->u.sta;
 	DECLARE_MAC_BUF(mac);
+#endif
 	u32 changed = 0;
 
 	if (use_protection != bss_conf->use_cts_prot) {
-- 
1.5.4.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[PATCH 2/2] iwlwifi: request Tx of bar if necessary

[Tomas Winkler]

From: Ron Rindjunsky <ron.rindjunsky@intel.com>

This patch sets the block ack request flag if needed

Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/net/wireless/iwlwifi/iwl-4965.c |    6 +++---
 drivers/net/wireless/iwlwifi/iwl-5000.c |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 8c93f8d..04365b3 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -2285,9 +2285,9 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 
 		iwl4965_tx_status_reply_tx(priv, agg, tx_resp, txq_id, index);
 
-		if ((tx_resp->frame_count == 1) && !iwl_is_tx_success(status)) {
-			/* TODO: send BAR */
-		}
+		/* check if BAR is needed */
+		if ((tx_resp->frame_count == 1) && !iwl_is_tx_success(status))
+			info->flags |= IEEE80211_TX_STAT_AMPDU_NO_BACK;
 
 		if (txq->q.read_ptr != (scd_ssn & 0xff)) {
 			int freed, ampdu_q;
diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index 7cc73e9..717db0d 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -1278,9 +1278,9 @@ static void iwl5000_rx_reply_tx(struct iwl_priv *priv,
 
 		iwl5000_tx_status_reply_tx(priv, agg, tx_resp, txq_id, index);
 
-		if ((tx_resp->frame_count == 1) && !iwl_is_tx_success(status)) {
-			/* TODO: send BAR */
-		}
+		/* check if BAR is needed */
+		if ((tx_resp->frame_count == 1) && !iwl_is_tx_success(status))
+			info->flags |= IEEE80211_TX_STAT_AMPDU_NO_BACK;
 
 		if (txq->q.read_ptr != (scd_ssn & 0xff)) {
 			int freed, ampdu_q;
-- 
1.5.4.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH 1/1] mac80211: fix warning: unused variable ifsta

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

On Tue, 2008-07-01 at 10:44 +0300, Tomas Winkler wrote:
> This patch fixes warning unused variable ifsta
> when compiling without CONFIG_MAC80211_VERBOSE_DEBUG

Thanks.

> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
>  net/mac80211/mlme.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 4fc843e..283e55f 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -366,8 +366,10 @@ static u32 ieee80211_handle_protect_preamb(struct ieee80211_sub_if_data *sdata,
>  					   bool use_short_preamble)
>  {
>  	struct ieee80211_bss_conf *bss_conf = &sdata->bss_conf;
> +#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
>  	struct ieee80211_if_sta *ifsta = &sdata->u.sta;
>  	DECLARE_MAC_BUF(mac);
> +#endif
>  	u32 changed = 0;
>  
>  	if (use_protection != bss_conf->use_cts_prot) {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] mac80211: add bar capability

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]

Can you capitalise BAR in the patch subjects please? I think of an
actual bar when reading that :)

> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -235,6 +235,8 @@ struct ieee80211_bss_conf {
>   * @IEEE80211_TX_STAT_ACK: Frame was acknowledged
>   * @IEEE80211_TX_STAT_AMPDU: The frame was aggregated, so status
>   * 	is for the whole aggregation.
> + * @IEEE80211_TX_STAT_AMPDU_NO_BACK: no block ack was returned,
> + * 	so consider using block ack request (BAR).

This flag I don't understand. When does the driver set this? And what
does mac80211 do to "consider", it seems to always just try to set up a
block-ack session.

I'd still loke to see a "HT operation" document that describes how all
this works together so other people can write HT capable drivers :)

> +	bar_control |= (u16)IEEE80211_BAR_CTRL_ACK_POLICY_NORMAL;

??

> +	bar_control |= (u16)IEEE80211_BAR_CTRL_CBMTID_COMPRESSED_BA;
> +	bar_control |= (u16)(tid << 12);
> +	bar->control = cpu_to_le16(bar_control);
> +	bar->start_seq_num = cpu_to_le16(ssn);
> +
> +	/* must be sent through voice AC */
> +	skb->priority = 3;

Does that actually work? The current wme.c code will still call
classify_1d() and that will use the IP TOS unless you set the priority
to 256..263.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] mac80211: add bar capability

[Tomas Winkler]

On Tue, Jul 1, 2008 at 11:03 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> Can you capitalise BAR in the patch subjects please? I think of an
> actual bar when reading that :)
>
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -235,6 +235,8 @@ struct ieee80211_bss_conf {
>>   * @IEEE80211_TX_STAT_ACK: Frame was acknowledged
>>   * @IEEE80211_TX_STAT_AMPDU: The frame was aggregated, so status
>>   *   is for the whole aggregation.
>> + * @IEEE80211_TX_STAT_AMPDU_NO_BACK: no block ack was returned,
>> + *   so consider using block ack request (BAR).
>
> This flag I don't understand. When does the driver set this? And what
> does mac80211 do to "consider", it seems to always just try to set up a
> block-ack session.

This is required during BA session. BAR solicited Block ACK for the SN
and on specified in BAR. Driver will send it if it doesn't hear BACK.
Iwlwifi uses this BAR just says to the recipient that we were not able
to transmit the frame and we won't retransmit. This immediately shifts
recipient reordering buffer not waiting for high SN numbers of the
next traffic otherwise there will be visible deep in the throughput as
packets are not released to upper layer till all are received.

>
> I'd still loke to see a "HT operation" document that describes how all
> this works together so other people can write HT capable drivers :)

Hope the above helps.

>> +     bar_control |= (u16)IEEE80211_BAR_CTRL_ACK_POLICY_NORMAL;
>
> ??
>
>> +     bar_control |= (u16)IEEE80211_BAR_CTRL_CBMTID_COMPRESSED_BA;
>> +     bar_control |= (u16)(tid << 12);
>> +     bar->control = cpu_to_le16(bar_control);
>> +     bar->start_seq_num = cpu_to_le16(ssn);
>> +
>> +     /* must be sent through voice AC */
>> +     skb->priority = 3;
>
> Does that actually work? The current wme.c code will still call
> classify_1d() and that will use the IP TOS unless you set the priority
> to 256..263.

Good point. Have to look at this again we might missed this point when
moving this out from the driver to the mac8011.
Not sure this is classified at all as this is a control frame.
Thanks for review
Tomas

Re: [PATCH 1/2] mac80211: add bar capability

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1961 bytes --]


> >> + * @IEEE80211_TX_STAT_AMPDU_NO_BACK: no block ack was returned,
> >> + *   so consider using block ack request (BAR).
> >
> > This flag I don't understand. When does the driver set this? And what
> > does mac80211 do to "consider", it seems to always just try to set up a
> > block-ack session.
> 
> This is required during BA session. BAR solicited Block ACK for the SN
> and on specified in BAR. Driver will send it if it doesn't hear BACK.
> Iwlwifi uses this BAR just says to the recipient that we were not able
> to transmit the frame and we won't retransmit. This immediately shifts
> recipient reordering buffer not waiting for high SN numbers of the
> next traffic otherwise there will be visible deep in the throughput as
> packets are not released to upper layer till all are received.

Hm, ok, I don't think I understand the subtleties of the protocol. So
when you have a BA session you have to send a BAR if you can't transmit
a frame for some reason? When would that happen?

> >> +     bar_control |= (u16)IEEE80211_BAR_CTRL_ACK_POLICY_NORMAL;
> >
> > ??
> >
> >> +     bar_control |= (u16)IEEE80211_BAR_CTRL_CBMTID_COMPRESSED_BA;
> >> +     bar_control |= (u16)(tid << 12);
> >> +     bar->control = cpu_to_le16(bar_control);
> >> +     bar->start_seq_num = cpu_to_le16(ssn);
> >> +
> >> +     /* must be sent through voice AC */
> >> +     skb->priority = 3;
> >
> > Does that actually work? The current wme.c code will still call
> > classify_1d() and that will use the IP TOS unless you set the priority
> > to 256..263.
> 
> Good point. Have to look at this again we might missed this point when
> moving this out from the driver to the mac8011.
> Not sure this is classified at all as this is a control frame.

Oh, hmm, yeah those work differently, non-data frames aren't classified
at all and always go out on AC_VO regardless of what you set there so
you can just remove that.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] mac80211: add bar capability

[Tomas Winkler]

On Tue, Jul 1, 2008 at 11:52 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>> >> + * @IEEE80211_TX_STAT_AMPDU_NO_BACK: no block ack was returned,
>> >> + *   so consider using block ack request (BAR).
>> >
>> > This flag I don't understand. When does the driver set this? And what
>> > does mac80211 do to "consider", it seems to always just try to set up a
>> > block-ack session.
>>
>> This is required during BA session. BAR solicited Block ACK for the SN
>> and on specified in BAR. Driver will send it if it doesn't hear BACK.
>> Iwlwifi uses this BAR just says to the recipient that we were not able
>> to transmit the frame and we won't retransmit. This immediately shifts
>> recipient reordering buffer not waiting for high SN numbers of the
>> next traffic otherwise there will be visible deep in the throughput as
>> packets are not released to upper layer till all are received.
>
> Hm, ok, I don't think I understand the subtleties of the protocol. So
> when you have a BA session you have to send a BAR if you can't transmit
> a frame for some reason? When would that happen?

Driver should send BAR when it doesn't hear BACK. Iwlwifi requests
this only  when both aggregation and single frame retries are
exhausted.  See some APs sends this more often.

>
>> >> +     bar_control |= (u16)IEEE80211_BAR_CTRL_ACK_POLICY_NORMAL;
>> >
>> > ??
>> >
>> >> +     bar_control |= (u16)IEEE80211_BAR_CTRL_CBMTID_COMPRESSED_BA;
>> >> +     bar_control |= (u16)(tid << 12);
>> >> +     bar->control = cpu_to_le16(bar_control);
>> >> +     bar->start_seq_num = cpu_to_le16(ssn);
>> >> +
>> >> +     /* must be sent through voice AC */
>> >> +     skb->priority = 3;
>> >
>> > Does that actually work? The current wme.c code will still call
>> > classify_1d() and that will use the IP TOS unless you set the priority
>> > to 256..263.
>>
>> Good point. Have to look at this again we might missed this point when
>> moving this out from the driver to the mac8011.
>> Not sure this is classified at all as this is a control frame.
>
> Oh, hmm, yeah those work differently, non-data frames aren't classified
> at all and always go out on AC_VO regardless of what you set there so
> you can just remove that.

..okay I will remove this
Tomas

RE: [PATCH 1/1] iwlwifi: fix error path of iwl_rfkill_init

[Zhu, Yi]

John, looks like you didn't merge the last patch "[PATCH 33/33] iwlwifi=
: remove the input device from rfkill_mngr" from me but merged an earli=
er patch from Adel directly. Tomas' this patch fixes the iwl-rfkill.c p=
art, but we should also remove the input layer for 3945. Do you want me=
 to submit another patch on top of the current tip or you can handle it=
 yourself.

http://marc.info/?l=3Dlinux-wireless&m=3D121481826531792&w=3D2

Thanks,
-yi

> -----Original Message-----
> From: Winkler, Tomas
> Sent: 2008=C4=EA7=D4=C21=C8=D5 15:45
> To: linville@tuxdriver.com; johannes@sipsolutions.net; Zhu, Yi
> Cc: linux-wireless@vger.kernel.org; Winkler, Tomas
> Subject: [PATCH 1/1] iwlwifi: fix error path of iwl_rfkill_init
>=20
> This patch cleans rfkill error path. The problem was result of removi=
ng
> the input device
>=20
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-rfkill.c |    8 ++------
>  1 files changed, 2 insertions(+), 6 deletions(-)
>=20
> diff --git a/drivers/net/wireless/iwlwifi/iwl-rfkill.c
> b/drivers/net/wireless/iwlwifi/iwl-rfkill.c
> index aa9f31e..eebaf43 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-rfkill.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-rfkill.c
> @@ -101,17 +101,13 @@ int iwl_rfkill_init(struct iwl_priv *priv)
>  	ret =3D rfkill_register(priv->rfkill_mngr.rfkill);
>  	if (ret) {
>  		IWL_ERROR("Unable to register rfkill: %d\n", ret);
> -		goto unregister_rfkill;
> +		goto free_rfkill;
>  	}
>=20
>  	IWL_DEBUG_RF_KILL("RFKILL initialization complete.\n");
>  	return ret;
>=20
> -unregister_rfkill:
> -	rfkill_unregister(priv->rfkill_mngr.rfkill);
> -	priv->rfkill_mngr.rfkill =3D NULL;
> -
> -freed_rfkill:
> +free_rfkill:
>  	if (priv->rfkill_mngr.rfkill !=3D NULL)
>  		rfkill_free(priv->rfkill_mngr.rfkill);
>  	priv->rfkill_mngr.rfkill =3D NULL;
> --
> 1.5.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] iwlwifi: remove input device from iwl3945

[John W. Linville]

I merged an early version of a patch and missed some bits included in a
later version.

	http://marc.info/?l=linux-wireless&m=121481826531792&w=2

I have replicated the missing bits here.

Cc: Adel Gadllah <adel.gadllah@gmail.com>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Ivo van Doorn <ivdoorn@gmail.com>
Cc: Fabien Crespel <fcrespel@gmail.com>
Cc: Zhu Yi <yi.zhu@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/wireless/iwlwifi/Kconfig        |    5 +++
 drivers/net/wireless/iwlwifi/iwl-3945.h     |    4 +-
 drivers/net/wireless/iwlwifi/iwl3945-base.c |   38 +-------------------------
 3 files changed, 9 insertions(+), 38 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/Kconfig b/drivers/net/wireless/iwlwifi/Kconfig
index b628a44..82b66a3 100644
--- a/drivers/net/wireless/iwlwifi/Kconfig
+++ b/drivers/net/wireless/iwlwifi/Kconfig
@@ -104,6 +104,7 @@ config IWL3945
 	select IWLWIFI
 	select MAC80211_LEDS if IWL3945_LEDS
 	select LEDS_CLASS if IWL3945_LEDS
+	select RFKILL if IWL3945_RFKILL
 	---help---
 	  Select to build the driver supporting the:
 
@@ -126,6 +127,10 @@ config IWL3945
 	  say M here and read <file:Documentation/kbuild/modules.txt>.  The
 	  module will be called iwl3945.ko.
 
+config IWL3945_RFKILL
+	bool "Enable RF kill support in iwl3945 drivers"
+	depends on IWL3945
+
 config IWL3945_SPECTRUM_MEASUREMENT
 	bool "Enable Spectrum Measurement in iwl3945 drivers"
 	depends on IWL3945
diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h b/drivers/net/wireless/iwlwifi/iwl-3945.h
index a774978..4df42ad 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.h
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
@@ -690,7 +690,7 @@ enum {
 
 #endif
 
-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
 struct iwl3945_priv;
 
 struct iwl3945_rfkill_mngr {
@@ -800,7 +800,7 @@ struct iwl3945_priv {
 	struct iwl3945_init_alive_resp card_alive_init;
 	struct iwl3945_alive_resp card_alive;
 
-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
 	struct iwl3945_rfkill_mngr rfkill_mngr;
 #endif
 
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 3bc2644..a604a29 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -8270,7 +8270,7 @@ static int iwl3945_pci_resume(struct pci_dev *pdev)
 #endif /* CONFIG_PM */
 
 /*************** RFKILL FUNCTIONS **********/
-#ifdef CONFIG_IWLWIFI_RFKILL
+#ifdef CONFIG_IWL3945_RFKILL
 /* software rf-kill from user */
 static int iwl3945_rfkill_soft_rf_kill(void *data, enum rfkill_state state)
 {
@@ -8331,44 +8331,15 @@ int iwl3945_rfkill_init(struct iwl3945_priv *priv)
 	priv->rfkill_mngr.rfkill->dev.class->suspend = NULL;
 	priv->rfkill_mngr.rfkill->dev.class->resume = NULL;
 
-	priv->rfkill_mngr.input_dev = input_allocate_device();
-	if (!priv->rfkill_mngr.input_dev) {
-		IWL_ERROR("Unable to allocate rfkill input device.\n");
-		ret = -ENOMEM;
-		goto freed_rfkill;
-	}
-
-	priv->rfkill_mngr.input_dev->name = priv->cfg->name;
-	priv->rfkill_mngr.input_dev->phys = wiphy_name(priv->hw->wiphy);
-	priv->rfkill_mngr.input_dev->id.bustype = BUS_HOST;
-	priv->rfkill_mngr.input_dev->id.vendor = priv->pci_dev->vendor;
-	priv->rfkill_mngr.input_dev->dev.parent = device;
-	priv->rfkill_mngr.input_dev->evbit[0] = BIT(EV_KEY);
-	set_bit(KEY_WLAN, priv->rfkill_mngr.input_dev->keybit);
-
 	ret = rfkill_register(priv->rfkill_mngr.rfkill);
 	if (ret) {
 		IWL_ERROR("Unable to register rfkill: %d\n", ret);
-		goto free_input_dev;
-	}
-
-	ret = input_register_device(priv->rfkill_mngr.input_dev);
-	if (ret) {
-		IWL_ERROR("Unable to register rfkill input device: %d\n", ret);
-		goto unregister_rfkill;
+		goto freed_rfkill;
 	}
 
 	IWL_DEBUG_RF_KILL("RFKILL initialization complete.\n");
 	return ret;
 
-unregister_rfkill:
-	rfkill_unregister(priv->rfkill_mngr.rfkill);
-	priv->rfkill_mngr.rfkill = NULL;
-
-free_input_dev:
-	input_free_device(priv->rfkill_mngr.input_dev);
-	priv->rfkill_mngr.input_dev = NULL;
-
 freed_rfkill:
 	if (priv->rfkill_mngr.rfkill != NULL)
 		rfkill_free(priv->rfkill_mngr.rfkill);
@@ -8381,14 +8352,9 @@ error:
 
 void iwl3945_rfkill_unregister(struct iwl3945_priv *priv)
 {
-
-	if (priv->rfkill_mngr.input_dev)
-		input_unregister_device(priv->rfkill_mngr.input_dev);
-
 	if (priv->rfkill_mngr.rfkill)
 		rfkill_unregister(priv->rfkill_mngr.rfkill);
 
-	priv->rfkill_mngr.input_dev = NULL;
 	priv->rfkill_mngr.rfkill = NULL;
 }
 
-- 
1.5.5.1

Re: [PATCH] iwlwifi: remove input device from iwl3945

[Ivo van Doorn]

On Tuesday 01 July 2008, John W. Linville wrote:
> I merged an early version of a patch and missed some bits included in a
> later version.
> 
> 	http://marc.info/?l=linux-wireless&m=121481826531792&w=2
> 
> I have replicated the missing bits here.

Not a problem in this or above mentioned patch, but more a comment
on rfkill handling in the iwlwifi driver.

How is the changed rfkill state of the driver reported to rfkill?
I see that the patch to remove the input device doesn't remove the
call input_report_key() by which I assume such call was never made.
This would mean rfkill support in iwlwifi has always been broken and
the entire input device wasn't used at all.

>From what I gather from the code itself is the function responsible
for changing the state iwl_rfkill_set_hw_state() but that only updates
the state, and doesn't report anything to the rfkill layer.

Ivo

> Cc: Adel Gadllah <adel.gadllah@gmail.com>
> Cc: Randy Dunlap <randy.dunlap@oracle.com>
> Cc: Ivo van Doorn <ivdoorn@gmail.com>
> Cc: Fabien Crespel <fcrespel@gmail.com>
> Cc: Zhu Yi <yi.zhu@intel.com>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---
>  drivers/net/wireless/iwlwifi/Kconfig        |    5 +++
>  drivers/net/wireless/iwlwifi/iwl-3945.h     |    4 +-
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |   38 +-------------------------
>  3 files changed, 9 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/Kconfig b/drivers/net/wireless/iwlwifi/Kconfig
> index b628a44..82b66a3 100644
> --- a/drivers/net/wireless/iwlwifi/Kconfig
> +++ b/drivers/net/wireless/iwlwifi/Kconfig
> @@ -104,6 +104,7 @@ config IWL3945
>  	select IWLWIFI
>  	select MAC80211_LEDS if IWL3945_LEDS
>  	select LEDS_CLASS if IWL3945_LEDS
> +	select RFKILL if IWL3945_RFKILL
>  	---help---
>  	  Select to build the driver supporting the:
>  
> @@ -126,6 +127,10 @@ config IWL3945
>  	  say M here and read <file:Documentation/kbuild/modules.txt>.  The
>  	  module will be called iwl3945.ko.
>  
> +config IWL3945_RFKILL
> +	bool "Enable RF kill support in iwl3945 drivers"
> +	depends on IWL3945
> +
>  config IWL3945_SPECTRUM_MEASUREMENT
>  	bool "Enable Spectrum Measurement in iwl3945 drivers"
>  	depends on IWL3945
> diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h b/drivers/net/wireless/iwlwifi/iwl-3945.h
> index a774978..4df42ad 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-3945.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
> @@ -690,7 +690,7 @@ enum {
>  
>  #endif
>  
> -#ifdef CONFIG_IWLWIFI_RFKILL
> +#ifdef CONFIG_IWL3945_RFKILL
>  struct iwl3945_priv;
>  
>  struct iwl3945_rfkill_mngr {
> @@ -800,7 +800,7 @@ struct iwl3945_priv {
>  	struct iwl3945_init_alive_resp card_alive_init;
>  	struct iwl3945_alive_resp card_alive;
>  
> -#ifdef CONFIG_IWLWIFI_RFKILL
> +#ifdef CONFIG_IWL3945_RFKILL
>  	struct iwl3945_rfkill_mngr rfkill_mngr;
>  #endif
>  
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index 3bc2644..a604a29 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -8270,7 +8270,7 @@ static int iwl3945_pci_resume(struct pci_dev *pdev)
>  #endif /* CONFIG_PM */
>  
>  /*************** RFKILL FUNCTIONS **********/
> -#ifdef CONFIG_IWLWIFI_RFKILL
> +#ifdef CONFIG_IWL3945_RFKILL
>  /* software rf-kill from user */
>  static int iwl3945_rfkill_soft_rf_kill(void *data, enum rfkill_state state)
>  {
> @@ -8331,44 +8331,15 @@ int iwl3945_rfkill_init(struct iwl3945_priv *priv)
>  	priv->rfkill_mngr.rfkill->dev.class->suspend = NULL;
>  	priv->rfkill_mngr.rfkill->dev.class->resume = NULL;
>  
> -	priv->rfkill_mngr.input_dev = input_allocate_device();
> -	if (!priv->rfkill_mngr.input_dev) {
> -		IWL_ERROR("Unable to allocate rfkill input device.\n");
> -		ret = -ENOMEM;
> -		goto freed_rfkill;
> -	}
> -
> -	priv->rfkill_mngr.input_dev->name = priv->cfg->name;
> -	priv->rfkill_mngr.input_dev->phys = wiphy_name(priv->hw->wiphy);
> -	priv->rfkill_mngr.input_dev->id.bustype = BUS_HOST;
> -	priv->rfkill_mngr.input_dev->id.vendor = priv->pci_dev->vendor;
> -	priv->rfkill_mngr.input_dev->dev.parent = device;
> -	priv->rfkill_mngr.input_dev->evbit[0] = BIT(EV_KEY);
> -	set_bit(KEY_WLAN, priv->rfkill_mngr.input_dev->keybit);
> -
>  	ret = rfkill_register(priv->rfkill_mngr.rfkill);
>  	if (ret) {
>  		IWL_ERROR("Unable to register rfkill: %d\n", ret);
> -		goto free_input_dev;
> -	}
> -
> -	ret = input_register_device(priv->rfkill_mngr.input_dev);
> -	if (ret) {
> -		IWL_ERROR("Unable to register rfkill input device: %d\n", ret);
> -		goto unregister_rfkill;
> +		goto freed_rfkill;
>  	}
>  
>  	IWL_DEBUG_RF_KILL("RFKILL initialization complete.\n");
>  	return ret;
>  
> -unregister_rfkill:
> -	rfkill_unregister(priv->rfkill_mngr.rfkill);
> -	priv->rfkill_mngr.rfkill = NULL;
> -
> -free_input_dev:
> -	input_free_device(priv->rfkill_mngr.input_dev);
> -	priv->rfkill_mngr.input_dev = NULL;
> -
>  freed_rfkill:
>  	if (priv->rfkill_mngr.rfkill != NULL)
>  		rfkill_free(priv->rfkill_mngr.rfkill);
> @@ -8381,14 +8352,9 @@ error:
>  
>  void iwl3945_rfkill_unregister(struct iwl3945_priv *priv)
>  {
> -
> -	if (priv->rfkill_mngr.input_dev)
> -		input_unregister_device(priv->rfkill_mngr.input_dev);
> -
>  	if (priv->rfkill_mngr.rfkill)
>  		rfkill_unregister(priv->rfkill_mngr.rfkill);
>  
> -	priv->rfkill_mngr.input_dev = NULL;
>  	priv->rfkill_mngr.rfkill = NULL;
>  }
>  

Re: [PATCH] iwlwifi: remove input device from iwl3945

[Zhu Yi]

On Tue, 2008-07-01 at 16:13 +0200, Ivo van Doorn wrote:
> How is the changed rfkill state of the driver reported to rfkill?
> I see that the patch to remove the input device doesn't remove the
> call input_report_key() by which I assume such call was never made.
> This would mean rfkill support in iwlwifi has always been broken and
> the entire input device wasn't used at all.

Yes, we don't use the input device before and I believe we shouldn't.
The patch just removes it.

> From what I gather from the code itself is the function responsible
> for changing the state iwl_rfkill_set_hw_state() but that only updates
> the state, and doesn't report anything to the rfkill layer.

Right, we will switch to use rfkill_force_state() instead.

Thanks,
-yi