* Re: [patch -next] wl1251: wl12xx_get_platform_data() returns an ERR_PTR
From: Kalle Valo @ 2010-12-18 9:12 UTC (permalink / raw)
To: Dan Carpenter; +Cc: John W. Linville, linux-wireless, kernel-janitors
In-Reply-To: <20101218085210.GO1639@bicker>
Dan Carpenter <error27@gmail.com> writes:
> wl12xx_get_platform_data() returns an ERR_PTR on failure and it never
> returns a NULL.
Yes, that's true. Good that you found this.
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Acked-by: Kalle Valo <kvalo@adurom.com>
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] ath9k: Properly initialize channel table for 2GHz
From: Mohammed Shafi @ 2010-12-18 10:39 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org
In-Reply-To: <AANLkTiky2ODWt-ocSy_+=9k9_E=7ekvLMLbQtir-cd-r@mail.gmail.com>
On Saturday 18 December 2010 01:21 AM, Luis R. Rodriguez wrote:
> On Fri, Dec 17, 2010 at 7:14 AM, Mohammed Shafi Shajakhan
> <mshajakhan@atheros.com> wrote:
>
>> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>
>> ath9k channel table for 2Ghz does not seems to initialize the 'band'
>> parameter.Though it does not seems to cause any visible issue it looks
>> odd when we initialize the 'band' parameter for 5Ghz channel table while
>> not so for 2Ghz.
>>
>> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>> ---
>> drivers/net/wireless/ath/ath9k/init.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
>> index 1238795..b0e5e71 100644
>> --- a/drivers/net/wireless/ath/ath9k/init.c
>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>> @@ -48,6 +48,7 @@ MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>> /* We use the hw_value as an index into our private channel structure */
>>
>> #define CHAN2G(_freq, _idx) { \
>> + .band = IEEE80211_BAND_2GHZ, \
>> .center_freq = (_freq), \
>> .hw_value = (_idx), \
>> .max_power = 20, \
>>
> That's fine but its not needed at all as the band is already
> kzalloc()'d so it gets zeroed out and the IEEE80211_BAND_2GHZ enum is
> 0, so its not required to be explicit and set it. But if it gives you
> the warm fuzzies sure.
>
Oh ok fine, then no problem. I could not trace any warnings if 'band'
parameter was not used, as the code looked suspicious I had sent a patch.
thanks,
shafi
> Luis
>
^ permalink raw reply
* Re: [PATCH] orinoco_cs: fix too early irq request
From: Dave Kilroy @ 2010-12-18 10:58 UTC (permalink / raw)
To: Meelis Roos; +Cc: linux-wireless
In-Reply-To: <alpine.SOC.1.00.1012180014390.26257@math.ut.ee>
Thanks Meelis. An equivalent patch has already been merged in Linus'
v2.6.37-rc6 (229bd792b), and wireless-next.
Does that version also work for you?
On Fri, Dec 17, 2010 at 10:18 PM, Meelis Roos <mroos@linux.ee> wrote:
> orinoco_cs requests its irq too early. This results in irq coming in
> right afrer request_irq and before initializing other data structures,
> resulting in NULL pointer dereference on module load.
>
> Fix it by moving request_irq after other initialization tasks.
>
> Tested to work with Orinoco Gold PCMCIA card.
>
> Signed-off-by: Meelis Roos <mroos@linux.ee>
>
> diff --git a/drivers/net/wireless/orinoco/orinoco_cs.c b/drivers/net/wireless/orinoco/orinoco_cs.c
> index 71b3d68..47fc408 100644
> --- a/drivers/net/wireless/orinoco/orinoco_cs.c
> +++ b/drivers/net/wireless/orinoco/orinoco_cs.c
> @@ -151,10 +151,6 @@ orinoco_cs_config(struct pcmcia_device *link)
> goto failed;
> }
>
> - ret = pcmcia_request_irq(link, orinoco_interrupt);
> - if (ret)
> - goto failed;
> -
> /* We initialize the hermes structure before completing PCMCIA
> * configuration just in case the interrupt handler gets
> * called. */
> @@ -182,6 +178,10 @@ orinoco_cs_config(struct pcmcia_device *link)
> goto failed;
> }
>
> + ret = pcmcia_request_irq(link, orinoco_interrupt);
> + if (ret)
> + goto failed;
> +
> return 0;
>
> failed:
>
> --
> Meelis Roos (mroos@linux.ee)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH] rndis_wlan: scanning, workaround device returning incorrect bssid-list item count.
From: Jussi Kivilinna @ 2010-12-18 14:59 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless
Sometimes device returns wrong number of items in bssid-list. Appears that some specific beacons
trigger this problem and leads to very poor scanning results. Workaround by ignoring num_items
received from device and walkthrough full bssid-list buffer.
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
---
drivers/net/wireless/rndis_wlan.c | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 4a4f005..8db8333 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1967,8 +1967,8 @@ static struct cfg80211_bss *rndis_bss_info_update(struct usbnet *usbdev,
int ie_len, bssid_len;
u8 *ie;
- netdev_dbg(usbdev->net, " found bssid: '%.32s' [%pM]\n",
- bssid->ssid.essid, bssid->mac);
+ netdev_dbg(usbdev->net, " found bssid: '%.32s' [%pM], len: %d\n",
+ bssid->ssid.essid, bssid->mac, le32_to_cpu(bssid->length));
/* parse bssid structure */
bssid_len = le32_to_cpu(bssid->length);
@@ -2008,10 +2008,10 @@ static int rndis_check_bssid_list(struct usbnet *usbdev, u8 *match_bssid,
void *buf = NULL;
struct ndis_80211_bssid_list_ex *bssid_list;
struct ndis_80211_bssid_ex *bssid;
- int ret = -EINVAL, len, count, bssid_len;
+ int ret = -EINVAL, len, count, bssid_len, real_count;
bool resized = false;
- netdev_dbg(usbdev->net, "check_bssid_list\n");
+ netdev_dbg(usbdev->net, "%s\n", __func__);
len = CONTROL_BUFFER_SIZE;
resize_buf:
@@ -2035,10 +2035,13 @@ resize_buf:
bssid = bssid_list->bssid;
bssid_len = le32_to_cpu(bssid->length);
count = le32_to_cpu(bssid_list->num_items);
- netdev_dbg(usbdev->net, "check_bssid_list: %d BSSIDs found (buflen: %d)\n",
- count, len);
+ real_count = 0;
+ netdev_dbg(usbdev->net, "%s, buflen: %d\n", __func__, len);
- while (count && ((void *)bssid + bssid_len) <= (buf + len)) {
+ /* Device returns incorrect 'num_items'. Workaround by ignoring the
+ * received 'num_items' and walking through full bssid buffer instead.
+ */
+ while (bssid_len > 0 && ((void *)bssid + bssid_len) <= (buf + len)) {
if (rndis_bss_info_update(usbdev, bssid) && match_bssid &&
matched) {
if (compare_ether_addr(bssid->mac, match_bssid))
@@ -2047,9 +2050,12 @@ resize_buf:
bssid = (void *)bssid + bssid_len;
bssid_len = le32_to_cpu(bssid->length);
- count--;
+ real_count++;
}
+ netdev_dbg(usbdev->net, "%s, num_items from device: %d, really found: "
+ "%d\n", __func__, count, real_count);
+
out:
kfree(buf);
return ret;
^ permalink raw reply related
* Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions
From: Ohad Ben-Cohen @ 2010-12-18 16:00 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-mmc, Linux-pm mailing list, Ido Yariv, linux-wireless,
Johannes Berg
In-Reply-To: <201012181607.26628.rjw@sisk.pl>
On Sat, Dec 18, 2010 at 5:07 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Saturday, December 18, 2010, Ohad Ben-Cohen wrote:
>> On Sat, Dec 11, 2010 at 4:50 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> >> Think of a device which you have no way to reset other than powering
>> >> it down and up again.
>> >>
>> >> If that device is managed by runtime PM, the only way to do that is by
>> >> using put_sync() followed by a get_sync(). Sure, if someone else
>> >> increased the usage_count of that device this won't work, but then of
>> >> course it means that the driver is not using runtime PM correctly.
>> >
>> > Not so. Even if a driver uses runtime PM correctly, there will still
>> > be times when someone else has increased the usage_count. This happens
>> > during probing and during system resume, for example.
>>
>> I'm aware of these two examples; normally we're good with them since
>> during probing we're not toggling the power, and during suspend/resume
>> the SDIO core is responsible for manipulating the power (and it does
>> so directly). Are there (or do you think there will be) additional
>> examples where this can happen ?
>>
>> But this leads me to a real problem which we have encountered.
>>
>> During system suspend, our driver is asked (by mac80211's suspend
>> handler) to power off its device. When this happens, the driver has no
>> idea that the system is suspending - regular driver code (responsible
>> to remove the wlan interface and stop the device) is being called.
>
> That's where the problem is. If there's a difference, from the driver's
> point of view, between suspend and some other operation, there should be a
> way to tell the driver what case it actually is dealing with.
Yes, the problem will be solved if the driver would bypass the runtime
PM framework on system suspend. mac80211 obviously has this
information, and technically it's very easy to let the driver know
about it.
But the difference between suspend and normal operation is really
artificial: in both cases mac80211 just asks the driver to power its
device down, and the end result is exactly the same (a GPIO line of
the device is de-asserted in our case). The difference between these
two scenarios
exist only because runtime PM is effectively disabled during system
suspend, and therefore the driver has to look for an alternative way
to power down the device.
> BTW, what would you do in that case if the runtime PM of the device were
> disabled by user space by writing "on" to the device's
> /sys/devices/.../power/control file?
That's a good point.
Blocking runtime PM for this device is fatal since this particular
device has functionality tied up with its power control (no other way
to reset it).
It might call for a device-specific dev_pm_info bit flag to prohibit this...
(Or.. not using runtime PM at all for this device. But that would call
for SDIO bus changes, because there is no other way to power off SDIO
devices in the absence of their drivers. Moreover, the device would
not gain from runtime PM's system integration, e.g. powertop
statistics, etc..)
^ permalink raw reply
* [PATCH 0/2] mac80211: hw offload for remain-on-channel
From: Johannes Berg @ 2010-12-18 16:20 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless
This adds functionality to enable remain-on-channel
hardware offload to mac80211 drivers. Any frames
transmitted during that time on the r-o-c channel
get a new TX flag and must be transmitted correctly
by the driver -- any other activity isn't stopped,
so the device must have different queues for the
different channels with the driver managing it.
johannes
^ permalink raw reply
* [PATCH 1/2] mac80211: implement hardware offload for remain-on-channel
From: Johannes Berg @ 2010-12-18 16:20 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless
In-Reply-To: <20101218162046.876589704@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
This allows drivers to support remain-on-channel
offload if they implement smarter timing or need
to use a device implementation like iwlwifi.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v2: - add missing ieee80211_recalc_idle()
include/net/mac80211.h | 19 ++++++++++
net/mac80211/cfg.c | 83 ++++++++++++++++++++++++++++++++++++++++++++
net/mac80211/driver-ops.h | 30 +++++++++++++++
net/mac80211/driver-trace.h | 80 ++++++++++++++++++++++++++++++++++++++++++
net/mac80211/ieee80211_i.h | 8 ++++
net/mac80211/iface.c | 9 +++-
net/mac80211/main.c | 5 ++
net/mac80211/offchannel.c | 75 +++++++++++++++++++++++++++++++++++++++
8 files changed, 306 insertions(+), 3 deletions(-)
--- wireless-testing.orig/include/net/mac80211.h 2010-12-17 19:34:51.000000000 +0100
+++ wireless-testing/include/net/mac80211.h 2010-12-17 19:55:54.000000000 +0100
@@ -365,6 +365,7 @@ enum mac80211_tx_control_flags {
IEEE80211_TX_INTFL_NL80211_FRAME_TX = BIT(21),
IEEE80211_TX_CTL_LDPC = BIT(22),
IEEE80211_TX_CTL_STBC = BIT(23) | BIT(24),
+ IEEE80211_TX_CTL_TX_OFFCHAN = BIT(25),
};
#define IEEE80211_TX_CTL_STBC_SHIFT 23
@@ -1824,6 +1825,12 @@ struct ieee80211_ops {
int (*napi_poll)(struct ieee80211_hw *hw, int budget);
int (*set_antenna)(struct ieee80211_hw *hw, u32 tx_ant, u32 rx_ant);
int (*get_antenna)(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant);
+
+ int (*remain_on_channel)(struct ieee80211_hw *hw,
+ struct ieee80211_channel *chan,
+ enum nl80211_channel_type channel_type,
+ int duration);
+ int (*cancel_remain_on_channel)(struct ieee80211_hw *hw);
};
/**
@@ -2729,6 +2736,18 @@ void ieee80211_request_smps(struct ieee8
*/
void ieee80211_key_removed(struct ieee80211_key_conf *key_conf);
+/**
+ * ieee80211_ready_on_channel - notification of remain-on-channel start
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ */
+void ieee80211_ready_on_channel(struct ieee80211_hw *hw);
+
+/**
+ * ieee80211_remain_on_channel_expired - remain_on_channel duration expired
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ */
+void ieee80211_remain_on_channel_expired(struct ieee80211_hw *hw);
+
/* Rate control API */
/**
--- wireless-testing.orig/net/mac80211/cfg.c 2010-12-17 19:33:22.000000000 +0100
+++ wireless-testing/net/mac80211/cfg.c 2010-12-18 17:06:35.000000000 +0100
@@ -1562,6 +1562,37 @@ static int ieee80211_set_bitrate_mask(st
return 0;
}
+static int ieee80211_remain_on_channel_hw(struct ieee80211_local *local,
+ struct net_device *dev,
+ struct ieee80211_channel *chan,
+ enum nl80211_channel_type chantype,
+ unsigned int duration, u64 *cookie)
+{
+ int ret;
+ u32 random_cookie;
+
+ lockdep_assert_held(&local->mtx);
+
+ if (local->hw_roc_cookie)
+ return -EBUSY;
+ /* must be nonzero */
+ random_cookie = random32() | 1;
+
+ *cookie = random_cookie;
+ local->hw_roc_dev = dev;
+ local->hw_roc_cookie = random_cookie;
+ local->hw_roc_channel = chan;
+ local->hw_roc_channel_type = chantype;
+ local->hw_roc_duration = duration;
+ ret = drv_remain_on_channel(local, chan, chantype, duration);
+ if (ret) {
+ local->hw_roc_channel = NULL;
+ local->hw_roc_cookie = 0;
+ }
+
+ return ret;
+}
+
static int ieee80211_remain_on_channel(struct wiphy *wiphy,
struct net_device *dev,
struct ieee80211_channel *chan,
@@ -1570,16 +1601,62 @@ static int ieee80211_remain_on_channel(s
u64 *cookie)
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = sdata->local;
+
+ if (local->ops->remain_on_channel) {
+ int ret;
+
+ mutex_lock(&local->mtx);
+ ret = ieee80211_remain_on_channel_hw(local, dev,
+ chan, channel_type,
+ duration, cookie);
+ mutex_unlock(&local->mtx);
+
+ return ret;
+ }
return ieee80211_wk_remain_on_channel(sdata, chan, channel_type,
duration, cookie);
}
+static int ieee80211_cancel_remain_on_channel_hw(struct ieee80211_local *local,
+ u64 cookie)
+{
+ int ret;
+
+ lockdep_assert_held(&local->mtx);
+
+ if (local->hw_roc_cookie != cookie)
+ return -ENOENT;
+
+ ret = drv_cancel_remain_on_channel(local);
+ if (ret)
+ return ret;
+
+ local->hw_roc_cookie = 0;
+ local->hw_roc_channel = NULL;
+
+ ieee80211_recalc_idle(local);
+
+ return 0;
+}
+
static int ieee80211_cancel_remain_on_channel(struct wiphy *wiphy,
struct net_device *dev,
u64 cookie)
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = sdata->local;
+
+ if (local->ops->cancel_remain_on_channel) {
+ int ret;
+
+ mutex_lock(&local->mtx);
+ ret = ieee80211_cancel_remain_on_channel_hw(local, cookie);
+ mutex_unlock(&local->mtx);
+
+ return ret;
+ }
return ieee80211_wk_cancel_remain_on_channel(sdata, cookie);
}
@@ -1631,6 +1708,12 @@ static int ieee80211_mgmt_tx(struct wiph
channel_type != local->_oper_channel_type))
is_offchan = true;
+ if (chan == local->hw_roc_channel) {
+ /* TODO: check channel type? */
+ is_offchan = false;
+ flags |= IEEE80211_TX_CTL_TX_OFFCHAN;
+ }
+
if (is_offchan && !offchan)
return -EBUSY;
--- wireless-testing.orig/net/mac80211/ieee80211_i.h 2010-12-17 19:33:22.000000000 +0100
+++ wireless-testing/net/mac80211/ieee80211_i.h 2010-12-18 17:06:23.000000000 +0100
@@ -938,6 +938,13 @@ struct ieee80211_local {
} debugfs;
#endif
+ struct ieee80211_channel *hw_roc_channel;
+ struct net_device *hw_roc_dev;
+ struct work_struct hw_roc_start, hw_roc_done;
+ enum nl80211_channel_type hw_roc_channel_type;
+ unsigned int hw_roc_duration;
+ u32 hw_roc_cookie;
+
/* dummy netdev for use w/ NAPI */
struct net_device napi_dev;
@@ -1129,6 +1136,7 @@ void ieee80211_offchannel_stop_beaconing
void ieee80211_offchannel_stop_station(struct ieee80211_local *local);
void ieee80211_offchannel_return(struct ieee80211_local *local,
bool enable_beaconing);
+void ieee80211_hw_roc_setup(struct ieee80211_local *local);
/* interface handling */
int ieee80211_iface_init(void);
--- wireless-testing.orig/net/mac80211/offchannel.c 2010-12-17 19:33:22.000000000 +0100
+++ wireless-testing/net/mac80211/offchannel.c 2010-12-18 17:06:23.000000000 +0100
@@ -14,6 +14,7 @@
*/
#include <net/mac80211.h>
#include "ieee80211_i.h"
+#include "driver-trace.h"
/*
* inform AP that we will go to sleep so that it will buffer the frames
@@ -190,3 +191,77 @@ void ieee80211_offchannel_return(struct
}
mutex_unlock(&local->iflist_mtx);
}
+
+static void ieee80211_hw_roc_start(struct work_struct *work)
+{
+ struct ieee80211_local *local =
+ container_of(work, struct ieee80211_local, hw_roc_start);
+
+ mutex_lock(&local->mtx);
+
+ if (!local->hw_roc_channel) {
+ mutex_unlock(&local->mtx);
+ return;
+ }
+
+ ieee80211_recalc_idle(local);
+
+ cfg80211_ready_on_channel(local->hw_roc_dev, local->hw_roc_cookie,
+ local->hw_roc_channel,
+ local->hw_roc_channel_type,
+ local->hw_roc_duration,
+ GFP_KERNEL);
+ mutex_unlock(&local->mtx);
+}
+
+void ieee80211_ready_on_channel(struct ieee80211_hw *hw)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+
+ trace_api_ready_on_channel(local);
+
+ ieee80211_queue_work(hw, &local->hw_roc_start);
+}
+EXPORT_SYMBOL_GPL(ieee80211_ready_on_channel);
+
+static void ieee80211_hw_roc_done(struct work_struct *work)
+{
+ struct ieee80211_local *local =
+ container_of(work, struct ieee80211_local, hw_roc_done);
+
+ mutex_lock(&local->mtx);
+
+ if (!local->hw_roc_channel) {
+ mutex_unlock(&local->mtx);
+ return;
+ }
+
+ cfg80211_remain_on_channel_expired(local->hw_roc_dev,
+ local->hw_roc_cookie,
+ local->hw_roc_channel,
+ local->hw_roc_channel_type,
+ GFP_KERNEL);
+
+ local->hw_roc_channel = NULL;
+ local->hw_roc_cookie = 0;
+
+ ieee80211_recalc_idle(local);
+
+ mutex_unlock(&local->mtx);
+}
+
+void ieee80211_remain_on_channel_expired(struct ieee80211_hw *hw)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+
+ trace_api_remain_on_channel_expired(local);
+
+ ieee80211_queue_work(hw, &local->hw_roc_done);
+}
+EXPORT_SYMBOL_GPL(ieee80211_remain_on_channel_expired);
+
+void ieee80211_hw_roc_setup(struct ieee80211_local *local)
+{
+ INIT_WORK(&local->hw_roc_start, ieee80211_hw_roc_start);
+ INIT_WORK(&local->hw_roc_done, ieee80211_hw_roc_done);
+}
--- wireless-testing.orig/net/mac80211/main.c 2010-12-17 19:34:51.000000000 +0100
+++ wireless-testing/net/mac80211/main.c 2010-12-17 19:55:54.000000000 +0100
@@ -603,6 +603,8 @@ struct ieee80211_hw *ieee80211_alloc_hw(
ieee80211_led_names(local);
+ ieee80211_hw_roc_setup(local);
+
return local_to_hw(local);
}
EXPORT_SYMBOL(ieee80211_alloc_hw);
@@ -747,7 +749,8 @@ int ieee80211_register_hw(struct ieee802
}
}
- local->hw.wiphy->max_remain_on_channel_duration = 5000;
+ if (!local->ops->remain_on_channel)
+ local->hw.wiphy->max_remain_on_channel_duration = 5000;
result = wiphy_register(local->hw.wiphy);
if (result < 0)
--- wireless-testing.orig/net/mac80211/iface.c 2010-12-17 19:33:22.000000000 +0100
+++ wireless-testing/net/mac80211/iface.c 2010-12-17 19:55:54.000000000 +0100
@@ -1264,7 +1264,7 @@ u32 __ieee80211_recalc_idle(struct ieee8
{
struct ieee80211_sub_if_data *sdata;
int count = 0;
- bool working = false, scanning = false;
+ bool working = false, scanning = false, hw_roc = false;
struct ieee80211_work *wk;
unsigned int led_trig_start = 0, led_trig_stop = 0;
@@ -1308,6 +1308,9 @@ u32 __ieee80211_recalc_idle(struct ieee8
local->scan_sdata->vif.bss_conf.idle = false;
}
+ if (local->hw_roc_channel)
+ hw_roc = true;
+
list_for_each_entry(sdata, &local->interfaces, list) {
if (sdata->old_idle == sdata->vif.bss_conf.idle)
continue;
@@ -1316,7 +1319,7 @@ u32 __ieee80211_recalc_idle(struct ieee8
ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_IDLE);
}
- if (working || scanning)
+ if (working || scanning || hw_roc)
led_trig_start |= IEEE80211_TPT_LEDTRIG_FL_WORK;
else
led_trig_stop |= IEEE80211_TPT_LEDTRIG_FL_WORK;
@@ -1328,6 +1331,8 @@ u32 __ieee80211_recalc_idle(struct ieee8
ieee80211_mod_tpt_led_trig(local, led_trig_start, led_trig_stop);
+ if (hw_roc)
+ return ieee80211_idle_off(local, "hw remain-on-channel");
if (working)
return ieee80211_idle_off(local, "working");
if (scanning)
--- wireless-testing.orig/net/mac80211/driver-ops.h 2010-12-17 19:33:22.000000000 +0100
+++ wireless-testing/net/mac80211/driver-ops.h 2010-12-17 19:55:54.000000000 +0100
@@ -465,4 +465,34 @@ static inline int drv_get_antenna(struct
return ret;
}
+static inline int drv_remain_on_channel(struct ieee80211_local *local,
+ struct ieee80211_channel *chan,
+ enum nl80211_channel_type chantype,
+ unsigned int duration)
+{
+ int ret;
+
+ might_sleep();
+
+ trace_drv_remain_on_channel(local, chan, chantype, duration);
+ ret = local->ops->remain_on_channel(&local->hw, chan, chantype,
+ duration);
+ trace_drv_return_int(local, ret);
+
+ return ret;
+}
+
+static inline int drv_cancel_remain_on_channel(struct ieee80211_local *local)
+{
+ int ret;
+
+ might_sleep();
+
+ trace_drv_cancel_remain_on_channel(local);
+ ret = local->ops->cancel_remain_on_channel(&local->hw);
+ trace_drv_return_int(local, ret);
+
+ return ret;
+}
+
#endif /* __MAC80211_DRIVER_OPS */
--- wireless-testing.orig/net/mac80211/driver-trace.h 2010-12-17 19:33:22.000000000 +0100
+++ wireless-testing/net/mac80211/driver-trace.h 2010-12-17 19:55:54.000000000 +0100
@@ -933,6 +933,50 @@ TRACE_EVENT(drv_get_antenna,
)
);
+TRACE_EVENT(drv_remain_on_channel,
+ TP_PROTO(struct ieee80211_local *local, struct ieee80211_channel *chan,
+ enum nl80211_channel_type chantype, unsigned int duration),
+
+ TP_ARGS(local, chan, chantype, duration),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ __field(int, center_freq)
+ __field(int, channel_type)
+ __field(unsigned int, duration)
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ __entry->center_freq = chan->center_freq;
+ __entry->channel_type = chantype;
+ __entry->duration = duration;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT " freq:%dMHz duration:%dms",
+ LOCAL_PR_ARG, __entry->center_freq, __entry->duration
+ )
+);
+
+TRACE_EVENT(drv_cancel_remain_on_channel,
+ TP_PROTO(struct ieee80211_local *local),
+
+ TP_ARGS(local),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT, LOCAL_PR_ARG
+ )
+);
+
/*
* Tracing for API calls that drivers call.
*/
@@ -1170,6 +1214,42 @@ TRACE_EVENT(api_chswitch_done,
)
);
+TRACE_EVENT(api_ready_on_channel,
+ TP_PROTO(struct ieee80211_local *local),
+
+ TP_ARGS(local),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT, LOCAL_PR_ARG
+ )
+);
+
+TRACE_EVENT(api_remain_on_channel_expired,
+ TP_PROTO(struct ieee80211_local *local),
+
+ TP_ARGS(local),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT, LOCAL_PR_ARG
+ )
+);
+
/*
* Tracing for internal functions
* (which may also be called in response to driver calls)
^ permalink raw reply
* [PATCH 2/2] mac80211: implement off-channel TX using hw r-o-c offload
From: Johannes Berg @ 2010-12-18 16:20 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless
In-Reply-To: <20101218162046.876589704@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
When the driver has remain-on-channel offload,
implement off-channel transmission using that
primitive.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/mac80211/cfg.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
net/mac80211/ieee80211_i.h | 2 +
net/mac80211/offchannel.c | 30 +++++++++++++++-------
3 files changed, 81 insertions(+), 10 deletions(-)
--- wireless-testing.orig/net/mac80211/cfg.c 2010-12-18 17:06:35.000000000 +0100
+++ wireless-testing/net/mac80211/cfg.c 2010-12-18 17:08:23.000000000 +0100
@@ -1610,6 +1610,7 @@ static int ieee80211_remain_on_channel(s
ret = ieee80211_remain_on_channel_hw(local, dev,
chan, channel_type,
duration, cookie);
+ local->hw_roc_for_tx = false;
mutex_unlock(&local->mtx);
return ret;
@@ -1751,6 +1752,49 @@ static int ieee80211_mgmt_tx(struct wiph
*cookie = (unsigned long) skb;
+ if (is_offchan && local->ops->remain_on_channel) {
+ unsigned int duration;
+ int ret;
+
+ mutex_lock(&local->mtx);
+ /*
+ * If the duration is zero, then the driver
+ * wouldn't actually do anything. Set it to
+ * 100 for now.
+ *
+ * TODO: cancel the off-channel operation
+ * when we get the SKB's TX status and
+ * the wait time was zero before.
+ */
+ duration = 100;
+ if (wait)
+ duration = wait;
+ ret = ieee80211_remain_on_channel_hw(local, dev, chan,
+ channel_type,
+ duration, cookie);
+ if (ret) {
+ kfree_skb(skb);
+ mutex_unlock(&local->mtx);
+ return ret;
+ }
+
+ local->hw_roc_for_tx = true;
+ local->hw_roc_duration = wait;
+
+ /*
+ * queue up frame for transmission after
+ * ieee80211_ready_on_channel call
+ */
+
+ /* modify cookie to prevent API mismatches */
+ *cookie ^= 2;
+ IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_CTL_TX_OFFCHAN;
+ local->hw_roc_skb = skb;
+ mutex_unlock(&local->mtx);
+
+ return 0;
+ }
+
/*
* Can transmit right away if the channel was the
* right one and there's no wait involved... If a
@@ -1791,6 +1835,21 @@ static int ieee80211_mgmt_tx_cancel_wait
int ret = -ENOENT;
mutex_lock(&local->mtx);
+
+ if (local->ops->cancel_remain_on_channel) {
+ cookie ^= 2;
+ ret = ieee80211_cancel_remain_on_channel_hw(local, cookie);
+
+ if (ret == 0) {
+ kfree_skb(local->hw_roc_skb);
+ local->hw_roc_skb = NULL;
+ }
+
+ mutex_unlock(&local->mtx);
+
+ return ret;
+ }
+
list_for_each_entry(wk, &local->work_list, list) {
if (wk->sdata != sdata)
continue;
--- wireless-testing.orig/net/mac80211/ieee80211_i.h 2010-12-18 17:06:23.000000000 +0100
+++ wireless-testing/net/mac80211/ieee80211_i.h 2010-12-18 17:08:23.000000000 +0100
@@ -940,10 +940,12 @@ struct ieee80211_local {
struct ieee80211_channel *hw_roc_channel;
struct net_device *hw_roc_dev;
+ struct sk_buff *hw_roc_skb;
struct work_struct hw_roc_start, hw_roc_done;
enum nl80211_channel_type hw_roc_channel_type;
unsigned int hw_roc_duration;
u32 hw_roc_cookie;
+ bool hw_roc_for_tx;
/* dummy netdev for use w/ NAPI */
struct net_device napi_dev;
--- wireless-testing.orig/net/mac80211/offchannel.c 2010-12-18 17:06:23.000000000 +0100
+++ wireless-testing/net/mac80211/offchannel.c 2010-12-18 17:08:23.000000000 +0100
@@ -196,6 +196,7 @@ static void ieee80211_hw_roc_start(struc
{
struct ieee80211_local *local =
container_of(work, struct ieee80211_local, hw_roc_start);
+ struct ieee80211_sub_if_data *sdata;
mutex_lock(&local->mtx);
@@ -206,11 +207,19 @@ static void ieee80211_hw_roc_start(struc
ieee80211_recalc_idle(local);
- cfg80211_ready_on_channel(local->hw_roc_dev, local->hw_roc_cookie,
- local->hw_roc_channel,
- local->hw_roc_channel_type,
- local->hw_roc_duration,
- GFP_KERNEL);
+ if (local->hw_roc_skb) {
+ sdata = IEEE80211_DEV_TO_SUB_IF(local->hw_roc_dev);
+ ieee80211_tx_skb(sdata, local->hw_roc_skb);
+ local->hw_roc_skb = NULL;
+ } else {
+ cfg80211_ready_on_channel(local->hw_roc_dev,
+ local->hw_roc_cookie,
+ local->hw_roc_channel,
+ local->hw_roc_channel_type,
+ local->hw_roc_duration,
+ GFP_KERNEL);
+ }
+
mutex_unlock(&local->mtx);
}
@@ -236,11 +245,12 @@ static void ieee80211_hw_roc_done(struct
return;
}
- cfg80211_remain_on_channel_expired(local->hw_roc_dev,
- local->hw_roc_cookie,
- local->hw_roc_channel,
- local->hw_roc_channel_type,
- GFP_KERNEL);
+ if (!local->hw_roc_for_tx)
+ cfg80211_remain_on_channel_expired(local->hw_roc_dev,
+ local->hw_roc_cookie,
+ local->hw_roc_channel,
+ local->hw_roc_channel_type,
+ GFP_KERNEL);
local->hw_roc_channel = NULL;
local->hw_roc_cookie = 0;
^ permalink raw reply
* Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions
From: Johannes Berg @ 2010-12-18 16:40 UTC (permalink / raw)
To: Ohad Ben-Cohen
Cc: Rafael J. Wysocki, linux-mmc, Linux-pm mailing list, Ido Yariv,
linux-wireless
In-Reply-To: <AANLkTin23vbbA7+UDzh=v060HbkLMrJjB38cs+AWScPJ@mail.gmail.com>
On Sat, 2010-12-18 at 18:00 +0200, Ohad Ben-Cohen wrote:
> > That's where the problem is. If there's a difference, from the driver's
> > point of view, between suspend and some other operation, there should be a
> > way to tell the driver what case it actually is dealing with.
>
> Yes, the problem will be solved if the driver would bypass the runtime
> PM framework on system suspend. mac80211 obviously has this
> information, and technically it's very easy to let the driver know
> about it.
>
> But the difference between suspend and normal operation is really
> artificial: in both cases mac80211 just asks the driver to power its
> device down, and the end result is exactly the same (a GPIO line of
> the device is de-asserted in our case). The difference between these
> two scenarios
> exist only because runtime PM is effectively disabled during system
> suspend, and therefore the driver has to look for an alternative way
> to power down the device.
Sounds to me like the difference isn't really in the driver, but the
core PM subsystem. Why does it care when powering off a device whether
it's during suspend, or during runtime?
johannes
^ permalink raw reply
* [PATCH 2/3] mac80211: skip unnecessary pskb_expand_head calls
From: Felix Fietkau @ 2010-12-18 18:30 UTC (permalink / raw)
To: linux-wireless; +Cc: linville, johannes, Felix Fietkau
In-Reply-To: <1292697050-25741-1-git-send-email-nbd@openwrt.org>
If the skb is not cloned and we don't need any extra headroom, there
is no point in reallocating the skb head.
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
net/mac80211/tx.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 7d78aca..56ae175 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1549,8 +1549,10 @@ static int ieee80211_skb_resize(struct ieee80211_local *local,
if (skb_header_cloned(skb))
I802_DEBUG_INC(local->tx_expand_skb_head_cloned);
- else
+ else if (head_need || tail_need)
I802_DEBUG_INC(local->tx_expand_skb_head);
+ else
+ return 0;
if (pskb_expand_head(skb, head_need, tail_need, GFP_ATOMIC)) {
wiphy_debug(local->hw.wiphy,
--
1.7.3.2
^ permalink raw reply related
* [PATCH 3/3] mac80211: fix potentially redundant skb data copying
From: Felix Fietkau @ 2010-12-18 18:30 UTC (permalink / raw)
To: linux-wireless; +Cc: linville, johannes, Felix Fietkau
In-Reply-To: <1292697050-25741-2-git-send-email-nbd@openwrt.org>
When an skb is shared, it needs to be duplicated, along with its data buffer.
If the skb does not have enough headroom, using skb_copy might cause the data
buffer to be copied twice (once by skb_copy and once by pskb_expand_head).
Fix this by using skb_clone initially and letting ieee80211_skb_resize sort
out the rest.
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
net/mac80211/tx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 56ae175..d484c07 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1939,7 +1939,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
*/
if (skb_shared(skb)) {
tmp_skb = skb;
- skb = skb_copy(skb, GFP_ATOMIC);
+ skb = skb_clone(skb, GFP_ATOMIC);
kfree_skb(tmp_skb);
if (!skb) {
--
1.7.3.2
^ permalink raw reply related
* [PATCH 1/3] mac80211: fix initialization of skb->cb in ieee80211_subif_start_xmit
From: Felix Fietkau @ 2010-12-18 18:30 UTC (permalink / raw)
To: linux-wireless; +Cc: linville, johannes, Felix Fietkau, Helmut Schaa
The change 'mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs'
added a check for copying the skb if it's shared, however the tx info variable
still points at the cb of the old skb
Cc: Helmut Schaa <helmut.schaa@googlemail.com>
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
net/mac80211/tx.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 898e864..7d78aca 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1742,7 +1742,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_local *local = sdata->local;
- struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ struct ieee80211_tx_info *info;
int ret = NETDEV_TX_BUSY, head_need;
u16 ethertype, hdrlen, meshhdrlen = 0;
__le16 fc;
@@ -2033,6 +2033,7 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
skb_set_network_header(skb, nh_pos);
skb_set_transport_header(skb, h_pos);
+ info = IEEE80211_SKB_CB(skb);
memset(info, 0, sizeof(*info));
dev->trans_start = jiffies;
--
1.7.3.2
^ permalink raw reply related
* Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions
From: Ohad Ben-Cohen @ 2010-12-18 19:08 UTC (permalink / raw)
To: Linux-pm mailing list
Cc: Rafael J. Wysocki, linux-mmc, Ido Yariv, linux-wireless,
Alan Stern, Johannes Berg
In-Reply-To: <1292690407.3653.2.camel@jlt3.sipsolutions.net>
On Sat, Dec 18, 2010 at 6:40 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Sat, 2010-12-18 at 18:00 +0200, Ohad Ben-Cohen wrote:
>
>> > That's where the problem is. If there's a difference, from the driver's
>> > point of view, between suspend and some other operation, there should be a
>> > way to tell the driver what case it actually is dealing with.
>>
>> Yes, the problem will be solved if the driver would bypass the runtime
>> PM framework on system suspend. mac80211 obviously has this
>> information, and technically it's very easy to let the driver know
>> about it.
>>
>> But the difference between suspend and normal operation is really
>> artificial: in both cases mac80211 just asks the driver to power its
>> device down, and the end result is exactly the same (a GPIO line of
>> the device is de-asserted in our case). The difference between these
>> two scenarios
>> exist only because runtime PM is effectively disabled during system
>> suspend, and therefore the driver has to look for an alternative way
>> to power down the device.
>
> Sounds to me like the difference isn't really in the driver, but the
> core PM subsystem. Why does it care when powering off a device whether
> it's during suspend, or during runtime?
Agree.
If we can add a dev_pm_info bit, that would allow using runtime PM API
during suspend/resume transitions, the driver will not have to care.
Rafael what do you think ? Is that totally unacceptable ?
Thanks,
Ohad.
^ permalink raw reply
* Compat-wireless release for 2010-12-18 is baked
From: Compat-wireless cronjob account @ 2010-12-18 20:03 UTC (permalink / raw)
To: linux-wireless
cat: /var/opt/compat/compat-wireless-2.6/compat_version: No such file or directory
cat: compat_base_tree: No such file or directory
cat: compat_base_tree_version: No such file or directory
cat: compat_version: No such file or directory
cat: /var/opt/compat/compat-wireless-2.6/compat_version: No such file or directory
scripts/Makefile.clean:17: /var/opt/compat/compat-wireless-2.6/drivers/net/wireless/hostap/Makefile: No such file or directory
make[4]: *** No rule to make target `/var/opt/compat/compat-wireless-2.6/drivers/net/wireless/hostap/Makefile'. Stop.
make[3]: *** [/var/opt/compat/compat-wireless-2.6/drivers/net/wireless/hostap] Error 2
make[2]: *** [/var/opt/compat/compat-wireless-2.6/drivers/net/wireless] Error 2
make[1]: *** [_clean_/var/opt/compat/compat-wireless-2.6] Error 2
make: *** [clean] Error 2
compat-wireless code metrics
753675 - Total upstream lines of code being pulled
^ permalink raw reply
* Re: [PATCH 1/3] compat: backport skb_checksum_start_offset
From: Luis R. Rodriguez @ 2010-12-18 20:08 UTC (permalink / raw)
To: Hauke Mehrtens
Cc: Luis Rodriguez, linux-wireless@vger.kernel.org,
mcgrof@infradead.org
In-Reply-To: <1292625698-17289-1-git-send-email-hauke@hauke-m.de>
On Fri, Dec 17, 2010 at 02:41:36PM -0800, Hauke Mehrtens wrote:
> This is needed by atlx, atl1c and atl1e.
Applied all and pushed, thanks!
Luis
^ permalink raw reply
* Re: [PATCH 1/3] compat-wireless: make patches apply again.
From: Luis R. Rodriguez @ 2010-12-18 20:08 UTC (permalink / raw)
To: Hauke Mehrtens
Cc: Luis Rodriguez, linux-wireless@vger.kernel.org,
mcgrof@infradead.org
In-Reply-To: <1292625768-17318-1-git-send-email-hauke@hauke-m.de>
On Fri, Dec 17, 2010 at 02:42:46PM -0800, Hauke Mehrtens wrote:
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Applied all and pushed, thanks!
Luis
^ permalink raw reply
* Re: [PATCH 2/3] compat-wireless: use pci_is_pcie()
From: Luis R. Rodriguez @ 2010-12-18 20:14 UTC (permalink / raw)
To: Hauke Mehrtens; +Cc: linux-wireless, mcgrof
In-Reply-To: <1292625768-17318-2-git-send-email-hauke@hauke-m.de>
On Fri, Dec 17, 2010 at 2:42 PM, Hauke Mehrtens <hauke@hauke-m.de> wrote:
> Use pci_is_pcie function and do not access the member of the struct directly.
So we can submit this upstream as well?
Luis
^ permalink raw reply
* Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions
From: Alan Stern @ 2010-12-18 21:29 UTC (permalink / raw)
To: Johannes Berg
Cc: Ohad Ben-Cohen, linux-wireless, Linux-pm mailing list, linux-mmc,
Ido Yariv
In-Reply-To: <1292690407.3653.2.camel@jlt3.sipsolutions.net>
On Sat, 18 Dec 2010, Johannes Berg wrote:
> Sounds to me like the difference isn't really in the driver, but the
> core PM subsystem. Why does it care when powering off a device whether
> it's during suspend, or during runtime?
The two operations are quite different. One involves preparing the
device to have its power removed (when the system goes to sleep), and
the other involves actually reducing the device's power. (In other
words, system suspend involves telling the driver "The computer is
going to sleep in a moment, so get ready", whereas runtime suspend
involves telling the driver "Your device isn't being used now so feel
free to reduce its power".) There also are differences with regard to
how wakeup events are signalled. Obviously there's a great deal of
similarity and overlap, but they are _not_ the same in general.
Now these differences don't matter so much to the PM core, but they
should matter to subsystems and drivers. The PM core cares just enough
to know that it has to invoke different callbacks for the different
operations.
Alan Stern
^ permalink raw reply
* Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions
From: Alan Stern @ 2010-12-18 21:30 UTC (permalink / raw)
To: Ohad Ben-Cohen
Cc: Linux-pm mailing list, Rafael J. Wysocki, linux-mmc, Ido Yariv,
linux-wireless, Johannes Berg
In-Reply-To: <AANLkTin6zdUb2C-3GU-FySb3Pmk5iHaG5neLNSG38Tdx@mail.gmail.com>
On Sat, 18 Dec 2010, Ohad Ben-Cohen wrote:
> > Sounds to me like the difference isn't really in the driver, but the
> > core PM subsystem. Why does it care when powering off a device whether
> > it's during suspend, or during runtime?
>
> Agree.
>
> If we can add a dev_pm_info bit, that would allow using runtime PM API
> during suspend/resume transitions, the driver will not have to care.
>
> Rafael what do you think ? Is that totally unacceptable ?
Have you forgotten about the "echo on >.../power/control" scenario?
Alan Stern
^ permalink raw reply
* Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions
From: Rafael J. Wysocki @ 2010-12-18 22:47 UTC (permalink / raw)
To: Ohad Ben-Cohen
Cc: linux-mmc, Linux-pm mailing list, Ido Yariv, linux-wireless,
Johannes Berg
In-Reply-To: <AANLkTin23vbbA7+UDzh=v060HbkLMrJjB38cs+AWScPJ@mail.gmail.com>
On Saturday, December 18, 2010, Ohad Ben-Cohen wrote:
> On Sat, Dec 18, 2010 at 5:07 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Saturday, December 18, 2010, Ohad Ben-Cohen wrote:
> >> On Sat, Dec 11, 2010 at 4:50 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> >> >> Think of a device which you have no way to reset other than powering
> >> >> it down and up again.
> >> >>
> >> >> If that device is managed by runtime PM, the only way to do that is by
> >> >> using put_sync() followed by a get_sync(). Sure, if someone else
> >> >> increased the usage_count of that device this won't work, but then of
> >> >> course it means that the driver is not using runtime PM correctly.
> >> >
> >> > Not so. Even if a driver uses runtime PM correctly, there will still
> >> > be times when someone else has increased the usage_count. This happens
> >> > during probing and during system resume, for example.
> >>
> >> I'm aware of these two examples; normally we're good with them since
> >> during probing we're not toggling the power, and during suspend/resume
> >> the SDIO core is responsible for manipulating the power (and it does
> >> so directly). Are there (or do you think there will be) additional
> >> examples where this can happen ?
> >>
> >> But this leads me to a real problem which we have encountered.
> >>
> >> During system suspend, our driver is asked (by mac80211's suspend
> >> handler) to power off its device. When this happens, the driver has no
> >> idea that the system is suspending - regular driver code (responsible
> >> to remove the wlan interface and stop the device) is being called.
> >
> > That's where the problem is. If there's a difference, from the driver's
> > point of view, between suspend and some other operation, there should be a
> > way to tell the driver what case it actually is dealing with.
>
> Yes, the problem will be solved if the driver would bypass the runtime
> PM framework on system suspend. mac80211 obviously has this
> information, and technically it's very easy to let the driver know
> about it.
>
> But the difference between suspend and normal operation is really
> artificial: in both cases mac80211 just asks the driver to power its
> device down, and the end result is exactly the same (a GPIO line of
> the device is de-asserted in our case). The difference between these
> two scenarios
> exist only because runtime PM is effectively disabled during system
> suspend, and therefore the driver has to look for an alternative way
> to power down the device.
That's not correct, sorry.
There is a bug and the bug is that you use the runtime PM to power down
the device in every situation, although you obviously shouldn't do that
(e.g. because it may be disabled by user space or it _is_ disabled by
the PM core during system suspend).
> > BTW, what would you do in that case if the runtime PM of the device were
> > disabled by user space by writing "on" to the device's
> > /sys/devices/.../power/control file?
>
> That's a good point.
>
> Blocking runtime PM for this device is fatal since this particular
> device has functionality tied up with its power control (no other way
> to reset it).
>
> It might call for a device-specific dev_pm_info bit flag to prohibit this...
No way.
> (Or.. not using runtime PM at all for this device. But that would call
> for SDIO bus changes, because there is no other way to power off SDIO
> devices in the absence of their drivers. Moreover, the device would
> not gain from runtime PM's system integration, e.g. powertop
> statistics, etc..)
Let me repeat, please. The runtime PM framework is _not_ _suitable_ for
_general_ runtime PM of devices, because (a) it may be disabled by user space
at any time and (b) it is disabled by the PM core during system suspend.
Thanks,
Rafael
^ permalink raw reply
* Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions
From: Rafael J. Wysocki @ 2010-12-18 22:50 UTC (permalink / raw)
To: Johannes Berg
Cc: Ohad Ben-Cohen, linux-mmc, Linux-pm mailing list, Ido Yariv,
linux-wireless
In-Reply-To: <1292690407.3653.2.camel@jlt3.sipsolutions.net>
On Saturday, December 18, 2010, Johannes Berg wrote:
> On Sat, 2010-12-18 at 18:00 +0200, Ohad Ben-Cohen wrote:
>
> > > That's where the problem is. If there's a difference, from the driver's
> > > point of view, between suspend and some other operation, there should be a
> > > way to tell the driver what case it actually is dealing with.
> >
> > Yes, the problem will be solved if the driver would bypass the runtime
> > PM framework on system suspend. mac80211 obviously has this
> > information, and technically it's very easy to let the driver know
> > about it.
> >
> > But the difference between suspend and normal operation is really
> > artificial: in both cases mac80211 just asks the driver to power its
> > device down, and the end result is exactly the same (a GPIO line of
> > the device is de-asserted in our case). The difference between these
> > two scenarios
> > exist only because runtime PM is effectively disabled during system
> > suspend, and therefore the driver has to look for an alternative way
> > to power down the device.
>
> Sounds to me like the difference isn't really in the driver, but the
> core PM subsystem. Why does it care when powering off a device whether
> it's during suspend, or during runtime?
The problem is that the driver is using something it shouldn't use for
the particular purpose. Plain and simple.
Namely, it uses the runtime PM _framework_ (which has specific and _documented_
limitations) for general power management of the device it handles. It is
wrong and it has nothing to do with the PM subsystem.
Thanks,
Rafael
^ permalink raw reply
* Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions
From: Rafael J. Wysocki @ 2010-12-18 22:52 UTC (permalink / raw)
To: Ohad Ben-Cohen
Cc: Linux-pm mailing list, linux-mmc, Ido Yariv, linux-wireless,
Alan Stern, Johannes Berg
In-Reply-To: <AANLkTin6zdUb2C-3GU-FySb3Pmk5iHaG5neLNSG38Tdx@mail.gmail.com>
On Saturday, December 18, 2010, Ohad Ben-Cohen wrote:
> On Sat, Dec 18, 2010 at 6:40 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Sat, 2010-12-18 at 18:00 +0200, Ohad Ben-Cohen wrote:
> >
> >> > That's where the problem is. If there's a difference, from the driver's
> >> > point of view, between suspend and some other operation, there should be a
> >> > way to tell the driver what case it actually is dealing with.
> >>
> >> Yes, the problem will be solved if the driver would bypass the runtime
> >> PM framework on system suspend. mac80211 obviously has this
> >> information, and technically it's very easy to let the driver know
> >> about it.
> >>
> >> But the difference between suspend and normal operation is really
> >> artificial: in both cases mac80211 just asks the driver to power its
> >> device down, and the end result is exactly the same (a GPIO line of
> >> the device is de-asserted in our case). The difference between these
> >> two scenarios
> >> exist only because runtime PM is effectively disabled during system
> >> suspend, and therefore the driver has to look for an alternative way
> >> to power down the device.
> >
> > Sounds to me like the difference isn't really in the driver, but the
> > core PM subsystem. Why does it care when powering off a device whether
> > it's during suspend, or during runtime?
>
> Agree.
>
> If we can add a dev_pm_info bit, that would allow using runtime PM API
> during suspend/resume transitions, the driver will not have to care.
>
> Rafael what do you think ? Is that totally unacceptable ?
Already said. It is not acceptable at all.
Thanks,
Rafael
^ permalink raw reply
* Re: [linux-pm] subtle pm_runtime_put_sync race and sdio functions
From: Rafael J. Wysocki @ 2010-12-18 22:57 UTC (permalink / raw)
To: Alan Stern
Cc: Ohad Ben-Cohen, Linux-pm mailing list, linux-mmc, Ido Yariv,
linux-wireless, Johannes Berg
In-Reply-To: <Pine.LNX.4.44L0.1012181629350.7965-100000@netrider.rowland.org>
On Saturday, December 18, 2010, Alan Stern wrote:
> On Sat, 18 Dec 2010, Ohad Ben-Cohen wrote:
>
> > > Sounds to me like the difference isn't really in the driver, but the
> > > core PM subsystem. Why does it care when powering off a device whether
> > > it's during suspend, or during runtime?
> >
> > Agree.
> >
> > If we can add a dev_pm_info bit, that would allow using runtime PM API
> > during suspend/resume transitions, the driver will not have to care.
> >
> > Rafael what do you think ? Is that totally unacceptable ?
>
> Have you forgotten about the "echo on >.../power/control" scenario?
Well, that change would basically require the runtime PM framework to ignore
the usage count for this particular device, which would defeat the framework's
purpose to some extent, but it would cover the "echo on > ..." case.
However, I'm not going to agree to make that change.
Thanks,
Rafael
^ permalink raw reply
* [PATCH 2/2] ath9k: do not limit the chainmask to 1 for legacy mode
From: Felix Fietkau @ 2010-12-18 23:31 UTC (permalink / raw)
To: linux-wireless; +Cc: linville, lrodriguez, Felix Fietkau
In-Reply-To: <1292715115-36506-1-git-send-email-nbd@openwrt.org>
Restricting the chainmask to 1 for legacy mode disables useful features
such as MRC, and it reduces the available transmit power.
I can't think of a good reason to do this in legacy mode, so let's just
get rid of that code.
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 1 -
drivers/net/wireless/ath/ath9k/main.c | 32 ------------------------------
drivers/net/wireless/ath/ath9k/virtual.c | 1 -
3 files changed, 0 insertions(+), 34 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 9fd9519..2c31f51 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -675,7 +675,6 @@ void ath9k_deinit_device(struct ath_softc *sc);
void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw);
void ath9k_update_ichannel(struct ath_softc *sc, struct ieee80211_hw *hw,
struct ath9k_channel *ichan);
-void ath_update_chainmask(struct ath_softc *sc, int is_ht);
int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
struct ath9k_channel *hchan);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index cb53fbb..8a1691d 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -538,32 +538,6 @@ set_timer:
}
}
-/*
- * Update tx/rx chainmask. For legacy association,
- * hard code chainmask to 1x1, for 11n association, use
- * the chainmask configuration, for bt coexistence, use
- * the chainmask configuration even in legacy mode.
- */
-void ath_update_chainmask(struct ath_softc *sc, int is_ht)
-{
- struct ath_hw *ah = sc->sc_ah;
- struct ath_common *common = ath9k_hw_common(ah);
-
- if ((sc->sc_flags & SC_OP_OFFCHANNEL) || is_ht ||
- (ah->btcoex_hw.scheme != ATH_BTCOEX_CFG_NONE)) {
- common->tx_chainmask = ah->caps.tx_chainmask;
- common->rx_chainmask = ah->caps.rx_chainmask;
- } else {
- common->tx_chainmask = 1;
- common->rx_chainmask = 1;
- }
-
- ath_dbg(common, ATH_DBG_CONFIG,
- "tx chmask: %d, rx chmask: %d\n",
- common->tx_chainmask,
- common->rx_chainmask);
-}
-
static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta)
{
struct ath_node *an;
@@ -1679,8 +1653,6 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
/* XXX: remove me eventualy */
ath9k_update_ichannel(sc, hw, &sc->sc_ah->channels[pos]);
- ath_update_chainmask(sc, conf_is_ht(conf));
-
/* update survey stats for the old channel before switching */
spin_lock_irqsave(&common->cc_lock, flags);
ath_update_survey_stats(sc);
@@ -1912,10 +1884,6 @@ static void ath9k_bss_info_changed(struct ieee80211_hw *hw,
/* Set aggregation protection mode parameters */
sc->config.ath_aggr_prot = 0;
- /* Only legacy IBSS for now */
- if (vif->type == NL80211_IFTYPE_ADHOC)
- ath_update_chainmask(sc, 0);
-
ath_dbg(common, ATH_DBG_CONFIG, "BSSID: %pM aid: 0x%x\n",
common->curbssid, common->curaid);
diff --git a/drivers/net/wireless/ath/ath9k/virtual.c b/drivers/net/wireless/ath/ath9k/virtual.c
index fbfbc82..2dc7095 100644
--- a/drivers/net/wireless/ath/ath9k/virtual.c
+++ b/drivers/net/wireless/ath/ath9k/virtual.c
@@ -288,7 +288,6 @@ void ath9k_wiphy_chan_work(struct work_struct *work)
/* sync hw configuration for hw code */
common->hw = aphy->hw;
- ath_update_chainmask(sc, sc->chan_is_ht);
if (ath_set_channel(sc, aphy->hw,
&sc->sc_ah->channels[sc->chan_idx]) < 0) {
printk(KERN_DEBUG "ath9k: Failed to set channel for new "
--
1.7.3.2
^ permalink raw reply related
* [PATCH 1/2] ath9k_hw: fix PA predistortion HT40 mask
From: Felix Fietkau @ 2010-12-18 23:31 UTC (permalink / raw)
To: linux-wireless
Cc: linville, lrodriguez, Felix Fietkau, Vasanthakumar Thiagarajan
The commit 'ath9k_hw: Disable PAPRD for rates with low Tx power' changed
the code that sets the PAPRD rate masks to use only either the HT20 mask
or the HT40 mask. This is wrong, as the hardware can still use HT20 rates
even when configured for HT40, and the operating channel mode does not
affect PAPRD operation.
The register for the HT40 rate mask is applied as a mask on top of the
other registers to selectively disable PAPRD for specific rates on HT40
packets only.
This patch changes the code back to the old behavior which matches the
intended use of these registers. While with current cards this should not
make any practical difference (according to Atheros, the HT20 and HT40
mask should always be equal), it is more correct that way, and maybe
the HT40 mask will be used for some rare corner cases in the future.
Cc: Vasanthakumar Thiagarajan <vasanth@atheros.com>
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 19 +++++++++++--------
drivers/net/wireless/ath/ath9k/ar9003_paprd.c | 2 +-
drivers/net/wireless/ath/ath9k/hw.h | 1 +
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index f80ec74..d7deae8 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -4762,6 +4762,7 @@ static void ath9k_hw_ar9300_set_txpower(struct ath_hw *ah,
struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
struct ath_common *common = ath9k_hw_common(ah);
struct ar9300_eeprom *eep = &ah->eeprom.ar9300_eep;
+ struct ar9300_modal_eep_header *modal_hdr;
u8 targetPowerValT2[ar9300RateSize];
u8 target_power_val_t2_eep[ar9300RateSize];
unsigned int i = 0, paprd_scale_factor = 0;
@@ -4771,15 +4772,17 @@ static void ath9k_hw_ar9300_set_txpower(struct ath_hw *ah,
if (ah->eep_ops->get_eeprom(ah, EEP_PAPRD)) {
if (IS_CHAN_2GHZ(chan))
- ah->paprd_ratemask = (IS_CHAN_HT40(chan) ?
- le32_to_cpu(eep->modalHeader2G.papdRateMaskHt40) :
- le32_to_cpu(eep->modalHeader2G.papdRateMaskHt20))
- & AR9300_PAPRD_RATE_MASK;
+ modal_hdr = &eep->modalHeader2G;
else
- ah->paprd_ratemask = (IS_CHAN_HT40(chan) ?
- le32_to_cpu(eep->modalHeader5G.papdRateMaskHt40) :
- le32_to_cpu(eep->modalHeader5G.papdRateMaskHt20))
- & AR9300_PAPRD_RATE_MASK;
+ modal_hdr = &eep->modalHeader5G;
+
+ ah->paprd_ratemask =
+ le32_to_cpu(modal_hdr->papdRateMaskHt20) &
+ AR9300_PAPRD_RATE_MASK;
+
+ ah->paprd_ratemask_ht40 =
+ le32_to_cpu(modal_hdr->papdRateMaskHt40) &
+ AR9300_PAPRD_RATE_MASK;
paprd_scale_factor = ar9003_get_paprd_scale_factor(ah, chan);
min_pwridx = IS_CHAN_HT40(chan) ? ALL_TARGET_HT40_0_8_16 :
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_paprd.c b/drivers/net/wireless/ath/ath9k/ar9003_paprd.c
index 79554c5..356d2fd7 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_paprd.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_paprd.c
@@ -134,7 +134,7 @@ static int ar9003_paprd_setup_single_table(struct ath_hw *ah)
REG_RMW_FIELD(ah, AR_PHY_PAPRD_AM2PM, AR_PHY_PAPRD_AM2PM_MASK,
ah->paprd_ratemask);
REG_RMW_FIELD(ah, AR_PHY_PAPRD_HT40, AR_PHY_PAPRD_HT40_MASK,
- AR_PHY_PAPRD_HT40_MASK);
+ ah->paprd_ratemask_ht40);
for (i = 0; i < ah->caps.max_txchains; i++) {
REG_RMW_FIELD(ah, ctrl0[i],
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index b98053f..b8ffaa5 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -835,6 +835,7 @@ struct ath_hw {
unsigned int paprd_target_power;
unsigned int paprd_training_power;
unsigned int paprd_ratemask;
+ unsigned int paprd_ratemask_ht40;
bool paprd_table_write_done;
u32 paprd_gain_table_entries[PAPRD_GAIN_TABLE_ENTRIES];
u8 paprd_gain_table_index[PAPRD_GAIN_TABLE_ENTRIES];
--
1.7.3.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox