From: Nick Kossifidis <mickflemm@gmail.com>
To: Zefir Kurtisi <zefir.kurtisi@neratec.com>
Cc: linux-wireless@vger.kernel.org, ath9k-devel@venema.h4ckr.net
Subject: Re: [PATCH] ath9k: spectral - simplify max_index calculation
Date: Thu, 18 Jun 2015 17:46:28 +0200 [thread overview]
Message-ID: <CAFtRNNzzmPGr8UBzULvvgcGFrUqY8nvi1sBAQx8zkfo2tuLg=Q@mail.gmail.com> (raw)
In-Reply-To: <CAFtRNNxnwTGXa8FS2n_chRZPrRffPtGrpZ96t7zFCDo2C7b8sw@mail.gmail.com>
2015-06-18 17:34 GMT+02:00 Nick Kossifidis <mickflemm@gmail.com>:
> 2015-06-18 16:13 GMT+02:00 Nick Kossifidis <mickflemm@gmail.com>:
>> 2015-06-18 12:36 GMT+02:00 Zefir Kurtisi <zefir.kurtisi@neratec.com>:
>>> On 06/18/2015 10:43 AM, Nick Kossifidis wrote:
>>>> max_index is a 6bit signed integer in both cases (sorry for the 5bit
>>>> typo in the comments), so the current function handles it correctly
>>>> for both HT20 and dynamic HT20/40 modes (I've tested it extensively).
>>>> Also you don't handle the negative indices we get from the hardware
>>>> (you just remove the sign). Have you tested this and if you did on
>>>> which hardware did you do the test ?
>>>>
>>>> 2015-06-16 11:21 GMT+02:00 Zefir Kurtisi <zefir.kurtisi@neratec.com>:
>>>>> [...]
>>>>> +/* return the max magnitude from the all/upper/lower bins
>>>>> + *
>>>>> + * in HT20: 6-bit signed number of range -28 to +27
>>>>> + * in HT40: 6-bit unsigned number of range 0 to +63
>>>>> + * (upper sub-channel index 0 is DC)
>>>>> + *
>>>>> + * Correct interpretation of the value has to be done at caller
>>>>> + */
>>>
>>> The comment above is taken from developer NDA documents and should be correct.
>>> With that, in HT40 mode the max_index value has to be taken as is, while in HT20
>>> it needs to be shifted to the unsigned range.
>>>
>>
>> I have NDA documents as well stating that the indices are from -64 to
>> 63 (-64 to -1, 1 to 63 and 0 is DC), you can check out for yourself
>> that we get 128bins on dynamic HT20/40, see the header files too:
>>
>> #define SPECTRAL_HT20_40_NUM_BINS 128
>>
>> and/or the received packet length. Maybe you are talking about
>> "static" HT40 (I don't see anything about that on the documents I
>> have) or something else.
>>
>
> To clarify this a bit: It's 0 - 63 for lower bins and 0 - 64 (not 63)
> for upper bins and since we want an array index of 0 - 128 we add the
> index of 0 to the upper max_idx (on the caller). You are right that
> the current comment there is wrong (it even mentions 5bit ints) so
> feel free to fix that but the code works as expected and it's much
> more readable than doing "^ 0x20 - 4" on the caller (plus it handles
> both signed and unsigned cases so no problem there).
>
Arghh, I need to sleep asap :P
0 - 64 for lower bins and 0 - 63 for upper bins...
--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick
WARNING: multiple messages have this Message-ID (diff)
From: Nick Kossifidis <mickflemm@gmail.com>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] [PATCH] ath9k: spectral - simplify max_index calculation
Date: Thu, 18 Jun 2015 17:46:28 +0200 [thread overview]
Message-ID: <CAFtRNNzzmPGr8UBzULvvgcGFrUqY8nvi1sBAQx8zkfo2tuLg=Q@mail.gmail.com> (raw)
In-Reply-To: <CAFtRNNxnwTGXa8FS2n_chRZPrRffPtGrpZ96t7zFCDo2C7b8sw@mail.gmail.com>
2015-06-18 17:34 GMT+02:00 Nick Kossifidis <mickflemm@gmail.com>:
> 2015-06-18 16:13 GMT+02:00 Nick Kossifidis <mickflemm@gmail.com>:
>> 2015-06-18 12:36 GMT+02:00 Zefir Kurtisi <zefir.kurtisi@neratec.com>:
>>> On 06/18/2015 10:43 AM, Nick Kossifidis wrote:
>>>> max_index is a 6bit signed integer in both cases (sorry for the 5bit
>>>> typo in the comments), so the current function handles it correctly
>>>> for both HT20 and dynamic HT20/40 modes (I've tested it extensively).
>>>> Also you don't handle the negative indices we get from the hardware
>>>> (you just remove the sign). Have you tested this and if you did on
>>>> which hardware did you do the test ?
>>>>
>>>> 2015-06-16 11:21 GMT+02:00 Zefir Kurtisi <zefir.kurtisi@neratec.com>:
>>>>> [...]
>>>>> +/* return the max magnitude from the all/upper/lower bins
>>>>> + *
>>>>> + * in HT20: 6-bit signed number of range -28 to +27
>>>>> + * in HT40: 6-bit unsigned number of range 0 to +63
>>>>> + * (upper sub-channel index 0 is DC)
>>>>> + *
>>>>> + * Correct interpretation of the value has to be done at caller
>>>>> + */
>>>
>>> The comment above is taken from developer NDA documents and should be correct.
>>> With that, in HT40 mode the max_index value has to be taken as is, while in HT20
>>> it needs to be shifted to the unsigned range.
>>>
>>
>> I have NDA documents as well stating that the indices are from -64 to
>> 63 (-64 to -1, 1 to 63 and 0 is DC), you can check out for yourself
>> that we get 128bins on dynamic HT20/40, see the header files too:
>>
>> #define SPECTRAL_HT20_40_NUM_BINS 128
>>
>> and/or the received packet length. Maybe you are talking about
>> "static" HT40 (I don't see anything about that on the documents I
>> have) or something else.
>>
>
> To clarify this a bit: It's 0 - 63 for lower bins and 0 - 64 (not 63)
> for upper bins and since we want an array index of 0 - 128 we add the
> index of 0 to the upper max_idx (on the caller). You are right that
> the current comment there is wrong (it even mentions 5bit ints) so
> feel free to fix that but the code works as expected and it's much
> more readable than doing "^ 0x20 - 4" on the caller (plus it handles
> both signed and unsigned cases so no problem there).
>
Arghh, I need to sleep asap :P
0 - 64 for lower bins and 0 - 63 for upper bins...
--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick
next prev parent reply other threads:[~2015-06-18 15:46 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-16 9:21 [PATCH] ath9k: spectral - simplify max_index calculation Zefir Kurtisi
2015-06-16 9:21 ` [ath9k-devel] " Zefir Kurtisi
2015-06-18 8:43 ` Nick Kossifidis
2015-06-18 8:43 ` [ath9k-devel] " Nick Kossifidis
2015-06-18 10:36 ` Zefir Kurtisi
2015-06-18 10:36 ` [ath9k-devel] " Zefir Kurtisi
2015-06-18 14:13 ` Nick Kossifidis
2015-06-18 14:13 ` [ath9k-devel] " Nick Kossifidis
2015-06-18 15:11 ` Zefir Kurtisi
2015-06-18 15:11 ` [ath9k-devel] " Zefir Kurtisi
2015-06-18 15:59 ` Nick Kossifidis
2015-06-18 15:59 ` [ath9k-devel] " Nick Kossifidis
2015-06-18 15:34 ` Nick Kossifidis
2015-06-18 15:34 ` [ath9k-devel] " Nick Kossifidis
2015-06-18 15:46 ` Nick Kossifidis [this message]
2015-06-18 15:46 ` Nick Kossifidis
2015-06-18 16:40 ` Zefir Kurtisi
2015-06-18 16:40 ` [ath9k-devel] " Zefir Kurtisi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFtRNNzzmPGr8UBzULvvgcGFrUqY8nvi1sBAQx8zkfo2tuLg=Q@mail.gmail.com' \
--to=mickflemm@gmail.com \
--cc=ath9k-devel@venema.h4ckr.net \
--cc=linux-wireless@vger.kernel.org \
--cc=zefir.kurtisi@neratec.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.