2013-10-14 11:16:33

by Bartosz Markowski

Subject: [PATCH] ath10k: do not warn about unsupported vdev param

10.X firmware does not support WMI_VDEV_PARAM_TX_ENCAP_TYPE.
It's a known limitation and we should not warn about this.

Signed-off-by: Bartosz Markowski <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 049eca2..9c800f8 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2085,7 +2085,9 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
vdev_param = ar->wmi.vdev_param->tx_encap_type;
ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
ATH10K_HW_TXRX_NATIVE_WIFI);
- if (ret)
+
+ /* 10.X firmware does not support this VDEV parameter. Do not warn */
+ if (ret && !test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
ath10k_warn("Failed to set TX encap: %d\n", ret);

if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
--
1.7.10

2013-10-14 16:06:08

by Bartosz Markowski

Subject: Re: [PATCH] ath10k: do not warn about unsupported vdev param

On 14 October 2013 17:46, Kalle Valo <[email protected]> wrote:
> Bartosz Markowski <[email protected]> writes:
>
>> 10.X firmware does not support WMI_VDEV_PARAM_TX_ENCAP_TYPE.
>> It's a known limitation and we should not warn about this.
>>
>> Signed-off-by: Bartosz Markowski <[email protected]>
>> ---
>> drivers/net/wireless/ath/ath10k/mac.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
>> index 049eca2..9c800f8 100644
>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>> @@ -2085,7 +2085,9 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
>> vdev_param = ar->wmi.vdev_param->tx_encap_type;
>> ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
>> ATH10K_HW_TXRX_NATIVE_WIFI);
>> - if (ret)
>> +
>> + /* 10.X firmware does not support this VDEV parameter. Do not warn */
>> + if (ret && !test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
>> ath10k_warn("Failed to set TX encap: %d\n", ret);
>
> But why call ath10k_wmi_vdev_set_param() even if we know it won't work?
> IMHO it's cleaner not to call it at all when ATH10K_FW_FEATURE_WMI_10X
> is set.

I tend to agree. Intention was to keep things generic and just hide the warning.

> Or if we want to avoid using ATH10K_FW_FEATURE_WMI_10X we could also
> change ath10k_wmi_pdev_set_param() to return -EOPNOTSUPP and check for
> that.

I like the second idea. I will send a v2 patch.

--
Bartosz

2013-10-14 15:46:22

by Kalle Valo

Subject: Re: [PATCH] ath10k: do not warn about unsupported vdev param

Bartosz Markowski <[email protected]> writes:

> 10.X firmware does not support WMI_VDEV_PARAM_TX_ENCAP_TYPE.
> It's a known limitation and we should not warn about this.
>
> Signed-off-by: Bartosz Markowski <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/mac.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 049eca2..9c800f8 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -2085,7 +2085,9 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
> vdev_param = ar->wmi.vdev_param->tx_encap_type;
> ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
> ATH10K_HW_TXRX_NATIVE_WIFI);
> - if (ret)
> +
> + /* 10.X firmware does not support this VDEV parameter. Do not warn */
> + if (ret && !test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
> ath10k_warn("Failed to set TX encap: %d\n", ret);

But why call ath10k_wmi_vdev_set_param() even if we know it won't work?
IMHO it's cleaner not to call it at all when ATH10K_FW_FEATURE_WMI_10X
is set.

Or if we want to avoid using ATH10K_FW_FEATURE_WMI_10X we could also
change ath10k_wmi_pdev_set_param() to return -EOPNOTSUPP and check for
that.

--
Kalle Valo