From: Ben Greear <greearb@candelatech.com>
To: Sebastian Gottschall <s.gottschall@newmedia-net.de>,
linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
Subject: Re: [PATCH] ath10k: Per-chain rssi should sum the secondary channels
Date: Tue, 17 Dec 2019 07:05:03 -0800 [thread overview]
Message-ID: <80700614-679a-336e-bd9a-e88622e75c9a@candelatech.com> (raw)
In-Reply-To: <b5d63d96-4ba6-bbab-bf1c-a61c6c437f37@newmedia-net.de>
On 12/17/2019 04:32 AM, Sebastian Gottschall wrote:
> result of my tests
>
> on qca988x rxd->ppdu_start.rssi_comb_ht is always zero. so you need to add a additional check
>
> Am 17.12.2019 um 13:02 schrieb Sebastian Gottschall:
>> i see a issue in your patch for qca988x chipsets
>>
>> + if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
>> + status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>> + rxd->ppdu_start.rssi_comb_ht;
>> + }
>>
>>
>> this is always true for qca988x, but the field is not provided on these older chipsets. so signal reporting will be broken
>> i'm currently debugging in your code, but i already have seen that the values are wrong now for this chipset
Thanks for testing. I'll add a check for 0 and ignore that value too. That seem OK?
Were the per-chain values OK?
Thanks,
Ben
>>
>> Am 16.12.2019 um 23:07 schrieb greearb@candelatech.com:
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> This makes per-chain RSSI be more consistent between HT20, HT40, HT80.
>>> Instead of doing precise log math for adding dbm, I did a rough estimate,
>>> it seems to work good enough.
>>>
>>> Tested on ath10k-ct 9984 firmware.
>>>
>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>> ---
>>> drivers/net/wireless/ath/ath10k/htt_rx.c | 64 ++++++++++++++++++++---
>>> drivers/net/wireless/ath/ath10k/rx_desc.h | 3 +-
>>> 2 files changed, 60 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index 13f652b622df..034d4ace228d 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -1167,6 +1167,44 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
>>> return true;
>>> }
>>> +static int ath10k_sum_sigs_2(int a, int b) {
>>> + int diff;
>>> +
>>> + if (b == 0x80)
>>> + return a;
>>> +
>>> + if (a >= b) {
>>> + diff = a - b;
>>> + if (diff == 0)
>>> + return a + 3;
>>> + else if (diff == 1)
>>> + return a + 2;
>>> + else if (diff == 2)
>>> + return a + 1;
>>> + return a;
>>> + }
>>> + else {
>>> + diff = b - a;
>>> + if (diff == 0)
>>> + return b + 3;
>>> + else if (diff == 1)
>>> + return b + 2;
>>> + else if (diff == 2)
>>> + return b + 1;
>>> + return b;
>>> + }
>>> +}
>>> +
>>> +static int ath10k_sum_sigs(int p20, int e20, int e40, int e80) {
>>> + /* Hacky attempt at summing dbm without resorting to log(10) business */
>>> + if (e40 != 0x80) {
>>> + return ath10k_sum_sigs_2(ath10k_sum_sigs_2(p20, e20), ath10k_sum_sigs_2(e40, e80));
>>> + }
>>> + else {
>>> + return ath10k_sum_sigs_2(p20, e20);
>>> + }
>>> +}
>>> +
>>> static void ath10k_htt_rx_h_signal(struct ath10k *ar,
>>> struct ieee80211_rx_status *status,
>>> struct htt_rx_desc *rxd)
>>> @@ -1177,18 +1215,32 @@ static void ath10k_htt_rx_h_signal(struct ath10k *ar,
>>> status->chains &= ~BIT(i);
>>> if (rxd->ppdu_start.rssi_chains[i].pri20_mhz != 0x80) {
>>> - status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR +
>>> - rxd->ppdu_start.rssi_chains[i].pri20_mhz;
>>> + status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR
>>> + + ath10k_sum_sigs(rxd->ppdu_start.rssi_chains[i].pri20_mhz,
>>> + rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>>> + rxd->ppdu_start.rssi_chains[i].ext40_mhz,
>>> + rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>>> + //ath10k_warn(ar, "rx-h-sig, chain[%i] pri20: %d ext20: %d ext40: %d ext80: %d\n",
>>> + // i, rxd->ppdu_start.rssi_chains[i].pri20_mhz, rxd->ppdu_start.rssi_chains[i].ext20_mhz,
>>> + // rxd->ppdu_start.rssi_chains[i].ext40_mhz, rxd->ppdu_start.rssi_chains[i].ext80_mhz);
>>> status->chains |= BIT(i);
>>> }
>>> }
>>> /* FIXME: Get real NF */
>>> - status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>> - rxd->ppdu_start.rssi_comb;
>>> - /* ath10k_warn(ar, "rx-h-sig, signal: %d chains: 0x%x chain[0]: %d chain[1]: %d chan[2]: %d\n",
>>> - status->signal, status->chains, status->chain_signal[0], status->chain_signal[1], status->chain_signal[2]); */
>>> + if (rxd->ppdu_start.rssi_comb_ht != 0x80) {
>>> + status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>> + rxd->ppdu_start.rssi_comb_ht;
>>> + }
>>> + else {
>>> + status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
>>> + rxd->ppdu_start.rssi_comb;
>>> + }
>>> +
>>> + //ath10k_warn(ar, "rx-h-sig, signal: %d chains: 0x%x chain[0]: %d chain[1]: %d chain[2]: %d chain[3]: %d\n",
>>> + // status->signal, status->chains, status->chain_signal[0],
>>> + // status->chain_signal[1], status->chain_signal[2], status->chain_signal[3]);
>>> status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
>>> }
>>> diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h b/drivers/net/wireless/ath/ath10k/rx_desc.h
>>> index dec1582005b9..6b44677474dd 100644
>>> --- a/drivers/net/wireless/ath/ath10k/rx_desc.h
>>> +++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
>>> @@ -726,7 +726,8 @@ struct rx_ppdu_start {
>>> u8 ext80_mhz;
>>> } rssi_chains[4];
>>> u8 rssi_comb;
>>> - __le16 rsvd0;
>>> + u8 rsvd0; /* first two bits are bandwidth, other 6 are reserved */
>>> + u8 rssi_comb_ht;
>>> u8 info0; /* %RX_PPDU_START_INFO0_ */
>>> __le32 info1; /* %RX_PPDU_START_INFO1_ */
>>> __le32 info2; /* %RX_PPDU_START_INFO2_ */
>>
>> _______________________________________________
>> ath10k mailing list
>> ath10k@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/ath10k
>>
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
next prev parent reply other threads:[~2019-12-17 15:05 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-16 22:07 [PATCH] ath10k: Per-chain rssi should sum the secondary channels greearb
2019-12-17 12:02 ` Sebastian Gottschall
2019-12-17 12:32 ` Sebastian Gottschall
2019-12-17 15:05 ` Ben Greear [this message]
2019-12-17 15:58 ` Sebastian Gottschall
2019-12-17 16:23 ` Justin Capella
2019-12-17 17:38 ` Ben Greear
2019-12-17 18:29 ` Tom Psyborg
2019-12-17 18:35 ` Adrian Chadd
2019-12-17 18:35 ` Ben Greear
2019-12-17 23:08 ` Tom Psyborg
2019-12-17 23:37 ` Tom Psyborg
2019-12-17 23:43 ` Ben Greear
2019-12-18 2:12 ` Sebastian Gottschall
2019-12-18 2:37 ` Ben Greear
2019-12-18 4:05 ` Sebastian Gottschall
2019-12-18 8:05 ` Justin Capella
2019-12-18 9:10 ` Sebastian Gottschall
2019-12-18 12:59 ` Ben Greear
2019-12-18 18:59 ` Sebastian Gottschall
2019-12-18 11:48 ` Tom Psyborg
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=80700614-679a-336e-bd9a-e88622e75c9a@candelatech.com \
--to=greearb@candelatech.com \
--cc=ath10k@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=s.gottschall@newmedia-net.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox