Re: [PATCH 4/7] ath6kl: Add new functions to handle wow suspend/resume operations

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

On 10/25/2011 01:37 PM, rmani@qca.qualcomm.com wrote:
> From: Raja Mani <rmani@qca.qualcomm.com>
> 
> Signed-off-by: Raja Mani <rmani@qca.qualcomm.com>

Empty commit log.

> +int ath6kl_pm_wow_suspend(struct ath6kl *ar, struct cfg80211_wowlan *wow)
> +{
> +	struct wmi_set_wow_mode_cmd wakeup_filter_cmd;
> +	struct wmi_add_wow_pattern_cmd add_pattern_cmd;
> +	struct wmi_del_wow_pattern_cmd del_pattern_cmd;
> +	struct wmi_set_host_sleep_mode_cmd hsleep_cmd;
> +	int i, ret, pos, left;
> +	u8 mask[WOW_PATTERN_SIZE];
> +
> +	if (WARN_ON(!wow))
> +		return -EINVAL;

Unnecessary null check, wow is non-zero here.

> +		for (pos = 0; pos < wow->patterns[i].pattern_len;
> +				pos++) {
> +			if (wow->patterns[i].mask[pos / 8] & (0x1 << (pos % 8)))
> +				mask[pos] = 0xFF;

This loop needs a comment.

> +	if (ar->tx_pending[ar->ctrl_ep]) {
> +		left = wait_event_interruptible_timeout(ar->event_wq,
> +				ar->tx_pending[ar->ctrl_ep] == 0, WMI_TIMEOUT);
> +		if (left == 0) {
> +			ret = -ETIMEDOUT;
> +			goto wow_setup_failed;
> +		} else if (left < 0) {
> +			ret = left;
> +			goto wow_setup_failed;

A warning message for both of these error case would be nice.

> +#define WOW_HOST_REQ_DELAY	500 /* 500 ms */

No point of duplicating the value, "/* ms */" is enough.

Kalle