* [PATCH net-next 0/4] net: stmmac: Improvements for -next
@ 2019-12-10 19:54 Jose Abreu
2019-12-10 19:54 ` [PATCH net-next 1/4] net: stmmac: Print more information in DebugFS DMA Capabilities file Jose Abreu
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Jose Abreu @ 2019-12-10 19:54 UTC (permalink / raw)
To: netdev
Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, David S. Miller, Maxime Ripard, Chen-Yu Tsai,
Maxime Coquelin, linux-arm-kernel, linux-stm32, linux-kernel
Improvements for stmmac.
1) Adds more information regarding HW Caps in the DebugFS file.
2) Prevents incostant bandwidth because of missing interrupts.
3) Allows interrupts to be independently enabled or disabled so that we don't
have to schedule both TX and RX NAPIs.
4) Stops using a magic number in coalesce timer re-arm.
---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-kernel@vger.kernel.org
---
Jose Abreu (4):
net: stmmac: Print more information in DebugFS DMA Capabilities file
net: stmmac: Always arm TX Timer at end of transmission start
net: stmmac: Let TX and RX interrupts be independently
enabled/disabled
net: stmmac: Always use TX coalesce timer value when rescheduling
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 24 +++++-
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h | 11 ++-
drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 47 +++++++++--
drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h | 6 +-
drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c | 22 ++++-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 +
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 24 +++++-
drivers/net/ethernet/stmicro/stmmac/hwif.h | 6 +-
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 +
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 94 +++++++++++++++-------
10 files changed, 183 insertions(+), 54 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH net-next 1/4] net: stmmac: Print more information in DebugFS DMA Capabilities file 2019-12-10 19:54 [PATCH net-next 0/4] net: stmmac: Improvements for -next Jose Abreu @ 2019-12-10 19:54 ` Jose Abreu 2019-12-10 19:54 ` [PATCH net-next 2/4] net: stmmac: Always arm TX Timer at end of transmission start Jose Abreu ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Jose Abreu @ 2019-12-10 19:54 UTC (permalink / raw) To: netdev Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel, linux-kernel DMA Capabilites have grown but the DebugFS that shows this info has not been updated. Lets add the missing information. Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> --- Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com> Cc: Alexandre Torgue <alexandre.torgue@st.com> Cc: Jose Abreu <joabreu@synopsys.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> Cc: netdev@vger.kernel.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 31 ++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index bbc65bd332a8..41d4188fc7bc 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -4238,9 +4238,38 @@ static int stmmac_dma_cap_show(struct seq_file *seq, void *v) priv->dma_cap.number_rx_channel); seq_printf(seq, "\tNumber of Additional TX channel: %d\n", priv->dma_cap.number_tx_channel); + seq_printf(seq, "\tNumber of Additional RX queues: %d\n", + priv->dma_cap.number_rx_queues); + seq_printf(seq, "\tNumber of Additional TX queues: %d\n", + priv->dma_cap.number_tx_queues); seq_printf(seq, "\tEnhanced descriptors: %s\n", (priv->dma_cap.enh_desc) ? "Y" : "N"); - + seq_printf(seq, "\tTX Fifo Size: %d\n", priv->dma_cap.tx_fifo_size); + seq_printf(seq, "\tRX Fifo Size: %d\n", priv->dma_cap.rx_fifo_size); + seq_printf(seq, "\tHash Table Size: %d\n", priv->dma_cap.hash_tb_sz); + seq_printf(seq, "\tTSO: %s\n", priv->dma_cap.tsoen ? "Y" : "N"); + seq_printf(seq, "\tNumber of PPS Outputs: %d\n", + priv->dma_cap.pps_out_num); + seq_printf(seq, "\tSafety Features: %s\n", + priv->dma_cap.asp ? "Y" : "N"); + seq_printf(seq, "\tFlexible RX Parser: %s\n", + priv->dma_cap.frpsel ? "Y" : "N"); + seq_printf(seq, "\tEnhanced Addressing: %d\n", + priv->dma_cap.addr64); + seq_printf(seq, "\tReceive Side Scaling: %s\n", + priv->dma_cap.rssen ? "Y" : "N"); + seq_printf(seq, "\tVLAN Hash Filtering: %s\n", + priv->dma_cap.vlhash ? "Y" : "N"); + seq_printf(seq, "\tSplit Header: %s\n", + priv->dma_cap.sphen ? "Y" : "N"); + seq_printf(seq, "\tVLAN TX Insertion: %s\n", + priv->dma_cap.vlins ? "Y" : "N"); + seq_printf(seq, "\tDouble VLAN: %s\n", + priv->dma_cap.dvlan ? "Y" : "N"); + seq_printf(seq, "\tNumber of L3/L4 Filters: %d\n", + priv->dma_cap.l3l4fnum); + seq_printf(seq, "\tARP Offloading: %s\n", + priv->dma_cap.arpoffsel ? "Y" : "N"); return 0; } DEFINE_SHOW_ATTRIBUTE(stmmac_dma_cap); -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next 2/4] net: stmmac: Always arm TX Timer at end of transmission start 2019-12-10 19:54 [PATCH net-next 0/4] net: stmmac: Improvements for -next Jose Abreu 2019-12-10 19:54 ` [PATCH net-next 1/4] net: stmmac: Print more information in DebugFS DMA Capabilities file Jose Abreu @ 2019-12-10 19:54 ` Jose Abreu 2019-12-10 19:54 ` [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled Jose Abreu 2019-12-10 19:54 ` [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling Jose Abreu 3 siblings, 0 replies; 11+ messages in thread From: Jose Abreu @ 2019-12-10 19:54 UTC (permalink / raw) To: netdev Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel, linux-kernel If TX Coalesce timer is enabled we should always arm it, otherwise we may hit the case where an interrupt is missed and the TX Queue will timeout. Arming the timer does not necessarly mean it will run the tx_clean() because this function is wrapped around NAPI launcher. Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> --- Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com> Cc: Alexandre Torgue <alexandre.torgue@st.com> Cc: Jose Abreu <joabreu@synopsys.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> Cc: netdev@vger.kernel.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 41d4188fc7bc..ae499fdd47bc 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3053,8 +3053,6 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev) tx_q->tx_count_frames = 0; stmmac_set_tx_ic(priv, desc); priv->xstats.tx_set_ic_bit++; - } else { - stmmac_tx_timer_arm(priv, queue); } /* We've used all descriptors we need for this skb, however, @@ -3125,6 +3123,7 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev) tx_q->tx_tail_addr = tx_q->dma_tx_phy + (tx_q->cur_tx * sizeof(*desc)); stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue); + stmmac_tx_timer_arm(priv, queue); return NETDEV_TX_OK; @@ -3276,8 +3275,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) tx_q->tx_count_frames = 0; stmmac_set_tx_ic(priv, desc); priv->xstats.tx_set_ic_bit++; - } else { - stmmac_tx_timer_arm(priv, queue); } /* We've used all descriptors we need for this skb, however, @@ -3366,6 +3363,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) tx_q->tx_tail_addr = tx_q->dma_tx_phy + (tx_q->cur_tx * sizeof(*desc)); stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue); + stmmac_tx_timer_arm(priv, queue); return NETDEV_TX_OK; -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled 2019-12-10 19:54 [PATCH net-next 0/4] net: stmmac: Improvements for -next Jose Abreu 2019-12-10 19:54 ` [PATCH net-next 1/4] net: stmmac: Print more information in DebugFS DMA Capabilities file Jose Abreu 2019-12-10 19:54 ` [PATCH net-next 2/4] net: stmmac: Always arm TX Timer at end of transmission start Jose Abreu @ 2019-12-10 19:54 ` Jose Abreu 2019-12-14 20:36 ` Jakub Kicinski 2019-12-10 19:54 ` [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling Jose Abreu 3 siblings, 1 reply; 11+ messages in thread From: Jose Abreu @ 2019-12-10 19:54 UTC (permalink / raw) To: netdev Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller, Maxime Ripard, Chen-Yu Tsai, Maxime Coquelin, linux-arm-kernel, linux-stm32, linux-kernel By using this mechanism we can get rid of the not so nice method of scheduling TX NAPI when the RX was scheduled. No bandwidth reduction was seen with this change. Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> --- Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com> Cc: Alexandre Torgue <alexandre.torgue@st.com> Cc: Jose Abreu <joabreu@synopsys.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Maxime Ripard <mripard@kernel.org> Cc: Chen-Yu Tsai <wens@csie.org> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> Cc: netdev@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 24 ++++++++-- drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h | 11 +++-- drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 47 ++++++++++++++---- drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h | 6 ++- drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c | 22 +++++++-- drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 + drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 24 ++++++++-- drivers/net/ethernet/stmicro/stmmac/hwif.h | 6 ++- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 + drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 55 +++++++++++++--------- 10 files changed, 150 insertions(+), 48 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index 1c8d84ed8410..6f834302fda3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -335,14 +335,30 @@ static void sun8i_dwmac_dump_mac_regs(struct mac_device_info *hw, } } -static void sun8i_dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan) +static void sun8i_dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan, + bool rx, bool tx) { - writel(EMAC_RX_INT | EMAC_TX_INT, ioaddr + EMAC_INT_EN); + u32 value = readl(ioaddr + EMAC_INT_EN); + + if (rx) + value |= EMAC_RX_INT; + if (tx) + value |= EMAC_TX_INT; + + writel(value, ioaddr + EMAC_INT_EN); } -static void sun8i_dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan) +static void sun8i_dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan, + bool rx, bool tx) { - writel(0, ioaddr + EMAC_INT_EN); + u32 value = readl(ioaddr + EMAC_INT_EN); + + if (rx) + value &= ~EMAC_RX_INT; + if (tx) + value &= ~EMAC_TX_INT; + + writel(value, ioaddr + EMAC_INT_EN); } static void sun8i_dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h index 589931795847..bcb6d5190f3d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h @@ -168,6 +168,8 @@ /* DMA default interrupt mask for 4.00 */ #define DMA_CHAN_INTR_DEFAULT_MASK (DMA_CHAN_INTR_NORMAL | \ DMA_CHAN_INTR_ABNORMAL) +#define DMA_CHAN_INTR_DEFAULT_RX (DMA_CHAN_INTR_ENA_RIE) +#define DMA_CHAN_INTR_DEFAULT_TX (DMA_CHAN_INTR_ENA_TIE) #define DMA_CHAN_INTR_NORMAL_4_10 (DMA_CHAN_INTR_ENA_NIE_4_10 | \ DMA_CHAN_INTR_ENA_RIE | \ @@ -178,6 +180,8 @@ /* DMA default interrupt mask for 4.10a */ #define DMA_CHAN_INTR_DEFAULT_MASK_4_10 (DMA_CHAN_INTR_NORMAL_4_10 | \ DMA_CHAN_INTR_ABNORMAL_4_10) +#define DMA_CHAN_INTR_DEFAULT_RX_4_10 (DMA_CHAN_INTR_ENA_RIE) +#define DMA_CHAN_INTR_DEFAULT_TX_4_10 (DMA_CHAN_INTR_ENA_TIE) /* channel 0 specific fields */ #define DMA_CHAN0_DBG_STAT_TPS GENMASK(15, 12) @@ -186,9 +190,10 @@ #define DMA_CHAN0_DBG_STAT_RPS_SHIFT 8 int dwmac4_dma_reset(void __iomem *ioaddr); -void dwmac4_enable_dma_irq(void __iomem *ioaddr, u32 chan); -void dwmac410_enable_dma_irq(void __iomem *ioaddr, u32 chan); -void dwmac4_disable_dma_irq(void __iomem *ioaddr, u32 chan); +void dwmac4_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx); +void dwmac410_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx); +void dwmac4_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx); +void dwmac410_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx); void dwmac4_dma_start_tx(void __iomem *ioaddr, u32 chan); void dwmac4_dma_stop_tx(void __iomem *ioaddr, u32 chan); void dwmac4_dma_start_rx(void __iomem *ioaddr, u32 chan); diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c index f2a29a90e085..9becca280074 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c @@ -97,21 +97,52 @@ void dwmac4_set_rx_ring_len(void __iomem *ioaddr, u32 len, u32 chan) writel(len, ioaddr + DMA_CHAN_RX_RING_LEN(chan)); } -void dwmac4_enable_dma_irq(void __iomem *ioaddr, u32 chan) +void dwmac4_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx) { - writel(DMA_CHAN_INTR_DEFAULT_MASK, ioaddr + - DMA_CHAN_INTR_ENA(chan)); + u32 value = readl(ioaddr + DMA_CHAN_INTR_ENA(chan)); + + if (rx) + value |= DMA_CHAN_INTR_DEFAULT_RX; + if (tx) + value |= DMA_CHAN_INTR_DEFAULT_TX; + + writel(value, ioaddr + DMA_CHAN_INTR_ENA(chan)); } -void dwmac410_enable_dma_irq(void __iomem *ioaddr, u32 chan) +void dwmac410_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx) { - writel(DMA_CHAN_INTR_DEFAULT_MASK_4_10, - ioaddr + DMA_CHAN_INTR_ENA(chan)); + u32 value = readl(ioaddr + DMA_CHAN_INTR_ENA(chan)); + + if (rx) + value |= DMA_CHAN_INTR_DEFAULT_RX_4_10; + if (tx) + value |= DMA_CHAN_INTR_DEFAULT_TX_4_10; + + writel(value, ioaddr + DMA_CHAN_INTR_ENA(chan)); } -void dwmac4_disable_dma_irq(void __iomem *ioaddr, u32 chan) +void dwmac4_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx) { - writel(0, ioaddr + DMA_CHAN_INTR_ENA(chan)); + u32 value = readl(ioaddr + DMA_CHAN_INTR_ENA(chan)); + + if (rx) + value &= ~DMA_CHAN_INTR_DEFAULT_RX; + if (tx) + value &= ~DMA_CHAN_INTR_DEFAULT_TX; + + writel(value, ioaddr + DMA_CHAN_INTR_ENA(chan)); +} + +void dwmac410_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx) +{ + u32 value = readl(ioaddr + DMA_CHAN_INTR_ENA(chan)); + + if (rx) + value &= ~DMA_CHAN_INTR_DEFAULT_RX_4_10; + if (tx) + value &= ~DMA_CHAN_INTR_DEFAULT_TX_4_10; + + writel(value, ioaddr + DMA_CHAN_INTR_ENA(chan)); } int dwmac4_dma_interrupt(void __iomem *ioaddr, diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h index 292b880f3f9f..e5dbd0bc257e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h @@ -96,6 +96,8 @@ /* DMA default interrupt mask */ #define DMA_INTR_DEFAULT_MASK (DMA_INTR_NORMAL | DMA_INTR_ABNORMAL) +#define DMA_INTR_DEFAULT_RX (DMA_INTR_ENA_RIE) +#define DMA_INTR_DEFAULT_TX (DMA_INTR_ENA_TIE) /* DMA Status register defines */ #define DMA_STATUS_GLPII 0x40000000 /* GMAC LPI interrupt */ @@ -130,8 +132,8 @@ #define NUM_DWMAC1000_DMA_REGS 23 void dwmac_enable_dma_transmission(void __iomem *ioaddr); -void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan); -void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan); +void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx); +void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx); void dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan); void dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan); void dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan); diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c index 1bc25aa86dbd..688d36095333 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c @@ -37,14 +37,28 @@ void dwmac_enable_dma_transmission(void __iomem *ioaddr) writel(1, ioaddr + DMA_XMT_POLL_DEMAND); } -void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan) +void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx) { - writel(DMA_INTR_DEFAULT_MASK, ioaddr + DMA_INTR_ENA); + u32 value = readl(ioaddr + DMA_INTR_ENA); + + if (rx) + value |= DMA_INTR_DEFAULT_RX; + if (tx) + value |= DMA_INTR_DEFAULT_TX; + + writel(value, ioaddr + DMA_INTR_ENA); } -void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan) +void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx) { - writel(0, ioaddr + DMA_INTR_ENA); + u32 value = readl(ioaddr + DMA_INTR_ENA); + + if (rx) + value &= ~DMA_INTR_DEFAULT_RX; + if (tx) + value &= ~DMA_INTR_DEFAULT_TX; + + writel(value, ioaddr + DMA_INTR_ENA); } void dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h index 3b6e559aa0b9..158cf4ad1596 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h @@ -361,6 +361,8 @@ #define XGMAC_TIE BIT(0) #define XGMAC_DMA_INT_DEFAULT_EN (XGMAC_NIE | XGMAC_AIE | XGMAC_RBUE | \ XGMAC_RIE | XGMAC_TIE) +#define XGMAC_DMA_INT_DEFAULT_RX (XGMAC_RBUE | XGMAC_RIE) +#define XGMAC_DMA_INT_DEFAULT_TX (XGMAC_TIE) #define XGMAC_DMA_CH_Rx_WATCHDOG(x) (0x0000313c + (0x80 * (x))) #define XGMAC_RWT GENMASK(7, 0) #define XGMAC_DMA_CH_STATUS(x) (0x00003160 + (0x80 * (x))) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c index 22a7f0cc1b90..ae066f4e99a8 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c @@ -248,14 +248,30 @@ static void dwxgmac2_dma_tx_mode(void __iomem *ioaddr, int mode, writel(value, ioaddr + XGMAC_MTL_TXQ_OPMODE(channel)); } -static void dwxgmac2_enable_dma_irq(void __iomem *ioaddr, u32 chan) +static void dwxgmac2_enable_dma_irq(void __iomem *ioaddr, u32 chan, + bool rx, bool tx) { - writel(XGMAC_DMA_INT_DEFAULT_EN, ioaddr + XGMAC_DMA_CH_INT_EN(chan)); + u32 value = readl(ioaddr + XGMAC_DMA_CH_INT_EN(chan)); + + if (rx) + value |= XGMAC_DMA_INT_DEFAULT_RX; + if (tx) + value |= XGMAC_DMA_INT_DEFAULT_TX; + + writel(value, ioaddr + XGMAC_DMA_CH_INT_EN(chan)); } -static void dwxgmac2_disable_dma_irq(void __iomem *ioaddr, u32 chan) +static void dwxgmac2_disable_dma_irq(void __iomem *ioaddr, u32 chan, + bool rx, bool tx) { - writel(0, ioaddr + XGMAC_DMA_CH_INT_EN(chan)); + u32 value = readl(ioaddr + XGMAC_DMA_CH_INT_EN(chan)); + + if (rx) + value &= ~XGMAC_DMA_INT_DEFAULT_RX; + if (tx) + value &= ~XGMAC_DMA_INT_DEFAULT_TX; + + writel(value, ioaddr + XGMAC_DMA_CH_INT_EN(chan)); } static void dwxgmac2_dma_start_tx(void __iomem *ioaddr, u32 chan) diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h index aa5b917398fe..098fbe7a5862 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h @@ -187,8 +187,10 @@ struct stmmac_dma_ops { void (*dma_diagnostic_fr) (void *data, struct stmmac_extra_stats *x, void __iomem *ioaddr); void (*enable_dma_transmission) (void __iomem *ioaddr); - void (*enable_dma_irq)(void __iomem *ioaddr, u32 chan); - void (*disable_dma_irq)(void __iomem *ioaddr, u32 chan); + void (*enable_dma_irq)(void __iomem *ioaddr, u32 chan, + bool rx, bool tx); + void (*disable_dma_irq)(void __iomem *ioaddr, u32 chan, + bool rx, bool tx); void (*start_tx)(void __iomem *ioaddr, u32 chan); void (*stop_tx)(void __iomem *ioaddr, u32 chan); void (*start_rx)(void __iomem *ioaddr, u32 chan); diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h index d993fc7e82c3..f98c5eefb382 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h @@ -88,6 +88,7 @@ struct stmmac_channel { struct napi_struct rx_napi ____cacheline_aligned_in_smp; struct napi_struct tx_napi ____cacheline_aligned_in_smp; struct stmmac_priv *priv_data; + spinlock_t lock; u32 index; }; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index ae499fdd47bc..f61780ae30ac 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -2069,17 +2069,25 @@ static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan) int status = stmmac_dma_interrupt_status(priv, priv->ioaddr, &priv->xstats, chan); struct stmmac_channel *ch = &priv->channel[chan]; + unsigned long flags; if ((status & handle_rx) && (chan < priv->plat->rx_queues_to_use)) { if (napi_schedule_prep(&ch->rx_napi)) { - stmmac_disable_dma_irq(priv, priv->ioaddr, chan); + spin_lock_irqsave(&ch->lock, flags); + stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 1, 0); + spin_unlock_irqrestore(&ch->lock, flags); __napi_schedule_irqoff(&ch->rx_napi); - status |= handle_tx; } } - if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use)) - napi_schedule_irqoff(&ch->tx_napi); + if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use)) { + if (napi_schedule_prep(&ch->tx_napi)) { + spin_lock_irqsave(&ch->lock, flags); + stmmac_disable_dma_irq(priv, priv->ioaddr, chan, 0, 1); + spin_unlock_irqrestore(&ch->lock, flags); + __napi_schedule_irqoff(&ch->tx_napi); + } + } return status; } @@ -2278,10 +2286,14 @@ static void stmmac_tx_timer(struct timer_list *t) * If NAPI is already running we can miss some events. Let's rearm * the timer and try again. */ - if (likely(napi_schedule_prep(&ch->tx_napi))) + if (likely(napi_schedule_prep(&ch->tx_napi))) { + unsigned long flags; + + spin_lock_irqsave(&ch->lock, flags); + stmmac_disable_dma_irq(priv, priv->ioaddr, ch->index, 0, 1); + spin_unlock_irqrestore(&ch->lock, flags); __napi_schedule(&ch->tx_napi); - else - mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10)); + } } /** @@ -3749,8 +3761,14 @@ static int stmmac_napi_poll_rx(struct napi_struct *napi, int budget) priv->xstats.napi_poll++; work_done = stmmac_rx(priv, budget, chan); - if (work_done < budget && napi_complete_done(napi, work_done)) - stmmac_enable_dma_irq(priv, priv->ioaddr, chan); + if (work_done < budget && napi_complete_done(napi, work_done)) { + unsigned long flags; + + spin_lock_irqsave(&ch->lock, flags); + stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 1, 0); + spin_unlock_irqrestore(&ch->lock, flags); + } + return work_done; } @@ -3759,24 +3777,18 @@ static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget) struct stmmac_channel *ch = container_of(napi, struct stmmac_channel, tx_napi); struct stmmac_priv *priv = ch->priv_data; - struct stmmac_tx_queue *tx_q; u32 chan = ch->index; int work_done; priv->xstats.napi_poll++; - work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan); - work_done = min(work_done, budget); - - if (work_done < budget) - napi_complete_done(napi, work_done); + work_done = stmmac_tx_clean(priv, budget, chan); + if (work_done < budget && napi_complete_done(napi, work_done)) { + unsigned long flags; - /* Force transmission restart */ - tx_q = &priv->tx_queue[chan]; - if (tx_q->cur_tx != tx_q->dirty_tx) { - stmmac_enable_dma_transmission(priv, priv->ioaddr); - stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, - chan); + spin_lock_irqsave(&ch->lock, flags); + stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 0, 1); + spin_unlock_irqrestore(&ch->lock, flags); } return work_done; @@ -4712,6 +4724,7 @@ int stmmac_dvr_probe(struct device *device, for (queue = 0; queue < maxq; queue++) { struct stmmac_channel *ch = &priv->channel[queue]; + spin_lock_init(&ch->lock); ch->priv_data = priv; ch->index = queue; -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled 2019-12-10 19:54 ` [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled Jose Abreu @ 2019-12-14 20:36 ` Jakub Kicinski 2019-12-16 9:18 ` Jose Abreu 0 siblings, 1 reply; 11+ messages in thread From: Jakub Kicinski @ 2019-12-14 20:36 UTC (permalink / raw) To: Jose Abreu Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue, David S. Miller, Maxime Ripard, Chen-Yu Tsai, Maxime Coquelin, linux-arm-kernel, linux-stm32, linux-kernel On Tue, 10 Dec 2019 20:54:43 +0100, Jose Abreu wrote: > @@ -2278,10 +2286,14 @@ static void stmmac_tx_timer(struct timer_list *t) > * If NAPI is already running we can miss some events. Let's rearm > * the timer and try again. > */ > - if (likely(napi_schedule_prep(&ch->tx_napi))) > + if (likely(napi_schedule_prep(&ch->tx_napi))) { > + unsigned long flags; > + > + spin_lock_irqsave(&ch->lock, flags); > + stmmac_disable_dma_irq(priv, priv->ioaddr, ch->index, 0, 1); > + spin_unlock_irqrestore(&ch->lock, flags); > __napi_schedule(&ch->tx_napi); > - else > - mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10)); You should also remove the comment above the if statement if it's really okay to no longer re-arm the timer. No? > + } > } > > /** > @@ -3759,24 +3777,18 @@ static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget) > struct stmmac_channel *ch = > container_of(napi, struct stmmac_channel, tx_napi); > struct stmmac_priv *priv = ch->priv_data; > - struct stmmac_tx_queue *tx_q; > u32 chan = ch->index; > int work_done; > > priv->xstats.napi_poll++; > > - work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan); > - work_done = min(work_done, budget); > - > - if (work_done < budget) > - napi_complete_done(napi, work_done); > + work_done = stmmac_tx_clean(priv, budget, chan); > + if (work_done < budget && napi_complete_done(napi, work_done)) { Not really related to this patch, but this looks a little suspicious. I think the TX completions should all be processed regardless of the budget. The budget is for RX. > + unsigned long flags; > > - /* Force transmission restart */ > - tx_q = &priv->tx_queue[chan]; > - if (tx_q->cur_tx != tx_q->dirty_tx) { > - stmmac_enable_dma_transmission(priv, priv->ioaddr); > - stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, > - chan); > + spin_lock_irqsave(&ch->lock, flags); > + stmmac_enable_dma_irq(priv, priv->ioaddr, chan, 0, 1); > + spin_unlock_irqrestore(&ch->lock, flags); > } > > return work_done; ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled 2019-12-14 20:36 ` Jakub Kicinski @ 2019-12-16 9:18 ` Jose Abreu 2019-12-16 20:14 ` Jakub Kicinski 0 siblings, 1 reply; 11+ messages in thread From: Jose Abreu @ 2019-12-16 9:18 UTC (permalink / raw) To: Jakub Kicinski Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue, David S. Miller, Maxime Ripard, Chen-Yu Tsai, Maxime Coquelin, linux-arm-kernel, linux-stm32, linux-kernel From: Jakub Kicinski <jakub.kicinski@netronome.com> Date: Dec/14/2019, 20:36:23 (UTC+00:00) > On Tue, 10 Dec 2019 20:54:43 +0100, Jose Abreu wrote: > > @@ -2278,10 +2286,14 @@ static void stmmac_tx_timer(struct timer_list *t) > > * If NAPI is already running we can miss some events. Let's rearm > > * the timer and try again. > > */ > > - if (likely(napi_schedule_prep(&ch->tx_napi))) > > + if (likely(napi_schedule_prep(&ch->tx_napi))) { > > + unsigned long flags; > > + > > + spin_lock_irqsave(&ch->lock, flags); > > + stmmac_disable_dma_irq(priv, priv->ioaddr, ch->index, 0, 1); > > + spin_unlock_irqrestore(&ch->lock, flags); > > __napi_schedule(&ch->tx_napi); > > - else > > - mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10)); > > You should also remove the comment above the if statement if it's > really okay to no longer re-arm the timer. No? Yeah, agreed! > > > + } > > } > > > > /** > > > @@ -3759,24 +3777,18 @@ static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget) > > struct stmmac_channel *ch = > > container_of(napi, struct stmmac_channel, tx_napi); > > struct stmmac_priv *priv = ch->priv_data; > > - struct stmmac_tx_queue *tx_q; > > u32 chan = ch->index; > > int work_done; > > > > priv->xstats.napi_poll++; > > > > - work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan); > > - work_done = min(work_done, budget); > > - > > - if (work_done < budget) > > - napi_complete_done(napi, work_done); > > + work_done = stmmac_tx_clean(priv, budget, chan); > > + if (work_done < budget && napi_complete_done(napi, work_done)) { > > Not really related to this patch, but this looks a little suspicious. > I think the TX completions should all be processed regardless of the > budget. The budget is for RX. Well but this is a TX NAPI ... Shouldn't it be limited to prevent CPU starvation ? --- Thanks, Jose Miguel Abreu ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled 2019-12-16 9:18 ` Jose Abreu @ 2019-12-16 20:14 ` Jakub Kicinski 0 siblings, 0 replies; 11+ messages in thread From: Jakub Kicinski @ 2019-12-16 20:14 UTC (permalink / raw) To: Jose Abreu Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue, David S. Miller, Maxime Ripard, Chen-Yu Tsai, Maxime Coquelin, linux-arm-kernel, linux-stm32, linux-kernel On Mon, 16 Dec 2019 09:18:50 +0000, Jose Abreu wrote: > > > @@ -3759,24 +3777,18 @@ static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget) > > > struct stmmac_channel *ch = > > > container_of(napi, struct stmmac_channel, tx_napi); > > > struct stmmac_priv *priv = ch->priv_data; > > > - struct stmmac_tx_queue *tx_q; > > > u32 chan = ch->index; > > > int work_done; > > > > > > priv->xstats.napi_poll++; > > > > > > - work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan); > > > - work_done = min(work_done, budget); > > > - > > > - if (work_done < budget) > > > - napi_complete_done(napi, work_done); > > > + work_done = stmmac_tx_clean(priv, budget, chan); > > > + if (work_done < budget && napi_complete_done(napi, work_done)) { > > > > Not really related to this patch, but this looks a little suspicious. > > I think the TX completions should all be processed regardless of the > > budget. The budget is for RX. > > Well but this is a TX NAPI ... Shouldn't it be limited to prevent CPU > starvation ? It is a bit confusing, but at least netpoll expects the TX completions to be processed with zero budget. Check out poll_one_napi(). ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling 2019-12-10 19:54 [PATCH net-next 0/4] net: stmmac: Improvements for -next Jose Abreu ` (2 preceding siblings ...) 2019-12-10 19:54 ` [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled Jose Abreu @ 2019-12-10 19:54 ` Jose Abreu 2019-12-14 20:28 ` Jakub Kicinski 3 siblings, 1 reply; 11+ messages in thread From: Jose Abreu @ 2019-12-10 19:54 UTC (permalink / raw) To: netdev Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel, linux-kernel When we have pending packets we re-arm the TX timer with a magic value. Change this from the hardcoded value to the pre-defined TX coalesce timer value. Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> --- Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com> Cc: Alexandre Torgue <alexandre.torgue@st.com> Cc: Jose Abreu <joabreu@synopsys.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> Cc: netdev@vger.kernel.org Cc: linux-stm32@st-md-mailman.stormreply.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index f61780ae30ac..726a17d9cc35 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1975,7 +1975,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue) /* We still have pending packets, let's call for a new scheduling */ if (tx_q->dirty_tx != tx_q->cur_tx) - mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10)); + mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(priv->tx_coal_timer)); __netif_tx_unlock_bh(netdev_get_tx_queue(priv->dev, queue)); -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling 2019-12-10 19:54 ` [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling Jose Abreu @ 2019-12-14 20:28 ` Jakub Kicinski 2019-12-16 9:20 ` Jose Abreu 0 siblings, 1 reply; 11+ messages in thread From: Jakub Kicinski @ 2019-12-14 20:28 UTC (permalink / raw) To: Jose Abreu Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue, David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel, linux-kernel On Tue, 10 Dec 2019 20:54:44 +0100, Jose Abreu wrote: > When we have pending packets we re-arm the TX timer with a magic value. > Change this from the hardcoded value to the pre-defined TX coalesce > timer value. s/pre-defined/user controlled/ ? > Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> > --- > Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com> > Cc: Alexandre Torgue <alexandre.torgue@st.com> > Cc: Jose Abreu <joabreu@synopsys.com> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> > Cc: netdev@vger.kernel.org > Cc: linux-stm32@st-md-mailman.stormreply.com > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index f61780ae30ac..726a17d9cc35 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -1975,7 +1975,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue) > > /* We still have pending packets, let's call for a new scheduling */ > if (tx_q->dirty_tx != tx_q->cur_tx) > - mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10)); > + mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(priv->tx_coal_timer)); I think intent of this code is to re-check the ring soon. The same value of 10 is used in stmmac_tx_timer() for quick re-check. tx_coal_timer defaults to 1000, so it's quite a jump from 10 to 1000. I think the commit message leaves too much unsaid. Also if you want to change to the ethtool timeout value, could you move stmmac_tx_timer_arm() and reuse that helper? > > __netif_tx_unlock_bh(netdev_get_tx_queue(priv->dev, queue)); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling 2019-12-14 20:28 ` Jakub Kicinski @ 2019-12-16 9:20 ` Jose Abreu 2019-12-16 20:16 ` Jakub Kicinski 0 siblings, 1 reply; 11+ messages in thread From: Jose Abreu @ 2019-12-16 9:20 UTC (permalink / raw) To: Jakub Kicinski Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue, David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel, linux-kernel From: Jakub Kicinski <jakub.kicinski@netronome.com> Date: Dec/14/2019, 20:28:37 (UTC+00:00) > On Tue, 10 Dec 2019 20:54:44 +0100, Jose Abreu wrote: > > When we have pending packets we re-arm the TX timer with a magic value. > > Change this from the hardcoded value to the pre-defined TX coalesce > > timer value. > > s/pre-defined/user controlled/ ? > > > Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> > > --- > > Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com> > > Cc: Alexandre Torgue <alexandre.torgue@st.com> > > Cc: Jose Abreu <joabreu@synopsys.com> > > Cc: "David S. Miller" <davem@davemloft.net> > > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> > > Cc: netdev@vger.kernel.org > > Cc: linux-stm32@st-md-mailman.stormreply.com > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: linux-kernel@vger.kernel.org > > --- > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > index f61780ae30ac..726a17d9cc35 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > @@ -1975,7 +1975,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue) > > > > /* We still have pending packets, let's call for a new scheduling */ > > if (tx_q->dirty_tx != tx_q->cur_tx) > > - mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10)); > > + mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(priv->tx_coal_timer)); > > I think intent of this code is to re-check the ring soon. The same > value of 10 is used in stmmac_tx_timer() for quick re-check. > > tx_coal_timer defaults to 1000, so it's quite a jump from 10 to 1000. > > I think the commit message leaves too much unsaid. > > Also if you want to change to the ethtool timeout value, could you move > stmmac_tx_timer_arm() and reuse that helper? Yeah, it's a quick re-check but 10us can be too low on some speeds and leads to CPU useless-looping. The intent is to let this always be configurable by user. --- Thanks, Jose Miguel Abreu ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling 2019-12-16 9:20 ` Jose Abreu @ 2019-12-16 20:16 ` Jakub Kicinski 0 siblings, 0 replies; 11+ messages in thread From: Jakub Kicinski @ 2019-12-16 20:16 UTC (permalink / raw) To: Jose Abreu Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue, David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel, linux-kernel On Mon, 16 Dec 2019 09:20:53 +0000, Jose Abreu wrote: > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > index f61780ae30ac..726a17d9cc35 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > @@ -1975,7 +1975,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue) > > > > > > /* We still have pending packets, let's call for a new scheduling */ > > > if (tx_q->dirty_tx != tx_q->cur_tx) > > > - mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10)); > > > + mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(priv->tx_coal_timer)); > > > > I think intent of this code is to re-check the ring soon. The same > > value of 10 is used in stmmac_tx_timer() for quick re-check. > > > > tx_coal_timer defaults to 1000, so it's quite a jump from 10 to 1000. > > > > I think the commit message leaves too much unsaid. > > > > Also if you want to change to the ethtool timeout value, could you move > > stmmac_tx_timer_arm() and reuse that helper? > > Yeah, it's a quick re-check but 10us can be too low on some speeds and > leads to CPU useless-looping. The intent is to let this always be > configurable by user. Okay, please do mention the bump from 10us to the default of 1ms in the commit message, though. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-12-16 20:16 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-10 19:54 [PATCH net-next 0/4] net: stmmac: Improvements for -next Jose Abreu 2019-12-10 19:54 ` [PATCH net-next 1/4] net: stmmac: Print more information in DebugFS DMA Capabilities file Jose Abreu 2019-12-10 19:54 ` [PATCH net-next 2/4] net: stmmac: Always arm TX Timer at end of transmission start Jose Abreu 2019-12-10 19:54 ` [PATCH net-next 3/4] net: stmmac: Let TX and RX interrupts be independently enabled/disabled Jose Abreu 2019-12-14 20:36 ` Jakub Kicinski 2019-12-16 9:18 ` Jose Abreu 2019-12-16 20:14 ` Jakub Kicinski 2019-12-10 19:54 ` [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling Jose Abreu 2019-12-14 20:28 ` Jakub Kicinski 2019-12-16 9:20 ` Jose Abreu 2019-12-16 20:16 ` Jakub Kicinski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).