On Tue, 2013-09-10 at 16:51 +0200, Eliad Peller wrote: > On Tue, Sep 10, 2013 at 10:39 AM, Luca Coelho <luca@xxxxxxxxx> wrote: > > On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote: > >> From: Ido Reis <idor@xxxxxx> > >> > >> This is a fix for channels 52,56,60,64 bit translation. > >> > >> Reported-by: Yaniv Machani <yanivma@xxxxxx> > >> Signed-off-by: Ido Reis <idor@xxxxxx> > >> Signed-off-by: Victor Goldenshtein <victorg@xxxxxx> > >> Signed-off-by: Eliad Peller <eliad@xxxxxxxxxx> > >> --- > >> drivers/net/wireless/ti/wlcore/cmd.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c > >> index e3ae425..1cb3296 100644 > >> --- a/drivers/net/wireless/ti/wlcore/cmd.c > >> +++ b/drivers/net/wireless/ti/wlcore/cmd.c > >> @@ -1613,8 +1613,10 @@ static int wlcore_get_reg_conf_ch_idx(enum ieee80211_band band, u16 ch) > >> case IEEE80211_BAND_5GHZ: > >> if (ch >= 8 && ch <= 16) > >> idx = ((ch-8)/4 + 18); > >> - else if (ch >= 34 && ch <= 64) > >> + else if (ch >= 34 && ch <= 48) > >> idx = ((ch-34)/2 + 3 + 18); > >> + else if (ch >= 52 && ch <= 64) > >> + idx = ((ch-52)/4 + 11 + 18); > >> else if (ch >= 100 && ch <= 140) > >> idx = ((ch-100)/4 + 15 + 18); > >> else if (ch >= 149 && ch <= 165) > > > > Hmmm... I don't have a clue what is going on here. I don't know how I > > let this function pass as is originally, shame on me. :) > > > :) > > > The change probably makes things work better, since someone apparently > > saw a bug in real life and reported it, but can anyone explain what is > > going on during this translation? Aren't we losing data here? Eg. > > channels 8, 9, 10 and 11 all use the same bit in the firmware command > > bitmask? > > > the 8-16 indeed looks weird, as the driver indeed declares supports > for channels 7,8,9,11... Yes, weird, because channel 7 would return an error even. And 9 and 11 would use 8... > the other conditions are a simple attempt to reduce the gap between > the channels (e.g. no channels between 64 and 100) in order to get a > minimized bitmap. Right, I should have looked into the channel list we advertise. > since the gap between the defined channels in the 5ghz band is usually > 2/4 (36->38 vs. 60->64) the other calculations seems fine (i didn't > dig into them as well, though). Yes, now that you mentioned it looks clearer, but still, this function sucks big time. If it takes more than a couple of minutes to understand what is going on (in such a simple thing as this is), it should be rewritten. Or at _least_ better documented. > i'll try getting some more information from the fw guys regarding the > 8-16 range. Great! Thanks. -- Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
