The controller does IP alignment (two bytes). However, we match that on
the software side with:
skb_reserve(skb, NET_IP_ALIGN);
The NET_IP_ALIGN value is arch-dependent and picked based on unaligned
CPU access performance. The hardware alignment value should be
compatible-specific rather than arch-specific. Offer a path forward by
adding a hw_ip_align field inside macb_config.
Values for macb_config->hw_ip_align are picked based on upstream
devicetrees:
Compatible | DTS folders | hw_ip_align
------------------------|---------------------------|----------------
cdns,at91sam9260-macb | arch/arm/ | 2
cdns,macb | arch/{arm,riscv}/ | NET_IP_ALIGN
cdns,np4-macb | NULL | NET_IP_ALIGN
cdns,pc302-gem | NULL | NET_IP_ALIGN
cdns,gem | arch/{arm,arm64}/ | NET_IP_ALIGN
cdns,sam9x60-macb | arch/arm/ | 2
atmel,sama5d2-gem | arch/arm/ | 2
atmel,sama5d29-gem | arch/arm/ | 2
atmel,sama5d3-gem | arch/arm/ | 2
atmel,sama5d3-macb | arch/arm/ | 2
atmel,sama5d4-gem | arch/arm/ | 2
cdns,at91rm9200-emac | arch/arm/ | 2
cdns,emac | arch/arm/ | 2
cdns,zynqmp-gem | *same as xlnx,zynqmp-gem* | 0
cdns,zynq-gem | *same as xlnx,zynq-gem* | 2
sifive,fu540-c000-gem | arch/riscv/ | 2
microchip,mpfs-macb | arch/riscv/ | 2
microchip,sama7g5-gem | arch/arm/ | 2
microchip,sama7g5-emac | arch/arm/ | 2
xlnx,zynqmp-gem | arch/arm64/ | 0
xlnx,zynq-gem | arch/arm/ | 2
xlnx,versal-gem | NULL | NET_IP_ALIGN
Considerations:
- cdns,macb has no match data config.
- cdns,np4-macb, cdns,pc302-gem and xlnx,versal-gem have no upstream
devicetrees using them.
- cdns,gem is used on both arm and arm64 platforms. Those do not have
the same NET_IP_ALIGN value. Also, it uses the same config as
cdns,pc302-gem.
- The default config (default_gem_config) behavior isn't changed:
it uses hw_ip_align == NET_IP_ALIGN.
Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
---
drivers/net/ethernet/cadence/macb.h | 2 ++
drivers/net/ethernet/cadence/macb_main.c | 40 ++++++++++++++++++++++++++++++--
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index e9da6e3b869fc772613a0d6b86308917c9bff7fe..5bf7e7ff70490cdb068bfdbe7cfd5bb8e1db7f86 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1191,6 +1191,7 @@ struct macb_usrio_config {
struct macb_config {
u32 caps;
+ int hw_ip_align;
unsigned int dma_burst_length;
int (*clk_init)(struct platform_device *pdev, struct clk **pclk,
struct clk **hclk, struct clk **tx_clk,
@@ -1295,6 +1296,7 @@ struct macb {
u32 caps;
unsigned int dma_burst_length;
+ int hw_ip_align;
phy_interface_t phy_interface;
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b206966178e3d49a084c754191f77205ff6dd466..b32363ba1ec3be0fc42866c8585f0b465d178220 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1352,8 +1352,26 @@ static void gem_rx_refill(struct macb_queue *queue)
dma_wmb();
macb_set_addr(bp, desc, paddr);
- /* properly align Ethernet header */
- skb_reserve(skb, NET_IP_ALIGN);
+ /* Properly align Ethernet header.
+ *
+ * Here be (small-ish) dragons. 3 features intertwine:
+ * (1) Hardware adds two dummy bytes. Notice how
+ * skb_reserve() is done AFTER dma_map_single().
+ * (2) The NET_IP_ALIGN value is arch dependent.
+ * (3) The low 2/3 bits cannot be picked.
+ * 3 bits if HW_DMA_CAP_PTP.
+ *
+ * Notice how (1) and (2) are unrelated (IP-specific
+ * versus arch-specific) but must agree for a working
+ * system.
+ *
+ * (3) implies we cannot align the IP header (ie respect
+ * NET_IP_ALIGN) if HW does not add two bytes.
+ *
+ * FIXME: migrate away from hw_ip_align == NET_IP_ALIGN
+ * for all compatibles.
+ */
+ skb_reserve(skb, bp->hw_ip_align);
} else {
desc->ctrl = 0;
dma_wmb();
@@ -4994,6 +5012,7 @@ static const struct macb_usrio_config sama7g5_usrio = {
static const struct macb_config fu540_c000_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
MACB_CAPS_GEM_HAS_PTP,
+ .hw_ip_align = 2,
.dma_burst_length = 16,
.clk_init = fu540_c000_clk_init,
.init = fu540_c000_init,
@@ -5003,6 +5022,7 @@ static const struct macb_config fu540_c000_config = {
static const struct macb_config at91sam9260_config = {
.caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
+ .hw_ip_align = 2,
.clk_init = macb_clk_init,
.init = macb_init,
.usrio = &macb_default_usrio,
@@ -5011,6 +5031,7 @@ static const struct macb_config at91sam9260_config = {
static const struct macb_config sama5d3macb_config = {
.caps = MACB_CAPS_SG_DISABLED |
MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
+ .hw_ip_align = 2,
.clk_init = macb_clk_init,
.init = macb_init,
.usrio = &macb_default_usrio,
@@ -5018,6 +5039,7 @@ static const struct macb_config sama5d3macb_config = {
static const struct macb_config pc302gem_config = {
.caps = MACB_CAPS_SG_DISABLED | MACB_CAPS_GIGABIT_MODE_AVAILABLE,
+ .hw_ip_align = NET_IP_ALIGN,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -5026,6 +5048,7 @@ static const struct macb_config pc302gem_config = {
static const struct macb_config sama5d2_config = {
.caps = MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII | MACB_CAPS_JUMBO,
+ .hw_ip_align = 2,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -5035,6 +5058,7 @@ static const struct macb_config sama5d2_config = {
static const struct macb_config sama5d29_config = {
.caps = MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII | MACB_CAPS_GEM_HAS_PTP,
+ .hw_ip_align = 2,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -5044,6 +5068,7 @@ static const struct macb_config sama5d29_config = {
static const struct macb_config sama5d3_config = {
.caps = MACB_CAPS_SG_DISABLED | MACB_CAPS_GIGABIT_MODE_AVAILABLE |
MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII | MACB_CAPS_JUMBO,
+ .hw_ip_align = 2,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -5053,6 +5078,7 @@ static const struct macb_config sama5d3_config = {
static const struct macb_config sama5d4_config = {
.caps = MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
+ .hw_ip_align = 2,
.dma_burst_length = 4,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -5061,6 +5087,7 @@ static const struct macb_config sama5d4_config = {
static const struct macb_config emac_config = {
.caps = MACB_CAPS_NEEDS_RSTONUBR | MACB_CAPS_MACB_IS_EMAC,
+ .hw_ip_align = 2,
.clk_init = at91ether_clk_init,
.init = at91ether_init,
.usrio = &macb_default_usrio,
@@ -5068,6 +5095,7 @@ static const struct macb_config emac_config = {
static const struct macb_config np4_config = {
.caps = MACB_CAPS_USRIO_DISABLED,
+ .hw_ip_align = NET_IP_ALIGN,
.clk_init = macb_clk_init,
.init = macb_init,
.usrio = &macb_default_usrio,
@@ -5077,6 +5105,7 @@ static const struct macb_config zynqmp_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
MACB_CAPS_JUMBO |
MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
+ .hw_ip_align = 0,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = init_reset_optional,
@@ -5087,6 +5116,7 @@ static const struct macb_config zynqmp_config = {
static const struct macb_config zynq_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF |
MACB_CAPS_NEEDS_RSTONUBR,
+ .hw_ip_align = 2,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -5097,6 +5127,7 @@ static const struct macb_config mpfs_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
MACB_CAPS_JUMBO |
MACB_CAPS_GEM_HAS_PTP,
+ .hw_ip_align = 2,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = init_reset_optional,
@@ -5108,6 +5139,7 @@ static const struct macb_config mpfs_config = {
static const struct macb_config sama7g5_gem_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_CLK_HW_CHG |
MACB_CAPS_MIIONRGMII | MACB_CAPS_GEM_HAS_PTP,
+ .hw_ip_align = 2,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -5118,6 +5150,7 @@ static const struct macb_config sama7g5_emac_config = {
.caps = MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII |
MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_MIIONRGMII |
MACB_CAPS_GEM_HAS_PTP,
+ .hw_ip_align = 2,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -5128,6 +5161,7 @@ static const struct macb_config versal_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH | MACB_CAPS_NEED_TSUCLK |
MACB_CAPS_QUEUE_DISABLE,
+ .hw_ip_align = NET_IP_ALIGN,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = init_reset_optional,
@@ -5167,6 +5201,7 @@ static const struct macb_config default_gem_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
MACB_CAPS_JUMBO |
MACB_CAPS_GEM_HAS_PTP,
+ .hw_ip_align = NET_IP_ALIGN,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -5244,6 +5279,7 @@ static int macb_probe(struct platform_device *pdev)
bp->tx_clk = tx_clk;
bp->rx_clk = rx_clk;
bp->tsu_clk = tsu_clk;
+ bp->hw_ip_align = macb_config->hw_ip_align;
bp->jumbo_max_len = macb_config->jumbo_max_len;
if (!hw_is_gem(bp->regs, bp->native_io))
--
2.48.1
