Ping-Ke Shih <pkshih@xxxxxxxxxxx> writes:
>> -----Original Message-----
>> From: Kalle Valo <kvalo@xxxxxxxxxx>
>> Sent: Saturday, August 13, 2022 12:10 AM
>> To: linux-wireless@xxxxxxxxxxxxxxx
>> Cc: ath12k@xxxxxxxxxxxxxxxxxxx
>> Subject: [PATCH 46/50] wifi: ath12k: add wmi.c
>>
>> From: Kalle Valo <quic_kvalo@xxxxxxxxxxx>
>>
>> (Patches split into one patch per file for easier review, but the final
>> commit will be one big patch. See the cover letter for more info.)
>>
>> Signed-off-by: Kalle Valo <quic_kvalo@xxxxxxxxxxx>
>> ---
>> drivers/net/wireless/ath/ath12k/wmi.c | 6558 +++++++++++++++++++++++++++++++++
>> 1 file changed, 6558 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c
>> b/drivers/net/wireless/ath/ath12k/wmi.c
>> new file mode 100644
>> index 000000000000..82aaedb5d0af
>> --- /dev/null
>> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
>> @@ -0,0 +1,6558 @@
>
> [...]
>
>> +
>> +static int ath12k_pull_svc_ready_ext(struct ath12k_wmi_pdev *wmi_handle,
>> + const void *ptr,
>> + struct ath12k_wmi_service_ext_arg *arg)
>> +{
>> + const struct wmi_service_ready_ext_event *ev = ptr;
>> +
>> + if (!ev)
>> + return -EINVAL;
>> +
>> + /* Move this to host based bitmap */
>> + arg->default_conc_scan_config_bits =
>> + le32_to_cpu(ev->default_conc_scan_config_bits);
>> + arg->default_fw_config_bits = le32_to_cpu(ev->default_fw_config_bits);
>> + arg->he_cap_info = le32_to_cpu(ev->he_cap_info);
>> + arg->mpdu_density = le32_to_cpu(ev->mpdu_density);
>> + arg->max_bssid_rx_filters = le32_to_cpu(ev->max_bssid_rx_filters);
>> + memcpy(&arg->ppet, &ev->ppet, sizeof(arg->ppet));
>
> Originally, I would like to say the memcpy() statement can be replaced by
> arg->ppet = ev->ppet;
>
> But I found the struct(s) are different:
>
> arg->ppet is
>
> struct ath12k_wmi_ppe_threshold_arg {
> u32 numss_m1;
> u32 ru_bit_mask;
> u32 ppet16_ppet8_ru3_ru0[PSOC_HOST_MAX_NUM_SS];
> };
>
> ev->ppet is
>
> struct ath12k_wmi_ppe_threshold_params {
> __le32 numss_m1; /** NSS - 1*/
> union {
> __le32 ru_count;
> __le32 ru_mask;
> } __packed;
> __le32 ppet16_ppet8_ru3_ru0[WMI_MAX_NUM_SS];
> } __packed;
>
> Then, using memcpy() doesn't work on big-endian machine.
This is now fixed like this:
arg->ppet.numss_m1 = le32_to_cpu(ev->ppet.numss_m1);
arg->ppet.ru_bit_mask = le32_to_cpu(ev->ppet.ru_info);
for (i = 0; i < PSOC_HOST_MAX_NUM_SS; i++)
arg->ppet.ppet16_ppet8_ru3_ru0[i] =
le32_to_cpu(ev->ppet.ppet16_ppet8_ru3_ru0[i]);
But now the code assumes that both PSOC_HOST_MAX_NUM_SS and
WMI_MAX_NUM_SS are 8, which is not a safe assumption without a proper
check. I think the best would be to remove PSOC_HOST_MAX_NUM_SS and use
MAX_HE_NSS everywhere. I added that to the todo list.
>> +static int
>> +ath12k_pull_mac_phy_cap_svc_ready_ext(struct ath12k_wmi_pdev *wmi_handle,
>> + struct ath12k_wmi_svc_rdy_ext_parse *svc,
>> + u8 hw_mode_id, u8 phy_id,
>> + struct ath12k_pdev *pdev)
>> +{
>> + const struct ath12k_wmi_mac_phy_caps_params *mac_phy_caps;
>> + const struct ath12k_wmi_soc_mac_phy_hw_mode_caps_params *hw_caps =
>> svc->hw_caps;
>> + const struct ath12k_wmi_hw_mode_cap_params *wmi_hw_mode_caps =
>> svc->hw_mode_caps;
>> + const struct ath12k_wmi_mac_phy_caps_params *wmi_mac_phy_caps =
>> svc->mac_phy_caps;
>> + struct ath12k_band_cap *cap_band;
>> + struct ath12k_pdev_cap *pdev_cap = &pdev->cap;
>> + u32 phy_map;
>> + u32 hw_idx, phy_idx = 0;
>> +
>> + if (!hw_caps || !wmi_hw_mode_caps || !svc->soc_hal_reg_caps)
>> + return -EINVAL;
>> +
>> + for (hw_idx = 0; hw_idx < le32_to_cpu(hw_caps->num_hw_modes); hw_idx++) {
>> + if (hw_mode_id == le32_to_cpu(wmi_hw_mode_caps[hw_idx].hw_mode_id))
>> + break;
>> +
>> + phy_map = le32_to_cpu(wmi_hw_mode_caps[hw_idx].phy_id_map);
>> + while (phy_map) {
>> + phy_map >>= 1;
>> + phy_idx++;
>> + }
>
> phy_idx = fls(phy_map)
Fixed.
>> + if (le32_to_cpu(mac_phy_caps->supported_bands) & WMI_HOST_WLAN_2G_CAP) {
>> + cap_band = &pdev_cap->band[NL80211_BAND_2GHZ];
>> + cap_band->phy_id = le32_to_cpu(mac_phy_caps->phy_id);
>> + cap_band->max_bw_supported =
>> + le32_to_cpu(mac_phy_caps->max_bw_supported_2g);
>> + cap_band->ht_cap_info = le32_to_cpu(mac_phy_caps->ht_cap_info_2g);
>> + cap_band->he_cap_info[0] = le32_to_cpu(mac_phy_caps->he_cap_info_2g);
>> + cap_band->he_cap_info[1] =
>> le32_to_cpu(mac_phy_caps->he_cap_info_2g_ext);
>> + cap_band->he_mcs = le32_to_cpu(mac_phy_caps->he_supp_mcs_2g);
>> + memcpy(cap_band->he_cap_phy_info, &mac_phy_caps->he_cap_phy_info_2g,
>> + sizeof(u32) * PSOC_HOST_MAX_PHY_SIZE);
>> + memcpy(&cap_band->he_ppet, &mac_phy_caps->he_ppet2g,
>> + sizeof(struct ath12k_wmi_ppe_threshold_arg));
>
> These two copy __le32 to u32.
These are now replaced with a loop using le32_to_cpu().
>> + if (le32_to_cpu(mac_phy_caps->supported_bands) & WMI_HOST_WLAN_5G_CAP) {
>> + cap_band = &pdev_cap->band[NL80211_BAND_5GHZ];
>> + cap_band->phy_id = le32_to_cpu(mac_phy_caps->phy_id);
>> + cap_band->max_bw_supported =
>> + le32_to_cpu(mac_phy_caps->max_bw_supported_5g);
>> + cap_band->ht_cap_info = le32_to_cpu(mac_phy_caps->ht_cap_info_5g);
>> + cap_band->he_cap_info[0] = le32_to_cpu(mac_phy_caps->he_cap_info_5g);
>> + cap_band->he_cap_info[1] =
>> le32_to_cpu(mac_phy_caps->he_cap_info_5g_ext);
>> + cap_band->he_mcs = le32_to_cpu(mac_phy_caps->he_supp_mcs_5g);
>> + memcpy(cap_band->he_cap_phy_info, &mac_phy_caps->he_cap_phy_info_5g,
>> + sizeof(u32) * PSOC_HOST_MAX_PHY_SIZE);
>> + memcpy(&cap_band->he_ppet, &mac_phy_caps->he_ppet5g,
>> + sizeof(struct ath12k_wmi_ppe_threshold_arg));
>
> copy __le32 to u32
This is also fixed.
>> +
>> + cap_band = &pdev_cap->band[NL80211_BAND_6GHZ];
>> + cap_band->max_bw_supported =
>> + le32_to_cpu(mac_phy_caps->max_bw_supported_5g);
>> + cap_band->ht_cap_info = le32_to_cpu(mac_phy_caps->ht_cap_info_5g);
>> + cap_band->he_cap_info[0] = le32_to_cpu(mac_phy_caps->he_cap_info_5g);
>> + cap_band->he_cap_info[1] =
>> le32_to_cpu(mac_phy_caps->he_cap_info_5g_ext);
>> + cap_band->he_mcs = le32_to_cpu(mac_phy_caps->he_supp_mcs_5g);
>> + memcpy(cap_band->he_cap_phy_info, &mac_phy_caps->he_cap_phy_info_5g,
>> + sizeof(u32) * PSOC_HOST_MAX_PHY_SIZE);
>> + memcpy(&cap_band->he_ppet, &mac_phy_caps->he_ppet5g,
>> + sizeof(struct ath12k_wmi_ppe_threshold_arg));
>
> and, these two.
And this is fixed as well.
> It seems like we don't have a simple way to convert struct with __le32 to
> another struct with cpu order. The only way is to convert fields of struct
> one by one.
Yeah, at least I can't think of any other solution.
>> +int ath12k_wmi_mgmt_send(struct ath12k *ar, u32 vdev_id, u32 buf_id,
>> + struct sk_buff *frame)
>> +{
>> + struct ath12k_wmi_pdev *wmi = ar->wmi;
>> + struct wmi_mgmt_send_cmd *cmd;
>> + struct wmi_tlv *frame_tlv;
>> + struct sk_buff *skb;
>> + u32 buf_len;
>> + int ret, len;
>> +
>> + buf_len = frame->len < WMI_MGMT_SEND_DOWNLD_LEN ?
>> + frame->len : WMI_MGMT_SEND_DOWNLD_LEN;
>
> buf_len = min(frame->len, WMI_MGMT_SEND_DOWNLD_LEN);
Fixed.
>> +int ath12k_wmi_vdev_install_key(struct ath12k *ar,
>> + struct wmi_vdev_install_key_arg *arg)
>> +{
>> + struct ath12k_wmi_pdev *wmi = ar->wmi;
>> + struct wmi_vdev_install_key_cmd *cmd;
>> + struct wmi_tlv *tlv;
>> + struct sk_buff *skb;
>> + int ret, len;
>> + int key_len_aligned = roundup(arg->key_len, sizeof(uint32_t));
>
> Not sure why 'uint32_t'? Maybe, just '4' like other wmi functions.
Yeah, we changed that to '4'.
> mac80211 defines
> struct ieee80211_key_conf {
> u8 key[];
> };
>
> And, this driver defines
> struct wmi_vdev_install_key_arg {
> const void *key_data;
> };
>
>> +
>> + len = sizeof(*cmd) + TLV_HDR_SIZE + key_len_aligned;
>> +
>> + skb = ath12k_wmi_alloc_skb(wmi->wmi_ab, len);
>> + if (!skb)
>> + return -ENOMEM;
>> +
>> + cmd = (struct wmi_vdev_install_key_cmd *)skb->data;
>> + cmd->tlv_header = ath12k_wmi_tlv_cmd_hdr(WMI_TAG_VDEV_INSTALL_KEY_CMD,
>> + sizeof(*cmd));
>> + cmd->vdev_id = cpu_to_le32(arg->vdev_id);
>> + ether_addr_copy(cmd->peer_macaddr.addr, arg->macaddr);
>> + cmd->key_idx = cpu_to_le32(arg->key_idx);
>> + cmd->key_flags = cpu_to_le32(arg->key_flags);
>> + cmd->key_cipher = cpu_to_le32(arg->key_cipher);
>> + cmd->key_len = cpu_to_le32(arg->key_len);
>> + cmd->key_txmic_len = cpu_to_le32(arg->key_txmic_len);
>> + cmd->key_rxmic_len = cpu_to_le32(arg->key_rxmic_len);
>> +
>> + if (arg->key_rsc_counter)
>> + cmd->key_rsc_counter = cpu_to_le64(arg->key_rsc_counter);
>> +
>> + tlv = (struct wmi_tlv *)(skb->data + sizeof(*cmd));
>> + tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_BYTE, key_len_aligned);
>> + memcpy(tlv->value, arg->key_data, key_len_aligned);
>
> If arg->key_len wasn't align of 4, this could access out of range of arg->key_data.
Yeah, this doesn't look right. Added this to the todo list.
>> +int ath12k_wmi_send_peer_assoc_cmd(struct ath12k *ar,
>> + struct ath12k_wmi_peer_assoc_arg *arg)
>> +{
>> + struct ath12k_wmi_pdev *wmi = ar->wmi;
>> + struct wmi_peer_assoc_complete_cmd *cmd;
>> + struct ath12k_wmi_vht_rate_set_params *mcs;
>> + struct ath12k_wmi_he_rate_set_params *he_mcs;
>> + struct sk_buff *skb;
>> + struct wmi_tlv *tlv;
>> + void *ptr;
>> + u32 peer_legacy_rates_align;
>> + u32 peer_ht_rates_align;
>> + int i, ret, len;
>> +
>> + peer_legacy_rates_align = roundup(arg->peer_legacy_rates.num_rates,
>> + sizeof(u32));
>> + peer_ht_rates_align = roundup(arg->peer_ht_rates.num_rates,
>> + sizeof(u32));
>> +
>> + len = sizeof(*cmd) +
>> + TLV_HDR_SIZE + (peer_legacy_rates_align * sizeof(u8)) +
>> + TLV_HDR_SIZE + (peer_ht_rates_align * sizeof(u8)) +
>> + sizeof(*mcs) + TLV_HDR_SIZE +
>> + (sizeof(*he_mcs) * arg->peer_he_mcs_count);
>> +
>> + skb = ath12k_wmi_alloc_skb(wmi->wmi_ab, len);
>> + if (!skb)
>> + return -ENOMEM;
>> +
>> + ptr = skb->data;
>> +
>> + cmd = ptr;
>> + cmd->tlv_header = ath12k_wmi_tlv_cmd_hdr(WMI_TAG_PEER_ASSOC_COMPLETE_CMD,
>> + sizeof(*cmd));
>> +
>> + cmd->vdev_id = cpu_to_le32(arg->vdev_id);
>> +
>> + cmd->peer_new_assoc = cpu_to_le32(arg->peer_new_assoc);
>> + cmd->peer_associd = cpu_to_le32(arg->peer_associd);
>> +
>> + ath12k_wmi_copy_peer_flags(cmd, arg,
>> + test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED,
>> + &ar->ab->dev_flags));
>> +
>> + ether_addr_copy(cmd->peer_macaddr.addr, arg->peer_mac);
>> +
>> + cmd->peer_rate_caps = cpu_to_le32(arg->peer_rate_caps);
>> + cmd->peer_caps = cpu_to_le32(arg->peer_caps);
>> + cmd->peer_listen_intval = cpu_to_le32(arg->peer_listen_intval);
>> + cmd->peer_ht_caps = cpu_to_le32(arg->peer_ht_caps);
>> + cmd->peer_max_mpdu = cpu_to_le32(arg->peer_max_mpdu);
>> + cmd->peer_mpdu_density = cpu_to_le32(arg->peer_mpdu_density);
>> + cmd->peer_vht_caps = cpu_to_le32(arg->peer_vht_caps);
>> + cmd->peer_phymode = cpu_to_le32(arg->peer_phymode);
>> +
>> + /* Update 11ax capabilities */
>> + cmd->peer_he_cap_info = cpu_to_le32(arg->peer_he_cap_macinfo[0]);
>> + cmd->peer_he_cap_info_ext = cpu_to_le32(arg->peer_he_cap_macinfo[1]);
>> + cmd->peer_he_cap_info_internal =
>> cpu_to_le32(arg->peer_he_cap_macinfo_internal);
>> + cmd->peer_he_caps_6ghz = cpu_to_le32(arg->peer_he_caps_6ghz);
>> + cmd->peer_he_ops = cpu_to_le32(arg->peer_he_ops);
>> + memcpy(&cmd->peer_he_cap_phy, &arg->peer_he_cap_phyinfo,
>> + sizeof(arg->peer_he_cap_phyinfo));
>> + memcpy(&cmd->peer_ppet, &arg->peer_ppet,
>> + sizeof(arg->peer_ppet));
>
> copy u32 to __le32
Fixed.
>> +int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>> + struct ath12k_wmi_scan_req_arg *arg)
>> +{
>> + struct ath12k_wmi_pdev *wmi = ar->wmi;
>> + struct wmi_start_scan_cmd *cmd;
>> + struct ath12k_wmi_ssid_params *ssid = NULL;
>> + struct ath12k_wmi_mac_addr_params *bssid;
>> + struct sk_buff *skb;
>> + struct wmi_tlv *tlv;
>> + void *ptr;
>> + int i, ret, len;
>> + u32 *tmp_ptr;
>> + u8 extraie_len_with_pad = 0;
>> + struct ath12k_wmi_hint_short_ssid_arg *s_ssid = NULL;
>> + struct ath12k_wmi_hint_bssid_arg *hint_bssid = NULL;
>> +
>> + len = sizeof(*cmd);
>> +
>> + len += TLV_HDR_SIZE;
>> + if (arg->num_chan)
>> + len += arg->num_chan * sizeof(u32);
>> +
>> + len += TLV_HDR_SIZE;
>> + if (arg->num_ssids)
>> + len += arg->num_ssids * sizeof(*ssid);
>> +
>> + len += TLV_HDR_SIZE;
>> + if (arg->num_bssid)
>> + len += sizeof(*bssid) * arg->num_bssid;
>> +
>> + len += TLV_HDR_SIZE;
>> + if (arg->extraie.len)
>> + extraie_len_with_pad =
>> + roundup(arg->extraie.len, sizeof(u32));
>> + len += extraie_len_with_pad;
>> +
>> + if (arg->num_hint_bssid)
>> + len += TLV_HDR_SIZE +
>> + arg->num_hint_bssid * sizeof(*hint_bssid);
>> +
>> + if (arg->num_hint_s_ssid)
>> + len += TLV_HDR_SIZE +
>> + arg->num_hint_s_ssid * sizeof(*s_ssid);
>> +
>> + skb = ath12k_wmi_alloc_skb(wmi->wmi_ab, len);
>> + if (!skb)
>> + return -ENOMEM;
>> +
>> + ptr = skb->data;
>> +
>> + cmd = ptr;
>> + cmd->tlv_header = ath12k_wmi_tlv_cmd_hdr(WMI_TAG_START_SCAN_CMD,
>> + sizeof(*cmd));
>> +
>> + cmd->scan_id = cpu_to_le32(arg->scan_id);
>> + cmd->scan_req_id = cpu_to_le32(arg->scan_req_id);
>> + cmd->vdev_id = cpu_to_le32(arg->vdev_id);
>> + cmd->scan_priority = cpu_to_le32(arg->scan_priority);
>> + cmd->notify_scan_events = cpu_to_le32(arg->notify_scan_events);
>> +
>> + ath12k_wmi_copy_scan_event_cntrl_flags(cmd, arg);
>> +
>> + cmd->dwell_time_active = cpu_to_le32(arg->dwell_time_active);
>> + cmd->dwell_time_active_2g = cpu_to_le32(arg->dwell_time_active_2g);
>> + cmd->dwell_time_passive = cpu_to_le32(arg->dwell_time_passive);
>> + cmd->dwell_time_active_6g = cpu_to_le32(arg->dwell_time_active_6g);
>> + cmd->dwell_time_passive_6g = cpu_to_le32(arg->dwell_time_passive_6g);
>> + cmd->min_rest_time = cpu_to_le32(arg->min_rest_time);
>> + cmd->max_rest_time = cpu_to_le32(arg->max_rest_time);
>> + cmd->repeat_probe_time = cpu_to_le32(arg->repeat_probe_time);
>> + cmd->probe_spacing_time = cpu_to_le32(arg->probe_spacing_time);
>> + cmd->idle_time = cpu_to_le32(arg->idle_time);
>> + cmd->max_scan_time = cpu_to_le32(arg->max_scan_time);
>> + cmd->probe_delay = cpu_to_le32(arg->probe_delay);
>> + cmd->burst_duration = cpu_to_le32(arg->burst_duration);
>> + cmd->num_chan = cpu_to_le32(arg->num_chan);
>> + cmd->num_bssid = cpu_to_le32(arg->num_bssid);
>> + cmd->num_ssids = cpu_to_le32(arg->num_ssids);
>> + cmd->ie_len = cpu_to_le32(arg->extraie.len);
>> + cmd->n_probes = cpu_to_le32(arg->n_probes);
>> +
>> + ptr += sizeof(*cmd);
>> +
>> + len = arg->num_chan * sizeof(u32);
>> +
>> + tlv = ptr;
>> + tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_UINT32, len);
>> + ptr += TLV_HDR_SIZE;
>> + tmp_ptr = (u32 *)ptr;
>> +
>> + for (i = 0; i < arg->num_chan; ++i)
>> + tmp_ptr[i] = arg->chan_list[i];
>
> nit.
> memcpy(tmp_ptr, arg->chan_list, arg->num_chan);
Heh, indeed. But we added '* 4':
memcpy(tmp_ptr, arg->chan_list, arg->num_chan * 4)
>> +int ath12k_wmi_send_scan_chan_list_cmd(struct ath12k *ar,
>> + struct ath12k_wmi_scan_chan_list_arg *arg)
>> +{
>> + struct ath12k_wmi_pdev *wmi = ar->wmi;
>> + struct wmi_scan_chan_list_cmd *cmd;
>> + struct sk_buff *skb;
>> + struct ath12k_wmi_channel_params *chan_info;
>> + struct ath12k_wmi_channel_arg *channel_arg;
>> + struct wmi_tlv *tlv;
>> + void *ptr;
>> + int i, ret, len;
>> + u16 num_send_chans, num_sends = 0, max_chan_limit = 0;
>> + __le32 *reg1, *reg2;
>> +
>> + channel_arg = &arg->channel[0];
>> + while (arg->nallchans) {
>> + len = sizeof(*cmd) + TLV_HDR_SIZE;
>> + max_chan_limit = (wmi->wmi_ab->max_msg_len[ar->pdev_idx] - len) /
>> + sizeof(*chan_info);
>> +
>> + if (arg->nallchans > max_chan_limit)
>> + num_send_chans = max_chan_limit;
>> + else
>> + num_send_chans = arg->nallchans;
>
> num_send_chans = min(arg->nallchans, max_chan_limit);
Fixed.
>> +static void
>> +ath12k_wmi_copy_resource_config(struct ath12k_wmi_resource_config_params *wmi_cfg,
>> + struct ath12k_wmi_resource_config_arg *tg_cfg)
>> +{
>> + wmi_cfg->num_vdevs = cpu_to_le32(tg_cfg->num_vdevs);
>> + wmi_cfg->num_peers = cpu_to_le32(tg_cfg->num_peers);
>> + wmi_cfg->num_offload_peers = cpu_to_le32(tg_cfg->num_offload_peers);
>> + wmi_cfg->num_offload_reorder_buffs =
>> + cpu_to_le32(tg_cfg->num_offload_reorder_buffs);
>> + wmi_cfg->num_peer_keys = cpu_to_le32(tg_cfg->num_peer_keys);
>> + wmi_cfg->num_tids = cpu_to_le32(tg_cfg->num_tids);
>> + wmi_cfg->ast_skid_limit = cpu_to_le32(tg_cfg->ast_skid_limit);
>> + wmi_cfg->tx_chain_mask = cpu_to_le32(tg_cfg->tx_chain_mask);
>> + wmi_cfg->rx_chain_mask = cpu_to_le32(tg_cfg->rx_chain_mask);
>> + wmi_cfg->rx_timeout_pri[0] = cpu_to_le32(tg_cfg->rx_timeout_pri[0]);
>> + wmi_cfg->rx_timeout_pri[1] = cpu_to_le32(tg_cfg->rx_timeout_pri[1]);
>> + wmi_cfg->rx_timeout_pri[2] = cpu_to_le32(tg_cfg->rx_timeout_pri[2]);
>> + wmi_cfg->rx_timeout_pri[3] = cpu_to_le32(tg_cfg->rx_timeout_pri[3]);
>> + wmi_cfg->rx_decap_mode = cpu_to_le32(tg_cfg->rx_decap_mode);
>> + wmi_cfg->scan_max_pending_req = cpu_to_le32(tg_cfg->scan_max_pending_req);
>> + wmi_cfg->bmiss_offload_max_vdev = cpu_to_le32(tg_cfg->bmiss_offload_max_vdev);
>> + wmi_cfg->roam_offload_max_vdev = cpu_to_le32(tg_cfg->roam_offload_max_vdev);
>> + wmi_cfg->roam_offload_max_ap_profiles =
>> + cpu_to_le32(tg_cfg->roam_offload_max_ap_profiles);
>> + wmi_cfg->num_mcast_groups = cpu_to_le32(tg_cfg->num_mcast_groups);
>> + wmi_cfg->num_mcast_table_elems = cpu_to_le32(tg_cfg->num_mcast_table_elems);
>> + wmi_cfg->mcast2ucast_mode = cpu_to_le32(tg_cfg->mcast2ucast_mode);
>> + wmi_cfg->tx_dbg_log_size = cpu_to_le32(tg_cfg->tx_dbg_log_size);
>> + wmi_cfg->num_wds_entries = cpu_to_le32(tg_cfg->num_wds_entries);
>> + wmi_cfg->dma_burst_size = cpu_to_le32(tg_cfg->dma_burst_size);
>> + wmi_cfg->mac_aggr_delim = cpu_to_le32(tg_cfg->mac_aggr_delim);
>> + wmi_cfg->rx_skip_defrag_timeout_dup_detection_check =
>> + cpu_to_le32(tg_cfg->rx_skip_defrag_timeout_dup_detection_check);
>> + wmi_cfg->vow_config = cpu_to_le32(tg_cfg->vow_config);
>> + wmi_cfg->gtk_offload_max_vdev = cpu_to_le32(tg_cfg->gtk_offload_max_vdev);
>> + wmi_cfg->num_msdu_desc = cpu_to_le32(tg_cfg->num_msdu_desc);
>> + wmi_cfg->max_frag_entries = cpu_to_le32(tg_cfg->max_frag_entries);
>> + wmi_cfg->num_tdls_vdevs = cpu_to_le32(tg_cfg->num_tdls_vdevs);
>> + wmi_cfg->num_tdls_conn_table_entries =
>> + cpu_to_le32(tg_cfg->num_tdls_conn_table_entries);
>> + wmi_cfg->beacon_tx_offload_max_vdev =
>> + cpu_to_le32(tg_cfg->beacon_tx_offload_max_vdev);
>> + wmi_cfg->num_multicast_filter_entries =
>> + cpu_to_le32(tg_cfg->num_multicast_filter_entries);
>> + wmi_cfg->num_wow_filters = cpu_to_le32(tg_cfg->num_wow_filters);
>> + wmi_cfg->num_keep_alive_pattern = cpu_to_le32(tg_cfg->num_keep_alive_pattern);
>> + wmi_cfg->keep_alive_pattern_size =
>> cpu_to_le32(tg_cfg->keep_alive_pattern_size);
>> + wmi_cfg->max_tdls_concurrent_sleep_sta =
>> + cpu_to_le32(tg_cfg->max_tdls_concurrent_sleep_sta);
>> + wmi_cfg->max_tdls_concurrent_buffer_sta =
>> + cpu_to_le32(tg_cfg->max_tdls_concurrent_buffer_sta);
>> + wmi_cfg->wmi_send_separate = cpu_to_le32(tg_cfg->wmi_send_separate);
>> + wmi_cfg->num_ocb_vdevs = cpu_to_le32(tg_cfg->num_ocb_vdevs);
>> + wmi_cfg->num_ocb_channels = cpu_to_le32(tg_cfg->num_ocb_channels);
>> + wmi_cfg->num_ocb_schedules = cpu_to_le32(tg_cfg->num_ocb_schedules);
>> + wmi_cfg->bpf_instruction_size = cpu_to_le32(tg_cfg->bpf_instruction_size);
>> + wmi_cfg->max_bssid_rx_filters = cpu_to_le32(tg_cfg->max_bssid_rx_filters);
>> + wmi_cfg->use_pdev_id = cpu_to_le32(tg_cfg->use_pdev_id);
>> + wmi_cfg->flag1 = cpu_to_le32(tg_cfg->atf_config);
>> + wmi_cfg->peer_map_unmap_version = cpu_to_le32(tg_cfg->peer_map_unmap_version);
>> + wmi_cfg->sched_params = cpu_to_le32(tg_cfg->sched_params);
>> + wmi_cfg->twt_ap_pdev_count = cpu_to_le32(tg_cfg->twt_ap_pdev_count);
>> + wmi_cfg->twt_ap_sta_count = cpu_to_le32(tg_cfg->twt_ap_sta_count);
>> + wmi_cfg->host_service_flags |=
>> + cpu_to_le32(1 << WMI_RSRC_CFG_HOST_SVC_FLAG_REG_CC_EXT_SUPPORT_BIT);
>
> Why not just '=' instead of '|=' ?
Fixed.
>> +int ath12k_wmi_pdev_lro_cfg(struct ath12k *ar,
>> + int pdev_id)
>> +{
>> + struct ath12k_wmi_pdev_lro_config_cmd *cmd;
>> + struct sk_buff *skb;
>> + int ret;
>> +
>> + skb = ath12k_wmi_alloc_skb(ar->wmi->wmi_ab, sizeof(*cmd));
>> + if (!skb)
>> + return -ENOMEM;
>> +
>> + cmd = (struct ath12k_wmi_pdev_lro_config_cmd *)skb->data;
>> + cmd->tlv_header = ath12k_wmi_tlv_cmd_hdr(WMI_TAG_LRO_INFO_CMD,
>> + sizeof(*cmd));
>> +
>> + get_random_bytes(cmd->th_4, sizeof(uint32_t) * ATH12K_IPV4_TH_SEED_SIZE);
>> + get_random_bytes(cmd->th_6, sizeof(uint32_t) * ATH12K_IPV6_TH_SEED_SIZE);
>
> get_random_bytes(cmd->th_4, sizeof(cmd->th_4));
> get_random_bytes(cmd->th_6, sizeof(cmd->th_6));
Fixed.
>> +static int ath12k_wmi_hw_mode_caps_parse(struct ath12k_base *soc,
>> + u16 tag, u16 len,
>> + const void *ptr, void *data)
>> +{
>> + struct ath12k_wmi_svc_rdy_ext_parse *svc_rdy_ext = data;
>> + struct ath12k_wmi_hw_mode_cap_params *hw_mode_cap;
>> + u32 phy_map = 0;
>> +
>> + if (tag != WMI_TAG_HW_MODE_CAPABILITIES)
>> + return -EPROTO;
>> +
>> + if (svc_rdy_ext->n_hw_mode_caps >= svc_rdy_ext->arg.num_hw_modes)
>> + return -ENOBUFS;
>> +
>> + hw_mode_cap = container_of(ptr, struct ath12k_wmi_hw_mode_cap_params,
>> + hw_mode_id);
>> + svc_rdy_ext->n_hw_mode_caps++;
>> +
>> + phy_map = le32_to_cpu(hw_mode_cap->phy_id_map);
>> + while (phy_map) {
>> + svc_rdy_ext->tot_phy_id++;
>> + phy_map = phy_map >> 1;
>> + }
>
> svc_rdy_ext->tot_phy_id = fls(phy_map);
> or
> svc_rdy_ext->tot_phy_id += fls(phy_map);
We chose the latter:
phy_map = le32_to_cpu(hw_mode_cap->phy_id_map);
svc_rdy_ext->tot_phy_id += fls(phy_map);
>> +static int ath12k_wmi_hw_mode_caps(struct ath12k_base *soc,
>> + u16 len, const void *ptr, void *data)
>> +{
>> + struct ath12k_wmi_svc_rdy_ext_parse *svc_rdy_ext = data;
>> + const struct ath12k_wmi_hw_mode_cap_params *hw_mode_caps;
>> + enum wmi_host_hw_mode_config_type mode, pref;
>> + u32 i;
>> + int ret;
>> +
>> + svc_rdy_ext->n_hw_mode_caps = 0;
>> + svc_rdy_ext->hw_mode_caps = ptr;
>> +
>> + ret = ath12k_wmi_tlv_iter(soc, ptr, len,
>> + ath12k_wmi_hw_mode_caps_parse,
>> + svc_rdy_ext);
>> + if (ret) {
>> + ath12k_warn(soc, "failed to parse tlv %d\n", ret);
>> + return ret;
>> + }
>> +
>> + i = 0;
>> + while (i < svc_rdy_ext->n_hw_mode_caps) {
>
> for (i = 0; i < svc_rdy_ext->n_hw_mode_caps; i++) {
Fixed.
>> + hw_mode_caps = &svc_rdy_ext->hw_mode_caps[i];
>> + mode = le32_to_cpu(hw_mode_caps->hw_mode_id);
>> + pref = soc->wmi_ab.preferred_hw_mode;
>> +
>> + if (ath12k_hw_mode_pri_map[mode] < ath12k_hw_mode_pri_map[pref]) {
>> + svc_rdy_ext->pref_hw_mode_caps = *hw_mode_caps;
>> + soc->wmi_ab.preferred_hw_mode = mode;
>> + }
>> + i++;
>
> With 'for', remove this i++.
Done.
>> +static int ath12k_wmi_ext_hal_reg_caps(struct ath12k_base *soc,
>> + u16 len, const void *ptr, void *data)
>> +{
>> + struct ath12k_wmi_pdev *wmi_handle = &soc->wmi_ab.wmi[0];
>> + struct ath12k_wmi_svc_rdy_ext_parse *svc_rdy_ext = data;
>> + struct ath12k_wmi_hal_reg_capabilities_ext_arg reg_cap;
>> + int ret;
>> + u32 i;
>> +
>> + svc_rdy_ext->n_ext_hal_reg_caps = 0;
>> + svc_rdy_ext->ext_hal_reg_caps = ptr;
>> + ret = ath12k_wmi_tlv_iter(soc, ptr, len,
>> + ath12k_wmi_ext_hal_reg_caps_parse,
>> + svc_rdy_ext);
>> + if (ret) {
>> + ath12k_warn(soc, "failed to parse tlv %d\n", ret);
>> + return ret;
>> + }
>> +
>> + for (i = 0; i < svc_rdy_ext->arg.num_phy; i++) {
>> + ret = ath12k_pull_reg_cap_svc_rdy_ext(wmi_handle,
>> + svc_rdy_ext->soc_hal_reg_caps,
>> + svc_rdy_ext->ext_hal_reg_caps, i,
>> + ®_cap);
>> + if (ret) {
>> + ath12k_warn(soc, "failed to extract reg cap %d\n", i);
>> + return ret;
>> + }
>> +
>> + memcpy(&soc->hal_reg_cap[reg_cap.phy_id],
>> + ®_cap, sizeof(reg_cap));
>
> soc->hal_reg_cap[reg_cap.phy_id] = reg_cap;
Fixed.
>> +static int ath12k_reg_chan_list_event(struct ath12k_base *ab, struct sk_buff *skb)
>> +{
>
> [...]
>
>> +
>> + /* Avoid multiple overwrites to default regd, during core
>> + * stop-start after mac registration.
>> + */
>> + if (ab->default_regd[pdev_idx] && !ab->new_regd[pdev_idx] &&
>> + !memcmp((char *)ab->default_regd[pdev_idx]->alpha2,
>> + (char *)reg_info->alpha2, 2))
>
> cast "(char *)" is unnecessary.
Fixed.
>> + goto mem_free;
>> +
>> + /* Intersect new rules with default regd if a new country setting was
>> + * requested, i.e a default regd was already set during initialization
>> + * and the regd coming from this event has a valid country info.
>> + */
>
> [...]
>
>> +
>> +static void ath12k_mgmt_rx_event(struct ath12k_base *ab, struct sk_buff *skb)
>> +{
>
> [...]
>
>> +
>> + if ((test_bit(ATH12K_CAC_RUNNING, &ar->dev_flags)) ||
>> + (rx_ev.status & (WMI_RX_STATUS_ERR_DECRYPT |
>> + WMI_RX_STATUS_ERR_KEY_CACHE_MISS | WMI_RX_STATUS_ERR_CRC))) {
>
> nit: coding style
>
> if ((test_bit(ATH12K_CAC_RUNNING, &ar->dev_flags)) ||
> (rx_ev.status & (WMI_RX_STATUS_ERR_DECRYPT |
> WMI_RX_STATUS_ERR_KEY_CACHE_MISS |
> WMI_RX_STATUS_ERR_CRC))) {
Fixed.
>> +static int ath12k_connect_pdev_htc_service(struct ath12k_base *ab,
>> + u32 pdev_idx)
>> +{
>> + int status;
>> + u32 svc_id[] = { ATH12K_HTC_SVC_ID_WMI_CONTROL,
>> + ATH12K_HTC_SVC_ID_WMI_CONTROL_MAC1,
>> + ATH12K_HTC_SVC_ID_WMI_CONTROL_MAC2 };
>> +
>
> nit.
> coding style: no blank line within declaration.
Fixed.
Thank you very much for the good comments!
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
