All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCHv2] ath10k : Fix channel survey dump
@ 2018-08-09  6:35 ` Venkateswara Naralasetty
  0 siblings, 0 replies; 13+ messages in thread
From: Venkateswara Naralasetty @ 2018-08-09  6:35 UTC (permalink / raw)
  To: Jasmine Strong; +Cc: Ben Greear, ath10k, linux-wireless, linux-wireless-owner

On 2018-08-01 00:03, Jasmine Strong wrote:
> On Tue, Jul 31, 2018 at 10:38 AM Ben Greear <greearb@candelatech.com>
> wrote:
> 
>> On 07/31/2018 05:11 AM, Venkateswara Naralasetty wrote:
>>> Channel active/busy time are showing incorrect(less than previous
>> or
>>> sometimes zero) for successive survey dump command.
>>> 
>>> example:
>>> Survey data from wlan0
>>> frequency:                      5180 MHz [in use]
>>> channel active time:            54995 ms
>>> channel busy time:              432 ms
>>> channel receive time:           0 ms
>>> channel transmit time:          59 ms
>>> Survey data from wlan0
>>> frequency:                      5180 MHz [in use]
>>> channel active time:            32592 ms
>>> channel busy time:              254 ms
>>> channel receive time:           0 ms
>>> channel transmit time:          0 ms
>>> 
>>> This patch fix this issue by assigning 'wmi_bss_survey_req_type'
>>> as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
>>> FW and send survey data to driver upon the driver request. Wrap
>> around
>>> is taken care by FW.
>>> 
>>> hardware used : QCA9984
>>> firmware ver  : ver 10.4-3.5.3-00057
>> 
>> Have you tested this on other firmwares?  I am pretty sure that at
>> least
>> some of them, probably 10.2, will have issues.
>> 
>> Maybe you could make this change specific to certain firmware that
>> is known to work with the change?
> 
> There are bigger problems with it than that;  even with firmwares that
> behave correctly,
> this changes the behavior that a lot of upper-layer software depends
> upon, and will likely
> make anything that actually uses the channel survey data misbehave.
> 
> It is therefore a breaking change for any device that actually
> implements ACS.
> 
> -J.

In ath9k driver also survey data being accumulated in driver and send 
survey data to user space is accumulated one. same thing we are 
implementing in FW(with WMI request type WMI_BSS_SURVEY_REQ_TYPE_READ) 
instead of doing it in driver.

Thanks,
Venkatesh.

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

* Re: [PATCHv2] ath10k : Fix channel survey dump
@ 2018-08-09  6:35 ` Venkateswara Naralasetty
  0 siblings, 0 replies; 13+ messages in thread
From: Venkateswara Naralasetty @ 2018-08-09  6:35 UTC (permalink / raw)
  To: Jasmine Strong; +Cc: linux-wireless-owner, Ben Greear, linux-wireless, ath10k

On 2018-08-01 00:03, Jasmine Strong wrote:
> On Tue, Jul 31, 2018 at 10:38 AM Ben Greear <greearb@candelatech.com>
> wrote:
> 
>> On 07/31/2018 05:11 AM, Venkateswara Naralasetty wrote:
>>> Channel active/busy time are showing incorrect(less than previous
>> or
>>> sometimes zero) for successive survey dump command.
>>> 
>>> example:
>>> Survey data from wlan0
>>> frequency:                      5180 MHz [in use]
>>> channel active time:            54995 ms
>>> channel busy time:              432 ms
>>> channel receive time:           0 ms
>>> channel transmit time:          59 ms
>>> Survey data from wlan0
>>> frequency:                      5180 MHz [in use]
>>> channel active time:            32592 ms
>>> channel busy time:              254 ms
>>> channel receive time:           0 ms
>>> channel transmit time:          0 ms
>>> 
>>> This patch fix this issue by assigning 'wmi_bss_survey_req_type'
>>> as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
>>> FW and send survey data to driver upon the driver request. Wrap
>> around
>>> is taken care by FW.
>>> 
>>> hardware used : QCA9984
>>> firmware ver  : ver 10.4-3.5.3-00057
>> 
>> Have you tested this on other firmwares?  I am pretty sure that at
>> least
>> some of them, probably 10.2, will have issues.
>> 
>> Maybe you could make this change specific to certain firmware that
>> is known to work with the change?
> 
> There are bigger problems with it than that;  even with firmwares that
> behave correctly,
> this changes the behavior that a lot of upper-layer software depends
> upon, and will likely
> make anything that actually uses the channel survey data misbehave.
> 
> It is therefore a breaking change for any device that actually
> implements ACS.
> 
> -J.

In ath9k driver also survey data being accumulated in driver and send 
survey data to user space is accumulated one. same thing we are 
implementing in FW(with WMI request type WMI_BSS_SURVEY_REQ_TYPE_READ) 
instead of doing it in driver.

Thanks,
Venkatesh.

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

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

* Re: [PATCHv2] ath10k : Fix channel survey dump
  2018-08-09  6:35 ` Venkateswara Naralasetty
@ 2018-08-09 18:46   ` Peter Oh
  -1 siblings, 0 replies; 13+ messages in thread
From: Peter Oh @ 2018-08-09 18:46 UTC (permalink / raw)
  To: Venkateswara Naralasetty, Jasmine Strong
  Cc: Ben Greear, ath10k, linux-wireless, linux-wireless-owner



On 08/08/2018 11:35 PM, Venkateswara Naralasetty wrote:
> On 2018-08-01 00:03, Jasmine Strong wrote:
>> On Tue, Jul 31, 2018 at 10:38 AM Ben Greear <greearb@candelatech.com>
>> wrote:
>>
>>> On 07/31/2018 05:11 AM, Venkateswara Naralasetty wrote:
>>>> Channel active/busy time are showing incorrect(less than previous
>>> or
>>>> sometimes zero) for successive survey dump command.
>>>>
>>>> example:
>>>> Survey data from wlan0
>>>> frequency:                      5180 MHz [in use]
>>>> channel active time:            54995 ms
>>>> channel busy time:              432 ms
>>>> channel receive time:           0 ms
>>>> channel transmit time:          59 ms
>>>> Survey data from wlan0
>>>> frequency:                      5180 MHz [in use]
>>>> channel active time:            32592 ms
>>>> channel busy time:              254 ms
>>>> channel receive time:           0 ms
>>>> channel transmit time:          0 ms
>>>>
>>>> This patch fix this issue by assigning 'wmi_bss_survey_req_type'
>>>> as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
>>>> FW and send survey data to driver upon the driver request. Wrap
>>> around
>>>> is taken care by FW.
>>>>
>>>> hardware used : QCA9984
>>>> firmware ver  : ver 10.4-3.5.3-00057
>>>
>>> Have you tested this on other firmwares?  I am pretty sure that at
>>> least
>>> some of them, probably 10.2, will have issues.
>>>
>>> Maybe you could make this change specific to certain firmware that
>>> is known to work with the change?
>>
>> There are bigger problems with it than that;  even with firmwares that
>> behave correctly,
>> this changes the behavior that a lot of upper-layer software depends
>> upon, and will likely
>> make anything that actually uses the channel survey data misbehave.
>>
>> It is therefore a breaking change for any device that actually
>> implements ACS.
>>
>> -J.
>
> In ath9k driver also survey data being accumulated in driver and send 
> survey data to user space is accumulated one. same thing we are 
> implementing in FW(with WMI request type WMI_BSS_SURVEY_REQ_TYPE_READ) 
> instead of doing it in driver.
It seems you're changing the behavior, not fixing a bug.
WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR is handled differently from 
WMI_BSS_SURVEY_REQ_TYPE_READ and as Jasmine addressed, userspace apps 
that's counting on WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR behavior will be 
broken their functions with your change.
This means this patch will break backward compatibility.

Thanks,
Peter

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

* Re: [PATCHv2] ath10k : Fix channel survey dump
@ 2018-08-09 18:46   ` Peter Oh
  0 siblings, 0 replies; 13+ messages in thread
From: Peter Oh @ 2018-08-09 18:46 UTC (permalink / raw)
  To: Venkateswara Naralasetty, Jasmine Strong
  Cc: linux-wireless-owner, Ben Greear, linux-wireless, ath10k



On 08/08/2018 11:35 PM, Venkateswara Naralasetty wrote:
> On 2018-08-01 00:03, Jasmine Strong wrote:
>> On Tue, Jul 31, 2018 at 10:38 AM Ben Greear <greearb@candelatech.com>
>> wrote:
>>
>>> On 07/31/2018 05:11 AM, Venkateswara Naralasetty wrote:
>>>> Channel active/busy time are showing incorrect(less than previous
>>> or
>>>> sometimes zero) for successive survey dump command.
>>>>
>>>> example:
>>>> Survey data from wlan0
>>>> frequency:                      5180 MHz [in use]
>>>> channel active time:            54995 ms
>>>> channel busy time:              432 ms
>>>> channel receive time:           0 ms
>>>> channel transmit time:          59 ms
>>>> Survey data from wlan0
>>>> frequency:                      5180 MHz [in use]
>>>> channel active time:            32592 ms
>>>> channel busy time:              254 ms
>>>> channel receive time:           0 ms
>>>> channel transmit time:          0 ms
>>>>
>>>> This patch fix this issue by assigning 'wmi_bss_survey_req_type'
>>>> as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
>>>> FW and send survey data to driver upon the driver request. Wrap
>>> around
>>>> is taken care by FW.
>>>>
>>>> hardware used : QCA9984
>>>> firmware ver  : ver 10.4-3.5.3-00057
>>>
>>> Have you tested this on other firmwares?  I am pretty sure that at
>>> least
>>> some of them, probably 10.2, will have issues.
>>>
>>> Maybe you could make this change specific to certain firmware that
>>> is known to work with the change?
>>
>> There are bigger problems with it than that;  even with firmwares that
>> behave correctly,
>> this changes the behavior that a lot of upper-layer software depends
>> upon, and will likely
>> make anything that actually uses the channel survey data misbehave.
>>
>> It is therefore a breaking change for any device that actually
>> implements ACS.
>>
>> -J.
>
> In ath9k driver also survey data being accumulated in driver and send 
> survey data to user space is accumulated one. same thing we are 
> implementing in FW(with WMI request type WMI_BSS_SURVEY_REQ_TYPE_READ) 
> instead of doing it in driver.
It seems you're changing the behavior, not fixing a bug.
WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR is handled differently from 
WMI_BSS_SURVEY_REQ_TYPE_READ and as Jasmine addressed, userspace apps 
that's counting on WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR behavior will be 
broken their functions with your change.
This means this patch will break backward compatibility.

Thanks,
Peter

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

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

* Re: [PATCHv2] ath10k : Fix channel survey dump
  2018-08-09 18:46   ` Peter Oh
@ 2018-08-09 22:26     ` Sven Eckelmann
  -1 siblings, 0 replies; 13+ messages in thread
From: Sven Eckelmann @ 2018-08-09 22:26 UTC (permalink / raw)
  To: Peter Oh
  Cc: Venkateswara Naralasetty, Jasmine Strong, Ben Greear, ath10k,
	linux-wireless, Johannes Berg, Felix Fietkau, Helmut Schaa,
	Michał Kazior

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]

On Donnerstag, 9. August 2018 20:46:26 CEST Peter Oh wrote:
> > In ath9k driver also survey data being accumulated in driver and send 
> > survey data to user space is accumulated one. same thing we are 
> > implementing in FW(with WMI request type WMI_BSS_SURVEY_REQ_TYPE_READ) 
> > instead of doing it in driver.
> It seems you're changing the behavior, not fixing a bug.
> WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR is handled differently from 
> WMI_BSS_SURVEY_REQ_TYPE_READ and as Jasmine addressed, userspace apps 
> that's counting on WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR behavior will be 
> broken their functions with your change.
> This means this patch will break backward compatibility.

Sounds to me like a bug when there is a driver independent API but two 
incompatible implementations:

* one driver using accumulated values over time
* one driver using non-accumulated values which are reset after each read to 0

Especially because there is already software which expects the accumulated 
values and which has no way to find out whether the just received data is now
accumulated or not. And it is also quite bad to have non-accumulated (ath10k's 
read+clear) data when multiple programs need to gather this data at the same 
time.

@Felix, @Johannes: Should &struct ieee80211_ops->get_survey/
NL80211_CMD_GET_SURVEY return "accumulated" channel utilizations values or 
clear the counters on each read? Might have missed the relevant phrase in 
include/uapi/linux/nl80211.h, include/net/cfg80211.h or
include/net/mac80211.h - so feel free to just point me to the already
existing documentation.


Btw. the same author also tried to implement the "accumulated data" behavior 
for get_survey in cfg80211 when a driver only supports read+clear:
https://patchwork.kernel.org/patch/10417673/

And here the earlier attempt (changing it from read_clear to read): 
https://patchwork.kernel.org/patch/9701459/ - the answer from Felix suggests 
that the accumulated values would be correct for the channel utilization 
related info in struct survey_info.

Kind regards,
	Sven


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCHv2] ath10k : Fix channel survey dump
@ 2018-08-09 22:26     ` Sven Eckelmann
  0 siblings, 0 replies; 13+ messages in thread
From: Sven Eckelmann @ 2018-08-09 22:26 UTC (permalink / raw)
  To: Peter Oh
  Cc: Jasmine Strong, Venkateswara Naralasetty, linux-wireless,
	Helmut Schaa, ath10k, Johannes Berg, Michał Kazior,
	Ben Greear, Felix Fietkau


[-- Attachment #1.1: Type: text/plain, Size: 2074 bytes --]

On Donnerstag, 9. August 2018 20:46:26 CEST Peter Oh wrote:
> > In ath9k driver also survey data being accumulated in driver and send 
> > survey data to user space is accumulated one. same thing we are 
> > implementing in FW(with WMI request type WMI_BSS_SURVEY_REQ_TYPE_READ) 
> > instead of doing it in driver.
> It seems you're changing the behavior, not fixing a bug.
> WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR is handled differently from 
> WMI_BSS_SURVEY_REQ_TYPE_READ and as Jasmine addressed, userspace apps 
> that's counting on WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR behavior will be 
> broken their functions with your change.
> This means this patch will break backward compatibility.

Sounds to me like a bug when there is a driver independent API but two 
incompatible implementations:

* one driver using accumulated values over time
* one driver using non-accumulated values which are reset after each read to 0

Especially because there is already software which expects the accumulated 
values and which has no way to find out whether the just received data is now
accumulated or not. And it is also quite bad to have non-accumulated (ath10k's 
read+clear) data when multiple programs need to gather this data at the same 
time.

@Felix, @Johannes: Should &struct ieee80211_ops->get_survey/
NL80211_CMD_GET_SURVEY return "accumulated" channel utilizations values or 
clear the counters on each read? Might have missed the relevant phrase in 
include/uapi/linux/nl80211.h, include/net/cfg80211.h or
include/net/mac80211.h - so feel free to just point me to the already
existing documentation.


Btw. the same author also tried to implement the "accumulated data" behavior 
for get_survey in cfg80211 when a driver only supports read+clear:
https://patchwork.kernel.org/patch/10417673/

And here the earlier attempt (changing it from read_clear to read): 
https://patchwork.kernel.org/patch/9701459/ - the answer from Felix suggests 
that the accumulated values would be correct for the channel utilization 
related info in struct survey_info.

Kind regards,
	Sven


[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 146 bytes --]

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

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

* RE: [PATCHv2] ath10k : Fix channel survey dump
  2018-07-31 17:37   ` Ben Greear
@ 2018-08-01  8:47     ` Venkateswara Naralasetty
  -1 siblings, 0 replies; 13+ messages in thread
From: Venkateswara Naralasetty @ 2018-08-01  8:47 UTC (permalink / raw)
  To: Ben Greear, Venkateswara Naralasetty, ath10k; +Cc: linux-wireless



> -----Original Message-----
> From: ath10k <ath10k-bounces@lists.infradead.org> On Behalf Of Ben
> Greear
> Sent: Tuesday, July 31, 2018 11:08 PM
> To: Venkateswara Naralasetty <vnaralas@codeaurora.org>;
> ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org
> Subject: Re: [PATCHv2] ath10k : Fix channel survey dump
>=20
> On 07/31/2018 05:11 AM, Venkateswara Naralasetty wrote:
> > Channel active/busy time are showing incorrect(less than previous or
> > sometimes zero) for successive survey dump command.
> >
> > example:
> > Survey data from wlan0
> > 	frequency:                      5180 MHz [in use]
> > 	channel active time:            54995 ms
> > 	channel busy time:              432 ms
> > 	channel receive time:           0 ms
> > 	channel transmit time:          59 ms
> > Survey data from wlan0
> > 	frequency:                      5180 MHz [in use]
> > 	channel active time:            32592 ms
> > 	channel busy time:              254 ms
> > 	channel receive time:           0 ms
> > 	channel transmit time:          0 ms
> >
> > This patch fix this issue by assigning 'wmi_bss_survey_req_type'
> > as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
> FW
> > and send survey data to driver upon the driver request. Wrap around is
> > taken care by FW.
> >
> > hardware used : QCA9984
> > firmware ver  : ver 10.4-3.5.3-00057
>=20
> Have you tested this on other firmwares?  I am pretty sure that at least =
some
> of them, probably 10.2, will have issues.
>=20
I have tested this change with firmware version 10.2.4-1.0-00036 (hw used Q=
CA988x) as well it's working fine for me without any issues.
=20
> Maybe you could make this change specific to certain firmware that is kno=
wn
> to work with the change?
>=20
> Thanks,
> Ben
>=20
> >
> > Signed-off-by: Venkateswara Naralasetty <vnaralas@codeaurora.org>
> > ---
> > v2:
> >  * updated commit log.
> >
> >  drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/mac.c
> > b/drivers/net/wireless/ath/ath10k/mac.c
> > index f068e2b..db93ab1 100644
> > --- a/drivers/net/wireless/ath/ath10k/mac.c
> > +++ b/drivers/net/wireless/ath/ath10k/mac.c
> > @@ -6837,7 +6837,7 @@ ath10k_mac_update_bss_chan_survey(struct
> ath10k *ar,
> >  				  struct ieee80211_channel *channel)  {
> >  	int ret;
> > -	enum wmi_bss_survey_req_type type =3D
> WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR;
> > +	enum wmi_bss_survey_req_type type =3D
> WMI_BSS_SURVEY_REQ_TYPE_READ;
> >
> >  	lockdep_assert_held(&ar->conf_mutex);
> >
> >
>=20
>=20
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>=20
>=20
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k

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

* RE: [PATCHv2] ath10k : Fix channel survey dump
@ 2018-08-01  8:47     ` Venkateswara Naralasetty
  0 siblings, 0 replies; 13+ messages in thread
From: Venkateswara Naralasetty @ 2018-08-01  8:47 UTC (permalink / raw)
  To: Ben Greear, Venkateswara Naralasetty, ath10k; +Cc: linux-wireless



> -----Original Message-----
> From: ath10k <ath10k-bounces@lists.infradead.org> On Behalf Of Ben
> Greear
> Sent: Tuesday, July 31, 2018 11:08 PM
> To: Venkateswara Naralasetty <vnaralas@codeaurora.org>;
> ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org
> Subject: Re: [PATCHv2] ath10k : Fix channel survey dump
> 
> On 07/31/2018 05:11 AM, Venkateswara Naralasetty wrote:
> > Channel active/busy time are showing incorrect(less than previous or
> > sometimes zero) for successive survey dump command.
> >
> > example:
> > Survey data from wlan0
> > 	frequency:                      5180 MHz [in use]
> > 	channel active time:            54995 ms
> > 	channel busy time:              432 ms
> > 	channel receive time:           0 ms
> > 	channel transmit time:          59 ms
> > Survey data from wlan0
> > 	frequency:                      5180 MHz [in use]
> > 	channel active time:            32592 ms
> > 	channel busy time:              254 ms
> > 	channel receive time:           0 ms
> > 	channel transmit time:          0 ms
> >
> > This patch fix this issue by assigning 'wmi_bss_survey_req_type'
> > as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
> FW
> > and send survey data to driver upon the driver request. Wrap around is
> > taken care by FW.
> >
> > hardware used : QCA9984
> > firmware ver  : ver 10.4-3.5.3-00057
> 
> Have you tested this on other firmwares?  I am pretty sure that at least some
> of them, probably 10.2, will have issues.
> 
I have tested this change with firmware version 10.2.4-1.0-00036 (hw used QCA988x) as well it's working fine for me without any issues.
 
> Maybe you could make this change specific to certain firmware that is known
> to work with the change?
> 
> Thanks,
> Ben
> 
> >
> > Signed-off-by: Venkateswara Naralasetty <vnaralas@codeaurora.org>
> > ---
> > v2:
> >  * updated commit log.
> >
> >  drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/mac.c
> > b/drivers/net/wireless/ath/ath10k/mac.c
> > index f068e2b..db93ab1 100644
> > --- a/drivers/net/wireless/ath/ath10k/mac.c
> > +++ b/drivers/net/wireless/ath/ath10k/mac.c
> > @@ -6837,7 +6837,7 @@ ath10k_mac_update_bss_chan_survey(struct
> ath10k *ar,
> >  				  struct ieee80211_channel *channel)  {
> >  	int ret;
> > -	enum wmi_bss_survey_req_type type =
> WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR;
> > +	enum wmi_bss_survey_req_type type =
> WMI_BSS_SURVEY_REQ_TYPE_READ;
> >
> >  	lockdep_assert_held(&ar->conf_mutex);
> >
> >
> 
> 
> --
> 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

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

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

* Re: [PATCHv2] ath10k : Fix channel survey dump
  2018-07-31 17:37   ` Ben Greear
  (?)
@ 2018-07-31 18:33   ` Jasmine Strong
  -1 siblings, 0 replies; 13+ messages in thread
From: Jasmine Strong @ 2018-07-31 18:33 UTC (permalink / raw)
  To: Ben Greear; +Cc: vnaralas, ath10k, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]

On Tue, Jul 31, 2018 at 10:38 AM Ben Greear <greearb@candelatech.com> wrote:

> On 07/31/2018 05:11 AM, Venkateswara Naralasetty wrote:
> > Channel active/busy time are showing incorrect(less than previous or
> > sometimes zero) for successive survey dump command.
> >
> > example:
> > Survey data from wlan0
> >       frequency:                      5180 MHz [in use]
> >       channel active time:            54995 ms
> >       channel busy time:              432 ms
> >       channel receive time:           0 ms
> >       channel transmit time:          59 ms
> > Survey data from wlan0
> >       frequency:                      5180 MHz [in use]
> >       channel active time:            32592 ms
> >       channel busy time:              254 ms
> >       channel receive time:           0 ms
> >       channel transmit time:          0 ms
> >
> > This patch fix this issue by assigning 'wmi_bss_survey_req_type'
> > as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
> > FW and send survey data to driver upon the driver request. Wrap around
> > is taken care by FW.
> >
> > hardware used : QCA9984
> > firmware ver  : ver 10.4-3.5.3-00057
>
> Have you tested this on other firmwares?  I am pretty sure that at least
> some of them, probably 10.2, will have issues.
>
> Maybe you could make this change specific to certain firmware that
> is known to work with the change?
>
>
There are bigger problems with it than that;  even with firmwares that
behave correctly,
this changes the behavior that a lot of upper-layer software depends upon,
and will likely
make anything that actually uses the channel survey data misbehave.

It is therefore a breaking change for any device that actually implements
ACS.

-J.

[-- Attachment #2: Type: text/html, Size: 2371 bytes --]

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

* Re: [PATCHv2] ath10k : Fix channel survey dump
  2018-07-31 12:11 ` Venkateswara Naralasetty
@ 2018-07-31 17:37   ` Ben Greear
  -1 siblings, 0 replies; 13+ messages in thread
From: Ben Greear @ 2018-07-31 17:37 UTC (permalink / raw)
  To: Venkateswara Naralasetty, ath10k; +Cc: linux-wireless

On 07/31/2018 05:11 AM, Venkateswara Naralasetty wrote:
> Channel active/busy time are showing incorrect(less than previous or
> sometimes zero) for successive survey dump command.
>
> example:
> Survey data from wlan0
> 	frequency:                      5180 MHz [in use]
> 	channel active time:            54995 ms
> 	channel busy time:              432 ms
> 	channel receive time:           0 ms
> 	channel transmit time:          59 ms
> Survey data from wlan0
> 	frequency:                      5180 MHz [in use]
> 	channel active time:            32592 ms
> 	channel busy time:              254 ms
> 	channel receive time:           0 ms
> 	channel transmit time:          0 ms
>
> This patch fix this issue by assigning 'wmi_bss_survey_req_type'
> as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
> FW and send survey data to driver upon the driver request. Wrap around
> is taken care by FW.
>
> hardware used : QCA9984
> firmware ver  : ver 10.4-3.5.3-00057

Have you tested this on other firmwares?  I am pretty sure that at least
some of them, probably 10.2, will have issues.

Maybe you could make this change specific to certain firmware that
is known to work with the change?

Thanks,
Ben

>
> Signed-off-by: Venkateswara Naralasetty <vnaralas@codeaurora.org>
> ---
> v2:
>  * updated commit log.
>
>  drivers/net/wireless/ath/ath10k/mac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index f068e2b..db93ab1 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -6837,7 +6837,7 @@ ath10k_mac_update_bss_chan_survey(struct ath10k *ar,
>  				  struct ieee80211_channel *channel)
>  {
>  	int ret;
> -	enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR;
> +	enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ;
>
>  	lockdep_assert_held(&ar->conf_mutex);
>
>


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

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

* Re: [PATCHv2] ath10k : Fix channel survey dump
@ 2018-07-31 17:37   ` Ben Greear
  0 siblings, 0 replies; 13+ messages in thread
From: Ben Greear @ 2018-07-31 17:37 UTC (permalink / raw)
  To: Venkateswara Naralasetty, ath10k; +Cc: linux-wireless

On 07/31/2018 05:11 AM, Venkateswara Naralasetty wrote:
> Channel active/busy time are showing incorrect(less than previous or
> sometimes zero) for successive survey dump command.
>
> example:
> Survey data from wlan0
> 	frequency:                      5180 MHz [in use]
> 	channel active time:            54995 ms
> 	channel busy time:              432 ms
> 	channel receive time:           0 ms
> 	channel transmit time:          59 ms
> Survey data from wlan0
> 	frequency:                      5180 MHz [in use]
> 	channel active time:            32592 ms
> 	channel busy time:              254 ms
> 	channel receive time:           0 ms
> 	channel transmit time:          0 ms
>
> This patch fix this issue by assigning 'wmi_bss_survey_req_type'
> as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
> FW and send survey data to driver upon the driver request. Wrap around
> is taken care by FW.
>
> hardware used : QCA9984
> firmware ver  : ver 10.4-3.5.3-00057

Have you tested this on other firmwares?  I am pretty sure that at least
some of them, probably 10.2, will have issues.

Maybe you could make this change specific to certain firmware that
is known to work with the change?

Thanks,
Ben

>
> Signed-off-by: Venkateswara Naralasetty <vnaralas@codeaurora.org>
> ---
> v2:
>  * updated commit log.
>
>  drivers/net/wireless/ath/ath10k/mac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index f068e2b..db93ab1 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -6837,7 +6837,7 @@ ath10k_mac_update_bss_chan_survey(struct ath10k *ar,
>  				  struct ieee80211_channel *channel)
>  {
>  	int ret;
> -	enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR;
> +	enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ;
>
>  	lockdep_assert_held(&ar->conf_mutex);
>
>


-- 
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] 13+ messages in thread

* [PATCHv2] ath10k : Fix channel survey dump
@ 2018-07-31 12:11 ` Venkateswara Naralasetty
  0 siblings, 0 replies; 13+ messages in thread
From: Venkateswara Naralasetty @ 2018-07-31 12:11 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Venkateswara Naralasetty

Channel active/busy time are showing incorrect(less than previous or
sometimes zero) for successive survey dump command.

example:
Survey data from wlan0
	frequency:                      5180 MHz [in use]
	channel active time:            54995 ms
	channel busy time:              432 ms
	channel receive time:           0 ms
	channel transmit time:          59 ms
Survey data from wlan0
	frequency:                      5180 MHz [in use]
	channel active time:            32592 ms
	channel busy time:              254 ms
	channel receive time:           0 ms
	channel transmit time:          0 ms

This patch fix this issue by assigning 'wmi_bss_survey_req_type'
as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
FW and send survey data to driver upon the driver request. Wrap around
is taken care by FW.

hardware used : QCA9984
firmware ver  : ver 10.4-3.5.3-00057

Signed-off-by: Venkateswara Naralasetty <vnaralas@codeaurora.org>
---
v2:
 * updated commit log.

 drivers/net/wireless/ath/ath10k/mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index f068e2b..db93ab1 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -6837,7 +6837,7 @@ ath10k_mac_update_bss_chan_survey(struct ath10k *ar,
 				  struct ieee80211_channel *channel)
 {
 	int ret;
-	enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR;
+	enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
-- 
2.7.4

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

* [PATCHv2] ath10k : Fix channel survey dump
@ 2018-07-31 12:11 ` Venkateswara Naralasetty
  0 siblings, 0 replies; 13+ messages in thread
From: Venkateswara Naralasetty @ 2018-07-31 12:11 UTC (permalink / raw)
  To: ath10k; +Cc: Venkateswara Naralasetty, linux-wireless

Channel active/busy time are showing incorrect(less than previous or
sometimes zero) for successive survey dump command.

example:
Survey data from wlan0
	frequency:                      5180 MHz [in use]
	channel active time:            54995 ms
	channel busy time:              432 ms
	channel receive time:           0 ms
	channel transmit time:          59 ms
Survey data from wlan0
	frequency:                      5180 MHz [in use]
	channel active time:            32592 ms
	channel busy time:              254 ms
	channel receive time:           0 ms
	channel transmit time:          0 ms

This patch fix this issue by assigning 'wmi_bss_survey_req_type'
as 'WMI_BSS_SURVEY_REQ_TYPE_READ' which accumulate survey data in
FW and send survey data to driver upon the driver request. Wrap around
is taken care by FW.

hardware used : QCA9984
firmware ver  : ver 10.4-3.5.3-00057

Signed-off-by: Venkateswara Naralasetty <vnaralas@codeaurora.org>
---
v2:
 * updated commit log.

 drivers/net/wireless/ath/ath10k/mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index f068e2b..db93ab1 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -6837,7 +6837,7 @@ ath10k_mac_update_bss_chan_survey(struct ath10k *ar,
 				  struct ieee80211_channel *channel)
 {
 	int ret;
-	enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ_CLEAR;
+	enum wmi_bss_survey_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
-- 
2.7.4


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

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

end of thread, other threads:[~2018-08-10  0:53 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-09  6:35 [PATCHv2] ath10k : Fix channel survey dump Venkateswara Naralasetty
2018-08-09  6:35 ` Venkateswara Naralasetty
2018-08-09 18:46 ` Peter Oh
2018-08-09 18:46   ` Peter Oh
2018-08-09 22:26   ` Sven Eckelmann
2018-08-09 22:26     ` Sven Eckelmann
  -- strict thread matches above, loose matches on Subject: below --
2018-07-31 12:11 Venkateswara Naralasetty
2018-07-31 12:11 ` Venkateswara Naralasetty
2018-07-31 17:37 ` Ben Greear
2018-07-31 17:37   ` Ben Greear
2018-07-31 18:33   ` Jasmine Strong
2018-08-01  8:47   ` Venkateswara Naralasetty
2018-08-01  8:47     ` Venkateswara Naralasetty

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.