From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kalle Valo Date: Thu, 9 May 2013 10:17:28 +0300 Subject: [ath9k-devel] [PATCH 3/3] ath10k: enable STA UAPSD support In-Reply-To: <1367999363-20424-4-git-send-email-janusz.dziedzic@tieto.com> (Janusz Dziedzic's message of "Wed, 8 May 2013 09:49:23 +0200") References: <1367999363-20424-1-git-send-email-janusz.dziedzic@tieto.com> <1367999363-20424-4-git-send-email-janusz.dziedzic@tieto.com> Message-ID: <8738twy647.fsf@kamboji.qca.qualcomm.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ath9k-devel@lists.ath9k.org Janusz Dziedzic writes: > Enable UAPSD support for STA mode. > > Signed-off-by: Janusz Dziedzic Same here, please explain briefly what you have tested. > --- a/drivers/net/wireless/ath/ath10k/mac.c > +++ b/drivers/net/wireless/ath/ath10k/mac.c > @@ -2296,6 +2296,68 @@ static int ath10k_sta_state(struct ieee80211_hw *hw, > return ret; > } > > +#define SET_UAPSD(enable, out, flags) do { \ > + if ((enable)) \ > + (out) |= (flags); \ > + else \ > + (out) &= ~(flags); \ > +} while (0) Why the macro? To workaround a long line warning from checkpatch? If that's a problem we could increate line length limit, for example to 85 or 90. In some cases the 80 char limit is a bit too excessive. Would that help? [...] > + if (ret) { > + ath10k_warn("could not set uapsd params (%d)\n", ret); "could not set uapsd params: %d\n" > + goto exit; > + } > + > + if (arvif->u.sta.uapsd) > + value = WMI_STA_PS_RX_WAKE_POLICY_POLL_UAPSD; > + else > + value = WMI_STA_PS_RX_WAKE_POLICY_WAKE; > + > + ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id, > + WMI_STA_PS_PARAM_RX_WAKE_POLICY, > + value); > + if (ret) > + ath10k_warn("could not set rx wake param (%d)\n", ret); > +exit: > + return ret; > +} Empty line before the exit label. > @@ -2335,13 +2399,20 @@ static int ath10k_conf_tx(struct ieee80211_hw *hw, > */ > p->txop = params->txop * 32; > > - /* FIXME: can we pass the params->uapsd to the FW? */ > + This now has two empty lines, one is enough. > - if (ret) > + if (ret) { > ath10k_warn("could not set wmm params (%d)\n", ret); > + goto exit; > + } The same comment as with the other warning message above. > + ret = ath10k_conf_tx_uapsd(ar, vif, ac, params->uapsd); > + if (ret) > + ath10k_warn("could not set sta uapsd (%d)\n", ret); Ditto. -- Kalle Valo