All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ath10k:  fix rssi reporting.
@ 2014-05-15 18:31 greearb
  2014-05-15 18:33 ` Ben Greear
  0 siblings, 1 reply; 9+ messages in thread
From: greearb @ 2014-05-15 18:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-wireless, Ben Greear

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] ath10k:  fix rssi reporting.
  2014-05-15 18:31 greearb
@ 2014-05-15 18:33 ` Ben Greear
  0 siblings, 0 replies; 9+ messages in thread
From: Ben Greear @ 2014-05-15 18:33 UTC (permalink / raw)
  To: greearb; +Cc: linux-kernel, linux-wireless

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] ath10k:  fix rssi reporting.
@ 2014-05-15 18:34 greearb
  2014-05-16 12:18 ` Janusz Dziedzic
  2014-05-23 11:06 ` Kalle Valo
  0 siblings, 2 replies; 9+ messages in thread
From: greearb @ 2014-05-15 18:34 UTC (permalink / raw)
  To: ath10k; +Cc: Ben Greear

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] ath10k: fix rssi reporting.
  2014-05-15 18:34 [PATCH] ath10k: fix rssi reporting greearb
@ 2014-05-16 12:18 ` Janusz Dziedzic
  2014-05-16 13:04   ` Ben Greear
  2014-05-23 11:06 ` Kalle Valo
  1 sibling, 1 reply; 9+ messages in thread
From: Janusz Dziedzic @ 2014-05-16 12:18 UTC (permalink / raw)
  To: Ben Greear; +Cc: ath10k@lists.infradead.org

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ath10k: fix rssi reporting.
  2014-05-16 12:18 ` Janusz Dziedzic
@ 2014-05-16 13:04   ` Ben Greear
  2014-05-16 13:16     ` Janusz Dziedzic
  0 siblings, 1 reply; 9+ messages in thread
From: Ben Greear @ 2014-05-16 13:04 UTC (permalink / raw)
  To: Janusz Dziedzic; +Cc: ath10k@lists.infradead.org



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ath10k: fix rssi reporting.
  2014-05-16 13:04   ` Ben Greear
@ 2014-05-16 13:16     ` Janusz Dziedzic
  2014-05-16 13:21       ` Ben Greear
  0 siblings, 1 reply; 9+ messages in thread
From: Janusz Dziedzic @ 2014-05-16 13:16 UTC (permalink / raw)
  To: Ben Greear; +Cc: ath10k@lists.infradead.org

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ath10k: fix rssi reporting.
  2014-05-16 13:16     ` Janusz Dziedzic
@ 2014-05-16 13:21       ` Ben Greear
  2014-05-16 15:36         ` Ben Greear
  0 siblings, 1 reply; 9+ messages in thread
From: Ben Greear @ 2014-05-16 13:21 UTC (permalink / raw)
  To: Janusz Dziedzic; +Cc: ath10k@lists.infradead.org



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ath10k: fix rssi reporting.
  2014-05-16 13:21       ` Ben Greear
@ 2014-05-16 15:36         ` Ben Greear
  0 siblings, 0 replies; 9+ messages in thread
From: Ben Greear @ 2014-05-16 15:36 UTC (permalink / raw)
  To: Janusz Dziedzic; +Cc: ath10k@lists.infradead.org

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ath10k:  fix rssi reporting.
  2014-05-15 18:34 [PATCH] ath10k: fix rssi reporting greearb
  2014-05-16 12:18 ` Janusz Dziedzic
@ 2014-05-23 11:06 ` Kalle Valo
  1 sibling, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2014-05-23 11:06 UTC (permalink / raw)
  To: greearb; +Cc: ath10k

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-05-23 11:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-15 18:34 [PATCH] ath10k: fix rssi reporting greearb
2014-05-16 12:18 ` Janusz Dziedzic
2014-05-16 13:04   ` Ben Greear
2014-05-16 13:16     ` Janusz Dziedzic
2014-05-16 13:21       ` Ben Greear
2014-05-16 15:36         ` Ben Greear
2014-05-23 11:06 ` Kalle Valo
  -- strict thread matches above, loose matches on Subject: below --
2014-05-15 18:31 greearb
2014-05-15 18:33 ` Ben Greear

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.