On 2/28/2025 6:37 PM, Johannes Berg wrote:
> On Wed, 2025-01-29 at 21:22 +0530, Roopni Devanathan wrote:
>>
>> --- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
>> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
>> @@ -126,7 +126,7 @@ struct iwl_mvm_time_event_data {
>> /* Power management */
>>
>> /**
>> - * enum iwl_power_scheme
>> + * enum iwl_power_scheme - enum iwl power sceme set by debugfs
>
> How does that belong here?!
>
There was a warning reported that the documentation is not carrying
description. Added this to remove that warning. I'll remove this in
my next version.
>> +++ b/net/mac80211/cfg.c
>> @@ -3038,7 +3038,13 @@ static int ieee80211_set_wiphy_params(struct wiphy *wiphy, u8 radio_id, u32 chan
>> }
>>
>> if (changed & WIPHY_PARAM_RTS_THRESHOLD) {
>> - err = drv_set_rts_threshold(local, wiphy->rts_threshold);
>> + u32 rts_threshold;
>> +
>> + if (radio_id >= wiphy->n_radio)
>> + rts_threshold = wiphy->rts_threshold;
>> + else
>> + rts_threshold = wiphy->radio_cfg[radio_id].rts_threshold;
>> + err = drv_set_rts_threshold(local, radio_id, rts_threshold);
>
> Should we really just leave it all up to the driver, or perhaps call it
> multiple times for each radio? Dunno.
>
Each time RTS threshold for a particular radio is changed, calling the function
with just that RTS threshold is beneficial, because the other radios will already
have their own data and we will be just rewriting the same values if we call the
function multiple times for each radio. Can we just stick to this method where we
are checking the number of radios and calling accordingly?
>> @@ -715,8 +716,14 @@ ieee80211_tx_h_rate_ctrl(struct ieee80211_tx_data *tx)
>> tx->sdata->vif.type == NL80211_IFTYPE_OCB);
>>
>> /* set up RTS protection if desired */
>> - if (len > tx->local->hw.wiphy->rts_threshold) {
>> - txrc.rts = true;
>> + if (tx->local->hw.wiphy->n_radio) {
>> + for (i = 0; i < tx->local->hw.wiphy->n_radio; i++) {
>> + if (len > tx->local->hw.wiphy->radio_cfg[i].rts_threshold)
>> + txrc.rts = true;
>> + }
>> + } else {
>> + if (len > tx->local->hw.wiphy->rts_threshold)
>> + txrc.rts = true;
>> }
>
> Are you sure you need this? Seems odd to me.
>
I'll remove thsi in my next version.
>> +static ssize_t rts_threshold_read(struct file *file, char __user *user_buf,
>> + size_t count, loff_t *ppos)
>
> I'm not convinced it's worth keeping this at all since you can retrieve
> it via nl80211?
>
Sure, I'll remove this in my next version.
> johannes
