@@ -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;
@@ -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))
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@bootlin.com> --- drivers/net/ethernet/cadence/macb.h | 2 ++ drivers/net/ethernet/cadence/macb_main.c | 40 ++++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-)