Re: [PATCH net-next 07/13] net: macb: move HW IP alignment value to macb_config

From
Andrew Lunn <andrew@lunn.ch>
Date
2025-03-24 18:36:28
Also in
linux-mips, linux-riscv, lkml, netdev
Reply to
Théo Lebrun, 2025-03-24
On Mon, Mar 24, 2025 at 06:49:05PM +0100, Théo Lebrun wrote:
Hello Andrew,

On Fri Mar 21, 2025 at 10:06 PM CET, Andrew Lunn wrote:
quoted
On Fri, Mar 21, 2025 at 08:09:38PM +0100, Théo Lebrun wrote:
quoted
The controller does IP alignment (two bytes).
I'm a bit confused here. Is this hard coded, baked into the silicon?
It will always do IP alignment? It cannot be turned off?
Yes, the alignment is baked inside the silicon.
I looked but haven't seen any register to configure the alignment.

Sorry the commit message isn't clear, it needs improvements.
quoted
quoted
	skb_reserve(skb, NET_IP_ALIGN);
Why not just replace this with

        skb_reserve(skb, 2);
On arm64, NET_IP_ALIGN=0. I don't have HW to test, but the current code
is telling us that the silicon doesn't do alignment on those:
This is part of the confusion. You say the hardware does alignment,
and then say it does not....
   skb = netdev_alloc_skb(...);
   paddr = dma_map_single(..., skb->data, ...);
   macb_set_addr(..., paddr);

   // arm   => NET_IP_ALIGN=2 => silicon does alignment
   // arm64 => NET_IP_ALIGN=0 => silicon doesn't do alignment
   skb_reserve(skb, NET_IP_ALIGN);

The platform we introduce is the first one where the silicon alignment
(0 bytes) is different from the NET_IP_ALIGN value (MIPS, 2 bytes).
This is starting to make it clearer. So the first statement that the
controller does IP alignment (two bytes) is not the full story. I
would start there, explain the full story, otherwise readers get the
wrong idea.
quoted
quoted
    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
I'm not sure this table is useful. What might be more interesting is

     Compatible             |  architecture |  hw_ip_align | value of NET_IP_ALIGN

We can then see if there are cases when the 3rd and 4th column differ.

	Andrew
Thread (38 messages) 38 messages, 6 authors, last reply 2025-03-27
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help