[PATCHv2 2/2] ath10k: Allow setting coverage class

Michal Kazior michal.kazior at tieto.com
Wed Aug 3 00:19:12 PDT 2016


On 1 August 2016 at 12:21, Benjamin Berg <benjamin at sipsolutions.net> wrote:
> Unfortunately ath10k does not generally allow modifying the coverage class
> with the stock firmware and Qualcomm has so far refused to implement this
> feature so that it can be properly supported in ath10k. If we however know
> the registers that need to be modified for proper operation with a higher
> coverage class, then we can do these modifications from the driver.
>
> This patch implements this hack for first generation cards which are based
> on a core that is similar to ath9k. The registers are modified in place and
> need to be re-written every time the firmware sets them. To achieve this
> the register status is verified after any WMI event from the firmware.
>
> The coverage class may not be modified temporarily right after the card
> re-initializes the registers. This is for example the case during scanning.
>
> Thanks to Sebastian Gottschall <s.gottschall at dd-wrt.com> for initially
> working on a userspace support for this. This patch wouldn't have been
> possible without this documentation.
>
> Signed-off-by: Benjamin Berg <benjamin at sipsolutions.net>
> Signed-off-by: Simon Wunderlich <sw at simonwunderlich.de>
> Signed-off-by: Mathias Kretschmer <mathias.kretschmer at fit.fraunhofer.de>
> ---
>  drivers/net/wireless/ath/ath10k/core.h |  10 +++
>  drivers/net/wireless/ath/ath10k/hw.c   | 112 +++++++++++++++++++++++++++++++++
>  drivers/net/wireless/ath/ath10k/hw.h   |  22 ++++++-
>  drivers/net/wireless/ath/ath10k/mac.c  |  19 ++++++
>  drivers/net/wireless/ath/ath10k/wmi.c  |  29 +++++++++
>  5 files changed, 191 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> index 5ace413..4ae730b 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -890,6 +890,16 @@ struct ath10k {
>         struct ath10k_thermal thermal;
>         struct ath10k_wow wow;
>
> +       /* protected by data_lock */
> +       struct {
> +               s16 coverage_class;
> +
> +               u32 reg_slottime_conf;
> +               u32 reg_slottime_orig;
> +               u32 reg_ack_cts_timeout_conf;
> +               u32 reg_ack_cts_timeout_orig;
> +       } fw_coverage;
> +
>         /* must be last */
>         u8 drv_priv[0] __aligned(sizeof(void *));
>  };
> diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
> index c2ecb9b..f1278ad 100644
> --- a/drivers/net/wireless/ath/ath10k/hw.c
> +++ b/drivers/net/wireless/ath/ath10k/hw.c
> @@ -17,11 +17,13 @@
>  #include <linux/types.h>
>  #include "core.h"
>  #include "hw.h"
> +#include "hif.h"
>
>  const struct ath10k_hw_regs qca988x_regs = {
>         .rtc_soc_base_address           = 0x00004000,
>         .rtc_wmac_base_address          = 0x00005000,
>         .soc_core_base_address          = 0x00009000,
> +       .wlan_mac_base_address          = 0x00020000,
>         .ce_wrapper_base_address        = 0x00057000,
>         .ce0_base_address               = 0x00057400,
>         .ce1_base_address               = 0x00057800,
> @@ -48,6 +50,7 @@ const struct ath10k_hw_regs qca6174_regs = {
>         .rtc_soc_base_address                   = 0x00000800,
>         .rtc_wmac_base_address                  = 0x00001000,
>         .soc_core_base_address                  = 0x0003a000,
> +       .wlan_mac_base_address                  = 0x00020000,
>         .ce_wrapper_base_address                = 0x00034000,
>         .ce0_base_address                       = 0x00034400,
>         .ce1_base_address                       = 0x00034800,
> @@ -74,6 +77,7 @@ const struct ath10k_hw_regs qca99x0_regs = {
>         .rtc_soc_base_address                   = 0x00080000,
>         .rtc_wmac_base_address                  = 0x00000000,
>         .soc_core_base_address                  = 0x00082000,
> +       .wlan_mac_base_address                  = 0x00030000,
>         .ce_wrapper_base_address                = 0x0004d000,
>         .ce0_base_address                       = 0x0004a000,
>         .ce1_base_address                       = 0x0004a400,
> @@ -109,6 +113,7 @@ const struct ath10k_hw_regs qca99x0_regs = {
>  const struct ath10k_hw_regs qca4019_regs = {
>         .rtc_soc_base_address                   = 0x00080000,
>         .soc_core_base_address                  = 0x00082000,
> +       .wlan_mac_base_address                  = 0x00030000,
>         .ce_wrapper_base_address                = 0x0004d000,
>         .ce0_base_address                       = 0x0004a000,
>         .ce1_base_address                       = 0x0004a400,
> @@ -220,7 +225,114 @@ void ath10k_hw_fill_survey_time(struct ath10k *ar, struct survey_info *survey,
>         survey->time_busy = CCNT_TO_MSEC(ar, rcc);
>  }
>
> +static void ath10k_qca988x_mac_op_set_coverage_class(struct ath10k *ar,
> +                                                    s16 value)

The naming is wrong. The general convention is:

 ath10k_{base_filename}_{purpose}

If a function is used as an _ops callback (ieee80211_ops, or wmi_ops)
the purpose should start with _ops.

In this case the name should be:

  ath10k_hw_qca988x_set_coverage_class()


> +       if (value < 0)
> +               value = ar->fw_coverage.coverage_class;
> +
> +       /* Break out if the coverage class and registers have the expected
> +        * value.
> +        */
> +       if (value == ar->fw_coverage.coverage_class &&
> +           slottime_reg == ar->fw_coverage.reg_slottime_conf &&
> +           timeout_reg == ar->fw_coverage.reg_ack_cts_timeout_conf)
> +               goto unlock;

I would move this part to the caller and keep the hardware-specific
function just compute and set registers, e.g. have
ath10k_mac_set_coverage_class (as opposed to
ath10k_mac_op_set_coverage_class). It should also reduce some code
duplication.


[...]
>  static int ath10k_qca99x0_rx_desc_get_l3_pad_bytes(struct htt_rx_desc *rxd)
> diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
> index 1ef7dc6..532eab5 100644
> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -230,6 +230,7 @@ struct ath10k_hw_regs {
>         u32 rtc_soc_base_address;
>         u32 rtc_wmac_base_address;
>         u32 soc_core_base_address;
> +       u32 wlan_mac_base_address;
>         u32 ce_wrapper_base_address;
>         u32 ce0_base_address;
>         u32 ce1_base_address;
> @@ -418,6 +419,7 @@ struct htt_rx_desc;
>  /* Defines needed for Rx descriptor abstraction */
>  struct ath10k_hw_ops {
>         int (*rx_desc_get_l3_pad_bytes)(struct htt_rx_desc *rxd);
> +       void (*mac_op_set_coverage_class)(struct ath10k *ar, s16 value);

Just "set_coverage_class" is fine.


[...]
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index 169cd2e7..700c430 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -4992,6 +4992,13 @@ static void ath10k_wmi_op_rx(struct ath10k *ar, struct sk_buff *skb)
>                 break;
>         }
>
> +       /* Check and possibly reset the coverage class configuration override.
> +        * There are many conditions (in particular internal card resets) that
> +        * can cause the registers to be re-initialized.
> +        */
> +       if (!ar->hw_params.hw_ops->mac_op_set_coverage_class)
> +               ar->hw_params.hw_ops->mac_op_set_coverage_class(ar, -1);

As discussed in the other thread - it may be better to implicitly
enable dbglog and hook the refresh call there.


Michał



More information about the ath10k mailing list