Re: [PATCH net-next v5 2/5] net: cadence: macb: implement EEE TX LPI support

[Jakub Kicinski]

On Tue, 03 Mar 2026 09:14:41 +0100 Théo Lebrun wrote:
> >> +/* Wake from LPI before transmitting. The MAC must deassert TXLPIEN
> >> + * and wait for the PHY to exit LPI before any frame can be sent.
> >> + * IEEE 802.3az Tw_sys is ~17us for 1000BASE-T, ~30us for 100BASE-TX;
> >> + * we use a conservative 50us.
> >> + */
> >> +static void macb_tx_lpi_wake(struct macb *bp)
> >> +{
> >> +	if (!macb_tx_lpi_set(bp, false))  
> >
> > Does this lpi_set() not have a relatively high cost, even if eee_active
> > is disabled? Reading registers is usually pretty slow. Can we add 
> > a eee_active check here as well to short cut the lpi check? 
> > If we do we probably want to make sure that the code paths setting
> > eee_active are also under bp->lock, otherwise this new check will be
> > racy.  
> 
> Funny how this discussion keeps coming up! I made the same remark on V3:
> https://lore.kernel.org/netdev/DGOXXGNSSMYK.2XNU9AQ6E077P@xxxxxxxxxxx/
> 
> And it had been discussed in a previous iteration before.
> 
> In theory I agree, in practice the optimization was statistically
> insignificant on my platform. The total time spent in macb_start_xmit()
> is tiny, so any optimization inside of it is even more so.

TBH I started looking around because the v5 implementation seems racy.
Probably not in practice but in theory on a multiple queue device you
clear LPI, release the lock, then another queue may schedule LPI again,
if the xmit path is delayed for a long time the LPI work may turn idle
on before xmit rings the doorbell.

The rework I suggested is not only more optimal (dare I say logical to
an experienced developer) but also I think it'd be more correct.

macb has a crazy number of locks so maybe i'm missing something.
But sooner or later someone will hopefully start removing those locks,
cause this driver gotta be dog slow right now :/