From: pradeepc@codeaurora.org
To: Sven Eckelmann <sven.eckelmann@openmesh.com>
Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
Subject: Re: [PATCH] ath10k:add support for multicast rate control
Date: Mon, 16 Apr 2018 22:10:50 -0700 [thread overview]
Message-ID: <fe1af65b1042bc3c2b551dcbe7d53f3c@codeaurora.org> (raw)
On 2018-04-12 01:00, Sven Eckelmann wrote:
> On Mittwoch, 11. April 2018 21:04:46 CEST Pradeep Kumar Chitrapu wrote:
>> Issues wmi command to firmware when multicast rate change is received
>> with the new BSS_CHANGED_MCAST_RATE flag.
> [...]
>>
>> + if (changed & BSS_CHANGED_MCAST_RATE &&
>> + !WARN_ON(ath10k_mac_vif_chan(arvif->vif, &def))) {
>> + band = def.chan->band;
>> + sband = &ar->mac.sbands[band];
>> + vdev_param = ar->wmi.vdev_param->mcast_data_rate;
>> + rate_index = vif->bss_conf.mcast_rate[band] - 1;
>> + rate = ATH10K_HW_MCS_RATE(sband->bitrates[rate_index].hw_value);
>> + ath10k_dbg(ar, ATH10K_DBG_MAC,
>> + "mac vdev %d mcast_rate %d\n",
>> + arvif->vdev_id, rate);
>> +
>> + ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
>> + vdev_param, rate);
>> + if (ret)
>> + ath10k_warn(ar,
>> + "failed to set mcast rate on vdev"
>> + " %i: %d\n", arvif->vdev_id, ret);
>> + }
>> +
>> mutex_unlock(&ar->conf_mutex);
>> }
>>
>>
>
> I see two major problems here without checking the actual
> implementation
> details:
>
> * hw_value is incorrect for a couple of devices. Some devices use a
> different
> mapping when they receive rates inforamtion (hw_value) then the ones
> you use
> for the mcast/bcast/beacon rate setting. I've handled in my POC patch
> like
> this:
>
> + if (ath10k_mac_bitrate_is_cck(sband->bitrates[i].bitrate)) {
> + preamble = WMI_RATE_PREAMBLE_CCK;
> +
> + /* QCA didn't use the correct rate values for CA99x0
> + * and above (ath10k_g_rates_rev2)
> + */
> + switch (sband->bitrates[i].bitrate) {
> + case 10:
> + hw_value = ATH10K_HW_RATE_CCK_LP_1M;
> + break;
> + case 20:
> + hw_value = ATH10K_HW_RATE_CCK_LP_2M;
> + break;
> + case 55:
> + hw_value = ATH10K_HW_RATE_CCK_LP_5_5M;
> + break;
> + case 110:
> + hw_value = ATH10K_HW_RATE_CCK_LP_11M;
> + break;
> + }
> + } else {
> + preamble = WMI_RATE_PREAMBLE_OFDM;
> + }
>
Isn't this already fixed in https://patchwork.kernel.org/patch/9150145/
?
> * bcast + mcast (+ mgmt) have to be set separately
Can you please let me know why this would be necessary?
Although I see minstrel rate control is currently seems using the
mcast_rate
setting to set MGMT/BCAST/MCAST rates, will this not be misleading to
user
passed value with 'iw set mcast_rate' for MGMT traffic as well?
>
> I have attached my POC patch (which I was using for packet loss based
> mesh
> metrics) and to work around (using debugfs) the silly mgmt vs.
> mcast/bcast
> settings of the QCA fw for APs.
>
> Many of the information came from Ben Greears ath10k-ct driver
> https://github.com/greearb/ath10k-ct
>
> Kind regards,
> Sven
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
WARNING: multiple messages have this Message-ID (diff)
From: pradeepc@codeaurora.org
To: Sven Eckelmann <sven.eckelmann@openmesh.com>
Cc: ath10k@lists.infradead.org, linux-wireless@vger.kernel.org
Subject: Re: [PATCH] ath10k:add support for multicast rate control
Date: Mon, 16 Apr 2018 22:10:50 -0700 [thread overview]
Message-ID: <fe1af65b1042bc3c2b551dcbe7d53f3c@codeaurora.org> (raw)
On 2018-04-12 01:00, Sven Eckelmann wrote:
> On Mittwoch, 11. April 2018 21:04:46 CEST Pradeep Kumar Chitrapu wrote:
>> Issues wmi command to firmware when multicast rate change is received
>> with the new BSS_CHANGED_MCAST_RATE flag.
> [...]
>>
>> + if (changed & BSS_CHANGED_MCAST_RATE &&
>> + !WARN_ON(ath10k_mac_vif_chan(arvif->vif, &def))) {
>> + band = def.chan->band;
>> + sband = &ar->mac.sbands[band];
>> + vdev_param = ar->wmi.vdev_param->mcast_data_rate;
>> + rate_index = vif->bss_conf.mcast_rate[band] - 1;
>> + rate = ATH10K_HW_MCS_RATE(sband->bitrates[rate_index].hw_value);
>> + ath10k_dbg(ar, ATH10K_DBG_MAC,
>> + "mac vdev %d mcast_rate %d\n",
>> + arvif->vdev_id, rate);
>> +
>> + ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
>> + vdev_param, rate);
>> + if (ret)
>> + ath10k_warn(ar,
>> + "failed to set mcast rate on vdev"
>> + " %i: %d\n", arvif->vdev_id, ret);
>> + }
>> +
>> mutex_unlock(&ar->conf_mutex);
>> }
>>
>>
>
> I see two major problems here without checking the actual
> implementation
> details:
>
> * hw_value is incorrect for a couple of devices. Some devices use a
> different
> mapping when they receive rates inforamtion (hw_value) then the ones
> you use
> for the mcast/bcast/beacon rate setting. I've handled in my POC patch
> like
> this:
>
> + if (ath10k_mac_bitrate_is_cck(sband->bitrates[i].bitrate)) {
> + preamble = WMI_RATE_PREAMBLE_CCK;
> +
> + /* QCA didn't use the correct rate values for CA99x0
> + * and above (ath10k_g_rates_rev2)
> + */
> + switch (sband->bitrates[i].bitrate) {
> + case 10:
> + hw_value = ATH10K_HW_RATE_CCK_LP_1M;
> + break;
> + case 20:
> + hw_value = ATH10K_HW_RATE_CCK_LP_2M;
> + break;
> + case 55:
> + hw_value = ATH10K_HW_RATE_CCK_LP_5_5M;
> + break;
> + case 110:
> + hw_value = ATH10K_HW_RATE_CCK_LP_11M;
> + break;
> + }
> + } else {
> + preamble = WMI_RATE_PREAMBLE_OFDM;
> + }
>
Isn't this already fixed in https://patchwork.kernel.org/patch/9150145/
?
> * bcast + mcast (+ mgmt) have to be set separately
Can you please let me know why this would be necessary?
Although I see minstrel rate control is currently seems using the
mcast_rate
setting to set MGMT/BCAST/MCAST rates, will this not be misleading to
user
passed value with 'iw set mcast_rate' for MGMT traffic as well?
>
> I have attached my POC patch (which I was using for packet loss based
> mesh
> metrics) and to work around (using debugfs) the silly mgmt vs.
> mcast/bcast
> settings of the QCA fw for APs.
>
> Many of the information came from Ben Greears ath10k-ct driver
> https://github.com/greearb/ath10k-ct
>
> Kind regards,
> Sven
next reply other threads:[~2018-04-17 5:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-17 5:10 pradeepc [this message]
2018-04-17 5:10 ` [PATCH] ath10k:add support for multicast rate control pradeepc
2018-04-17 6:33 ` Sven Eckelmann
2018-04-17 6:33 ` Sven Eckelmann
-- strict thread matches above, loose matches on Subject: below --
2018-04-12 4:04 Pradeep Kumar Chitrapu
2018-04-12 4:04 ` Pradeep Kumar Chitrapu
2018-04-12 8:00 ` Sven Eckelmann
2018-04-12 8:00 ` Sven Eckelmann
2020-12-21 17:23 ` Kalle Valo
2020-12-21 17:23 ` Kalle Valo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fe1af65b1042bc3c2b551dcbe7d53f3c@codeaurora.org \
--to=pradeepc@codeaurora.org \
--cc=ath10k@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=sven.eckelmann@openmesh.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.