Baochen Qiang <quic_bqiang@xxxxxxxxxxx> writes:
> Implement net-detect feature by setting flag
> WIPHY_WOWLAN_NET_DETECT if firmware supports this
> feature. Driver sets the related PNO configuration
> to firmware before entering WoW and firmware then
> scans periodically and wakes up host if a specific
> SSID is found.
>
> Note that firmware crashes if we enable it for both
> P2P vdev and station vdev simultaneously because
> firmware can only support one vdev at a time. Since
> there is rare scenario for a P2P vdev to do net-detect,
> skip it for P2P vdevs.
>
> Tested-on: WCN7850 hw2.0 PCI
> WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Baochen Qiang <quic_bqiang@xxxxxxxxxxx>
[...]
> +struct wmi_wow_nlo_config_cmd {
> + __le32 tlv_header;
> + __le32 flags;
> + __le32 vdev_id;
> + __le32 fast_scan_max_cycles;
> + __le32 active_dwell_time;
> + __le32 passive_dwell_time;
> + __le32 probe_bundle_size;
> +
> + /* ART = IRT */
> + __le32 rest_time;
What's ART and IRT in this context? The comments are supposed to answer
to questions but this is just adding more questions.
> +
> + /* Max value that can be reached after SBM */
> + __le32 max_rest_time;
It's good to avoid acronyms so I changed this to:
/* max value that can be reached after scan_backoff_multiplier */
__le32 max_rest_time;
> + /* SBM */
> + __le32 scan_backoff_multiplier;
> +
> + /* SCBM */
> + __le32 fast_scan_period;
These two comments are not really providing any extra information I
removed them.
> +static int
> +ath12k_wow_pno_check_and_convert(struct ath12k *ar, u32 vdev_id,
> + const struct cfg80211_sched_scan_request *nd_config,
> + struct wmi_pno_scan_req_arg *pno)
> +{
> + int i, j;
> + u8 ssid_len;
> +
> + pno->enable = 1;
> + pno->vdev_id = vdev_id;
> + pno->uc_networks_count = nd_config->n_match_sets;
> +
> + if (!pno->uc_networks_count ||
> + pno->uc_networks_count > WMI_PNO_MAX_SUPP_NETWORKS)
> + return -EINVAL;
> +
> + if (nd_config->n_channels > WMI_PNO_MAX_NETW_CHANNELS_EX)
> + return -EINVAL;
> +
> + /* Filling per profile params */
> + for (i = 0; i < pno->uc_networks_count; i++) {
> + ssid_len = nd_config->match_sets[i].ssid.ssid_len;
> +
> + if (ssid_len == 0 || ssid_len > 32)
> + return -EINVAL;
> +
> + pno->a_networks[i].ssid.ssid_len = ssid_len;
> +
> + memcpy(pno->a_networks[i].ssid.ssid,
> + nd_config->match_sets[i].ssid.ssid,
> + ssid_len);
> + pno->a_networks[i].authentication = 0;
> + pno->a_networks[i].encryption = 0;
> + pno->a_networks[i].bcast_nw_type = 0;
> +
> + /* Copying list of valid channel into request */
> + pno->a_networks[i].channel_count = nd_config->n_channels;
> + pno->a_networks[i].rssi_threshold = nd_config->match_sets[i].rssi_thold;
> +
> + for (j = 0; j < nd_config->n_channels; j++) {
> + pno->a_networks[i].channels[j] =
> + nd_config->channels[j]->center_freq;
> + }
> + }
> +
> + /* set scan to passive if no SSIDs are specified in the request */
> + if (nd_config->n_ssids == 0)
> + pno->do_passive_scan = true;
> + else
> + pno->do_passive_scan = false;
> +
> + for (i = 0; i < nd_config->n_ssids; i++) {
> + j = 0;
> + while (j < pno->uc_networks_count) {
> + if (pno->a_networks[j].ssid.ssid_len ==
> + nd_config->ssids[i].ssid_len &&
> + !memcmp(pno->a_networks[j].ssid.ssid,
> + nd_config->ssids[i].ssid,
> + pno->a_networks[j].ssid.ssid_len)) {
> + pno->a_networks[j].bcast_nw_type = BCAST_HIDDEN;
> + break;
> + }
> + j++;
> + }
> + }
The while loop is just a simple for loop so I changed it to use for statement.
> +
> + if (nd_config->n_scan_plans == 2) {
> + pno->fast_scan_period = nd_config->scan_plans[0].interval * MSEC_PER_SEC;
> + pno->fast_scan_max_cycles = nd_config->scan_plans[0].iterations;
> + pno->slow_scan_period =
> + nd_config->scan_plans[1].interval * MSEC_PER_SEC;
> + } else if (nd_config->n_scan_plans == 1) {
> + pno->fast_scan_period = nd_config->scan_plans[0].interval * MSEC_PER_SEC;
> + pno->fast_scan_max_cycles = 1;
> + pno->slow_scan_period = nd_config->scan_plans[0].interval * MSEC_PER_SEC;
> + } else {
> + ath12k_warn(ar->ab, "Invalid number of scan plans %d !!",
> + nd_config->n_scan_plans);
> + }
I cleaned up the error message here.
> +static int ath12k_wow_vdev_clean_nlo(struct ath12k *ar, u32 vdev_id)
> +{
> + int ret = 0;
> +
> + if (ar->nlo_enabled) {
> + struct wmi_pno_scan_req_arg *pno =
> + kzalloc(sizeof(*pno), GFP_KERNEL);
> + if (!pno)
> + return -ENOMEM;
> +
> + pno->enable = 0;
> + ar->nlo_enabled = false;
> + ret = ath12k_wmi_wow_config_pno(ar, vdev_id, pno);
> + kfree(pno);
> + }
> +
> + return ret;
> +}
Avoid initialising ret variables and minimise the indentation. I changed
this to:
static int ath12k_wow_vdev_clean_nlo(struct ath12k *ar, u32 vdev_id)
{
struct wmi_pno_scan_req_arg *pno;
int ret;
if (!ar->nlo_enabled)
return 0;
pno = kzalloc(sizeof(*pno), GFP_KERNEL);
if (!pno)
return -ENOMEM;
pno->enable = 0;
ret = ath12k_wmi_wow_config_pno(ar, vdev_id, pno);
if (ret) {
ath12k_warn(ar->ab, "failed to disable PNO: %d", ret);
goto out;
}
ar->nlo_enabled = false;
out:
kfree(pno);
return ret;
}
> +static int ath12k_wow_vif_clean_nlo(struct ath12k_vif *arvif)
> +{
> + struct ath12k *ar = arvif->ar;
> + int ret = 0;
> +
> + switch (arvif->vdev_type) {
> + case WMI_VDEV_TYPE_STA:
> + ret = ath12k_wow_vdev_clean_nlo(ar, arvif->vdev_id);
> + break;
> + default:
> + break;
> + }
> + return ret;
> +}
ret variable is not really needed:
static int ath12k_wow_vif_clean_nlo(struct ath12k_vif *arvif)
{
struct ath12k *ar = arvif->ar;
switch (arvif->vdev_type) {
case WMI_VDEV_TYPE_STA:
return ath12k_wow_vdev_clean_nlo(ar, arvif->vdev_id);
default:
return 0;
}
}
In the pending branch I did also some minor cosmetic changes to this and
earlier patches, too many to list here.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
