Senthil Balasubramanian <senthilkumar@xxxxxxxxxxx> writes:
> On Tue, May 10, 2011 at 10:10:23AM +0530, Kalle Valo wrote:
>> From: Naveen Singh <nsingh@xxxxxxxxxxx>
>>
>> implementing the cfg ops that gets called when iw dev wlan0
>> link is issued by user. The ops that needs to be implemented
>> is get_station.
>>
>> kvalo: check the mac address and fix style issues
[...]
>> +int ar6k_get_station(struct wiphy *wiphy, struct net_device *dev,
>> + u8 *mac, struct station_info *sinfo)
>> +{
>> + struct ar6_softc *ar = ar6k_priv(dev);
>> + u8 mcs, ret, timeout = false;
>> +
>> + if (compare_ether_addr(mac, ar->arBssid))
>
> this may result in unaligned memeory access on some platforms as mac
> may not be 2 byte aligned. This is the case with the caller as per
> cfg80211_wext_giwrate() and cfg80211_wireless_stats().
I didn't think of this. I'll change it to use memcmp() instead.
>> + wait_event_interruptible_timeout(arEvent,
>> + ar->statsUpdatePending == false,
>> + wmitimeout * HZ);
>> +
>> + if (signal_pending(current)) {
>
> wait_event_interruptible_tiemout() itself checks for signal_pending
> and so this is not necessary. You can check for the return status of
> wait_event_interruptible_timeout() to know whether the task was
> interrupted by a signal.
Will fix.
>> + timeout = true;
>> + AR_DEBUG_PRINTF(ATH_DEBUG_ERR,
>> + ("ar6000 : WMI get stats timeout\n"));
>> + }
>> +
>> + up(&ar->arSem);
>
> I believe we can release the semaphore early on. I don't see any reason..
What do you mean? I'm not the author of the patch, but I assume that
the idea is to protect the access to ar->statsUpdatePending.
Thanks for the review, I'll submit v2.
--
Kalle Valo
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
