Subject: Re: [PATCH 4/8] staging: et131x: Use for loop to initialise contiguous macstat registers to zero

From: Mark Einon <[email-masked]>
Date: Thu, 21 Aug 2014 11:05:14 +0100

On Thu, Aug 21, 2014 at 08:40:20AM +0000, David Laight wrote:
> From: Mark Einon
> > Replace a long list of contiguous writel() calls with a for loop iterating
> > over the same address values.
> > 
> > Also remove redundant comments on the macstat registers, the variable names
> > are good enough.
> ...
> > -	writel(0, &macstat->txrx_0_64_byte_frames);
> ...
> > -	writel(0, &macstat->carry_reg2);
> > +	/* initialize all the macstat registers to zero on the device  */
> > +	for (reg = &macstat->txrx_0_64_byte_frames;
> > +	     reg <= &macstat->carry_reg2; reg++)
> > +		writel(0, reg);
> ...
> >  struct macstat_regs {			/* Location: */
> >  	u32 pad[32];			/*  0x6000 - 607C */
> > 
> > -	/* Tx/Rx 0-64 Byte Frame Counter */
> > +	/* counters */
> >  	u32 txrx_0_64_byte_frames;	/*  0x6080 */
> > -
> > -	/* Tx/Rx 65-127 Byte Frame Counter */
> >  	u32 txrx_65_127_byte_frames;	/*  0x6084 */
> 
> I think it would be best to also convert the stats counters to an array.

Hi David, thanks for the review.

There's other code that accesses these registers individually, taking into
account carries - so I don't think using an array would change much, as
we'd still need a way of identifying individual indices.

Cheers,

Mark