| Message ID | 20241023133004.2253830-2-kvalo@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Jeff Johnson |
| Headers | show |
| Series | wifi: ath12k: MLO support part 2 | expand |
On 10/23/2024 6:29 AM, Kalle Valo wrote: > From: Kalle Valo <quic_kvalo@quicinc.com> > > In commit 477cabfdb776 ("wifi: ath12k: modify link arvif creation and removal > for MLO") I had accidentally left one personal TODO comment about using goto > instead of ret. Switch to use goto to be consistent with the error handling in > the function. > > Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3 > > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> > --- > drivers/net/wireless/ath/ath12k/mac.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c > index f5f96a8b1d61..f45f32f3b5f6 100644 > --- a/drivers/net/wireless/ath/ath12k/mac.c > +++ b/drivers/net/wireless/ath/ath12k/mac.c > @@ -7047,8 +7047,7 @@ int ath12k_mac_vdev_create(struct ath12k *ar, struct ath12k_link_vif *arvif) > ret = ath12k_wait_for_peer_delete_done(ar, arvif->vdev_id, > arvif->bssid); > if (ret) > - /* KVALO: why not goto err? */ > - return ret; > + goto err; why does this goto err instead of err_vdev_del? > > ar->num_peers--; > } looking at the context for this patch I have a question about a different part of this function: param_id = WMI_VDEV_PARAM_RTS_THRESHOLD; param_value = hw->wiphy->rts_threshold; ret = ath12k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id, param_id, param_value); if (ret) { ath12k_warn(ar->ab, "failed to set rts threshold for vdev %d: %d\n", arvif->vdev_id, ret); NOTE: no return or goto } ath12k_dp_vdev_tx_attach(ar, arvif); if (vif->type != NL80211_IFTYPE_MONITOR && ar->monitor_conf_enabled) ath12k_mac_monitor_vdev_create(ar); return ret; NOTE: this can return an error if the RTS threshold set fails, but fails without cleaning up (dp vdev still attached and monitor vdev created) Seems either we need error handling if the set param fails, or we should ret 0 at this point
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c index f5f96a8b1d61..f45f32f3b5f6 100644 --- a/drivers/net/wireless/ath/ath12k/mac.c +++ b/drivers/net/wireless/ath/ath12k/mac.c @@ -7047,8 +7047,7 @@ int ath12k_mac_vdev_create(struct ath12k *ar, struct ath12k_link_vif *arvif) ret = ath12k_wait_for_peer_delete_done(ar, arvif->vdev_id, arvif->bssid); if (ret) - /* KVALO: why not goto err? */ - return ret; + goto err; ar->num_peers--; }