> Date: Wed, 24 Oct 2012 22:39:46 +0200
> From: Claudio Jeker <cje...@diehard.n-r-g.com>
>
> This diff implements the bits needed to support power management on acx(4)
> based APs. In other words with this I can use my OpenBSD acx111 hostap
> just fine with my mobile devices that would fail before.
>
> Tested on acx111 but not on acx100 based cards.
A few notes:
> Index: dev/ic//acx.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/acx.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 acx.c
> --- dev/ic//acx.c 27 Aug 2010 17:08:00 -0000 1.97
> +++ dev/ic//acx.c 24 Oct 2012 20:35:57 -0000
> @@ -202,6 +202,9 @@ int acx_init_radio(struct acx_softc *,
> void acx_iter_func(void *, struct ieee80211_node *);
> void acx_amrr_timeout(void *);
> void acx_newassoc(struct ieee80211com *, struct ieee80211_node *, int);
> +#ifndef IEEE80211_STA_ONLY
> +void acx_set_tim(struct ieee80211com *, int, int);
> +#endif
>
> int acx_beacon_intvl = 100; /* 100 TU */
>
> @@ -316,6 +319,7 @@ acx_attach(struct acx_softc *sc)
> #ifndef IEEE80211_STA_ONLY
> IEEE80211_C_IBSS | /* IBSS mode */
> IEEE80211_C_HOSTAP | /* Access Point */
> + IEEE80211_C_APPMGT | /* AP Power Mgmt */
> #endif
> IEEE80211_C_SHPREAMBLE; /* Short preamble */
>
> @@ -343,6 +347,11 @@ acx_attach(struct acx_softc *sc)
> ic->ic_node_alloc = acx_node_alloc;
> ic->ic_newassoc = acx_newassoc;
>
> +#ifndef IEEE80211_STA_ONLY
> + /* Override set TIM */
> + ic->ic_set_tim = acx_set_tim;
> +#endif
> +
> /* Override newstate */
> sc->sc_newstate = ic->ic_newstate;
> ic->ic_newstate = acx_newstate;
> @@ -935,10 +944,12 @@ acx_start(struct ifnet *ifp)
> buf = &bd->tx_buf[idx]) {
> struct ieee80211_frame *wh;
> struct ieee80211_node *ni = NULL;
> + struct ieee80211_node *pni = NULL;
As far as I can tell pni is never set...
> struct mbuf *m;
> int rate;
>
> IF_DEQUEUE(&ic->ic_mgtq, m);
> + /* first dequeue management frames */
> if (m != NULL) {
> ni = (struct ieee80211_node *)m->m_pkthdr.rcvif;
> m->m_pkthdr.rcvif = NULL;
> @@ -962,13 +973,20 @@ acx_start(struct ifnet *ifp)
> */
> rate = ni->ni_rates.rs_rates[0];
> rate &= IEEE80211_RATE_VAL;
> - } else if (!IFQ_IS_EMPTY(&ifp->if_snd)) {
> + } else {
> struct ether_header *eh;
>
> - IFQ_DEQUEUE(&ifp->if_snd, m);
> - if (m == NULL)
> - break;
> -
> + /* then dequeue packets on the powersave queue */
> + IF_DEQUEUE(&ic->ic_pwrsaveq, m);
> + if (m != NULL) {
> + ni = (struct ieee80211_node *)m->m_pkthdr.rcvif;
> + m->m_pkthdr.rcvif = NULL;
> + goto encapped;
> + } else {
> + IFQ_DEQUEUE(&ifp->if_snd, m);
> + if (m == NULL)
> + break;
> + }
> if (ic->ic_state != IEEE80211_S_RUN) {
> DPRINTF(("%s: data packet dropped due to "
> "not RUN. Current state %d\n",
> @@ -981,6 +999,8 @@ acx_start(struct ifnet *ifp)
> m = m_pullup(m, sizeof(struct ether_header));
> if (m == NULL) {
> ifp->if_oerrors++;
> + if (pni != NULL)
> + ieee80211_release_node(ic, pni);
..so this bit...
> continue;
> }
> }
> @@ -995,22 +1015,23 @@ acx_start(struct ifnet *ifp)
>
> if ((m = ieee80211_encap(ifp, m, &ni)) == NULL) {
> ifp->if_oerrors++;
> + if (pni != NULL)
> + ieee80211_release_node(ic, pni);
...and this bit can go away.
> @@ -1064,7 +1085,7 @@ acx_start(struct ifnet *ifp)
> * acx_txeof(), so it is not freed here. acx_txeof()
> * will free it for us
> */
> - trans = 1;
> + trans++;
> bd->tx_used_count++;
> idx = (idx + 1) % ACX_TX_DESC_CNT;
> }
> @@ -1116,14 +1137,19 @@ acx_intr(void *arg)
> return (0);
> }
>
> + /* Acknowledge all interrupts */
> + CSR_WRITE_2(sc, ACXREG_INTR_ACK, ACXRV_INTR_ALL);
> +
> intr_status &= sc->chip_intr_enable;
> if (intr_status == 0) {
> /* not interrupts we care about */
> return (1);
> }
>
> - /* Acknowledge all interrupts */
> - CSR_WRITE_2(sc, ACXREG_INTR_ACK, ACXRV_INTR_ALL);
This change isn't really related to the powersave stuff isn't it? We
should never see interrupts that we haven't enabled, but I suppose
there is no harm in acknowledging them all.
But something is fishy here. We should return 0 if the crad didn't
interrupt. And I don't really see the point in masking off the
interrupts that aren't enabled. So I think this should be something
like:
if (intr_status == 0) {
/* not for us */
return (0);
}
/* Acknowledge all interrupts */
CSR_WRITE_2(sc, ACXREG_INTR_ACK, ACXRV_INTR_ALL);
or perhaps that last line should actually be removed since the
ACXREG_INTR_STATUS_CLR strongly suggests that reading it already
clears the interrupts. Or is the hardware buggy? In that case you
should probably change the last line into:
/* Acknowledge all interrupts */
CSR_WRITE_2(sc, ACXREG_INTR_ACK, intr_status);
to avoid accidentally acknowledging interrupts that came in after we
read the interrupt status.
> @@ -2733,3 +2759,23 @@ acx_newassoc(struct ieee80211com *ic, st
> i--);
> ni->ni_txrate = i;
> }
> +
> +#ifndef IEEE80211_STA_ONLY
> +void
> +acx_set_tim(struct ieee80211com *ic, int aid, int set)
> +{
> + struct acx_softc *sc = ic->ic_if.if_softc;
> + struct acx_tmplt_tim tim;
> + u_int8_t *ep;
> +
> + if (set)
> + setbit(ic->ic_tim_bitmap, aid & ~0xc000);
> + else
> + clrbit(ic->ic_tim_bitmap, aid & ~0xc000);
Probably should call ieee80211_set_tim() instead of inlining it here.
That way you hide all dependencies of the implementation of the
ieee80211 TIM code.
