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

[Meghana Malladi]

On 6/17/26 10:58, MD Danish Anwar wrote:
> Meghana,
>
> On 16/06/26 6:24 pm, Meghana Malladi wrote:
> > Hi Jakub,
> >
> > On 6/16/26 05:09, Jakub Kicinski wrote:
> > > 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.
> >
> > I agree that this is legit, but right now there is no other place holder
> > other than pa stats to put the mac merge firmware counters. I believe
>
> You can put a boolean is_standard_stats. Only those where
> is_standard_stats=false will be populated via ethtool. Others will be
> populated via the standard interface.
>
> Look at icssg_miig_stats for reference.

Sure, since you were already doing some refactoring w.r.t HSR standard 
stats I thought this could also be covered there.

I will send out another version addressing this then.

> > the effort needs to go in re-structuring the hardware and firmware stats
> > implementation to address this issue.