Return-path: Received: from mail-eopbgr60088.outbound.protection.outlook.com ([40.107.6.88]:7072 "EHLO EUR04-DB3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727211AbeHIVMt (ORCPT ); Thu, 9 Aug 2018 17:12:49 -0400 Subject: Re: [PATCHv2] ath10k : Fix channel survey dump To: Venkateswara Naralasetty , Jasmine Strong References: Cc: Ben Greear , ath10k , linux-wireless@vger.kernel.org, linux-wireless-owner@vger.kernel.org From: Peter Oh Message-ID: <619039a5-3b30-b49d-dc85-9ccfa4403251@bowerswilkins.com> (sfid-20180809_204654_059331_F43538EF) Date: Thu, 9 Aug 2018 11:46:26 -0700 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: 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 >> 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