[PATCH 2/6] usb: eth: asix88179: Use a fixed URB buffer size of 16 KiB

[Marek Vasut]

On 1/30/26 9:13 PM, Jonas Karlman wrote:

Hello Jonas,

>>> Is this a USB controller related problem/limitation ? Maybe the DWC2
>>> driver needs fixing instead, to handler larger buffers ?
>>
>> I accidentally mixed up what host controller was being used, the board in
>> question is using a generic-echi controller not the DWC2 as I stated.
> 
> I dove deeper

Thank you for doing that.

> and it looks like there could be an issue with ehci-hcd
> and handling of short packets, see below debug print that dumps the qTD
> and where it can be seen that the first packet is short and using the
> xfr_bytes - totalbytes of the first qTD with a short packet fixes my
> issue.
> 
>    BOOTP broadcast 4
>    ** asix_send_common(), len 342
>    ehci_submit_bulk_msg: dev='usb at ff100000', udev=00000000f9f2d520
>    dev=00000000f9f2d520, pipe=c0018303, buffer=00000000f9ee13c0, length=350, req=0000000000000000
>    QH: next=0x1, alt=0x11, token=0x8c00, totalbytes=0, xfr_bytes=0
>    qTD[0]: next=0x1, alt=0x1, token=0x8c00, totalbytes=0, xfr_bytes=350
>    TOKEN=0x8c00
>    Tx: len = 346, actual = 350, err = 0
> 
>    ax88179_eth_recv: first try, len=0
>    ehci_submit_bulk_msg: dev='usb at ff100000', udev=00000000f9f2d520
>    dev=00000000f9f2d520, pipe=c0010383, buffer=00000000f9f2df00, length=26624, req=0000000000000000
>    QH: next=0x1, alt=0x11, token=0xa7808d00, totalbytes=10112
>    qTD[0]: next=0xf9f2a140, alt=0x1, token=0xbe989d00, totalbytes=16024, xfr_bytes=16384 <<-- SHORT
>    qTD[1]: next=0x1, alt=0x1, token=0xa7808d00, totalbytes=10112, xfr_bytes=10240 <<-- SHORT
>    TOKEN=0xa7808d00
>    ax88179_eth_recv: second try, len=16512
>    ax88179_eth_recv: 2 packets received, pkt header at 112
>    ax88179_eth_recv: return packet of 0 bytes (1 packets left)
>    ax88179_eth_recv: return packet of 0 bytes (0 packets left)
> 
>    ax88179_eth_recv: first try, len=0
>    ehci_submit_bulk_msg: dev='usb at ff100000', udev=00000000f9f2d520
>    dev=00000000f9f2d520, pipe=c0010383, buffer=00000000f9f2df00, length=26624, req=0000000000000000
>    QH: next=0x1, alt=0x11, token=0x27a08d00, totalbytes=10144
>    qTD[0]: next=0xf9f2a140, alt=0x1, token=0x3e889d00, totalbytes=16008, xfr_bytes=16384 <<-- SHORT
>    qTD[1]: next=0x1, alt=0x1, token=0x27a08d00, totalbytes=10144, xfr_bytes=10240 <<-- SHORT
>    TOKEN=0x27a08d00
>    ax88179_eth_recv: second try, len=16480
>    ax88179_eth_recv: 2 packets received, pkt header at 80
>    ax88179_eth_recv: return packet of 5429 bytes (1 packets left)
>    ax88179_eth_recv: return packet of 4149 bytes (0 packets left)

Could you please share the printing diff ?

Maybe I am missing something, but the transfers above are longer than 20 
kiB ? Also, how does the printout look _after_ the change below was 
applied ?

> The diff below seem to solve my issue even better, aligns the ehci
> transfers to only use one qTD with a 20kb buffer for optimal performance.
> 
> But that is probably still not a good solution, ehci-hcd should probably
> gain proper support for short packet handling and usb_ether_register()
> could start using a 4kb aligned rxbuf to make sure 20kb transfers instead
> of 16kb transfers are used for ehci controllers.
> 
> 
> Maybe I should just drop this patch and address your feedback on
> remaining patches for v2?

Maybe at least a subset of these patches can be picked that way, yes.

> diff --git a/drivers/usb/eth/asix88179.c b/drivers/usb/eth/asix88179.c
> index 69d3073b669a..a04952b5eb74 100644
> --- a/drivers/usb/eth/asix88179.c
> +++ b/drivers/usb/eth/asix88179.c
> @@ -475,7 +475,6 @@ static int asix_init_common(struct ueth_data *dev,
>   	/* RX bulk configuration */
>   	asix_write_cmd(dev, AX_ACCESS_MAC, AX_RX_BULKIN_QCTRL, 5, 5, tmp);
>   
> -	dev_priv->rx_urb_size = (1024 * (tmp[3] + 2));
>   	if (*tmp16 & GMII_PHY_PHYSR_FULL)
>   		mode |= AX_MEDIUM_FULL_DUPLEX;
>   	asix_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
> diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c
> index 8bba3e0974e6..02c93b40dd3e 100644
> --- a/drivers/usb/eth/usb_ether.c
> +++ b/drivers/usb/eth/usb_ether.c
> @@ -72,7 +72,7 @@ int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize)
>   	}
>   
>   	ueth->rxsize = rxsize;
> -	ueth->rxbuf = memalign(ARCH_DMA_MINALIGN, rxsize);
> +	ueth->rxbuf = memalign(4096, rxsize);

What is your current ARCH_DMA_MINALIGN ?