* Re: [PATCH 09/29] wl12xx: make event handling support multirole
From: Luciano Coelho @ 2011-10-10 18:58 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In-Reply-To: <1318234397-21081-10-git-send-email-eliad@wizery.com>
On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
> Some events don't indicate the role they are intended for.
> In these cases, iterate through all the relevant vifs,
> and pass the event to each one of them.
>
> This is only a workaround. future fw releases should indicate
> the relevant role_id for such events.
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
Is this workaround really necessary? Are we going to have a firmware
that supports multi-role but does *not* pass the role_id for these
events?
--
Cheers,
Luca.
^ permalink raw reply
* Compat-wireless release for 2011-10-10 is baked
From: Compat-wireless cronjob account @ 2011-10-10 19:03 UTC (permalink / raw)
To: linux-wireless
compat-wireless code metrics
811845 - Total upstream lines of code being pulled
2431 - backport code changes
2113 - backport code additions
318 - backport code deletions
8588 - backport from compat module
11019 - total backport code
1.3573 - % of code consists of backport work
^ permalink raw reply
* Re: [PATCH 0/8] wireless: add DFS master support
From: Luis R. Rodriguez @ 2011-10-10 19:05 UTC (permalink / raw)
To: Zefir Kurtisi; +Cc: linux-wireless, linville, Johannes Berg
In-Reply-To: <4E92B74F.9030901@neratec.com>
On Mon, Oct 10, 2011 at 2:13 AM, Zefir Kurtisi
<zefir.kurtisi@neratec.com> wrote:
> On 10/08/2011 08:26 PM, Luis R. Rodriguez wrote:
>> On Sat, Oct 8, 2011 at 10:46 AM, Zefir Kurtisi
>> <zefir.kurtisi@neratec.com> wrote:
>>> On 08.10.2011 00:32, Luis R. Rodriguez wrote:
>>>>
>>>> On Fri, Oct 7, 2011 at 3:29 PM, Luis R. Rodriguez
>>>> <mcgrof@qca.qualcomm.com> wrote:
>>>>>
>>>>> On Fri, Oct 7, 2011 at 2:11 PM, Luis R. Rodriguez
>>>>> <mcgrof@qca.qualcomm.com> wrote:
>>>>>>
>>>>>> On Tue, Oct 4, 2011 at 5:14 PM, Luis R. Rodriguez
>>>>>> <mcgrof@qca.qualcomm.com> wrote:
>>>>>>>
>>>>>>> On Tue, Oct 4, 2011 at 4:47 PM, Luis R. Rodriguez
>>>>>>> <mcgrof@qca.qualcomm.com> wrote:
>>>>>>>>
>>>>>>>> This set of 8 patches adds DFS master support to the Linux wireless
>>>>>>>> subsystem.
>>>>>>>> I've reviewed future possible changes to DFS master regions and it
>>>>>>>> seems that
>>>>>>>> we are not going to be having multiple DFS regions for one country,
>>>>>>>> instead
>>>>>>>> we'll always have one DFS region for one country.
>>>>>>>>
>>>>>>>> The changes here are spread out throughout wireless-regdb, crda the
>>>>>>>> kernel and
>>>>>>>> lastly iw. The changes made allow for older verions of CRDA to work
>>>>>>>> with new
>>>>>>>> wireless-regdb files with DFS region support. If you want DFS master
>>>>>>>> region
>>>>>>>> support you'll need to upgrade your CRDA, your kernel and then hope
>>>>>>>> someone
>>>>>>>> implements DFS master support for your respective driver.
>>>>>>>>
>>>>>>>> This patch series does not have specific driver changes, although some
>>>>>>>> seem to
>>>>>>>> be backing in the oven right now.
>>>>>>>
>>>>>>> Here's a puzzle though... If we change this series to use the other
>>>>>>> pad byte that was available, the first pad byte, instead of the last
>>>>>>> one, we loose backward compatibility support and I cannot figure out
>>>>>>> why. What I ended up seeing was that crda sends the message, and for
>>>>>>> some reason (return code is 222 from nl_wait_for_ack(), whatever that
>>>>>>> is) the kernel rejects it. I suspect it may have to do with some sort
>>>>>>> of offset to the *other* data that makes some of the rules output
>>>>>>> invalid data for the attribute policy, but at least when I hexdump the
>>>>>>> wireless-regdb the only changes I see are in the signature and the pad
>>>>>>> shift.
>>>>>>>
>>>>>>> I got tired of trying though and after seeing flipping the pad bytes
>>>>>>> things worked decided to stay with it. In my original RFC in December
>>>>>>> I had used u16 instead, but since the data was in the last pad byte
>>>>>>> things still worked. So something is fishy about only using the first
>>>>>>> pad byte. The change below, as far as I can tell, should not have any
>>>>>>> issues but it does with the older version of CRDA and even a new one.
>>>>>>
>>>>>> Johannes spotted the issue, I'll send the fix, thanks to Johannes.
>>>>>> John, Johannes the patches still apply my fix goes on top of these
>>>>>> changes, the fix is not addressing a regression introduced by this
>>>>>> patchset, instead it fixes a long standing issue which would prevent
>>>>>> us from using the next available pad byte.
>>>>>
>>>>> I'm going to respin this to make use of 2 bits:
>>>>>
>>>>> 00 unset
>>>>> 01 FCC
>>>>> 10 ETSI
>>>>> 11 JP
>>>>>
>>>>> We may need some more DFS values later but
>>>>
>>>> Sorry I did not finish this e-mail I meant that we may later have a
>>>> requirement for more DFS values but at this time we don't, we should
>>>> consider whether or not we will want to leave more bits for usage of
>>>> more DFS values and if so how many? Using two bits will give us
>>>> support for what we know today but nothing for the future.
>>>>
>>> For the future things we do not know of today we should add another bit and
>>> define
>>>
>>> 111 unknown / other
>>>
>>> to be able to mark special countrycodes that do not fully belong to the
>>> three known domains.
>>
>> I thought about this a bit more, so we'd have to define this as a U8
>> attribute either way, so I would instead of calling NL80211_DFS_REGION
>> call it something like NL80211_CTRY_REQS and for now only use 2 bits
>> for the known DFS regions. We'd mask out the rest of the values. If we
>> ever decide we need a new DFS region we could just extend the values
>> on NL80211_CTRY_REQS after the last region. If we want to add a new
>> requirement that is country specific other than DFS we could start at
>> the end of the u8 instead of after the last DFS region. We could do
>> this without making any explicit reservations.
>>
>>> We could use some invalid coding for those CCs (like no
>>> DFS domain set in CC
>>
>> Sure, I should point out today every single regulatory domain will
>> have this set except US, only because I added a patch to match US to
>> DFS-FCC DFS region.
>>
>>> but flag set for frequency band) to identify those
>>> special domains,
>>
>> So you're saying in case later we find out we need band specific DFS data later?
>>
> No, I guess I initially misinterpreted the meaning of '00 unset' as a flag for countrycodes not restricted by DFS at all, but in fact this is done by not setting the per-band DFS flags.
>
> So, if we consider the per-country DFS domain as an enabler for DFS support on top of per-band DFS restrictions (that otherwise remain disabled), we can support 'fancy' CCs by not setting the domain. The concern I brought up is therefore void.
>
> WRT band specific DFS data, we agreed that this will break backwards compatibility and therefore needs some detailed planning. Adrian already brought up some issues we will face and why he is using per-band (or even per-channel) DFS properties.
>
> I opt for keeping this proposed per-country DFS domain scheme until we get DFS fully working and find its limitations significant enough to start a major revision.
Sure, I'll respin my patches to use only 2 bits for now on the
wireless-regdb. As far as nl80211 is concerned though since we can
only send U8 as a minimum I'll define this as a NL80211_CTRY_REQS and
we can stuff whatever we want into it later on CRDA.
Luis
^ permalink raw reply
* Re: [PATCH 02/29] wl12xx: move last_tx_hlid into wlvif
From: Eliad Peller @ 2011-10-10 19:16 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
In-Reply-To: <1318269135.9690.369.camel@cumari>
On Mon, Oct 10, 2011 at 7:52 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
>> move last_tx_hlid into the per-interface data, rather than
>> being global.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>> drivers/net/wireless/wl12xx/debugfs.c | 1 -
>> drivers/net/wireless/wl12xx/main.c | 1 -
>> drivers/net/wireless/wl12xx/tx.c | 10 +++++-----
>> drivers/net/wireless/wl12xx/wl12xx.h | 6 +++---
>> 4 files changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/wireless/wl12xx/debugfs.c b/drivers/net/wireless/wl12xx/debugfs.c
>> index f0398d0..bbc8004 100644
>> --- a/drivers/net/wireless/wl12xx/debugfs.c
>> +++ b/drivers/net/wireless/wl12xx/debugfs.c
>> @@ -355,7 +355,6 @@ static ssize_t driver_state_read(struct file *file, char __user *user_buf,
>> DRIVER_STATE_PRINT_INT(sg_enabled);
>> DRIVER_STATE_PRINT_INT(enable_11a);
>> DRIVER_STATE_PRINT_INT(noise);
>> - DRIVER_STATE_PRINT_INT(last_tx_hlid);
>> DRIVER_STATE_PRINT_HEX(ap_fw_ps_map);
>> DRIVER_STATE_PRINT_LHEX(ap_ps_map);
>> DRIVER_STATE_PRINT_HEX(quirks);
>
> No big deal at this point, so I'll apply this, but maybe we should print
> the last_tx_hlid from inside all the vifs as well?
>
yes.
i plan to add a new vif_state debugfs key that will print all the relevant info.
Eliad.
^ permalink raw reply
* Re: [PATCH 05/29] wl12xx: move bitrate_masks into wlvif
From: Eliad Peller @ 2011-10-10 19:21 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
In-Reply-To: <1318270806.9690.373.camel@cumari>
On Mon, Oct 10, 2011 at 8:20 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
>> move bitrate_masks into the per-interface data, rather than
>> being global.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>
> [...]
>
>> diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
>> index 102a8a5..ff653e8 100644
>> --- a/drivers/net/wireless/wl12xx/cmd.c
>> +++ b/drivers/net/wireless/wl12xx/cmd.c
>> @@ -1111,15 +1111,16 @@ out:
>> return ret;
>> }
>>
>> -int wl1271_cmd_build_probe_req(struct wl1271 *wl,
>> +int wl1271_cmd_build_probe_req(struct wl1271 *wl, struct wl12xx_vif *wlvif,
>> const u8 *ssid, size_t ssid_len,
>> const u8 *ie, size_t ie_len, u8 band)
>
> If struct wl12xx_vif contains a pointer to wl, can't you pass only wlvif
> to all these functions instead of passing both?
>
yes, it's possible.
but maybe leave it for some future overhaul?
Eliad.
^ permalink raw reply
* Re: [PATCH 10/29] wl12xx: move tx_security_seq into wlvif
From: Luciano Coelho @ 2011-10-10 19:25 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In-Reply-To: <1318234397-21081-11-git-send-email-eliad@wizery.com>
On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
> The last security seq num has to be saved across reconfigs.
> Add a new "persistent" struct into wlvif, which won't get
> deleted on wl12xx_init_vif_data()
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
> @@ -1910,8 +1914,8 @@ static int wl12xx_init_vif_data(struct wl1271 *wl, struct ieee80211_vif *vif)
> {
> struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
>
> - /* make sure wlvif is zeroed */
> - memset(wlvif, 0, sizeof(*wlvif));
> + /* clear everything but the persistent data */
> + memset(wlvif, 0, offsetof(struct wl12xx_vif, persistent));
Nice trick! :)
> diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
> index c7be151..0a5fef5 100644
> --- a/drivers/net/wireless/wl12xx/tx.c
> +++ b/drivers/net/wireless/wl12xx/tx.c
> @@ -755,6 +755,8 @@ static void wl1271_tx_complete_packet(struct wl1271 *wl,
> struct wl1271_tx_hw_res_descr *result)
> {
> struct ieee80211_tx_info *info;
> + struct ieee80211_vif *vif;
> + struct wl12xx_vif *wlvif;
> struct sk_buff *skb;
> int id = result->id;
> int rate = -1;
> @@ -774,6 +776,10 @@ static void wl1271_tx_complete_packet(struct wl1271 *wl,
> return;
> }
>
> + /* info->control is valid as long as we don't update info->status */
> + vif = info->control.vif;
> + wlvif = wl12xx_vif_to_data(vif);
>From a comment in the ieee80211_tx_info struct in mac80211.h:
/* NB: vif can be NULL for injected frames */
Should we check the vif here...
> /* update the TX status info */
> if (result->status == TX_SUCCESS) {
> if (!(info->flags & IEEE80211_TX_CTL_NO_ACK))
> @@ -801,14 +807,14 @@ static void wl1271_tx_complete_packet(struct wl1271 *wl,
> info->control.hw_key->cipher == WLAN_CIPHER_SUITE_CCMP ||
> info->control.hw_key->cipher == WL1271_CIPHER_SUITE_GEM)) {
> u8 fw_lsb = result->tx_security_sequence_number_lsb;
> - u8 cur_lsb = wl->tx_security_last_seq_lsb;
> + u8 cur_lsb = wlvif->tx_security_last_seq_lsb;
...because we're accessing it directly here?
And don't we get TX complete events for dummy packets?
> /*
> * update security sequence number, taking care of potential
> * wrap-around
> */
> - wl->tx_security_seq += (fw_lsb - cur_lsb + 256) % 256;
> - wl->tx_security_last_seq_lsb = fw_lsb;
> + wlvif->tx_security_seq += (fw_lsb - cur_lsb + 256) % 256;
> + wlvif->tx_security_last_seq_lsb = fw_lsb;
We could change this (... + 256) % 256 here for & 0xFF as Ido suggested
for some other code earlier.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH 05/29] wl12xx: move bitrate_masks into wlvif
From: Luciano Coelho @ 2011-10-10 19:25 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In-Reply-To: <CAB3XZEejP2iELFGd+s+-4fBPTxDk_iuNPhU1AZdmCxneBKGF-Q@mail.gmail.com>
On Mon, 2011-10-10 at 21:21 +0200, Eliad Peller wrote:
> On Mon, Oct 10, 2011 at 8:20 PM, Luciano Coelho <coelho@ti.com> wrote:
> > On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
> >> move bitrate_masks into the per-interface data, rather than
> >> being global.
> >>
> >> Signed-off-by: Eliad Peller <eliad@wizery.com>
> >> ---
> >
> > [...]
> >
> >> diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
> >> index 102a8a5..ff653e8 100644
> >> --- a/drivers/net/wireless/wl12xx/cmd.c
> >> +++ b/drivers/net/wireless/wl12xx/cmd.c
> >> @@ -1111,15 +1111,16 @@ out:
> >> return ret;
> >> }
> >>
> >> -int wl1271_cmd_build_probe_req(struct wl1271 *wl,
> >> +int wl1271_cmd_build_probe_req(struct wl1271 *wl, struct wl12xx_vif *wlvif,
> >> const u8 *ssid, size_t ssid_len,
> >> const u8 *ie, size_t ie_len, u8 band)
> >
> > If struct wl12xx_vif contains a pointer to wl, can't you pass only wlvif
> > to all these functions instead of passing both?
> >
> yes, it's possible.
> but maybe leave it for some future overhaul?
Yes, it's okay. Otherwise the rebasing of this would be annoying to
do. ;)
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH 06/29] wl12xx: add vifs list
From: Eliad Peller @ 2011-10-10 19:29 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
In-Reply-To: <1318271328.9690.381.camel@cumari>
On Mon, Oct 10, 2011 at 8:28 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
>> keep a list of all the vifs associated with our hw.
>> it will be later used in order to iterate through vifs.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>
> [...]
>
>> diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
>> index 33ccdf8..55561c5 100644
>> --- a/drivers/net/wireless/wl12xx/wl12xx.h
>> +++ b/drivers/net/wireless/wl12xx/wl12xx.h
>> @@ -394,6 +394,8 @@ struct wl1271 {
>> unsigned long roles_map[BITS_TO_LONGS(WL12XX_MAX_ROLES)];
>> unsigned long roc_map[BITS_TO_LONGS(WL12XX_MAX_ROLES)];
>>
>> + struct list_head wlvif_list;
>> +
>> struct wl1271_acx_mem_map *target_mem_map;
>>
>> /* Accounting for allocated / available TX blocks on HW */
>> @@ -564,6 +566,7 @@ struct wl1271_station {
>>
>> struct wl12xx_vif {
>> struct wl1271 *wl;
>> + struct list_head list;
>
> I know we alredy call this kind of thing "list" struct wl1271 for the
> ARP stuff, but I prefer "node" so it's easier to follow when calling
> list_add and friends. Mind changing?
>
i think "list" is the standard convention (e.g. see sta_info,
ieee80211_key, etc.) and more intuitive.
>
>> @@ -653,6 +656,9 @@ struct ieee80211_vif *wl12xx_wlvif_to_vif(struct wl12xx_vif *wlvif)
>> return container_of((void *)wlvif, struct ieee80211_vif, drv_priv);
>> }
>>
>> +#define wl12xx_for_each_wlvif(wl, wlvif) \
>> + list_for_each_entry(wlvif, &wl->wlvif_list, list)
>> +
>
> This looks fine, but is it really necessary? I think the call to
> list_for_each_entry is clear enough already, isn't it?
>
i find it easier to understand this way.
also, future patches will add wl12xx_for_each_wlvif_sta and
wl12xx_for_each_wlvif_ap, so it even makes more sense to use this
macro.
Eliad.
^ permalink raw reply
* Re: [alsa-devel] Using spdiff for backporting
From: Luis R. Rodriguez @ 2011-10-10 19:30 UTC (permalink / raw)
To: Takashi Iwai
Cc: Jesper Andersen, linux-wireless, alsa-devel, julia, linux-kernel
In-Reply-To: <s5hy5wtxbmm.wl%tiwai@suse.de>
On Mon, Oct 10, 2011 at 5:41 AM, Takashi Iwai <tiwai@suse.de> wrote:
> At Fri, 7 Oct 2011 10:41:06 -0700,
> Luis R. Rodriguez wrote:
>>
>> > Also, in the slides from the plumbers-conf. I think I saw it mentioned
>> > that there's also an alsa-compat git-tree somewhere. Maybe that is a
>> > more compelling usecase? I was unable to find it though so I'd
>> > appreciate a link.
>>
>> It was on kernel.org, but if it got rm -rf'd then its gone as I do not
>> have a local copy.
>
> Oh, I didn't know of such a tree. So you created alsa driver build
> system with your spdiff? That's interesting.
Nope, this was pre-spdiff days. It was just a proof of concept I
wanted to make just to see how long it would take me to do it and how
clean it was. It was pretty trivial. I'm now working with Jesper on
seeing how best we can use spdiff for a unified backport of the
kernel.
>> The compat-alsa stuff though was determined to be
>> superflous with the ALSA's team's own backport work which is currently
>> independent.
>
> Yes, the external alsa-driver build tree has existed since 10 years
> ago :) It was even possible to build with 2.2/2.4 kernels until
> recently.
>
> The current tree is found in github,
> git://github.com/tiwai/alsa-driver-build.git
That's pretty darn impressive.
>> My hope though is to unify these through the compat.git /
>> compat-kernel (currently just called compat-wireless) effort.
>
> The common framework would be really nice to have.
Agreed!
> V4L also have own build system, and possible other subsystem trees too.
Heh, yeah for Video at least Ubuntu guys just backported a newer DRM
into their distro kernel. I feel as if a generic backport would make
more sense. I have already split out all generic kernel backport stuff
into its own module, compat.git, and anything else that cannot be
backported within an independent module we'e kept on compat-wireless.
compat-wireless at this point should really be called compat-net but
I've been lazy to rename it.
At plumbers I put together slides for metrics of what goes into compat
or compat-wireles, how many lines of code we pull and how much code
consists of backport work. The conclusion was with compat backporting
a new subsystem becomes proportionally easier. Then the legwork to
keep compat-wireless up to date can benefit from spdiff as the
spatches can eseentially generate new patches to help backport new
drivers after just two drivers get backported for a condition which
compat cannot address.
The slides are at:
http://bombadil.infradead.org/~mcgrof/presentations/backport/kernel-backport-for-good.pdf
Luis
^ permalink raw reply
* Re: [PATCH 08/29] wl12xx: configure sleep_policy according to active roles
From: Eliad Peller @ 2011-10-10 19:32 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
In-Reply-To: <1318272712.9690.384.camel@cumari>
On Mon, Oct 10, 2011 at 8:51 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
>> If there is an active AP role, stay always on.
>> Otherwise, allow chip to enter elp.
>>
>> (Note that this is a global configuration, so if the
>> device is already configured according to our policy,
>> we don't have to configure it again)
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>
> [...]
>
>> @@ -573,13 +563,38 @@ static int wl12xx_init_ap_role(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>>
>> int wl1271_init_vif_specific(struct wl1271 *wl, struct ieee80211_vif *vif)
>> {
>> - struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
>> + struct wl12xx_vif *iter, *wlvif = wl12xx_vif_to_data(vif);
>> struct conf_tx_ac_category *conf_ac;
>> struct conf_tx_tid *conf_tid;
>> bool is_ap = (wlvif->bss_type == BSS_TYPE_AP_BSS);
>> -
>> + u8 sta_roles_cnt = 0, ap_roles_cnt = 0;
>> int ret, i;
>>
>> + wl12xx_for_each_wlvif(wl, iter) {
>> + if (iter->bss_type == BSS_TYPE_AP_BSS)
>> + ap_roles_cnt++;
>> + else
>> + sta_roles_cnt++;
>> + }
>> +
>> + /*
>> + * consider all existing roles before configuring psm.
>> + * TODO: reconfigure on interface removal.
>> + */
>> + if (!ap_roles_cnt) {
>> + if (is_ap) {
>> + /* Configure for power always on */
>> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
>> + if (ret < 0)
>> + return ret;
>> + } else if (!sta_roles_cnt) {
>> + /* Configure for ELP power saving */
>> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
>> + if (ret < 0)
>> + return ret;
>> + }
>> + }
>> +
>
> Wouldn't it be much simpler to have a "have_ap" element in the wl
> struct?
>
>
have_ap is not enough, because there might be multiple aps, and we
need to consider it on removal.
we can use ap_count, if you prefer it that way.
Eliad.
^ permalink raw reply
* Re: [PATCH 09/29] wl12xx: make event handling support multirole
From: Eliad Peller @ 2011-10-10 19:36 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
In-Reply-To: <1318273094.9690.385.camel@cumari>
On Mon, Oct 10, 2011 at 8:58 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
>> Some events don't indicate the role they are intended for.
>> In these cases, iterate through all the relevant vifs,
>> and pass the event to each one of them.
>>
>> This is only a workaround. future fw releases should indicate
>> the relevant role_id for such events.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>
> Is this workaround really necessary? Are we going to have a firmware
> that supports multi-role but does *not* pass the role_id for these
> events?
>
i hope not. :)
but in the meantime, we use this workaround to keep compatibility with
the current (single-role) fw.
Eliad.
^ permalink raw reply
* Re: [PATCH 06/29] wl12xx: add vifs list
From: Luciano Coelho @ 2011-10-10 19:40 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In-Reply-To: <CAB3XZEfA2woetrXDycv3xcBYGBvyYJQoLz969Rp8k8WjaG3FFw@mail.gmail.com>
On Mon, 2011-10-10 at 21:29 +0200, Eliad Peller wrote:
> On Mon, Oct 10, 2011 at 8:28 PM, Luciano Coelho <coelho@ti.com> wrote:
> > On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
> >> keep a list of all the vifs associated with our hw.
> >> it will be later used in order to iterate through vifs.
> >>
> >> Signed-off-by: Eliad Peller <eliad@wizery.com>
> >> ---
> >
> > [...]
> >
> >> diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
> >> index 33ccdf8..55561c5 100644
> >> --- a/drivers/net/wireless/wl12xx/wl12xx.h
> >> +++ b/drivers/net/wireless/wl12xx/wl12xx.h
> >> @@ -394,6 +394,8 @@ struct wl1271 {
> >> unsigned long roles_map[BITS_TO_LONGS(WL12XX_MAX_ROLES)];
> >> unsigned long roc_map[BITS_TO_LONGS(WL12XX_MAX_ROLES)];
> >>
> >> + struct list_head wlvif_list;
> >> +
> >> struct wl1271_acx_mem_map *target_mem_map;
> >>
> >> /* Accounting for allocated / available TX blocks on HW */
> >> @@ -564,6 +566,7 @@ struct wl1271_station {
> >>
> >> struct wl12xx_vif {
> >> struct wl1271 *wl;
> >> + struct list_head list;
> >
> > I know we alredy call this kind of thing "list" struct wl1271 for the
> > ARP stuff, but I prefer "node" so it's easier to follow when calling
> > list_add and friends. Mind changing?
> >
> i think "list" is the standard convention (e.g. see sta_info,
> ieee80211_key, etc.) and more intuitive.
Yeah, it makes a bit of sense, because each node is actually a head of a
list. Still, I don't like it because it's annoying to read it a
list_add() with two lists. It's not intuitive to know which one is
which. Anyway, it's fine to leave it like this.
> >> @@ -653,6 +656,9 @@ struct ieee80211_vif *wl12xx_wlvif_to_vif(struct wl12xx_vif *wlvif)
> >> return container_of((void *)wlvif, struct ieee80211_vif, drv_priv);
> >> }
> >>
> >> +#define wl12xx_for_each_wlvif(wl, wlvif) \
> >> + list_for_each_entry(wlvif, &wl->wlvif_list, list)
> >> +
> >
> > This looks fine, but is it really necessary? I think the call to
> > list_for_each_entry is clear enough already, isn't it?
> >
> i find it easier to understand this way.
> also, future patches will add wl12xx_for_each_wlvif_sta and
> wl12xx_for_each_wlvif_ap, so it even makes more sense to use this
> macro.
Yeah, I now saw the other macros and it now makes a bit more sense for
the other ones. But I dislike simple macros whose content are almost as
easy to understand as the macro name, because one needs to go dig the
macro out to figure out exactly what it is doing. With no macro, it
would be much easier to figure out that this is a simple
list_for_each_entry(), because you would simply call that explicitly. ;)
It's as annoying as unnecessary typedefs.
But I'm in a temporary and unusual mood of accepting stuff easier than
usual right now, so it's fine to keep it like this. ;)
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH 09/29] wl12xx: make event handling support multirole
From: Luciano Coelho @ 2011-10-10 19:47 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In-Reply-To: <CAB3XZEc9DcYHmph9wH=+8jQkjBPAiGqHW7P-oVcQJcBHeONNWQ@mail.gmail.com>
On Mon, 2011-10-10 at 21:36 +0200, Eliad Peller wrote:
> On Mon, Oct 10, 2011 at 8:58 PM, Luciano Coelho <coelho@ti.com> wrote:
> > On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
> >> Some events don't indicate the role they are intended for.
> >> In these cases, iterate through all the relevant vifs,
> >> and pass the event to each one of them.
> >>
> >> This is only a workaround. future fw releases should indicate
> >> the relevant role_id for such events.
> >>
> >> Signed-off-by: Eliad Peller <eliad@wizery.com>
> >> ---
> >
> > Is this workaround really necessary? Are we going to have a firmware
> > that supports multi-role but does *not* pass the role_id for these
> > events?
> >
> i hope not. :)
> but in the meantime, we use this workaround to keep compatibility with
> the current (single-role) fw.
What I meant is that you could keep this global, without moving it
temporarily to the wlvif and traversing the list, which has always only
a single element anyway. But now I see that you want to get rid of the
global wl->vif, so it's fine. It's only the list traversal that is
actually a workaround, the rest of the changes are actually fine.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH 08/29] wl12xx: configure sleep_policy according to active roles
From: Luciano Coelho @ 2011-10-10 19:54 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In-Reply-To: <CAB3XZEcXGyia5r1jrfjYTwrNvFDpxpckY=jX6qYyWuHg-iBYvg@mail.gmail.com>
On Mon, 2011-10-10 at 21:32 +0200, Eliad Peller wrote:
> On Mon, Oct 10, 2011 at 8:51 PM, Luciano Coelho <coelho@ti.com> wrote:
> > On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
> >> If there is an active AP role, stay always on.
> >> Otherwise, allow chip to enter elp.
> >>
> >> (Note that this is a global configuration, so if the
> >> device is already configured according to our policy,
> >> we don't have to configure it again)
> >>
> >> Signed-off-by: Eliad Peller <eliad@wizery.com>
> >> ---
> >
> > [...]
> >
> >> @@ -573,13 +563,38 @@ static int wl12xx_init_ap_role(struct wl1271 *wl, struct wl12xx_vif *wlvif)
> >>
> >> int wl1271_init_vif_specific(struct wl1271 *wl, struct ieee80211_vif *vif)
> >> {
> >> - struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
> >> + struct wl12xx_vif *iter, *wlvif = wl12xx_vif_to_data(vif);
> >> struct conf_tx_ac_category *conf_ac;
> >> struct conf_tx_tid *conf_tid;
> >> bool is_ap = (wlvif->bss_type == BSS_TYPE_AP_BSS);
> >> -
> >> + u8 sta_roles_cnt = 0, ap_roles_cnt = 0;
> >> int ret, i;
> >>
> >> + wl12xx_for_each_wlvif(wl, iter) {
> >> + if (iter->bss_type == BSS_TYPE_AP_BSS)
> >> + ap_roles_cnt++;
> >> + else
> >> + sta_roles_cnt++;
> >> + }
> >> +
> >> + /*
> >> + * consider all existing roles before configuring psm.
> >> + * TODO: reconfigure on interface removal.
> >> + */
> >> + if (!ap_roles_cnt) {
> >> + if (is_ap) {
> >> + /* Configure for power always on */
> >> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
> >> + if (ret < 0)
> >> + return ret;
> >> + } else if (!sta_roles_cnt) {
> >> + /* Configure for ELP power saving */
> >> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
> >> + if (ret < 0)
> >> + return ret;
> >> + }
> >> + }
> >> +
> >
> > Wouldn't it be much simpler to have a "have_ap" element in the wl
> > struct?
> >
> >
> have_ap is not enough, because there might be multiple aps, and we
> need to consider it on removal.
Right...
> we can use ap_count, if you prefer it that way.
Yeah, I'd still prefer that.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH 06/29] wl12xx: add vifs list
From: Johannes Berg @ 2011-10-10 20:04 UTC (permalink / raw)
To: Eliad Peller; +Cc: Luciano Coelho, linux-wireless
In-Reply-To: <1318234397-21081-7-git-send-email-eliad@wizery.com>
On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
> keep a list of all the vifs associated with our hw.
> it will be later used in order to iterate through vifs.
What's wrong with ieee80211_iterate_active_interfaces() :-)
johannes
^ permalink raw reply
* Re: [PATCH 06/29] wl12xx: add vifs list
From: Eliad Peller @ 2011-10-10 20:07 UTC (permalink / raw)
To: Johannes Berg; +Cc: Luciano Coelho, linux-wireless
In-Reply-To: <1318277096.4001.1.camel@jlt3.sipsolutions.net>
On Mon, Oct 10, 2011 at 10:04 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
>> keep a list of all the vifs associated with our hw.
>> it will be later used in order to iterate through vifs.
>
> What's wrong with ieee80211_iterate_active_interfaces() :-)
defining iterator for each time we want to iterate is pretty cumbersome :)
^ permalink raw reply
* Re: [PATCH 21/29] wl12xx: add band field to wlvif
From: Luciano Coelho @ 2011-10-10 20:38 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In-Reply-To: <1318234397-21081-22-git-send-email-eliad@wizery.com>
On Mon, 2011-10-10 at 10:13 +0200, Eliad Peller wrote:
> add band field into the per-interface data.
>
> mac80211 configures some values (e.g. band, channel)
> globally, while we configure them per-interface.
>
> In order to make it easier to keep track of the
> configured value for each value while keeping sync
> with mac80211, save these values both globally
> and per-vif.
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
[...]
> -static int wl12xx_cmd_roc(struct wl1271 *wl, u8 role_id)
> +static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
> + u8 role_id)
[...]
> -int wl12xx_roc(struct wl1271 *wl, u8 role_id)
> +int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id)
Seems like wl12xx_roc(), which is the only caller of wl12xx_cmd_roc(),
is always called with wlvif->role_id as the role, so it seems a bit
useless to pass role_id separately all the way.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH 21/29] wl12xx: add band field to wlvif
From: Eliad Peller @ 2011-10-10 20:47 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
In-Reply-To: <1318279138.9690.421.camel@cumari>
On Mon, Oct 10, 2011 at 10:38 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Mon, 2011-10-10 at 10:13 +0200, Eliad Peller wrote:
>> add band field into the per-interface data.
>>
>> mac80211 configures some values (e.g. band, channel)
>> globally, while we configure them per-interface.
>>
>> In order to make it easier to keep track of the
>> configured value for each value while keeping sync
>> with mac80211, save these values both globally
>> and per-vif.
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>
> [...]
>
>
>> -static int wl12xx_cmd_roc(struct wl1271 *wl, u8 role_id)
>> +static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
>> + u8 role_id)
>
> [...]
>
>> -int wl12xx_roc(struct wl1271 *wl, u8 role_id)
>> +int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id)
>
> Seems like wl12xx_roc(), which is the only caller of wl12xx_cmd_roc(),
> is always called with wlvif->role_id as the role, so it seems a bit
> useless to pass role_id separately all the way.
>
in fact, it's usually being called with wlvif->dev_role_id, so we do
need this param.
(otoh, wl12xx_roc is usually get called along with start_role_dev. i
have some pending patch that couples them together and cleans the code
a bit. i'll send it soon).
Eliad.
^ permalink raw reply
* Re: [PATCH 0/8] wireless: add DFS master support
From: Luis R. Rodriguez @ 2011-10-10 23:32 UTC (permalink / raw)
To: Zefir Kurtisi; +Cc: linux-wireless, linville, Johannes Berg
In-Reply-To: <CAB=NE6XU57NLU=Tq_eqWoUtK_ppdd-PBH=tGCbEu7vQ1cPzpyQ@mail.gmail.com>
On Mon, Oct 10, 2011 at 12:05 PM, Luis R. Rodriguez
<mcgrof@qca.qualcomm.com> wrote:
> Sure, I'll respin my patches to use only 2 bits for now on the
> wireless-regdb. As far as nl80211 is concerned though since we can
> only send U8 as a minimum I'll define this as a NL80211_CTRY_REQS and
> we can stuff whatever we want into it later on CRDA.
And more to Johannes' point, my kernel patches can go as-is, the only
one that needs to deal with the nastiness is crda.c, not even the
wireless-regdb patches need to be respin'd. I did make a typo on
nl80211.h though so I will have to respin, also for the sake of
clarity.
Luis
^ permalink raw reply
* Re: [PATCH 21/29] wl12xx: add band field to wlvif
From: Victor Goldenshtein @ 2011-10-11 9:48 UTC (permalink / raw)
To: Eliad Peller, Luciano Coelho; +Cc: linux-wireless
In-Reply-To: <CAB3XZEf+YgQMdsduXb1eL5r_E+A-Y0EdyP-4map4ZqmQbLTweA@mail.gmail.com>
On 10/10/2011 22:47, Eliad Peller wrote:
> On Mon, Oct 10, 2011 at 10:38 PM, Luciano Coelho<coelho@ti.com> wrote:
>> On Mon, 2011-10-10 at 10:13 +0200, Eliad Peller wrote:
> (otoh, wl12xx_roc is usually get called along with start_role_dev. i
> have some pending patch that couples them together and cleans the code
> a bit. i'll send it soon).
>
> Eliad.
> --
> 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
Guys,
Just fyi we will use the roc/croc also in DFS implementation.
Thanks,
Victor.
^ permalink raw reply
* [PATCH] wl12xx: couple role_start_dev with roc
From: Eliad Peller @ 2011-10-11 9:55 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
Device role is always started along with ROC.
Couple them together by introducing new wl12xx_start_dev
and wl12xx_stop_dev functions.
By using these functions, we solve a bug that occured during
channel switch - we started the dev role on one channel, and
ROCed on a different one.
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/cmd.c | 53 ++++++++++++++++++++++++++++++++++-
drivers/net/wireless/wl12xx/cmd.h | 4 +-
drivers/net/wireless/wl12xx/main.c | 40 +++++++--------------------
drivers/net/wireless/wl12xx/scan.c | 3 +-
drivers/net/wireless/wl12xx/tx.c | 6 +---
5 files changed, 65 insertions(+), 41 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
index 65bf952..8ea295e 100644
--- a/drivers/net/wireless/wl12xx/cmd.c
+++ b/drivers/net/wireless/wl12xx/cmd.c
@@ -467,7 +467,8 @@ static int wl12xx_get_new_session_id(struct wl1271 *wl,
return wlvif->session_counter;
}
-int wl12xx_cmd_role_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+static int wl12xx_cmd_role_start_dev(struct wl1271 *wl,
+ struct wl12xx_vif *wlvif)
{
struct wl12xx_cmd_role_start *cmd;
int ret;
@@ -515,7 +516,8 @@ out:
return ret;
}
-int wl12xx_cmd_role_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+static int wl12xx_cmd_role_stop_dev(struct wl1271 *wl,
+ struct wl12xx_vif *wlvif)
{
struct wl12xx_cmd_role_stop *cmd;
int ret;
@@ -1775,3 +1777,50 @@ out_free:
out:
return ret;
}
+
+/* start dev role and roc on its channel */
+int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+{
+ int ret;
+
+ if (WARN_ON(!(wlvif->bss_type == BSS_TYPE_STA_BSS ||
+ wlvif->bss_type == BSS_TYPE_IBSS)))
+ return -EINVAL;
+
+ ret = wl12xx_cmd_role_start_dev(wl, wlvif);
+ if (ret < 0)
+ goto out;
+
+ ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ if (ret < 0)
+ goto out_stop;
+
+ return 0;
+
+out_stop:
+ wl12xx_cmd_role_stop_dev(wl, wlvif);
+out:
+ return ret;
+}
+
+/* croc dev hlid, and stop the role */
+int wl12xx_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+{
+ int ret;
+
+ if (WARN_ON(!(wlvif->bss_type == BSS_TYPE_STA_BSS ||
+ wlvif->bss_type == BSS_TYPE_IBSS)))
+ return -EINVAL;
+
+ if (test_bit(wlvif->dev_role_id, wl->roc_map)) {
+ ret = wl12xx_croc(wl, wlvif->dev_role_id);
+ if (ret < 0)
+ goto out;
+ }
+
+ ret = wl12xx_cmd_role_stop_dev(wl, wlvif);
+ if (ret < 0)
+ goto out;
+out:
+ return ret;
+}
diff --git a/drivers/net/wireless/wl12xx/cmd.h b/drivers/net/wireless/wl12xx/cmd.h
index 968d5bd..3f7d0b9 100644
--- a/drivers/net/wireless/wl12xx/cmd.h
+++ b/drivers/net/wireless/wl12xx/cmd.h
@@ -39,13 +39,13 @@ int wl1271_cmd_ext_radio_parms(struct wl1271 *wl);
int wl12xx_cmd_role_enable(struct wl1271 *wl, u8 *addr, u8 role_type,
u8 *role_id);
int wl12xx_cmd_role_disable(struct wl1271 *wl, u8 *role_id);
-int wl12xx_cmd_role_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
-int wl12xx_cmd_role_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_stop_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_start_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_stop_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_start_ibss(struct wl1271 *wl, struct wl12xx_vif *wlvif);
+int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
+int wl12xx_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl1271_cmd_test(struct wl1271 *wl, void *buf, size_t buf_len, u8 answer);
int wl1271_cmd_interrogate(struct wl1271 *wl, u16 id, void *buf, size_t len);
int wl1271_cmd_configure(struct wl1271 *wl, u16 id, void *buf, size_t len);
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index f29d18d..2037208 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2441,11 +2441,7 @@ static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
if (idle) {
/* no need to croc if we weren't busy (e.g. during boot) */
if (wl12xx_is_roc(wl)) {
- ret = wl12xx_croc(wl, wlvif->dev_role_id);
- if (ret < 0)
- goto out;
-
- ret = wl12xx_cmd_role_stop_dev(wl, wlvif);
+ ret = wl12xx_stop_dev(wl, wlvif);
if (ret < 0)
goto out;
}
@@ -2467,11 +2463,7 @@ static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
ieee80211_sched_scan_stopped(wl->hw);
}
- ret = wl12xx_cmd_role_start_dev(wl, wlvif);
- if (ret < 0)
- goto out;
-
- ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ ret = wl12xx_start_dev(wl, wlvif);
if (ret < 0)
goto out;
clear_bit(WL1271_FLAG_IDLE, &wl->flags);
@@ -2537,16 +2529,13 @@ static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
*/
if (wl12xx_is_roc(wl) &&
!(conf->flags & IEEE80211_CONF_IDLE)) {
- ret = wl12xx_croc(wl,
- wlvif->dev_role_id);
+ ret = wl12xx_stop_dev(wl, wlvif);
if (ret < 0)
return ret;
- ret = wl12xx_roc(wl, wlvif,
- wlvif->dev_role_id);
+ ret = wl12xx_start_dev(wl, wlvif);
if (ret < 0)
- wl1271_warning("roc failed %d",
- ret);
+ return ret;
}
}
}
@@ -3099,8 +3088,7 @@ static int wl1271_op_hw_scan(struct ieee80211_hw *hw,
ret = -EBUSY;
goto out_sleep;
}
- wl12xx_croc(wl, wlvif->dev_role_id);
- wl12xx_cmd_role_stop_dev(wl, wlvif);
+ wl12xx_stop_dev(wl, wlvif);
}
ret = wl1271_scan(hw->priv, vif, ssid, len, req);
@@ -3611,8 +3599,7 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
if (test_and_clear_bit(WLVIF_FLAG_IBSS_JOINED,
&wlvif->flags)) {
wl1271_unjoin(wl, wlvif);
- wl12xx_cmd_role_start_dev(wl, wlvif);
- wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ wl12xx_start_dev(wl, wlvif);
}
}
}
@@ -3788,11 +3775,8 @@ sta_not_found:
}
wl1271_unjoin(wl, wlvif);
- if (!(conf_flags & IEEE80211_CONF_IDLE)) {
- wl12xx_cmd_role_start_dev(wl, wlvif);
- wl12xx_roc(wl, wlvif,
- wlvif->dev_role_id);
- }
+ if (!(conf_flags & IEEE80211_CONF_IDLE))
+ wl12xx_start_dev(wl, wlvif);
}
}
}
@@ -3871,11 +3855,7 @@ sta_not_found:
* STA role). TODO: make it better.
*/
if (wlvif->dev_role_id != WL12XX_INVALID_ROLE_ID) {
- ret = wl12xx_croc(wl, wlvif->dev_role_id);
- if (ret < 0)
- goto out;
-
- ret = wl12xx_cmd_role_stop_dev(wl, wlvif);
+ ret = wl12xx_stop_dev(wl, wlvif);
if (ret < 0)
goto out;
}
diff --git a/drivers/net/wireless/wl12xx/scan.c b/drivers/net/wireless/wl12xx/scan.c
index 2711438..96d804f 100644
--- a/drivers/net/wireless/wl12xx/scan.c
+++ b/drivers/net/wireless/wl12xx/scan.c
@@ -76,8 +76,7 @@ void wl1271_scan_complete_work(struct work_struct *work)
(is_ibss && !test_bit(WLVIF_FLAG_IBSS_JOINED, &wlvif->flags))) &&
!test_bit(wlvif->dev_role_id, wl->roc_map)) {
/* restore remain on channel */
- wl12xx_cmd_role_start_dev(wl, wlvif);
- wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ wl12xx_start_dev(wl, wlvif);
}
wl1271_ps_elp_sleep(wl);
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 05968c6..02d606f 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -98,11 +98,7 @@ static int wl1271_tx_update_filters(struct wl1271 *wl,
goto out;
wl1271_debug(DEBUG_CMD, "starting device role for roaming");
- ret = wl12xx_cmd_role_start_dev(wl, wlvif);
- if (ret < 0)
- goto out;
-
- ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ ret = wl12xx_start_dev(wl, wlvif);
if (ret < 0)
goto out;
out:
--
1.7.6.401.g6a319
^ permalink raw reply related
* [PATCH v2 08/29] wl12xx: configure sleep_policy according to active roles
From: Eliad Peller @ 2011-10-11 9:49 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
If there is an active AP role, stay always on.
Otherwise, allow chip to enter elp.
(Note that this is a global configuration, so if the
device is already configured according to our policy,
we don't have to configure it again)
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
v2: use ap_count/sta_count instead of iterating
drivers/net/wireless/wl12xx/init.c | 29 ++++++++++++++++++-----------
drivers/net/wireless/wl12xx/main.c | 10 ++++++++++
drivers/net/wireless/wl12xx/wl12xx.h | 2 ++
3 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/init.c b/drivers/net/wireless/wl12xx/init.c
index 80e89e3..4af7e2f 100644
--- a/drivers/net/wireless/wl12xx/init.c
+++ b/drivers/net/wireless/wl12xx/init.c
@@ -343,11 +343,6 @@ static int wl1271_sta_hw_init(struct wl1271 *wl, struct wl12xx_vif *wlvif)
if (ret < 0)
return ret;
- /* Configure for ELP power saving */
- ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
- if (ret < 0)
- return ret;
-
ret = wl1271_acx_sta_rate_policies(wl, wlvif);
if (ret < 0)
return ret;
@@ -382,11 +377,6 @@ static int wl1271_ap_hw_init(struct wl1271 *wl, struct wl12xx_vif *wlvif)
{
int ret;
- /* Configure for power always on */
- ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
- if (ret < 0)
- return ret;
-
ret = wl1271_init_ap_rates(wl, wlvif);
if (ret < 0)
return ret;
@@ -577,9 +567,26 @@ int wl1271_init_vif_specific(struct wl1271 *wl, struct ieee80211_vif *vif)
struct conf_tx_ac_category *conf_ac;
struct conf_tx_tid *conf_tid;
bool is_ap = (wlvif->bss_type == BSS_TYPE_AP_BSS);
-
int ret, i;
+ /*
+ * consider all existing roles before configuring psm.
+ * TODO: reconfigure on interface removal.
+ */
+ if (!wl->ap_count) {
+ if (is_ap) {
+ /* Configure for power always on */
+ ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
+ if (ret < 0)
+ return ret;
+ } else if (!wl->sta_count) {
+ /* Configure for ELP power saving */
+ ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
/* Mode specific init */
if (is_ap) {
ret = wl1271_ap_hw_init(wl, wlvif);
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 0623f5d..b52deac 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2117,6 +2117,11 @@ static int wl1271_op_add_interface(struct ieee80211_hw *hw,
wl->vif = vif;
list_add(&wlvif->list, &wl->wlvif_list);
set_bit(WL1271_FLAG_IF_INITIALIZED, &wl->flags);
+
+ if (wlvif->bss_type == BSS_TYPE_AP_BSS)
+ wl->ap_count++;
+ else
+ wl->sta_count++;
out:
mutex_unlock(&wl->mutex);
@@ -2188,6 +2193,11 @@ deinit:
wlvif->role_id = WL12XX_INVALID_ROLE_ID;
wlvif->dev_role_id = WL12XX_INVALID_ROLE_ID;
+ if (wlvif->bss_type == BSS_TYPE_AP_BSS)
+ wl->ap_count--;
+ else
+ wl->sta_count--;
+
mutex_unlock(&wl->mutex);
cancel_delayed_work_sync(&wlvif->pspoll_work);
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index 55561c5..fd78f8c 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -395,6 +395,8 @@ struct wl1271 {
unsigned long roc_map[BITS_TO_LONGS(WL12XX_MAX_ROLES)];
struct list_head wlvif_list;
+ u8 sta_count;
+ u8 ap_count;
struct wl1271_acx_mem_map *target_mem_map;
--
1.7.6.401.g6a319
^ permalink raw reply related
* [PATCH] wl12xx: reconfigure rate policies on set_bitrate_mask
From: Eliad Peller @ 2011-10-11 9:57 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
The rate policies are configured only after association,
resulting in auth req being sent in wrong rates.
Reconfigure rate policies on bitrate mask change.
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/main.c | 23 +++++++++++++++++++++--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 2037208..d2b13ca 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -4330,7 +4330,7 @@ static int wl12xx_set_bitrate_mask(struct ieee80211_hw *hw,
{
struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
struct wl1271 *wl = hw->priv;
- int i;
+ int i, ret = 0;
wl1271_debug(DEBUG_MAC80211, "mac80211 set_bitrate_mask 0x%x 0x%x",
mask->control[NL80211_BAND_2GHZ].legacy,
@@ -4343,9 +4343,28 @@ static int wl12xx_set_bitrate_mask(struct ieee80211_hw *hw,
wl1271_tx_enabled_rates_get(wl,
mask->control[i].legacy,
i);
+
+ if (unlikely(wl->state == WL1271_STATE_OFF))
+ goto out;
+
+ if (wlvif->bss_type == BSS_TYPE_STA_BSS &&
+ !test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags)) {
+
+ ret = wl1271_ps_elp_wakeup(wl);
+ if (ret < 0)
+ goto out;
+
+ wl1271_set_band_rate(wl, wlvif);
+ wlvif->basic_rate =
+ wl1271_tx_min_rate_get(wl, wlvif->basic_rate_set);
+ ret = wl1271_acx_sta_rate_policies(wl, wlvif);
+
+ wl1271_ps_elp_sleep(wl);
+ }
+out:
mutex_unlock(&wl->mutex);
- return 0;
+ return ret;
}
static void wl12xx_op_channel_switch(struct ieee80211_hw *hw,
--
1.7.6.401.g6a319
^ permalink raw reply related
* Re: [PATCH v2 08/29] wl12xx: configure sleep_policy according to active roles
From: Arik Nemtsov @ 2011-10-11 10:16 UTC (permalink / raw)
To: Eliad Peller; +Cc: Luciano Coelho, linux-wireless
In-Reply-To: <1318326555-13905-1-git-send-email-eliad@wizery.com>
On Tue, Oct 11, 2011 at 11:49, Eliad Peller <eliad@wizery.com> wrote:
> If there is an active AP role, stay always on.
> Otherwise, allow chip to enter elp.
>
> (Note that this is a global configuration, so if the
> device is already configured according to our policy,
> we don't have to configure it again)
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
> v2: use ap_count/sta_count instead of iterating
> + if (!wl->ap_count) {
> + if (is_ap) {
> + /* Configure for power always on */
> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
> + if (ret < 0)
> + return ret;
> + } else if (!wl->sta_count) {
> + /* Configure for ELP power saving */
> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
> + if (ret < 0)
> + return ret;
> + }
> + }
Probably need to add a symmetric part on remove_interface(). If ap was
shutdown we can return to normal ELP.
btw, elp_work() still gets called even in AP-mode. I'm guessing it has
no effect, but if you want to prevent the SDIO transaction from taking
place (like you said on IRC), you can check for it :)
(Just nitpicking here)
^ permalink raw reply
* Re: [PATCH v2 08/29] wl12xx: configure sleep_policy according to active roles
From: Eliad Peller @ 2011-10-11 10:46 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: Luciano Coelho, linux-wireless
In-Reply-To: <CA+XVXfdM7=G1YdWVYpzfKajRYu+sAJcfKSFkbtgneUPTPabQ=w@mail.gmail.com>
On Tue, Oct 11, 2011 at 12:16 PM, Arik Nemtsov <anamtsov@gmail.com> wrote:
> On Tue, Oct 11, 2011 at 11:49, Eliad Peller <eliad@wizery.com> wrote:
>> If there is an active AP role, stay always on.
>> Otherwise, allow chip to enter elp.
>>
>> (Note that this is a global configuration, so if the
>> device is already configured according to our policy,
>> we don't have to configure it again)
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>> v2: use ap_count/sta_count instead of iterating
>> + if (!wl->ap_count) {
>> + if (is_ap) {
>> + /* Configure for power always on */
>> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
>> + if (ret < 0)
>> + return ret;
>> + } else if (!wl->sta_count) {
>> + /* Configure for ELP power saving */
>> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
>> + if (ret < 0)
>> + return ret;
>> + }
>> + }
>
> Probably need to add a symmetric part on remove_interface(). If ap was
> shutdown we can return to normal ELP.
>
right. that's why i added:
+ /*
+ * consider all existing roles before configuring psm.
+ * TODO: reconfigure on interface removal.
+ */
> btw, elp_work() still gets called even in AP-mode. I'm guessing it has
> no effect, but if you want to prevent the SDIO transaction from taking
> place (like you said on IRC), you can check for it :)
> (Just nitpicking here)
we are not in idle-on while beaconing, so elp_work should return
before configuring the chip.
Eliad.
^ permalink raw reply
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