On 6/27/25 05:08, Théo Lebrun wrote:
> If HW is RSC capable, it cannot add dummy bytes at the start of IP
Receive-side coalescing? Can you add a brief description of this
feature to your commit message?
> packets. Alignment (ie number of dummy bytes) is configured using the
> RBOF field inside the NCFGR register.
>
> On the software side, the skb_reserve(skb, NET_IP_ALIGN) call must only
> be done if those dummy bytes are added by the hardware; notice the
> skb_reserve() is done AFTER writing the address to the device.
>
> We cannot do the skb_reserve() call BEFORE writing the address because
> the address field ignores the low 2/3 bits. Conclusion: in some cases,
> we risk not being able to respect the NET_IP_ALIGN value (which is
> picked based on unaligned CPU access performance).
>
> Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")
Do any existing MACBs support RSC? Is this a fix?
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> drivers/net/ethernet/cadence/macb.h | 3 +++
> drivers/net/ethernet/cadence/macb_main.c | 21 ++++++++++++++++++---
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index adc70b6efd52b0b11e436c2c95bb5108c40f3490..d42c81cf441ce435cad38e2dfd779b0e6a141bf3 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -523,6 +523,8 @@
> /* Bitfields in DCFG6. */
> #define GEM_PBUF_LSO_OFFSET 27
> #define GEM_PBUF_LSO_SIZE 1
> +#define GEM_PBUF_RSC_OFFSET 26
> +#define GEM_PBUF_RSC_SIZE 1
> #define GEM_PBUF_CUTTHRU_OFFSET 25
> #define GEM_PBUF_CUTTHRU_SIZE 1
> #define GEM_DAW64_OFFSET 23
> @@ -733,6 +735,7 @@
> #define MACB_CAPS_MIIONRGMII BIT(9)
> #define MACB_CAPS_NEED_TSUCLK BIT(10)
> #define MACB_CAPS_QUEUE_DISABLE BIT(11)
> +#define MACB_CAPS_RSC_CAPABLE BIT(12)
No need to be _CAPABLE, we're already _CAPS_
> #define MACB_CAPS_PCS BIT(24)
> #define MACB_CAPS_HIGH_SPEED BIT(25)
> #define MACB_CAPS_CLK_HW_CHG BIT(26)
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 48b75d95861317b9925b366446c7572c7e186628..578e72c7727d4f578478ff2b3d0a6316327271b1 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1317,8 +1317,19 @@ 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.
> + *
> + * Hardware can add dummy bytes if asked using the RBOF
> + * field inside the NCFGR register. That feature isn't
> + * available if hardware is RSC capable.
> + *
> + * We cannot fallback to doing the 2-byte shift before
> + * DMA mapping because the address field does not allow
> + * setting the low 2/3 bits.
> + * It is 3 bits if HW_DMA_CAP_PTP, else 2 bits.
> + */
> + if (!(bp->caps & MACB_CAPS_RSC_CAPABLE))
> + skb_reserve(skb, NET_IP_ALIGN);
> } else {
> desc->ctrl = 0;
> dma_wmb();
> @@ -2787,7 +2798,9 @@ static void macb_init_hw(struct macb *bp)
> macb_set_hwaddr(bp);
>
> config = macb_mdc_clk_div(bp);
> - config |= MACB_BF(RBOF, NET_IP_ALIGN); /* Make eth data aligned */
> + /* Make eth data aligned. If RSC capable, that offset is ignored by HW. */
> + if (!(bp->caps & MACB_CAPS_RSC_CAPABLE))
> + config |= MACB_BF(RBOF, NET_IP_ALIGN);
> config |= MACB_BIT(DRFCS); /* Discard Rx FCS */
> if (bp->caps & MACB_CAPS_JUMBO)
> config |= MACB_BIT(JFRAME); /* Enable jumbo frames */
> @@ -4108,6 +4121,8 @@ static void macb_configure_caps(struct macb *bp,
> dcfg = gem_readl(bp, DCFG2);
> if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0)
> bp->caps |= MACB_CAPS_FIFO_MODE;
> + if (GEM_BFEXT(PBUF_RSC, gem_readl(bp, DCFG6)))
> + bp->caps |= MACB_CAPS_RSC_CAPABLE;
> if (gem_has_ptp(bp)) {
> if (!GEM_BFEXT(TSU, gem_readl(bp, DCFG5)))
> dev_err(&bp->pdev->dev,
>