Re: [PATCH v4 net-next 10/19] net: mvpp2: add FCA RXQ non occupied descriptor threshold

[Willem de Bruijn]

On Wed, Jan 27, 2021 at 7:26 AM <stefanc@xxxxxxxxxxx> wrote:
>
> From: Stefan Chulski <stefanc@xxxxxxxxxxx>
>
> RXQ non occupied descriptor threshold would be used by
> Flow Control Firmware feature to move to the XOFF mode.
> RXQ non occupied threshold would change interrupt cause
> that polled by CM3 Firmware.
> Actual non occupied interrupt masked and won't trigger interrupt.

Does this mean that this change enables a feature, but it is unused
due to a masked interrupt?

>
> Signed-off-by: Stefan Chulski <stefanc@xxxxxxxxxxx>
> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2.h      |  3 ++
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 46 +++++++++++++++++---
>  2 files changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> index 73f087c..9d8993f 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> @@ -295,6 +295,8 @@
>  #define     MVPP2_PON_CAUSE_TXP_OCCUP_DESC_ALL_MASK    0x3fc00000
>  #define     MVPP2_PON_CAUSE_MISC_SUM_MASK              BIT(31)
>  #define MVPP2_ISR_MISC_CAUSE_REG               0x55b0
> +#define MVPP2_ISR_RX_ERR_CAUSE_REG(port)       (0x5520 + 4 * (port))
> +#define            MVPP2_ISR_RX_ERR_CAUSE_NONOCC_MASK  0x00ff

The indentation in this file is inconsistent. Here even between the
two newly introduced lines.

>  /* Buffer Manager registers */
>  #define MVPP2_BM_POOL_BASE_REG(pool)           (0x6000 + ((pool) * 4))
> @@ -764,6 +766,7 @@
>  #define MSS_SRAM_SIZE          0x800
>  #define FC_QUANTA              0xFFFF
>  #define FC_CLK_DIVIDER         100
> +#define MSS_THRESHOLD_STOP     768
>
>  /* RX buffer constants */
>  #define MVPP2_SKB_SHINFO_SIZE \
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 8f40293a..a4933c4 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1144,14 +1144,19 @@ static inline void mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec)
>  static void mvpp2_interrupts_mask(void *arg)
>  {
>         struct mvpp2_port *port = arg;
> +       int cpu = smp_processor_id();
> +       u32 thread;
>
>         /* If the thread isn't used, don't do anything */
> -       if (smp_processor_id() > port->priv->nthreads)
> +       if (cpu >= port->priv->nthreads)
>                 return;

Here and below, the change from greater than to greater than is really
a (standalone) fix?

> -       mvpp2_thread_write(port->priv,
> -                          mvpp2_cpu_to_thread(port->priv, smp_processor_id()),
> +       thread = mvpp2_cpu_to_thread(port->priv, cpu);
> +
> +       mvpp2_thread_write(port->priv, thread,
>                            MVPP2_ISR_RX_TX_MASK_REG(port->id), 0);
> +       mvpp2_thread_write(port->priv, thread,
> +                          MVPP2_ISR_RX_ERR_CAUSE_REG(port->id), 0);
>  }
>
>  /* Unmask the current thread's Rx/Tx interrupts.
> @@ -1161,20 +1166,25 @@ static void mvpp2_interrupts_mask(void *arg)
>  static void mvpp2_interrupts_unmask(void *arg)
>  {
>         struct mvpp2_port *port = arg;
> -       u32 val;
> +       int cpu = smp_processor_id();
> +       u32 val, thread;
>
>         /* If the thread isn't used, don't do anything */
> -       if (smp_processor_id() > port->priv->nthreads)
> +       if (cpu >= port->priv->nthreads)
>                 return;
>
> +       thread = mvpp2_cpu_to_thread(port->priv, cpu);
> +
>         val = MVPP2_CAUSE_MISC_SUM_MASK |
>                 MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK(port->priv->hw_version);
>         if (port->has_tx_irqs)
>                 val |= MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_MASK;
>
> -       mvpp2_thread_write(port->priv,
> -                          mvpp2_cpu_to_thread(port->priv, smp_processor_id()),
> +       mvpp2_thread_write(port->priv, thread,
>                            MVPP2_ISR_RX_TX_MASK_REG(port->id), val);
> +       mvpp2_thread_write(port->priv, thread,
> +                          MVPP2_ISR_RX_ERR_CAUSE_REG(port->id),
> +                          MVPP2_ISR_RX_ERR_CAUSE_NONOCC_MASK);
>  }
>
>  static void
> @@ -1199,6 +1209,9 @@ static void mvpp2_interrupts_unmask(void *arg)
>
>                 mvpp2_thread_write(port->priv, v->sw_thread_id,
>                                    MVPP2_ISR_RX_TX_MASK_REG(port->id), val);
> +               mvpp2_thread_write(port->priv, v->sw_thread_id,
> +                                  MVPP2_ISR_RX_ERR_CAUSE_REG(port->id),
> +                                  MVPP2_ISR_RX_ERR_CAUSE_NONOCC_MASK);
>         }
>  }
>
> @@ -2404,6 +2417,22 @@ static void mvpp2_txp_max_tx_size_set(struct mvpp2_port *port)
>         }
>  }
>
> +/* Routine set the number of non-occupied descriptors threshold that change
> + * interrupt error cause polled by FW Flow Control
> + */

nit: no need for "Routine". Also, does "change .. cause" mean "that
triggers an interrupt"?