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

[Andrew Lunn]

On Fri, Mar 21, 2025 at 08:09:38PM +0100, Théo Lebrun wrote:
> 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?

> 	skb_reserve(skb, NET_IP_ALIGN);

Why not just replace this with

        skb_reserve(skb, 2);

> 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

I don't remember seeing any other driver doing anything like
this. That often means it is wrong....

      Andrew