Search Linux Wireless

Re: [PATCH v2 3/4] ath9k: fix the .flush driver op implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Felix Fietkau wrote:
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
[...]
>  static void ath9k_flush(struct ieee80211_hw *hw, bool drop)
>  {
>        struct ath_softc *sc = hw->priv;
> +       int timeout = 200; /* ms */
> +       int i, j;
>
> +       ath9k_ps_wakeup(sc);
>        mutex_lock(&sc->mutex);
>
>        cancel_delayed_work_sync(&sc->tx_complete_work);
>
> +       if (drop)
> +               timeout = 1;
>
> +       for (j = 0; j < timeout; j++) {
> +               int npend = 0;
> +               for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
> +                       if (!ATH_TXQ_SETUP(sc, i))
> +                               continue;
>
> +                       npend += ath9k_has_pending_frames(sc, &sc->tx.txq[i]);
>                }
> +
> +               if (!npend)
> +                   goto out;
> +
> +               usleep_range(1000, 2000);
>        }
>
> +       if (!ath_drain_all_txq(sc, false))
>                ath_reset(sc, false);
>
> +out:
>        ieee80211_queue_delayed_work(hw, &sc->tx_complete_work, 0);
>        mutex_unlock(&sc->mutex);
> +       ath9k_ps_restore(sc);
>  }

My only comment here is that you still hit the sleep even on the last
pass through the loop when it isn’t strictly necessary. Practically,
the only place this would be realized is the case where |drop| is set.
In this scenario, the code formerly didn’t sleep at all, but now can
sleep for 1-2ms. I couldn’t find any calls to drv_flush with |drop|
set, though, so it might just be an academic concern.

The delay loops in v2 patches 2/4 and 4/4 also share this
characteristic (although with shorter timeouts, and delays instead of
sleeps). Have you considered restructuring these loops to avoid the
final waits?

This is admittedly a pretty minor thing.
--
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


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux