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

Andrew Lunn
2025-03-21T21:07:52Z

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