| Message ID | 1611488647-12478-10-git-send-email-stefanc@marvell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: mvpp2: Add TX Flow Control support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | fail | Series longer than 15 patches |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 5 of 5 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 5 this patch: 5 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 74 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 5 this patch: 5 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Sun, Jan 24, 2021 at 01:43:58PM +0200, stefanc@marvell.com wrote: > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > index 9d69752..0f5069f 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > @@ -1154,6 +1154,9 @@ static void mvpp2_interrupts_mask(void *arg) > mvpp2_thread_write(port->priv, > mvpp2_cpu_to_thread(port->priv, smp_processor_id()), > MVPP2_ISR_RX_TX_MASK_REG(port->id), 0); > + mvpp2_thread_write(port->priv, > + mvpp2_cpu_to_thread(port->priv, smp_processor_id()), > + MVPP2_ISR_RX_ERR_CAUSE_REG(port->id), 0); I wonder if this should be refactored: u32 thread = mvpp2_cpu_to_thread(port->priv, smp_processor_id()); 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); to avoid having to recompute mvpp2_cpu_to_thread() for each write? However, looking deeper... static void mvpp2_interrupts_mask(void *arg) { struct mvpp2_port *port = arg; u32 thread; int cpu; cpu = smp_processor_id(); if (cpu > port->priv->nthreads) return thread = mvpp2_cpu_to_thread(port->priv, cpu); ... and I wonder about that condition - "cpu > port->priv->nthreads". If cpu == port->priv->nthreads, then mvpp2_cpu_to_thread() will return zero, just like the cpu=0 case. This leads me to suspect that this comparison off by one.
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > @@ -1154,6 +1154,9 @@ static void mvpp2_interrupts_mask(void *arg) > > mvpp2_thread_write(port->priv, > > mvpp2_cpu_to_thread(port->priv, > smp_processor_id()), > > MVPP2_ISR_RX_TX_MASK_REG(port->id), 0); > > + mvpp2_thread_write(port->priv, > > + mvpp2_cpu_to_thread(port->priv, > smp_processor_id()), > > + MVPP2_ISR_RX_ERR_CAUSE_REG(port->id), 0); > > I wonder if this should be refactored: > > u32 thread = mvpp2_cpu_to_thread(port->priv, > smp_processor_id()); > > 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); > > to avoid having to recompute mvpp2_cpu_to_thread() for each write? > > However, looking deeper... > > static void mvpp2_interrupts_mask(void *arg) { > struct mvpp2_port *port = arg; > u32 thread; > int cpu; > > cpu = smp_processor_id(); > if (cpu > port->priv->nthreads) > return > > thread = mvpp2_cpu_to_thread(port->priv, cpu); > ... > > and I wonder about that condition - "cpu > port->priv->nthreads". If cpu == > port->priv->nthreads, then mvpp2_cpu_to_thread() will return zero, just like > the cpu=0 case. This leads me to suspect that this comparison off by one. I can push patch that make it if (cpu => port->priv->nthreads). Or even remove this if. Anyway on current Armada platforms we have only 4 CPU's and maximum 9 PPv2 threads(nthreads is min between num_present_cpus and maximum HW PPv2 threads), so this would be always false. Regards, Stefan.
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h index 0861c0b..3df8f60 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 /* 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 0x140 +#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 9d69752..0f5069f 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -1154,6 +1154,9 @@ static void mvpp2_interrupts_mask(void *arg) mvpp2_thread_write(port->priv, mvpp2_cpu_to_thread(port->priv, smp_processor_id()), MVPP2_ISR_RX_TX_MASK_REG(port->id), 0); + mvpp2_thread_write(port->priv, + mvpp2_cpu_to_thread(port->priv, smp_processor_id()), + MVPP2_ISR_RX_ERR_CAUSE_REG(port->id), 0); } /* Unmask the current thread's Rx/Tx interrupts. @@ -1177,6 +1180,10 @@ static void mvpp2_interrupts_unmask(void *arg) mvpp2_thread_write(port->priv, mvpp2_cpu_to_thread(port->priv, smp_processor_id()), MVPP2_ISR_RX_TX_MASK_REG(port->id), val); + mvpp2_thread_write(port->priv, + mvpp2_cpu_to_thread(port->priv, smp_processor_id()), + MVPP2_ISR_RX_ERR_CAUSE_REG(port->id), + MVPP2_ISR_RX_ERR_CAUSE_NONOCC_MASK); } static void @@ -1201,6 +1208,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); } } @@ -2406,6 +2416,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 + */ +static void mvpp2_set_rxq_free_tresh(struct mvpp2_port *port, + struct mvpp2_rx_queue *rxq) +{ + u32 val; + + mvpp2_write(port->priv, MVPP2_RXQ_NUM_REG, rxq->id); + + val = mvpp2_read(port->priv, MVPP2_RXQ_THRESH_REG); + val &= ~MVPP2_RXQ_NON_OCCUPIED_MASK; + val |= MSS_THRESHOLD_STOP << MVPP2_RXQ_NON_OCCUPIED_OFFSET; + mvpp2_write(port->priv, MVPP2_RXQ_THRESH_REG, val); +} + /* Set the number of packets that will be received before Rx interrupt * will be generated by HW. */ @@ -2661,6 +2687,9 @@ static int mvpp2_rxq_init(struct mvpp2_port *port, mvpp2_rx_pkts_coal_set(port, rxq); mvpp2_rx_time_coal_set(port, rxq); + /* Set the number of non occupied descriptors threshold */ + mvpp2_set_rxq_free_tresh(port, rxq); + /* Add number of descriptors ready for receiving packets */ mvpp2_rxq_status_update(port, rxq->id, 0, rxq->size);