| Message ID | 20230802085852.19821-2-quic_mdharane@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8b8b990fe495e9be057249e1651b59b5ebacf2ef |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath12k: fix radar detection in secondary 80 MHz | expand |
On 8/2/2023 1:58 AM, Manish Dharanenthiran wrote: > Fix WARN_ON() from ath12k_mac_update_vif_chan() if vdev is not up. > Since change_chanctx can be called even before vdev_up. > > Do vdev stop followed by a vdev start in case of vdev is down. > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0-02903-QCAHKSWPL_SILICONZ-1 Has this been tested on WCN7850? My understanding is that firmware may expect vdev down and then vdev restart > > Signed-off-by: Manish Dharanenthiran <quic_mdharane@quicinc.com> > --- > drivers/net/wireless/ath/ath12k/mac.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c > index 1bb9802ef569..0ab95e138d1d 100644 > --- a/drivers/net/wireless/ath/ath12k/mac.c > +++ b/drivers/net/wireless/ath/ath12k/mac.c > @@ -5698,13 +5698,28 @@ ath12k_mac_update_vif_chan(struct ath12k *ar, > if (WARN_ON(!arvif->is_started)) > continue; > > - if (WARN_ON(!arvif->is_up)) > - continue; > + /* Firmware expect vdev_restart only if vdev is up. > + * If vdev is down then it expect vdev_stop->vdev_start. > + */ > + if (arvif->is_up) { > + ret = ath12k_mac_vdev_restart(arvif, &vifs[i].new_ctx->def); > + if (ret) { > + ath12k_warn(ab, "failed to restart vdev %d: %d\n", > + arvif->vdev_id, ret); > + continue; > + } > + } else { > + ret = ath12k_mac_vdev_stop(arvif); > + if (ret) { > + ath12k_warn(ab, "failed to stop vdev %d: %d\n", > + arvif->vdev_id, ret); > + continue; > + } > > - ret = ath12k_mac_vdev_restart(arvif, &vifs[i].new_ctx->def); > - if (ret) { > - ath12k_warn(ab, "failed to restart vdev %d: %d\n", > - arvif->vdev_id, ret); > + ret = ath12k_mac_vdev_start(arvif, &vifs[i].new_ctx->def); > + if (ret) > + ath12k_warn(ab, "failed to start vdev %d: %d\n", > + arvif->vdev_id, ret); > continue; > } >
On 8/2/2023 8:29 PM, Jeff Johnson wrote: > On 8/2/2023 1:58 AM, Manish Dharanenthiran wrote: >> Fix WARN_ON() from ath12k_mac_update_vif_chan() if vdev is not up. >> Since change_chanctx can be called even before vdev_up. >> >> Do vdev stop followed by a vdev start in case of vdev is down. >> >> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0-02903-QCAHKSWPL_SILICONZ-1 > > Has this been tested on WCN7850? My understanding is that firmware may > expect vdev down and then vdev restart Hi Jeff, No, not tested with WCN7850 chip-set. But, we will be sending vdev_down before starting the restart sequence. Let me get help from MCC team to test this patch and update the changes, if needed. > >> >> Signed-off-by: Manish Dharanenthiran <quic_mdharane@quicinc.com> >> --- >> drivers/net/wireless/ath/ath12k/mac.c | 27 +++++++++++++++++++++------ >> 1 file changed, 21 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath12k/mac.c >> b/drivers/net/wireless/ath/ath12k/mac.c >> index 1bb9802ef569..0ab95e138d1d 100644 >> --- a/drivers/net/wireless/ath/ath12k/mac.c >> +++ b/drivers/net/wireless/ath/ath12k/mac.c >> @@ -5698,13 +5698,28 @@ ath12k_mac_update_vif_chan(struct ath12k *ar, >> if (WARN_ON(!arvif->is_started)) >> continue; >> - if (WARN_ON(!arvif->is_up)) >> - continue; >> + /* Firmware expect vdev_restart only if vdev is up. >> + * If vdev is down then it expect vdev_stop->vdev_start. >> + */ >> + if (arvif->is_up) { >> + ret = ath12k_mac_vdev_restart(arvif, &vifs[i].new_ctx->def); >> + if (ret) { >> + ath12k_warn(ab, "failed to restart vdev %d: %d\n", >> + arvif->vdev_id, ret); >> + continue; >> + } >> + } else { >> + ret = ath12k_mac_vdev_stop(arvif); >> + if (ret) { >> + ath12k_warn(ab, "failed to stop vdev %d: %d\n", >> + arvif->vdev_id, ret); >> + continue; >> + } >> - ret = ath12k_mac_vdev_restart(arvif, &vifs[i].new_ctx->def); >> - if (ret) { >> - ath12k_warn(ab, "failed to restart vdev %d: %d\n", >> - arvif->vdev_id, ret); >> + ret = ath12k_mac_vdev_start(arvif, &vifs[i].new_ctx->def); >> + if (ret) >> + ath12k_warn(ab, "failed to start vdev %d: %d\n", >> + arvif->vdev_id, ret); >> continue; >> } >
On 8/2/2023 10:59 PM, Jeff Johnson wrote: > On 8/2/2023 1:58 AM, Manish Dharanenthiran wrote: >> Fix WARN_ON() from ath12k_mac_update_vif_chan() if vdev is not up. >> Since change_chanctx can be called even before vdev_up. >> >> Do vdev stop followed by a vdev start in case of vdev is down. >> >> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0-02903-QCAHKSWPL_SILICONZ-1 > > Has this been tested on WCN7850? My understanding is that firmware may > expect vdev down and then vdev restart > For WCN7850, firmware do not expect vdev down for station vdev type here. And this patch only take effect when arvif->is_up is not set, it is hard to happen that for MCC station mode. Because this function is entered for channel switch for MCC station mode, it means station is connected to the AP, then arvif->is_up is set, then this patch not take effect. [...] >
On 8/8/2023 3:56 PM, Wen Gong wrote: > On 8/2/2023 10:59 PM, Jeff Johnson wrote: >> On 8/2/2023 1:58 AM, Manish Dharanenthiran wrote: >>> Fix WARN_ON() from ath12k_mac_update_vif_chan() if vdev is not up. >>> Since change_chanctx can be called even before vdev_up. >>> >>> Do vdev stop followed by a vdev start in case of vdev is down. >>> >>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0-02903-QCAHKSWPL_SILICONZ-1 >> >> Has this been tested on WCN7850? My understanding is that firmware may >> expect vdev down and then vdev restart >> > For WCN7850, firmware do not expect vdev down for station vdev type here. > > And this patch only take effect when arvif->is_up is not set, > > it is hard to happen that for MCC station mode. > > Because this function is entered for channel switch for MCC station mode, > > it means station is connected to the AP, then arvif->is_up is set, then > this patch not take effect. > > [...] > >> Thanks Wen for checking this change! Hi Jeff, Kindly review the changes and provide your comments. Regards Manish Dharanenthiran
On 8/2/2023 1:58 AM, Manish Dharanenthiran wrote: > Fix WARN_ON() from ath12k_mac_update_vif_chan() if vdev is not up. > Since change_chanctx can be called even before vdev_up. > > Do vdev stop followed by a vdev start in case of vdev is down. > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0-02903-QCAHKSWPL_SILICONZ-1 > > Signed-off-by: Manish Dharanenthiran <quic_mdharane@quicinc.com> Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com>
Manish Dharanenthiran <quic_mdharane@quicinc.com> wrote: > Fix WARN_ON() from ath12k_mac_update_vif_chan() if vdev is not up. > Since change_chanctx can be called even before vdev_up. > > Do vdev stop followed by a vdev start in case of vdev is down. > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0-02903-QCAHKSWPL_SILICONZ-1 > > Signed-off-by: Manish Dharanenthiran <quic_mdharane@quicinc.com> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> 2 patches applied to ath-next branch of ath.git, thanks. 8b8b990fe495 wifi: ath12k: fix WARN_ON during ath12k_mac_update_vif_chan 3f53624f74f4 wifi: ath12k: fix radar detection in 160 MHz
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c index 1bb9802ef569..0ab95e138d1d 100644 --- a/drivers/net/wireless/ath/ath12k/mac.c +++ b/drivers/net/wireless/ath/ath12k/mac.c @@ -5698,13 +5698,28 @@ ath12k_mac_update_vif_chan(struct ath12k *ar, if (WARN_ON(!arvif->is_started)) continue; - if (WARN_ON(!arvif->is_up)) - continue; + /* Firmware expect vdev_restart only if vdev is up. + * If vdev is down then it expect vdev_stop->vdev_start. + */ + if (arvif->is_up) { + ret = ath12k_mac_vdev_restart(arvif, &vifs[i].new_ctx->def); + if (ret) { + ath12k_warn(ab, "failed to restart vdev %d: %d\n", + arvif->vdev_id, ret); + continue; + } + } else { + ret = ath12k_mac_vdev_stop(arvif); + if (ret) { + ath12k_warn(ab, "failed to stop vdev %d: %d\n", + arvif->vdev_id, ret); + continue; + } - ret = ath12k_mac_vdev_restart(arvif, &vifs[i].new_ctx->def); - if (ret) { - ath12k_warn(ab, "failed to restart vdev %d: %d\n", - arvif->vdev_id, ret); + ret = ath12k_mac_vdev_start(arvif, &vifs[i].new_ctx->def); + if (ret) + ath12k_warn(ab, "failed to start vdev %d: %d\n", + arvif->vdev_id, ret); continue; }
Fix WARN_ON() from ath12k_mac_update_vif_chan() if vdev is not up. Since change_chanctx can be called even before vdev_up. Do vdev stop followed by a vdev start in case of vdev is down. Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0-02903-QCAHKSWPL_SILICONZ-1 Signed-off-by: Manish Dharanenthiran <quic_mdharane@quicinc.com> --- drivers/net/wireless/ath/ath12k/mac.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)