[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CO6PR18MB3873573FA21D4B82A32948B3B0BB9@CO6PR18MB3873.namprd18.prod.outlook.com>
Date: Wed, 27 Jan 2021 18:41:32 +0000
From: Stefan Chulski <stefanc@...vell.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC: Network Development <netdev@...r.kernel.org>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
David Miller <davem@...emloft.net>,
"Nadav Haklai" <nadavh@...vell.com>,
Yan Markman <ymarkman@...vell.com>,
LKML <linux-kernel@...r.kernel.org>,
Jakub Kicinski <kuba@...nel.org>,
"linux@...linux.org.uk" <linux@...linux.org.uk>,
"mw@...ihalf.com" <mw@...ihalf.com>, Andrew Lunn <andrew@...n.ch>,
"rmk+kernel@...linux.org.uk" <rmk+kernel@...linux.org.uk>,
Antoine Tenart <atenart@...nel.org>
Subject: RE: [EXT] Re: [PATCH v4 net-next 10/19] net: mvpp2: add FCA RXQ non
occupied descriptor threshold
>
> > 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?
Firmware poll RXQ non occupied cause register to indicate if number of registers bellow threshold.
We do not trigger any interrupt, just poll this bit in CM3. So this cause always masked.
> >
> > 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.
Ok, I would fix this.
> > /* 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?
Ok, I would move this to separate patch.
> > +/* 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"?
Ok.
Thanks,
Stefan.
Powered by blists - more mailing lists