RE: [PATCH] ath10k: support PCIe enter L1 state

2018-11-14 Thread Wen Gong
> -Original Message-
> From: ath10k  On Behalf Of Brian
> Norris
> Sent: Thursday, November 15, 2018 8:29 AM
> To: Wen Gong 
> Cc: linux-wirel...@vger.kernel.org; ath10k@lists.infradead.org
> Subject: [EXTERNAL] Re: [PATCH] ath10k: support PCIe enter L1 state
> 
> Hi Wen,
> 
> >
> > Signed-off-by: Wen Gong 
> 
> Is there some reason L1 was disabled in the first place? Wa
s it known to be
> unreliable?
> 
Hi Brian,
It is a BUG for power, and it is not considered this BUG before.
So this change will fix the bug.

> Brian
> 
> ___
> 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 v4 2/3] dts: arm64/sdm845: Add WCN3990 WLAN module device node

2018-11-14 Thread Brian Norris
Hi Govind,

On Mon, Nov 05, 2018 at 06:38:37PM +0530, Govind Singh wrote:
> Add device node for the ath10k SNOC platform driver probe
> and add resources required for WCN3990 on SDM845 soc.
> 
> Signed-off-by: Govind Singh 
> Reviewed-by: Brian Norris 
> Tested-by: Brian Norris 
> ---
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |  8 
>  arch/arm64/boot/dts/qcom/sdm845.dtsi| 26 ++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts 
> b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index eedfaf8..c062c5c 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -440,3 +440,11 @@
>   bias-pull-up;
>   };
>  };
> +
> +&wifi {
> + status = "okay";
> + vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
> + vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
> + vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
> + vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
> +};

This node should be above the PINCTRL section.

Brian

> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index b72bdb0..324be5b 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -87,6 +87,11 @@
>   reg = <0 0x8620 0 0x2d0>;
>   no-map;
>   };
> +
> + wlan_msa_mem: memory@9670 {
> + reg = <0 0x9670 0 0x10>;
> + no-map;
> + };
>   };
>  
>   cpus {
> @@ -1403,5 +1408,26 @@
>   status = "disabled";
>   };
>   };
> +
> + wifi: wifi@1880 {
> + compatible = "qcom,wcn3990-wifi";
> + status = "disabled";
> + reg = <0x1880 0x80>;
> + reg-names = "membase";
> + memory-region = <&wlan_msa_mem>;
> + interrupts =
> + ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ;
> + };
>   };
>  };
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

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


Re: atk10k Unknown eventid and WARN() on resume with kernel 4.19

2018-11-14 Thread Gabriel C
Am Mi., 14. Nov. 2018 um 23:16 Uhr schrieb Gabriel C :
> > > Hello,
> > >
> > > starting with kernel 4.19 we noticed  'Unknown eventid: ' warnings
> > > on our Laptops.
> > >
> > > ...
> > >
> > > crazy@devnull:~$ dmesg | grep Unknow
> > > [7.144998] ath10k_pci :03:00.0: Unknown eventid: 118809
> > > [7.147758] ath10k_pci :03:00.0: Unknown eventid: 90118
> > > [9.441654] ath10k_pci :03:00.0: Unknown eventid: 118809
> > > [9.46] ath10k_pci :03:00.0: Unknown eventid: 90118
> > > [   11.770997] ath10k_pci :03:00.0: Unknown eventid: 118809
> > > [   11.773787] ath10k_pci :03:00.0: Unknown eventid: 90118
> > > [320339.374108] ath10k_pci :03:00.0: Unknown eventid: 118809
> > > [320339.377020] ath10k_pci :03:00.0: Unknown eventid: 90118
> > > [320342.128664] ath10k_pci :03:00.0: Unknown eventid: 118809
> > > [320342.131447] ath10k_pci :03:00.0: Unknown eventid: 90118
> > > [320344.452864] ath10k_pci :03:00.0: Unknown eventid: 118809
> > > [320344.455696] ath10k_pci :03:00.0: Unknown eventid: 90118
> > >
> > > 
> > >
> > > Both boxes also hits the following WARN*() on resume from S3:
> > >
> >  Uh sorry .. Gmail doesn't like long lines it seems.
> > There the WARN() message:
> >
> > http://ftp.frugalware.org/pub/other/people/crazy/warn.txt
> >
> > >
> > > Please let me know if you need an dmesg or any other informations.
>
>
> Added pm list maybe someone there got some idea ?
>
> Also same issue reported there :
> https://bugzilla.kernel.org/show_bug.cgi?id=201499
>

The WARN_ON() on resume seems to be triggered  after commit cd93b83ad927b.

BR,

Gabriel C

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


Re: [PATCH] ath10k: support PCIe enter L1 state

2018-11-14 Thread Brian Norris
Hi Wen,

On Wed, Nov 14, 2018 at 10:50:48AM +0800, Wen Gong wrote:
> QCA6174A/QCA9377 PCIe chips support PCIe L1 and L1SS, and indicate the
> L1/L1SS capabilities in PCI configuration space. Currently ath10k driver
> write target PCIe config flags to disallow HW enter into L1, this leads
> some HW modules are still powered up even when both system PCIe RC and
> QCA6174A/QCA9377 endpoint decides to enter into L1 or L1SS.
> 
> This cause ~12 mA power drain of bottom power consumption for all scenarios.
> Fix this issue by removing the drive code to write PCIe config flags.
> 
> Tested with QCA6174 PCI with firmware
> WLAN.RM.4.4.1-00109-QCARMSWPZ-1, but this will also affect QCA9377 PCI.
> It's not a regression with new firmware releases.
> 
> Signed-off-by: Wen Gong 

Is there some reason L1 was disabled in the first place? Was it known to
be unreliable?

Brian

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


Re: [PATCH v3] ath10k: support NET_DETECT WoWLAN feature

2018-11-14 Thread Brian Norris
Hi Wen,

You've introduced a regression in 4.20-rc1:

On Thu, Aug 16, 2018 at 02:48:33PM +0800, Wen Gong wrote:
> For WoWLAN support, it expect to support wake up based on discovery of
> one or more known SSIDs. This is the WIPHY_WOWLAN_NET_DETECT feature,
> which shows up as an NL80211 feature flag.
> 
> With an upgrade iw, this shows up in 'iw phy' as:
> WoWLAN support:
> * wake up on network detection, up to 16 match sets
> And it can use command:
> "iw phy0 wowlan enable net-detect interval 5000 delay 30 freqs 2412
> matches ssid foo" to configure the parameters of net detect.
> 
> Firmware will do scan by the configured parameters after suspend and
> wakeup if it found matched SSIDs. Tested with QCA6174 hw3.0 with
> firmware WLAN.RM.4.4.1-00110-QCARMSWPZ-1.
> 
> Signed-off-by: Wen Gong 
> ---
> V3:
> -fix the waring of alloc with no test
>  drivers/net/wireless/ath/ath10k/core.h|   1 +
>  drivers/net/wireless/ath/ath10k/mac.c |  12 ++
>  drivers/net/wireless/ath/ath10k/wmi-ops.h |  21 +++
>  drivers/net/wireless/ath/ath10k/wmi-tlv.c | 180 +++-
>  drivers/net/wireless/ath/ath10k/wmi-tlv.h | 226 
> ++
>  drivers/net/wireless/ath/ath10k/wmi.h |  57 
>  drivers/net/wireless/ath/ath10k/wow.c | 174 +++
>  7 files changed, 670 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/core.h 
> b/drivers/net/wireless/ath/ath10k/core.h
> index 427ee57..7885462 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -904,6 +904,7 @@ struct ath10k {
>   u32 high_5ghz_chan;
>   bool ani_enabled;
>  
> + bool nlo_enabled;
>   bool p2p;
>  
>   struct {
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
> b/drivers/net/wireless/ath/ath10k/mac.c
> index 95243b4..ba9b9af 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -8361,6 +8361,18 @@ int ath10k_mac_register(struct ath10k *ar)
>   ar->hw->wiphy->max_scan_ssids = WLAN_SCAN_PARAMS_MAX_SSID;
>   ar->hw->wiphy->max_scan_ie_len = WLAN_SCAN_PARAMS_MAX_IE_LEN;
>  
> + if (test_bit(WMI_SERVICE_NLO, ar->wmi.svc_map)) {
> + ar->hw->wiphy->max_sched_scan_reqs = 1;
> + ar->hw->wiphy->max_sched_scan_ssids = WMI_PNO_MAX_SUPP_NETWORKS;
> + ar->hw->wiphy->max_match_sets   = WMI_PNO_MAX_SUPP_NETWORKS;
> + ar->hw->wiphy->max_sched_scan_ie_len = WMI_PNO_MAX_IE_LENGTH;
> + ar->hw->wiphy->max_sched_scan_plans = 
> WMI_PNO_MAX_SCHED_SCAN_PLANS;
> + ar->hw->wiphy->max_sched_scan_plan_interval =
> + WMI_PNO_MAX_SCHED_SCAN_PLAN_INT;
> + ar->hw->wiphy->max_sched_scan_plan_iterations =
> + WMI_PNO_MAX_SCHED_SCAN_PLAN_ITRNS;

It seems like youre enabling SCHED_SCAN support? But you're not adding
the NL80211_FEATURE_SCHED_SCAN_RANDOM_MAC_ADDR feature flag. So it puts
us in a tough place on using randomization -- we either can't trust the
FEATURE flags, or else we can't use both SCHED_SCAN and scan
randomization.

I haven't played with this much at all yet (except to notice that my
tests no longer pass), but maybe you just need to add the FEATURE flag.

Brian

> + }
> +
>   ar->hw->vif_data_size = sizeof(struct ath10k_vif);
>   ar->hw->sta_data_size = sizeof(struct ath10k_sta);
>   ar->hw->txq_data_size = sizeof(struct ath10k_txq);

...

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


Re: atk10k Unknown eventid and WARN() on resume with kernel 4.19

2018-11-14 Thread Gabriel C
Am Sa., 3. Nov. 2018 um 21:11 Uhr schrieb Gabriel C :
>
> Am Sa., 3. Nov. 2018 um 21:01 Uhr schrieb Gabriel C :
> >
> > Hello,
> >
> > starting with kernel 4.19 we noticed  'Unknown eventid: ' warnings
> > on our Laptops.
> >
> > ...
> >
> > crazy@devnull:~$ dmesg | grep Unknow
> > [7.144998] ath10k_pci :03:00.0: Unknown eventid: 118809
> > [7.147758] ath10k_pci :03:00.0: Unknown eventid: 90118
> > [9.441654] ath10k_pci :03:00.0: Unknown eventid: 118809
> > [9.46] ath10k_pci :03:00.0: Unknown eventid: 90118
> > [   11.770997] ath10k_pci :03:00.0: Unknown eventid: 118809
> > [   11.773787] ath10k_pci :03:00.0: Unknown eventid: 90118
> > [320339.374108] ath10k_pci :03:00.0: Unknown eventid: 118809
> > [320339.377020] ath10k_pci :03:00.0: Unknown eventid: 90118
> > [320342.128664] ath10k_pci :03:00.0: Unknown eventid: 118809
> > [320342.131447] ath10k_pci :03:00.0: Unknown eventid: 90118
> > [320344.452864] ath10k_pci :03:00.0: Unknown eventid: 118809
> > [320344.455696] ath10k_pci :03:00.0: Unknown eventid: 90118
> >
> > 
> >
> > Both boxes also hits the following WARN*() on resume from S3:
> >
>  Uh sorry .. Gmail doesn't like long lines it seems.
> There the WARN() message:
>
> http://ftp.frugalware.org/pub/other/people/crazy/warn.txt
>
> >
> > Please let me know if you need an dmesg or any other informations.


Added pm list maybe someone there got some idea ?

Also same issue reported there :
https://bugzilla.kernel.org/show_bug.cgi?id=201499

BR,

Gabriel C

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


Re: [PATCH v3 3/6] mac80211: Add airtime accounting and scheduling to TXQs

2018-11-14 Thread Toke Høiland-Jørgensen
Felix Fietkau  writes:

> On 2018-11-12 23:51, Rajkumar Manoharan wrote:
>> From: Toke Høiland-Jørgensen 
>> 
>> This adds airtime accounting and scheduling to the mac80211 TXQ
>> scheduler. A new callback, ieee80211_sta_register_airtime(), is added
>> that drivers can call to report airtime usage for stations.
>> 
>> When airtime information is present, mac80211 will schedule TXQs
>> (through ieee80211_next_txq()) in a way that enforces airtime fairness
>> between active stations. This scheduling works the same way as the ath9k
>> in-driver airtime fairness scheduling. If no airtime usage is reported
>> by the driver, the scheduler will default to round-robin scheduling.
>> 
>> For drivers that don't control TXQ scheduling in software, a new API
>> function, ieee80211_txq_may_transmit(), is added which the driver can use
>> to check if the TXQ is eligible for transmission, or should be throttled to
>> enforce fairness. Calls to this function must also be enclosed in
>> ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
>> 
>> The API ieee80211_txq_may_transmit() also ensures that TXQ list will be
>> aligned aginst driver's own round-robin scheduler list. i.e it rotates
>> the TXQ list till it makes the requested node becomes the first entry
>> in TXQ list. Thus both the TXQ list and driver's list are in sync.
>> 
>> Co-Developed-by: Rajkumar Manoharan 
>> Signed-off-by: Toke Høiland-Jørgensen 
>> Signed-off-by: Rajkumar Manoharan 
>> ---
>>  include/net/mac80211.h | 59 ++
>>  net/mac80211/cfg.c |  3 ++
>>  net/mac80211/debugfs.c |  3 ++
>>  net/mac80211/debugfs_sta.c | 50 --
>>  net/mac80211/ieee80211_i.h |  2 ++
>>  net/mac80211/main.c|  4 +++
>>  net/mac80211/sta_info.c| 44 +--
>>  net/mac80211/sta_info.h| 13 +++
>>  net/mac80211/status.c  |  6 
>>  net/mac80211/tx.c  | 90 
>> +++---
>>  10 files changed, 264 insertions(+), 10 deletions(-)
>> 
>> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
>> index aa4afbf0abaf..a1f1256448f5 100644
>> --- a/net/mac80211/status.c
>> +++ b/net/mac80211/status.c
>> @@ -818,6 +818,12 @@ static void __ieee80211_tx_status(struct ieee80211_hw 
>> *hw,
>>  ieee80211_sta_tx_notify(sta->sdata, (void *) skb->data,
>>  acked, info->status.tx_time);
>>  
>> +if (info->status.tx_time &&
>> +wiphy_ext_feature_isset(local->hw.wiphy,
>> +
>> NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
>> +ieee80211_sta_register_airtime(&sta->sta, tid,
>> +   info->status.tx_time, 0);
>> +
>>  if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
>>  if (info->flags & IEEE80211_TX_STAT_ACK) {
>>  if (sta->status_stats.lost_packets)
> I think the same is needed in ieee80211_tx_status_ext.

Right, good point.

>> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
>> index 305965283506..3f417e80e041 100644
>> --- a/net/mac80211/tx.c
>> +++ b/net/mac80211/tx.c
>> @@ -3660,12 +3680,74 @@ void ieee80211_return_txq(struct ieee80211_hw *hw,
>>  lockdep_assert_held(&local->active_txq_lock[txq->ac]);
>>  
>>  if (list_empty(&txqi->schedule_order) &&
>> -(!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets))
>> -list_add_tail(&txqi->schedule_order,
>> -  &local->active_txqs[txq->ac]);
>> +(!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) {
>> +/* If airtime accounting is active, always enqueue STAs at the
>> + * head of the list to ensure that they only get moved to the
>> + * back by the airtime DRR scheduler once they have a negative
>> + * deficit. A station that already has a negative deficit will
>> + * get immediately moved to the back of the list on the next
>> + * call to ieee80211_next_txq().
>> + */
>> +if (txqi->txq.sta &&
>> +wiphy_ext_feature_isset(local->hw.wiphy,
>> +
>> NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
>> +list_add(&txqi->schedule_order,
>> + &local->active_txqs[txq->ac]);
>> +else
>> +list_add_tail(&txqi->schedule_order,
>> +  &local->active_txqs[txq->ac]);
>> +}
>>  }
> This part doesn't really make much sense to me, but maybe I'm
> misunderstanding how the code works.
> Let's assume we have a driver like ath9k or mt76, which tries to keep a
> number of aggregates in the hardware queue, and the hardware queue is
> currently empty.
> If the current txq entry is kept at the head of the schedule list,

Re: [PATCH v3 3/6] mac80211: Add airtime accounting and scheduling to TXQs

2018-11-14 Thread Felix Fietkau
On 2018-11-12 23:51, Rajkumar Manoharan wrote:
> From: Toke Høiland-Jørgensen 
> 
> This adds airtime accounting and scheduling to the mac80211 TXQ
> scheduler. A new callback, ieee80211_sta_register_airtime(), is added
> that drivers can call to report airtime usage for stations.
> 
> When airtime information is present, mac80211 will schedule TXQs
> (through ieee80211_next_txq()) in a way that enforces airtime fairness
> between active stations. This scheduling works the same way as the ath9k
> in-driver airtime fairness scheduling. If no airtime usage is reported
> by the driver, the scheduler will default to round-robin scheduling.
> 
> For drivers that don't control TXQ scheduling in software, a new API
> function, ieee80211_txq_may_transmit(), is added which the driver can use
> to check if the TXQ is eligible for transmission, or should be throttled to
> enforce fairness. Calls to this function must also be enclosed in
> ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
> 
> The API ieee80211_txq_may_transmit() also ensures that TXQ list will be
> aligned aginst driver's own round-robin scheduler list. i.e it rotates
> the TXQ list till it makes the requested node becomes the first entry
> in TXQ list. Thus both the TXQ list and driver's list are in sync.
> 
> Co-Developed-by: Rajkumar Manoharan 
> Signed-off-by: Toke Høiland-Jørgensen 
> Signed-off-by: Rajkumar Manoharan 
> ---
>  include/net/mac80211.h | 59 ++
>  net/mac80211/cfg.c |  3 ++
>  net/mac80211/debugfs.c |  3 ++
>  net/mac80211/debugfs_sta.c | 50 --
>  net/mac80211/ieee80211_i.h |  2 ++
>  net/mac80211/main.c|  4 +++
>  net/mac80211/sta_info.c| 44 +--
>  net/mac80211/sta_info.h| 13 +++
>  net/mac80211/status.c  |  6 
>  net/mac80211/tx.c  | 90 
> +++---
>  10 files changed, 264 insertions(+), 10 deletions(-)
> 
> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
> index aa4afbf0abaf..a1f1256448f5 100644
> --- a/net/mac80211/status.c
> +++ b/net/mac80211/status.c
> @@ -818,6 +818,12 @@ static void __ieee80211_tx_status(struct ieee80211_hw 
> *hw,
>   ieee80211_sta_tx_notify(sta->sdata, (void *) skb->data,
>   acked, info->status.tx_time);
>  
> + if (info->status.tx_time &&
> + wiphy_ext_feature_isset(local->hw.wiphy,
> + 
> NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
> + ieee80211_sta_register_airtime(&sta->sta, tid,
> +info->status.tx_time, 0);
> +
>   if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
>   if (info->flags & IEEE80211_TX_STAT_ACK) {
>   if (sta->status_stats.lost_packets)
I think the same is needed in ieee80211_tx_status_ext.

> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 305965283506..3f417e80e041 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -3660,12 +3680,74 @@ void ieee80211_return_txq(struct ieee80211_hw *hw,
>   lockdep_assert_held(&local->active_txq_lock[txq->ac]);
>  
>   if (list_empty(&txqi->schedule_order) &&
> - (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets))
> - list_add_tail(&txqi->schedule_order,
> -   &local->active_txqs[txq->ac]);
> + (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) {
> + /* If airtime accounting is active, always enqueue STAs at the
> +  * head of the list to ensure that they only get moved to the
> +  * back by the airtime DRR scheduler once they have a negative
> +  * deficit. A station that already has a negative deficit will
> +  * get immediately moved to the back of the list on the next
> +  * call to ieee80211_next_txq().
> +  */
> + if (txqi->txq.sta &&
> + wiphy_ext_feature_isset(local->hw.wiphy,
> + 
> NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
> + list_add(&txqi->schedule_order,
> +  &local->active_txqs[txq->ac]);
> + else
> + list_add_tail(&txqi->schedule_order,
> +   &local->active_txqs[txq->ac]);
> + }
>  }
This part doesn't really make much sense to me, but maybe I'm
misunderstanding how the code works.
Let's assume we have a driver like ath9k or mt76, which tries to keep a
number of aggregates in the hardware queue, and the hardware queue is
currently empty.
If the current txq entry is kept at the head of the schedule list,
wouldn't the code just pull from that one over and over again, until
enough packets are transmitted by the hardware a