Jouni Malinen <jouni@xxxxxxxxxxxxxx> writes:
> From: Seevalamuthu Mariappan <seevalam@xxxxxxxxxxxxxx>
>
> Add separate APIs for monitor_vdev_create/monitor_vdev_delete
> and monitor_vdev_start/monitor_vdev_stop.
>
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.4.0.1-01725-QCAHKSWPL_SILICONZ-1
>
> Co-developed-by: Miles Hu <milehu@xxxxxxxxxxxxxx>
> Signed-off-by: Miles Hu <milehu@xxxxxxxxxxxxxx>
> Co-developed-by: Vasanthakumar Thiagarajan <vthiagar@xxxxxxxxxxxxxx>
> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@xxxxxxxxxxxxxx>
> Signed-off-by: Seevalamuthu Mariappan <seevalam@xxxxxxxxxxxxxx>
> Signed-off-by: Jouni Malinen <jouni@xxxxxxxxxxxxxx>
> ---
> drivers/net/wireless/ath/ath11k/core.h | 5 +-
> drivers/net/wireless/ath/ath11k/mac.c | 313 ++++++++++++++++++++++++-
> 2 files changed, 312 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
> index 6a6cabdd3e30..3cddab695031 100644
> --- a/drivers/net/wireless/ath/ath11k/core.h
> +++ b/drivers/net/wireless/ath/ath11k/core.h
> @@ -488,7 +488,8 @@ struct ath11k {
> u32 chan_tx_pwr;
> u32 num_stations;
> u32 max_num_stations;
> - bool monitor_present;
> + bool monitor_conf_enabled;
> + bool monitor_started;
> /* To synchronize concurrent synchronous mac80211 callback operations,
> * concurrent debugfs configuration and concurrent FW statistics events.
> */
> @@ -563,6 +564,7 @@ struct ath11k {
> struct ath11k_per_peer_tx_stats cached_stats;
> u32 last_ppdu_id;
> u32 cached_ppdu_id;
> + int monitor_vdev_id;
> #ifdef CONFIG_ATH11K_DEBUGFS
> struct ath11k_debug debug;
> #endif
> @@ -571,6 +573,7 @@ struct ath11k {
> #endif
> bool dfs_block_radar_events;
> struct ath11k_thermal thermal;
> + bool monitor_vdev_created;
> };
>
> struct ath11k_band_cap {
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
> index 3fd9a79801cb..e446817ac8b0 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -745,14 +745,314 @@ static inline int ath11k_mac_vdev_setup_sync(struct ath11k *ar)
> return ar->last_wmi_vdev_start_status ? -EINVAL : 0;
> }
>
> -static int ath11k_mac_op_config(struct ieee80211_hw *hw, u32 changed)
> +static void
> +ath11k_mac_get_any_chandef_iter(struct ieee80211_hw *hw,
> + struct ieee80211_chanctx_conf *conf,
> + void *data)
> {
> - /* mac80211 requires this op to be present and that's why
> - * there's an empty function, this can be extended when
> - * required.
> - */
> + struct cfg80211_chan_def **def = data;
> +
> + *def = &conf->def;
> +}
> +
> +static int ath11k_mac_monitor_vdev_start(struct ath11k *ar, int vdev_id,
> + struct cfg80211_chan_def *chandef)
> +{
> + struct ieee80211_channel *channel;
> + struct wmi_vdev_start_req_arg arg = {};
> + int ret;
> +
> + lockdep_assert_held(&ar->conf_mutex);
> +
> + channel = chandef->chan;
> +
> + arg.vdev_id = vdev_id;
> + arg.channel.freq = channel->center_freq;
> + arg.channel.band_center_freq1 = chandef->center_freq1;
> + arg.channel.band_center_freq2 = chandef->center_freq2;
> +
> + arg.channel.mode = ath11k_phymodes[chandef->chan->band][chandef->width];
> + arg.channel.chan_radar =
> + !!(channel->flags & IEEE80211_CHAN_RADAR);
> +
> + arg.channel.min_power = 0;
> + arg.channel.max_power = channel->max_power * 2;
> + arg.channel.max_reg_power = channel->max_reg_power * 2;
> + arg.channel.max_antenna_gain = channel->max_antenna_gain * 2;
> +
> + arg.pref_tx_streams = ar->num_tx_chains;
> + arg.pref_rx_streams = ar->num_rx_chains;
> +
> + arg.channel.passive = !!(chandef->chan->flags & IEEE80211_CHAN_NO_IR);
> +
> + reinit_completion(&ar->vdev_setup_done);
> + reinit_completion(&ar->vdev_delete_done);
>
> + ret = ath11k_wmi_vdev_start(ar, &arg, false);
> + if (ret) {
> + ath11k_warn(ar->ab, "failed to request monitor vdev %i start: %d\n",
> + vdev_id, ret);
> + return ret;
> + }
> + ret = ath11k_mac_vdev_setup_sync(ar);
> + if (ret) {
> + ath11k_warn(ar->ab, "failed to synchronize setup for monitor vdev %i start: %d\n",
> + vdev_id, ret);
> + return ret;
> + }
> + ret = ath11k_wmi_vdev_up(ar, vdev_id, 0, ar->mac_addr);
> + if (ret) {
> + ath11k_warn(ar->ab, "failed to put up monitor vdev %i: %d\n",
> + vdev_id, ret);
> + goto vdev_stop;
> + }
> + ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "mac monitor vdev %i started\n",
> + vdev_id);
> return 0;
> +
> +vdev_stop:
> + reinit_completion(&ar->vdev_setup_done);
> +
> + ret = ath11k_wmi_vdev_stop(ar, vdev_id);
> + if (ret) {
> + ath11k_warn(ar->ab, "failed to stop monitor vdev %i after start failure: %d\n",
> + vdev_id, ret);
> + return ret;
> + }
> +
> + ret = ath11k_mac_vdev_setup_sync(ar);
> + if (ret)
> + ath11k_warn(ar->ab, "failed to synchronize setup for vdev %i stop: %d\n",
> + vdev_id, ret);
I added return ret here for consistency..
> + return ret;
I don't thinks this is right, in an error path (vdev_stop label) we
return 0? I changed this to -EIO.
> +static int ath11k_mac_monitor_vdev_stop(struct ath11k *ar)
> +{
> + int ret;
> +
> + lockdep_assert_held(&ar->conf_mutex);
> +
> + reinit_completion(&ar->vdev_setup_done);
> +
> + ret = ath11k_wmi_vdev_stop(ar, ar->monitor_vdev_id);
> + if (ret)
> + ath11k_warn(ar->ab, "failed to request monitor vdev %i stop: %d\n",
> + ar->monitor_vdev_id, ret);
> +
> + ret = ath11k_mac_vdev_setup_sync(ar);
> + if (ret)
> + ath11k_warn(ar->ab, "failed to synchronize monitor vdev %i stop: %d\n",
> + ar->monitor_vdev_id, ret);
> +
> + ret = ath11k_wmi_vdev_down(ar, ar->monitor_vdev_id);
> + if (ret)
> + ath11k_warn(ar->ab, "failed to put down monitor vdev %i: %d\n",
> + ar->monitor_vdev_id, ret);
> +
> + ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "mac monitor vdev %i stopped\n",
> + ar->monitor_vdev_id);
> + return ret;
> +}
I was not sure what's the idea of error path handling here, we print
warnings but still continue the normal execution. I changed this so that
we bail out in the first error and if everything goes well we return 0.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
