Re: [PATCH 11/20] ath6kl: Support channel set request for startscan command

[Kalle Valo <kvalo@qca.qualcomm.com>]

On 08/29/2011 03:23 PM, Jouni Malinen wrote:
> From: Edward Lu <elu@qca.qualcomm.com>
>
> @@ -785,14 +787,33 @@ static int ath6kl_cfg80211_scan(struct wiphy *wiphy, struct net_device *ndev,
>  		}
>  	}
>  
> +	if (request->n_channels > 0) {
> +		u8 i;
> +
> +		n_channels = (request->n_channels > 127) ?
> +				127 : request->n_channels;

min()

> +
> +		channels = kzalloc(n_channels * sizeof(u16), GFP_KERNEL);
> +		if (channels == NULL) {
> +			ath6kl_err("failed to set scan channels, "
> +				   "scan all channels");

ath6kl_warn() would be more approriate here as the driver can still
continue.

> +			n_channels = 0;
> +		}
> +
> +		for (i = 0; i < n_channels; i++)
> +			channels[i] = request->channels[i]->center_freq;
> +	}
> +
>  	if (ath6kl_wmi_startscan_cmd(ar->wmi, WMI_LONG_SCAN, 0,
> -				     false, 0, 0, 0, NULL) != 0) {
> +				     false, 0, 0, n_channels, channels) != 0) {
>  		ath6kl_err("wmi_startscan_cmd failed\n");
>  		ret = -EIO;

Again I would prefer to return the error code from the wmi call.

> @@ -1734,8 +1734,10 @@ int ath6kl_wmi_startscan_cmd(struct wmi *wmi, enum wmi_scan_type scan_type,
>  	sc->force_scan_intvl = cpu_to_le32(force_scan_interval);
>  	sc->num_ch = num_chan;
>  
> -	if (num_chan)
> -		memcpy(sc->ch_list, ch_list, num_chan * sizeof(u16));
> +	if (num_chan) {
> +		for (i = 0; i < num_chan; i++)
> +			sc->ch_list[i] = cpu_to_le16(ch_list[i]);
> +	}

if (num_chan) is not needed, the for loop is enough.

Kalle