Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <dedd1b4c-4d72-ac36-865e-f02614dfe3da@gmail.com>
Date:   Mon, 21 Mar 2022 23:47:01 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.2
Subject: Re: [PATCH RFT] mwifiex: add mutex lock for call in
 mwifiex_dfs_chan_sw_work_queue
Content-Language: en-US
To:     Brian Norris <briannorris@chromium.org>
Cc:     linux-wireless@vger.kernel.org,
        Amitkumar Karwar <amitkarwar@gmail.com>,
        Ganapathi Bhat <ganapathi017@gmail.com>,
        Sharvari Harisangam <sharvari.harisangam@nxp.com>,
        Xinming Hu <huxinming820@gmail.com>,
        Kalle Valo <kvalo@kernel.org>
References: <20220319224751.72241-1-dossche.niels@gmail.com>
 <YjjdcG5NmzLOmjEu@google.com>
From:   Niels Dossche <dossche.niels@gmail.com>
In-Reply-To: <YjjdcG5NmzLOmjEu@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 21/03/2022 21:17, Brian Norris wrote:
> On Sat, Mar 19, 2022 at 11:47:52PM +0100, Niels Dossche wrote:
>> --- a/drivers/net/wireless/marvell/mwifiex/11h.c
>> +++ b/drivers/net/wireless/marvell/mwifiex/11h.c
>> @@ -285,6 +285,7 @@ void mwifiex_dfs_chan_sw_work_queue(struct work_struct *work)
>>  	struct mwifiex_private *priv =
>>  			container_of(delayed_work, struct mwifiex_private,
>>  				     dfs_chan_sw_work);
>> +	struct net_device *netdev;
>>  
>>  	bss_cfg = &priv->bss_cfg;
>>  	if (!bss_cfg->beacon_period) {
>> @@ -301,7 +302,11 @@ void mwifiex_dfs_chan_sw_work_queue(struct work_struct *work)
>>  		return;
>>  	}
>>  
>> +	netdev = priv->netdev;
>> +
>>  	mwifiex_dbg(priv->adapter, MSG,
>>  		    "indicating channel switch completion to kernel\n");
>> -	cfg80211_ch_switch_notify(priv->netdev, &priv->dfs_chandef);
>> +	mutex_lock(&netdev->ieee80211_ptr->mtx);
> 
> A more appropriate route to this object might be priv->wdev.mtx. But
> otherwise, I think this makes sense, and matches what
> ath6kl_cfg80211_ch_switch_notify() and qtnf_event_handle_freq_change()
> do. With the suggested change:

Thanks for the review, will change that and send a v2.

> 
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> 
> Thanks.
> 
>> +	cfg80211_ch_switch_notify(netdev, &priv->dfs_chandef);
>> +	mutex_unlock(&netdev->ieee80211_ptr->mtx);
>>  }
>> -- 
>> 2.35.1
>>