Re: [PATCH net v4] net: airoha: fix MIB stats collection to be lossless

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]

> Hi Lorenzo,
> Thanks for the review.
> 
> > > +	for (i = AIROHA_GDM1_IDX; i <= AIROHA_GDM4_IDX; i++)
> > > +		airoha_fe_set(eth, REG_FE_GDM_MIB_CLEAR(i),
> > > +			      FE_GDM_MIB_RX_CLEAR_MASK | FE_GDM_MIB_TX_CLEAR_MASK);
> >
> > This configuration is not needed since we reset FE at module load.
> 
> Agreed. The SCU FE reset clears MIB counters, so the explicit loop
> is redundant. Will drop it in v5.
> 
> On Mon, 2 Jul 2026 22:59:32 +0000, Sashiko <sashiko@kernel.org> wrote:
> > With this patch the driver no longer has any users of
> > REG_FE_GDM_MIB_CLEAR, FE_GDM_MIB_RX_CLEAR_MASK or
> > FE_GDM_MIB_TX_CLEAR_MASK; grep in drivers/net/ethernet/airoha/
> > matches only the definitions in airoha_regs.h. Should these macros
> > be removed in the same patch that eliminates their only caller?
> 
> What is your suggestions here, I prefer that definitions should 
> be retained in airoha_regs.h as register documentation for future 
> reference. Only the loop will be dropped.

correct.

Regards,
Lorenzo

> 
> > nit: tmp is not so meaningful, maybe better something like data?
> > nit: I would prefer "+" instead of "|"
> > nit: please drop prev and just do:
> >       dev->stats.tx_ok_pkts = max(data, dev->stats.tx_ok_pkts);
> > please redo it for all the occurrences.
> 
> Understood. Will rename tmp to data, and use '+' instead of '|' in v5.
> 
> Thanks,
> Aniket

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]