[PATCH] ath10k: fix rssi reporting.

[PATCH] ath10k: fix rssi reporting.

[greearb]

From: Ben Greear <greearb@candelatech.com>

When the driver cannot provide proper rssi, mark
status with RX_FLAG_NO_SIGNAL_VAL so that stack
properly ignores it.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

Patch is against my tree, hopefully will apply OK against upstream
but have not tested that yet.

 drivers/net/wireless/ath/ath10k/htt_rx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 0c83ffb..d8ec8dd 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1216,11 +1216,12 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
 
 	/* Fill this once, while this is per-ppdu */
-	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
-		memset(rx_status, 0, sizeof(*rx_status));
+	memset(rx_status, 0, sizeof(*rx_status));
+	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID)
 		rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
 				     rx->ppdu.combined_rssi;
-	}
+	else
+		rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
 
 	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
 		/* TSF available only in 32-bit */
-- 
1.7.11.7


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: fix rssi reporting.

[Janusz Dziedzic]

On 15 May 2014 20:34,  <greearb@candelatech.com> wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> When the driver cannot provide proper rssi, mark
> status with RX_FLAG_NO_SIGNAL_VAL so that stack
> properly ignores it.
>
I think we should skip this one while we know rssi/rates.
They are correct for all packets between START_VALID and END_VALID flags.

BR
Janusz

> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
> Patch is against my tree, hopefully will apply OK against upstream
> but have not tested that yet.
>
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 0c83ffb..d8ec8dd 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1216,11 +1216,12 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>         mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
>
>         /* Fill this once, while this is per-ppdu */
> -       if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
> -               memset(rx_status, 0, sizeof(*rx_status));
> +       memset(rx_status, 0, sizeof(*rx_status));
> +       if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID)
>                 rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
>                                      rx->ppdu.combined_rssi;
> -       }
> +       else
> +               rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
>
>         if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
>                 /* TSF available only in 32-bit */
> --
> 1.7.11.7
>
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: fix rssi reporting.

[Ben Greear]

On 05/16/2014 05:18 AM, Janusz Dziedzic wrote:
> On 15 May 2014 20:34,  <greearb@candelatech.com> wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> When the driver cannot provide proper rssi, mark
>> status with RX_FLAG_NO_SIGNAL_VAL so that stack
>> properly ignores it.
>>
> I think we should skip this one while we know rssi/rates.
> They are correct for all packets between START_VALID and END_VALID flags.

Skip what, the patch?  With current code, you are sending packets up
the tree without signal being set and yet without the flag set that says
to ignore the (unset) signal value.

Thanks,
Ben

>
> BR
> Janusz
>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>
>> Patch is against my tree, hopefully will apply OK against upstream
>> but have not tested that yet.
>>
>>   drivers/net/wireless/ath/ath10k/htt_rx.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index 0c83ffb..d8ec8dd 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -1216,11 +1216,12 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>>          mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
>>
>>          /* Fill this once, while this is per-ppdu */
>> -       if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
>> -               memset(rx_status, 0, sizeof(*rx_status));
>> +       memset(rx_status, 0, sizeof(*rx_status));
>> +       if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID)
>>                  rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
>>                                       rx->ppdu.combined_rssi;
>> -       }
>> +       else
>> +               rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
>>
>>          if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
>>                  /* TSF available only in 32-bit */
>> --
>> 1.7.11.7
>>
>>
>> _______________________________________________
>> 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

Re: [PATCH] ath10k: fix rssi reporting.

[Janusz Dziedzic]

On 16 May 2014 15:04, Ben Greear <greearb@candelatech.com> wrote:
>
>
> On 05/16/2014 05:18 AM, Janusz Dziedzic wrote:
>>
>> On 15 May 2014 20:34,  <greearb@candelatech.com> wrote:
>>>
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> When the driver cannot provide proper rssi, mark
>>> status with RX_FLAG_NO_SIGNAL_VAL so that stack
>>> properly ignores it.
>>>
>> I think we should skip this one while we know rssi/rates.
>> They are correct for all packets between START_VALID and END_VALID flags.
>
>
> Skip what, the patch?  With current code, you are sending packets up
> the tree without signal being set and yet without the flag set that says
> to ignore the (unset) signal value.
>
Didn't reproduce this yet, while we have this values saved when
START_VALID - rx status is an template in htt structure.

struct ieee80211_rx_status *rx_status = &htt->rx_status;

Are you sure you have all patches?
This could be passible if we will get first packet with only flag
END_VALID (not sure this is even possible). In case we get packets
with START valid before, we will use template correctly. In case we
will get packets with flags START and END valid we will also reports
this correctly.
Did you reproduce this with official firmware?

BTW
your patch break rates info based also on htt->rx_status template.

BR
Janusz


>>
>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>> ---
>>>
>>> Patch is against my tree, hopefully will apply OK against upstream
>>> but have not tested that yet.
>>>
>>>   drivers/net/wireless/ath/ath10k/htt_rx.c | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index 0c83ffb..d8ec8dd 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -1216,11 +1216,12 @@ static void ath10k_htt_rx_handler(struct
>>> ath10k_htt *htt,
>>>          mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
>>>
>>>          /* Fill this once, while this is per-ppdu */
>>> -       if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
>>> -               memset(rx_status, 0, sizeof(*rx_status));
>>> +       memset(rx_status, 0, sizeof(*rx_status));
>>> +       if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID)
>>>                  rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
>>>                                       rx->ppdu.combined_rssi;
>>> -       }
>>> +       else
>>> +               rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
>>>
>>>          if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
>>>                  /* TSF available only in 32-bit */
>>> --
>>> 1.7.11.7
>>>
>>>
>>> _______________________________________________
>>> 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

Re: [PATCH] ath10k: fix rssi reporting.

[Ben Greear]

On 05/16/2014 06:16 AM, Janusz Dziedzic wrote:
> On 16 May 2014 15:04, Ben Greear <greearb@candelatech.com> wrote:
>>
>>
>> On 05/16/2014 05:18 AM, Janusz Dziedzic wrote:
>>>
>>> On 15 May 2014 20:34,  <greearb@candelatech.com> wrote:
>>>>
>>>> From: Ben Greear <greearb@candelatech.com>
>>>>
>>>> When the driver cannot provide proper rssi, mark
>>>> status with RX_FLAG_NO_SIGNAL_VAL so that stack
>>>> properly ignores it.
>>>>
>>> I think we should skip this one while we know rssi/rates.
>>> They are correct for all packets between START_VALID and END_VALID flags.
>>
>>
>> Skip what, the patch?  With current code, you are sending packets up
>> the tree without signal being set and yet without the flag set that says
>> to ignore the (unset) signal value.
>>
> Didn't reproduce this yet, while we have this values saved when
> START_VALID - rx status is an template in htt structure.
>
> struct ieee80211_rx_status *rx_status = &htt->rx_status;
>
> Are you sure you have all patches?
> This could be passible if we will get first packet with only flag
> END_VALID (not sure this is even possible). In case we get packets
> with START valid before, we will use template correctly. In case we
> will get packets with flags START and END valid we will also reports
> this correctly.
> Did you reproduce this with official firmware?
>
> BTW
> your patch break rates info based also on htt->rx_status template.
>
> BR
> Janusz

I will test again without my patch, and if you think current code is
correct, then just drop this patch and I'll retest everything when I
move back to testing on kalle's tree.

Thanks,
Ben

-- 
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

Re: [PATCH] ath10k: fix rssi reporting.

[Ben Greear]

On 05/16/2014 06:21 AM, Ben Greear wrote:
> 
> 
> On 05/16/2014 06:16 AM, Janusz Dziedzic wrote:
>> On 16 May 2014 15:04, Ben Greear <greearb@candelatech.com> wrote:
>>>
>>>
>>> On 05/16/2014 05:18 AM, Janusz Dziedzic wrote:
>>>>
>>>> On 15 May 2014 20:34,  <greearb@candelatech.com> wrote:
>>>>>
>>>>> From: Ben Greear <greearb@candelatech.com>
>>>>>
>>>>> When the driver cannot provide proper rssi, mark
>>>>> status with RX_FLAG_NO_SIGNAL_VAL so that stack
>>>>> properly ignores it.
>>>>>
>>>> I think we should skip this one while we know rssi/rates.
>>>> They are correct for all packets between START_VALID and END_VALID flags.
>>>
>>>
>>> Skip what, the patch?  With current code, you are sending packets up
>>> the tree without signal being set and yet without the flag set that says
>>> to ignore the (unset) signal value.
>>>
>> Didn't reproduce this yet, while we have this values saved when
>> START_VALID - rx status is an template in htt structure.
>>
>> struct ieee80211_rx_status *rx_status = &htt->rx_status;
>>
>> Are you sure you have all patches?
>> This could be passible if we will get first packet with only flag
>> END_VALID (not sure this is even possible). In case we get packets
>> with START valid before, we will use template correctly. In case we
>> will get packets with flags START and END valid we will also reports
>> this correctly.
>> Did you reproduce this with official firmware?
>>
>> BTW
>> your patch break rates info based also on htt->rx_status template.
>>
>> BR
>> Janusz
> 
> I will test again without my patch, and if you think current code is
> correct, then just drop this patch and I'll retest everything when I
> move back to testing on kalle's tree.

I tested again without my patch, and the graph looks smooth against
my attenuator's settings, so looks like my patch is not needed.

Thanks,
Ben


-- 
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

Re: [PATCH] ath10k: fix rssi reporting.

[Kalle Valo]

greearb@candelatech.com writes:

> From: Ben Greear <greearb@candelatech.com>
>
> When the driver cannot provide proper rssi, mark
> status with RX_FLAG_NO_SIGNAL_VAL so that stack
> properly ignores it.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>

Based on the discussion I'm dropping this patch.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH] ath10k: fix rssi reporting.

[greearb]

From: Ben Greear <greearb@candelatech.com>

When the driver cannot provide proper rssi, mark
status with RX_FLAG_NO_SIGNAL_VAL so that stack
properly ignores it.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

Patch is against my tree, hopefully will apply OK against upstream
but have not tested that yet.

 drivers/net/wireless/ath/ath10k/htt_rx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 0c83ffb..d8ec8dd 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1216,11 +1216,12 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
 
 	/* Fill this once, while this is per-ppdu */
-	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
-		memset(rx_status, 0, sizeof(*rx_status));
+	memset(rx_status, 0, sizeof(*rx_status));
+	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID)
 		rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
 				     rx->ppdu.combined_rssi;
-	}
+	else
+		rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
 
 	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
 		/* TSF available only in 32-bit */
-- 
1.7.11.7

Re: [PATCH] ath10k: fix rssi reporting.

[Ben Greear]

Sorry, ...this and previous patch should not have gone to LKML.

Will send it over to ath10k list where it was supposed to go.

Thanks,
Ben

On 05/15/2014 11:31 AM, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> When the driver cannot provide proper rssi, mark
> status with RX_FLAG_NO_SIGNAL_VAL so that stack
> properly ignores it.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> 
> Patch is against my tree, hopefully will apply OK against upstream
> but have not tested that yet.
> 
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 0c83ffb..d8ec8dd 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1216,11 +1216,12 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
>  	mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
>  
>  	/* Fill this once, while this is per-ppdu */
> -	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
> -		memset(rx_status, 0, sizeof(*rx_status));
> +	memset(rx_status, 0, sizeof(*rx_status));
> +	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID)
>  		rx_status->signal  = ATH10K_DEFAULT_NOISE_FLOOR +
>  				     rx->ppdu.combined_rssi;
> -	}
> +	else
> +		rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
>  
>  	if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
>  		/* TSF available only in 32-bit */
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com