Re: [PATCH v3 net-next 2/2] net: mscc: ocelot: use bulk reads for stats

[Colin Foster]

Hi Vladimir,

I'm sending out v5 shortly. Sorry I seem to have forgotten to respond.
All changes made, except the SYS << TARGET_OFFSET. The macros for *_rix
use reg##_RSZ for expansion.

On Mon, Jan 31, 2022 at 10:22:55AM +0000, Vladimir Oltean wrote:
> On Mon, Jan 24, 2022 at 11:15:31PM -0800, Colin Foster wrote:
[ ... ]
> >  	mutex_lock(&ocelot->stats_lock);
> >  
> >  	for (i = 0; i < ocelot->num_phys_ports; i++) {
> > +		unsigned int idx = 0;
> 
> It is usual to leave a blank line between variable declarations and code.
> 
> >  		/* Configure the port to read the stats from */
> >  		ocelot_write(ocelot, SYS_STAT_CFG_STAT_VIEW(i), SYS_STAT_CFG);
> >  
> > -		for (j = 0; j < ocelot->num_stats; j++) {
> > -			u32 val;
> > -			unsigned int idx = i * ocelot->num_stats + j;
> > +		list_for_each_entry(region, &ocelot->stats_regions, node) {
> > +			err = ocelot_bulk_read_rix(ocelot, SYS_COUNT_RX_OCTETS,
> 
> I'd be tempted to pass SYS << TARGET_OFFSET here.

Expands to SYS_COUNT_RX_OCTETS_RSZ, defined in
include/soc/mscc/ocelot_sys.h.

> 
> > +						   region->offset, region->buf,
> > +						   region->count);
[ ... ]
> > +@ -2799,6 +2845,10 @@ int ocelot_init(struct ocelot *ocelot)
> >  				 ANA_CPUQ_8021_CFG_CPUQ_BPDU_VAL(6),
> >  				 ANA_CPUQ_8021_CFG, i);
> >  
> > +	ret = ocelot_prepare_stats_regions(ocelot);
> > +	if (ret)
> > +		return ret;
> > +
> 
> Destroy ocelot->stats_queue and ocelot->owq.
> 
> >  	INIT_DELAYED_WORK(&ocelot->stats_work, ocelot_check_stats_work);
> >  	queue_delayed_work(ocelot->stats_queue, &ocelot->stats_work,
> >  			   OCELOT_STATS_CHECK_DELAY);
> > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> > index b66e5abe04a7..837450fdea57 100644
> > --- a/include/soc/mscc/ocelot.h
> > +++ b/include/soc/mscc/ocelot.h
> > @@ -542,6 +542,13 @@ struct ocelot_stat_layout {
> >  	char name[ETH_GSTRING_LEN];
> >  };
> >  
> > +struct ocelot_stats_region {
> > +	struct list_head node;
> > +	u32 offset;
> > +	int count;
> > +	u32 *buf;
> > +};
> > +
> >  enum ocelot_tag_prefix {
> >  	OCELOT_TAG_PREFIX_DISABLED	= 0,
> >  	OCELOT_TAG_PREFIX_NONE,
> > @@ -673,6 +680,7 @@ struct ocelot {
> >  	struct regmap_field		*regfields[REGFIELD_MAX];
> >  	const u32 *const		*map;
> >  	const struct ocelot_stat_layout	*stats_layout;
> > +	struct list_head		stats_regions;
> >  	unsigned int			num_stats;
> >  
> >  	u32				pool_size[OCELOT_SB_NUM][OCELOT_SB_POOL_NUM];
> > -- 
> > 2.25.1
> >