On Tue, Aug 30, 2011 at 12:33:46PM +0300, Kalle Valo wrote:
> On 08/29/2011 03:23 PM, Jouni Malinen wrote:
> > @@ -785,14 +787,33 @@ static int ath6kl_cfg80211_scan(struct wiphy *wiphy, struct net_device *ndev,
> > + n_channels = (request->n_channels > 127) ?
> > + 127 : request->n_channels;
>
> min()
It looks kind of silly to have 127U here, but I can do that if it is the
preferred way, i.e., min(127U, request->n_channels). min(127,
request->n_channels) complains because of the strict type-checking in
the macro (request->n_channels is u32).
> > + 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.
OK.
> > 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.
OK.
> > - 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.
OK.
--
Jouni Malinen PGP id EFC895FA
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
