On 5/21/2025 2:32 PM, Johannes Berg wrote:
> On Wed, 2025-05-21 at 14:30 +0530, Roopni Devanathan wrote:
>>
>> On 5/21/2025 12:49 PM, Johannes Berg wrote:
>>> On Tue, 2025-04-29 at 09:30 +0530, Roopni Devanathan wrote:
>>>
>>>> +++ b/include/net/mac80211.h
>>>> @@ -4569,7 +4569,8 @@ struct ieee80211_ops {
>>>> struct ieee80211_key_conf *key,
>>>> struct ieee80211_key_seq *seq);
>>>> int (*set_frag_threshold)(struct ieee80211_hw *hw, u32 value);
>>>> - int (*set_rts_threshold)(struct ieee80211_hw *hw, u32 value);
>>>> + int (*set_rts_threshold)(struct ieee80211_hw *hw, u8 radio_id,
>>>> + u32 value);
>>>
>>> That should probably have documentation updates. And passing a negative
>>> value to a u8 seems awkward? Maybe that should just be 'int'? For a
>>> value that's likely passed in a register, u8 will probably require more
>>> (machine) code anyway.
>>>
>> The following snippet of code tests the value of radio_id in
>> nl80211_set_wiphy() in net/wireless/nl80211.c:
>> + /* Radio idx is not expected for non-multi radio wiphy */
>> + if (rdev->wiphy.n_radio <= 0)
>> + return -EINVAL;
>>
>> This snippet returns an error if the radio_id is negative, so radio_id
>> passed to set_rts_threshold() will always be positive. So can we retain
>> u8 data type for radio_id?
>
> I quote from the commit message:
>
>> A value of NL80211_WIPHY_RADIO_ID_INVALID(-1) indicates radio index is not
>> mentioned and the configuration applies to all radio(s) of the wiphy.
>
> So now something is at least _very_ confusing.
>
I can rewrite the code to something like this:
- Change the data type of radio_id to int.
- NL80211_WIPHY_RADIO_ID_INVALID value assigned a -1.
- Set RTS threshold for all radios when radio_id is -1.
Is this okay?
> johannes
