[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAF=yD-Lohx+1DRijK5=qgTj0uctBkS-Loh20zrMF7_Ditb2+pQ@mail.gmail.com>
Date: Wed, 27 Jan 2021 12:36:46 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: stefanc@...vell.com
Cc: Network Development <netdev@...r.kernel.org>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
David Miller <davem@...emloft.net>, nadavh@...vell.com,
ymarkman@...vell.com, LKML <linux-kernel@...r.kernel.org>,
Jakub Kicinski <kuba@...nel.org>, linux@...linux.org.uk,
mw@...ihalf.com, Andrew Lunn <andrew@...n.ch>,
rmk+kernel@...linux.org.uk, Antoine Tenart <atenart@...nel.org>
Subject: Re: [PATCH v4 net-next 10/19] net: mvpp2: add FCA RXQ non occupied
descriptor threshold
On Wed, Jan 27, 2021 at 7:26 AM <stefanc@...vell.com> wrote:
>
> From: Stefan Chulski <stefanc@...vell.com>
>
> 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@...vell.com>
> ---
> 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"?
Powered by blists - more mailing lists