Wright Feng <Wright.Feng@xxxxxxxxxxx> writes:
>>>>> +brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev,
>>>>> + struct bss_parameters *params)
>>>>> +{
>>>>> + struct brcmf_if *ifp;
>>>>> + int ret = 0;
>>>>> + u32 ap_isolate;
>>>>> +
>>>>> + brcmf_dbg(TRACE, "Enter\n");
>>>>> + ifp = netdev_priv(dev);
>>>>> + if (params->ap_isolate >= 0) {
>>>>> + ap_isolate = (u32)params->ap_isolate;
>>>>> + ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", ap_isolate);
>>>>
>>>> Is the cast to u32 really necessary? Please avoid casts as much as
>>>> possible.
>>>
>>> It seems to me. struct bss_parameters::ap_isolate is typed as int. It
>>> is passed to brcmf_fil_iovar_int_set() which requires a u32 (maybe
>>> function name is causing the confusion).
>>
>> What extra value does this explicit type casting bring here? I don't see
>> it. Implicit type casting would work the same, no?
>
> The value will be -1, 0 or 1.
> I will submit v2 patch that ignores doing iovar if getting
> params->ap_isolate -1 and removes explicit type casting. Thanks for the
> comment.
Oh, I didn't realise ap_isolate can be -1 as struct bss_parameters
didn't document that. Can someone submit a patch to fix that?
* @ap_isolate: do not forward packets between connected stations
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
