Re: [PATCH net v2 1/6] net: bcm4908_enet: fix the TX ring wrap

[Kordian Chmielewski <kudzo51@gmail.com>]

On Fri, 13 Nov 2026 20:42:07 +0000, netdev-bot+sashiko@kernel.org wrote:
> - [Medium] net: bcm4908_enet: missing barriers and late write_idx update
>   on TX ring

Both parts are older than this series, and this patch doesn't change
them.

For the barriers, v3 adds a patch in which each side publishes its
index with smp_store_release() and reads the other side's index with
smp_load_acquire() before it touches the slots. It also covers the
bcm4908_enet_poll_tx() issue below.

As for the late write_idx update, start_xmit() has updated write_idx
after kicking the DMA since the driver was added, but that only became
a race when the TX interrupt support moved the TX cleanup out of
start_xmit(). In v3, start_xmit() publishes write_idx right after the
descriptor, before the MMIO write that kicks the DMA, so the window no
longer spans that write. As the review says, though, it can still be
there if the channel is busy with earlier frames and picks up the
descriptor as soon as it sees OWN. I'll look at it separately.

> Pre-existing issues:
> - [High] When the device tree has no "tx" interrupt, TX completions never
>   run, so the queue stop/wake design this patch adds cannot work.

The v1 review found this too. Without the TX interrupt, open() never
enables the TX NAPI, so nothing frees the sent frames and TX stops for
good once the queue is stopped, and stop() hangs in napi_disable().
Every in-tree device tree that uses this driver has the TX interrupt.
Florian, Rafał, would you rather have the driver require it (and the
binding say so), or poll the ring on a timer when it is missing? I'd
send that after this series.

> - [High] bcm4908_enet_poll_tx() reads the producer index without a lock
>   or ordering, then reads descriptor and slot contents with no read…

Covered by the new patch in v3, as above.