Re: [PATCH 12/19] wilc: add wilc_wfi_cfgoperations.c

[<Adham.Abozaeid@microchip.com>]

On 10/08/2018 07:57 AM, Johannes Berg wrote:
> On Wed, 2018-09-26 at 15:55 +0530, Ajay Singh wrote:
>>
>> +static void clear_shadow_scan(struct wilc_priv *priv)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < priv->scanned_cnt; i++) {
>> +		kfree(priv->scanned_shadow[i].ies);
>> +		priv->scanned_shadow[i].ies = NULL;
>> +
>> +		kfree(priv->scanned_shadow[i].join_params);
>> +		priv->scanned_shadow[i].join_params = NULL;
>> +	}
>> +	priv->scanned_cnt = 0;
>> +}
> 
> This seems unlikely to be a good idea - why keep things around in the
> driver?
> 
> 
>> +static void refresh_scan(struct wilc_priv *priv, bool direct_scan)
>> +{
>> +	struct wiphy *wiphy = priv->dev->ieee80211_ptr->wiphy;
>> +	int i;
>> +
>> +	for (i = 0; i < priv->scanned_cnt; i++) {
>> +		struct network_info *network_info;
>> +		s32 freq;
>> +		struct ieee80211_channel *channel;
>> +		int rssi;
>> +		struct cfg80211_bss *bss;
>> +
>> +		network_info = &priv->scanned_shadow[i];
>> +
>> +		if (!memcmp("DIRECT-", network_info->ssid, 7) && !direct_scan)
>> +			continue;
> 
> Err, no? Don't do that? What's the point?
> 
> I don't know what you need the shadow stuff for, but you should remove
> it anyway, and use the cfg80211 functionality instead. If not
> sufficient, propose patches to improve it?

The point behind using a shadow buffer was to keep the scan results consistent between consecutive scans, and smooth out the cases where an AP isn't found momentarily during scanning.
In this implementation, APs found during scanning are added to the shadow list, and removed if not found again for a period of time.

I'm not much in favour of this implementation neither since it complicates the driver's logic, but it was serving the purpose.

> 
>> +			if (memcmp("DIRECT-", network_info->ssid, 7))
>> +				return;
> 
> same here
> 
> 
> johannes
> 

Thanks,
Adham