lkml.org 
[lkml]   [2020]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] ath10k: Keep track of which interrupts fired, don't poll them
    On Tue, Jul 7, 2020 at 10:18 AM Douglas Anderson <dianders@chromium.org> wrote:
    > diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
    > index a440aaf74aa4..666ce384a1d8 100644
    > --- a/drivers/net/wireless/ath/ath10k/ce.h
    > +++ b/drivers/net/wireless/ath/ath10k/ce.h
    ...
    > @@ -376,12 +377,9 @@ static inline u32 ath10k_ce_interrupt_summary(struct ath10k *ar)
    > {
    > struct ath10k_ce *ce = ath10k_ce_priv(ar);
    >
    > - if (!ar->hw_params.per_ce_irq)

    If I'm reading correctly, you're removing the only remaining use of
    'per_ce_irq'. Should we kill the field entirely? Or perhaps we should
    leave some kind of WARN_ON() (BUG_ON()?) if this function is called
    erroneously with per_ce_irq==true? But I suppose this driver is full
    of landmines if the CE API is used incorrectly.

    > - return CE_WRAPPER_INTERRUPT_SUMMARY_HOST_MSI_GET(
    > - ce->bus_ops->read32((ar), CE_WRAPPER_BASE_ADDRESS +
    > - CE_WRAPPER_INTERRUPT_SUMMARY_ADDRESS));
    > - else
    > - return ath10k_ce_gen_interrupt_summary(ar);
    > + return CE_WRAPPER_INTERRUPT_SUMMARY_HOST_MSI_GET(
    > + ce->bus_ops->read32((ar), CE_WRAPPER_BASE_ADDRESS +
    > + CE_WRAPPER_INTERRUPT_SUMMARY_ADDRESS));
    > }
    >
    > /* Host software's Copy Engine configuration. */

    > diff --git a/drivers/net/wireless/ath/ath10k/snoc.h b/drivers/net/wireless/ath/ath10k/snoc.h
    > index a3dd06f6ac62..5095d1893681 100644
    > --- a/drivers/net/wireless/ath/ath10k/snoc.h
    > +++ b/drivers/net/wireless/ath/ath10k/snoc.h
    > @@ -78,6 +78,7 @@ struct ath10k_snoc {
    > unsigned long flags;
    > bool xo_cal_supported;
    > u32 xo_cal_data;
    > + DECLARE_BITMAP(pending_ce_irqs, CE_COUNT_MAX);

    Do you need to clear this map if the interface goes down or if there's
    a firmware crash? Right now, I don't think there's a guarantee that
    we'll run through a NAPI poll in those cases, which is the only place
    you clear the map, and if the hardware/firmware has been reset, the
    state map is probably not valid.

    Otherwise, looks OK to me:

    Reviewed-by: Brian Norris <briannorris@chromium.org>

    \
     
     \ /
      Last update: 2020-07-09 01:04    [W:2.655 / U:0.132 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site