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

[Stefan Chulski]

 >
> > 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?

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@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.

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.