* 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
* [PATCH] iw: fix HT PHY BSS Membership selector value encoding
From: Christian Lamparter @ 2011-10-11 11:15 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
IEEE 802.11n 7.3.2.2 extended the supported rate IE to
support a special encoding for HT rate support.
iw needs to be updated in order to recognize
the magic value and parse it accordingly.
e.g.:
> Extended supported rates: 63.5*
now becomes:
> Extended supported rates: HT*
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
diff --git a/scan.c b/scan.c
index d083591..2220f26 100644
--- a/scan.c
+++ b/scan.c
@@ -158,6 +158,8 @@ static void print_ssid(const uint8_t type, uint8_t len, const uint8_t *data)
printf("\n");
}
+#define BSS_MEMBERSHIP_SELECTOR_HT_PHY 127
+
static void print_supprates(const uint8_t type, uint8_t len, const uint8_t *data)
{
int i;
@@ -166,7 +168,13 @@ static void print_supprates(const uint8_t type, uint8_t len, const uint8_t *data
for (i = 0; i < len; i++) {
int r = data[i] & 0x7f;
- printf("%d.%d%s ", r/2, 5*(r&1), data[i] & 0x80 ? "*":"");
+
+ if (r == BSS_MEMBERSHIP_SELECTOR_HT_PHY)
+ printf("HT");
+ else
+ printf("%d.%d", r/2, 5*(r&1));
+
+ printf("%s ", data[i] & 0x80 ? "*" : "");
}
printf("\n");
}
^ permalink raw reply related
* Re: [PATCH 1/1] ath6kl: Add WSC IE on the associate message
From: Kalle Valo @ 2011-10-11 11:24 UTC (permalink / raw)
To: Kevin Fang; +Cc: linux-wireless
In-Reply-To: <1317948679-10299-1-git-send-email-kevin.fang@qca.qualcomm.com>
On 10/07/2011 03:51 AM, Kevin Fang wrote:
> For some WPS test items, such as item "5.1.14"
> STAUT must include the WSC IE in the 802.11 Association Request frame.
> Therefore, add the corresponding IE in association message.
Applied, thanks!
Kalle
^ permalink raw reply
* Re: [PATCH] ath6kl: fix null skb dereference in ath6kl_rx()
From: Kalle Valo @ 2011-10-11 11:23 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
In-Reply-To: <20111006113232.24700.12168.stgit@localhost6.localdomain6>
On 10/06/2011 02:32 PM, Kalle Valo wrote:
> smatch found that skb might be null in some cases in ath6kl_rx():
>
> ath6kl/txrx.c +1252 ath6kl_rx(222) error: potential null derefence 'skb'.
>
> This will happen when ath6kl is in AP mode and two clients send traffic
> to each other.
Applied to ath6kl.git.
Kalle
^ permalink raw reply
* [PATCH] mac80211: Populate radiotap header with MCS info for tx'ed frames
From: Helmut Schaa @ 2011-10-11 11:28 UTC (permalink / raw)
To: linux-wireless; +Cc: linville, johannes, Helmut Schaa
mac80211 already filled in the MCS rate info for rx'ed frames but tx'ed
frames that are sent to a monitor interface during the status callback
lack this information.
Add the radiotap fields for MCS info to ieee80211_tx_status_rtap_hdr
and populate them when sending tx'ed frames to the monitors.
One minor flaw is that the radiotap header now includes both, the rate
field and the mcs field. For HT frames the rate field will be zero and
for legacy frames the mcs fields will be zero but still this could
be improved.
Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
---
Wireshark will display the MCS rate correctly but will show the zero'd rate
field as well with 0Mbps. For legacy frames the MCS field won't be shown
since IEEE80211_RADIOTAP_MCS_HAVE_MCS isn't set. Only the MCS flags will be
shown (all zero of course).
I still think it is justified to use it like this as otherwise we would have
to build the radiotap header in a more generic fashion and we won't be able to
easily catch build bugs like
BUILD_BUG_ON(IEEE80211_TX_STATUS_HEADROOM !=
sizeof(struct ieee80211_tx_status_rtap_hdr));
include/net/mac80211.h | 2 +-
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/status.c | 26 +++++++++++++++++++++-----
3 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index cd108df..a500c7b 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2520,7 +2520,7 @@ static inline int ieee80211_sta_ps_transition_ni(struct ieee80211_sta *sta,
* The TX headroom reserved by mac80211 for its own tx_status functions.
* This is enough for the radiotap header.
*/
-#define IEEE80211_TX_STATUS_HEADROOM 13
+#define IEEE80211_TX_STATUS_HEADROOM 16
/**
* ieee80211_sta_set_buffered - inform mac80211 about driver-buffered frames
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9fa5f8a..41dc565 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1187,6 +1187,9 @@ struct ieee80211_tx_status_rtap_hdr {
u8 padding_for_rate;
__le16 tx_flags;
u8 data_retries;
+ u8 mcs_known;
+ u8 mcs_flags;
+ u8 mcs;
} __packed;
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 864a9c3..34319fe 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -475,7 +475,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
rthdr->hdr.it_present =
cpu_to_le32((1 << IEEE80211_RADIOTAP_TX_FLAGS) |
(1 << IEEE80211_RADIOTAP_DATA_RETRIES) |
- (1 << IEEE80211_RADIOTAP_RATE));
+ (1 << IEEE80211_RADIOTAP_RATE) |
+ (1 << IEEE80211_RADIOTAP_MCS));
if (!(info->flags & IEEE80211_TX_STAT_ACK) &&
!is_multicast_ether_addr(hdr->addr1))
@@ -491,10 +492,25 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
rthdr->tx_flags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_CTS);
else if (info->status.rates[0].flags & IEEE80211_TX_RC_USE_RTS_CTS)
rthdr->tx_flags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_RTS);
- if (info->status.rates[0].idx >= 0 &&
- !(info->status.rates[0].flags & IEEE80211_TX_RC_MCS))
- rthdr->rate = sband->bitrates[
- info->status.rates[0].idx].bitrate / 5;
+ if (info->status.rates[0].idx >= 0) {
+ if (!(info->status.rates[0].flags & IEEE80211_TX_RC_MCS)) {
+ /* legacy rate */
+ rthdr->rate = sband->bitrates[
+ info->status.rates[0].idx].bitrate / 5;
+ } else {
+ /* HT rate */
+ rthdr->mcs_known = IEEE80211_RADIOTAP_MCS_HAVE_MCS |
+ IEEE80211_RADIOTAP_MCS_HAVE_GI |
+ IEEE80211_RADIOTAP_MCS_HAVE_BW;
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_SHORT_GI)
+ rthdr->mcs_flags |= IEEE80211_RADIOTAP_MCS_SGI;
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_40_MHZ_WIDTH)
+ rthdr->mcs_flags |= IEEE80211_RADIOTAP_MCS_BW_40;
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_GREEN_FIELD)
+ rthdr->mcs_flags |= IEEE80211_RADIOTAP_MCS_FMT_GF;
+ rthdr->mcs = info->status.rates[0].idx;
+ }
+ }
/* for now report the total retry_count */
rthdr->data_retries = retry_count;
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] mac80211: Populate radiotap header with MCS info for tx'ed frames
From: Johannes Berg @ 2011-10-11 11:35 UTC (permalink / raw)
To: Helmut Schaa; +Cc: linux-wireless, linville
In-Reply-To: <1318332535-8815-1-git-send-email-helmut.schaa@googlemail.com>
On Tue, 2011-10-11 at 13:28 +0200, Helmut Schaa wrote:
> mac80211 already filled in the MCS rate info for rx'ed frames but tx'ed
> frames that are sent to a monitor interface during the status callback
> lack this information.
>
> Add the radiotap fields for MCS info to ieee80211_tx_status_rtap_hdr
> and populate them when sending tx'ed frames to the monitors.
>
> One minor flaw is that the radiotap header now includes both, the rate
> field and the mcs field. For HT frames the rate field will be zero and
> for legacy frames the mcs fields will be zero but still this could
> be improved.
> Wireshark will display the MCS rate correctly but will show the zero'd rate
> field as well with 0Mbps. For legacy frames the MCS field won't be shown
> since IEEE80211_RADIOTAP_MCS_HAVE_MCS isn't set. Only the MCS flags will be
> shown (all zero of course).
>
> I still think it is justified to use it like this as otherwise we would have
> to build the radiotap header in a more generic fashion and we won't be able to
> easily catch build bugs like
>
> BUILD_BUG_ON(IEEE80211_TX_STATUS_HEADROOM !=
> sizeof(struct ieee80211_tx_status_rtap_hdr));
Hmm. I don't like it much, but it's a bug right now too. Too bad the
rate field doesn't just fit into the padding :-)
Actually though, the MCS field is three bytes. So using something like
this would allow us to save some space as well:
struct ieee80211_tx_status_rtap_hdr {
struct ieee80211_radiotap_header hdr;
union {
struct {
u8 rate;
u8 padding_for_rate;
__le16 tx_flags;
u8 data_retries;
} non_mcs;
struct {
__le16 tx_flags;
u8 data_retries;
u8 mcs_known, mcs_flags, mcs;
} mcs;
}
} __packed;
johannes
^ permalink raw reply
* Re: [PATCH 2/5] ath6kl: Add debugfs file for target roam table
From: Kalle Valo @ 2011-10-11 11:39 UTC (permalink / raw)
To: Jouni Malinen; +Cc: linux-wireless
In-Reply-To: <1318243411-16110-3-git-send-email-jouni@qca.qualcomm.com>
On 10/10/2011 01:43 PM, Jouni Malinen wrote:
> The new roam_table debugfs file can be used to display the current
> roam table from the target.
One comment:
> +static int ath6kl_wmi_roam_tbl_event_rx(struct wmi *wmi, u8 *datap, int len)
> +{
> +#ifdef CONFIG_ATH6KL_DEBUG
> + struct ath6kl *ar = wmi->parent_dev;
> + struct wmi_target_roam_tbl *tbl;
> + u16 num_entries;
> +
> + if (len < sizeof(*tbl))
> + return -EINVAL;
> +
> + tbl = (struct wmi_target_roam_tbl *) datap;
> + num_entries = le16_to_cpu(tbl->num_entries);
> + if (sizeof(*tbl) + num_entries * sizeof(struct wmi_bss_roam_info) > len)
> + return -EINVAL;
> +
> + if (ar->debug.roam_tbl == NULL ||
> + ar->debug.roam_tbl_len < (unsigned int) len) {
> + kfree(ar->debug.roam_tbl);
> + ar->debug.roam_tbl = kmalloc(len, GFP_ATOMIC);
> + if (ar->debug.roam_tbl == NULL)
> + return -ENOMEM;
> + }
> +
> + memcpy(ar->debug.roam_tbl, datap, len);
> + ar->debug.roam_tbl_len = len;
> +
> + if (test_bit(ROAM_TBL_PEND, &ar->flag)) {
> + clear_bit(ROAM_TBL_PEND, &ar->flag);
> + wake_up(&ar->event_wq);
> + }
> +#endif /* CONFIG_ATH6KL_DEBUG */
> +
> + return 0;
> +}
I would prefer to have the part inside ifdef in debug.c, for example
like ath6kl_debug_fwlog_event() is implemented. That way we can get rid
of the ifdef inside code and related functinality would be in the same file.
Kalle
^ 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