| Message ID | 1381749381-15459-1-git-send-email-bartosz.markowski@tieto.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Bartosz Markowski <bartosz.markowski@tieto.com> 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 <bartosz.markowski@tieto.com> > --- > 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.
On 14 October 2013 17:46, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Bartosz Markowski <bartosz.markowski@tieto.com> 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 <bartosz.markowski@tieto.com> >> --- >> 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.
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) {
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 <bartosz.markowski@tieto.com> --- drivers/net/wireless/ath/ath10k/mac.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)