tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

net: stmmac: fix dwmac4 transmit performance regression

dwmac4's transmit performance dropped by a factor of four due to an
incorrect assumption about which definitions are for what. This
highlights the need for sane register macros.

Commit 8409495bf6c9 ("net: stmmac: cores: remove many xxx_SHIFT
definitions") changed the way the txpbl value is merged into the
register:

value = readl(ioaddr + DMA_CHAN_TX_CONTROL(dwmac4_addrs, chan));
- value = value | (txpbl << DMA_BUS_MODE_PBL_SHIFT);
+ value = value | FIELD_PREP(DMA_BUS_MODE_PBL, txpbl);

With the following in the header file:

#define DMA_BUS_MODE_PBL BIT(16)
-#define DMA_BUS_MODE_PBL_SHIFT 16

The assumption here was that DMA_BUS_MODE_PBL was the mask for
DMA_BUS_MODE_PBL_SHIFT, but this turns out not to be the case.

The field is actually six bits wide, buts 21:16, and is called
TXPBL.

What's even more confusing is, there turns out to be a PBLX8
single bit in the DMA_CHAN_CONTROL register (0x1100 for channel 0),
and DMA_BUS_MODE_PBL seems to be used for that. However, this bit
et.al. was listed under a comment "/* DMA SYS Bus Mode bitmap */"
which is for register 0x1004.

Fix this up by adding an appropriately named field definition under
the DMA_CHAN_TX_CONTROL() register address definition.

Move the RPBL mask definition under DMA_CHAN_RX_CONTROL(), correctly
renaming it as well.

Also move the PBL bit definition under DMA_CHAN_CONTROL(), correctly
renaming it.

This removes confusion over the PBL fields.

Fixes: 8409495bf6c9 ("net: stmmac: cores: remove many xxx_SHIFT definitions")
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Bisected-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Link: https://lore.kernel.org/51859704-57fd-4913-b09d-9ac58a57f185@bootlin.com
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Link: https://patch.msgid.link/E1vgY1k-00000003vOC-0Z1H@rmk-PC.armlinux.org.uk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Russell King (Oracle) and committed by
Jakub Kicinski 5ccde4c8 f10ab9d3

+8 -7
+4 -4
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
··· 52 52 u32 rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl; 53 53 54 54 value = readl(ioaddr + DMA_CHAN_RX_CONTROL(dwmac4_addrs, chan)); 55 - value = value | FIELD_PREP(DMA_BUS_MODE_RPBL_MASK, rxpbl); 55 + value = value | FIELD_PREP(DMA_CHAN_RX_CTRL_RXPBL_MASK, rxpbl); 56 56 writel(value, ioaddr + DMA_CHAN_RX_CONTROL(dwmac4_addrs, chan)); 57 57 58 58 if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) && likely(dma_cfg->eame)) ··· 73 73 u32 txpbl = dma_cfg->txpbl ?: dma_cfg->pbl; 74 74 75 75 value = readl(ioaddr + DMA_CHAN_TX_CONTROL(dwmac4_addrs, chan)); 76 - value = value | FIELD_PREP(DMA_BUS_MODE_PBL, txpbl); 76 + value = value | FIELD_PREP(DMA_CHAN_TX_CTRL_TXPBL_MASK, txpbl); 77 77 78 78 /* Enable OSP to get best performance */ 79 79 value |= DMA_CONTROL_OSP; ··· 98 98 /* common channel control register config */ 99 99 value = readl(ioaddr + DMA_CHAN_CONTROL(dwmac4_addrs, chan)); 100 100 if (dma_cfg->pblx8) 101 - value = value | DMA_BUS_MODE_PBL; 101 + value = value | DMA_CHAN_CTRL_PBLX8; 102 102 writel(value, ioaddr + DMA_CHAN_CONTROL(dwmac4_addrs, chan)); 103 103 104 104 /* Mask interrupts by writing to CSR7 */ ··· 116 116 /* common channel control register config */ 117 117 value = readl(ioaddr + DMA_CHAN_CONTROL(dwmac4_addrs, chan)); 118 118 if (dma_cfg->pblx8) 119 - value = value | DMA_BUS_MODE_PBL; 119 + value = value | DMA_CHAN_CTRL_PBLX8; 120 120 121 121 writel(value, ioaddr + DMA_CHAN_CONTROL(dwmac4_addrs, chan)); 122 122
+4 -3
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
··· 24 24 25 25 #define DMA_SYS_BUS_MODE 0x00001004 26 26 27 - #define DMA_BUS_MODE_PBL BIT(16) 28 - #define DMA_BUS_MODE_RPBL_MASK GENMASK(21, 16) 29 27 #define DMA_BUS_MODE_MB BIT(14) 30 28 #define DMA_BUS_MODE_FB BIT(0) 31 29 ··· 66 68 67 69 #define DMA_CHAN_CONTROL(addrs, x) dma_chanx_base_addr(addrs, x) 68 70 71 + #define DMA_CHAN_CTRL_PBLX8 BIT(16) 69 72 #define DMA_CONTROL_SPH BIT(24) 70 73 71 74 #define DMA_CHAN_TX_CONTROL(addrs, x) (dma_chanx_base_addr(addrs, x) + 0x4) 72 75 73 76 #define DMA_CONTROL_EDSE BIT(28) 77 + #define DMA_CHAN_TX_CTRL_TXPBL_MASK GENMASK(21, 16) 74 78 #define DMA_CONTROL_TSE BIT(12) 75 79 #define DMA_CONTROL_OSP BIT(4) 76 80 #define DMA_CONTROL_ST BIT(0) 77 81 78 82 #define DMA_CHAN_RX_CONTROL(addrs, x) (dma_chanx_base_addr(addrs, x) + 0x8) 79 83 80 - #define DMA_CONTROL_SR BIT(0) 84 + #define DMA_CHAN_RX_CTRL_RXPBL_MASK GENMASK(21, 16) 81 85 #define DMA_RBSZ_MASK GENMASK(14, 1) 86 + #define DMA_CONTROL_SR BIT(0) 82 87 83 88 #define DMA_CHAN_TX_BASE_ADDR_HI(addrs, x) (dma_chanx_base_addr(addrs, x) + 0x10) 84 89 #define DMA_CHAN_TX_BASE_ADDR(addrs, x) (dma_chanx_base_addr(addrs, x) + 0x14)