Re: [PATCH net-next v7 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 15 Jun 2026 16:10:41 -0700 Jakub Kicinski wrote:
> > diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
> > index 5ec0b38e0c67..8073deac35c3 100644
> > --- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
> > +++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
> > @@ -189,6 +187,11 @@ static const struct icssg_pa_stats icssg_all_pa_stats[] = {
> >  	ICSSG_PA_STATS(FW_INF_DROP_PRIOTAGGED),
> >  	ICSSG_PA_STATS(FW_INF_DROP_NOTAG),
> >  	ICSSG_PA_STATS(FW_INF_DROP_NOTMEMBER),
> > +	ICSSG_PA_STATS(FW_PREEMPT_BAD_FRAG),
> > +	ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_ERR),
> > +	ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_TX),
> > +	ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_OK),
> > +	ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_RX),
> >  	ICSSG_PA_STATS(FW_RX_EOF_SHORT_FRMERR),
> >  	ICSSG_PA_STATS(FW_RX_B0_DROP_EARLY_EOF),
> >  	ICSSG_PA_STATS(FW_TX_JUMBO_FRM_CUTOFF),  
> 
> [Medium]
> Are these five new entries duplicating values that already have a
> standard uAPI?
> 
> The same five firmware counters are exposed through the new
> .get_mm_stats callback as the standardized MAC Merge stats
> (MACMergeFrameAssOkCount, MACMergeFrameAssErrorCount, MACMergeFragCountRx,
> MACMergeFragCountTx, MACMergeFrameSmdErrorCount in struct
> ethtool_mm_stats), and adding them to icssg_all_pa_stats[] also
> publishes them via emac_get_strings() / emac_get_ethtool_stats() as
> ethtool -S strings.
> 
> Documentation/networking/statistics.rst describes ethtool -S as the
> private-driver-stats interface; counters that have a standard uAPI are
> expected to flow only through that uAPI.
> 
> Could the firmware-register lookup table used by emac_get_stat_by_name()
> be separated from the ethtool -S string table, so the new preemption
> counters feed get_mm_stats without also showing up under ethtool -S?

This -- not sure about the other complaints but this one looks legit.