From: Michael Buesch <mb@bu3sch.de>
To: bcm43xx-dev@lists.berlios.de
Subject: Re: [RFT] [PATCH] bcm43xx: ACI fixes
Date: Wed, 14 Mar 2007 15:17:52 +0100
Cc: Stefano Brivio <stefano.brivio@polimi.it>,
	Larry Finger <larry.finger@lwfinger.net>,
	linux-wireless@vger.kernel.org
References: <20070312010420.353a59c0@localhost> <45F78498.8080608@lwfinger.net> <20070314074118.466bb9f0@localhost>
In-Reply-To: <20070314074118.466bb9f0@localhost>
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-1"
Message-Id: <200703141517.52624.mb@bu3sch.de>
Sender: linux-wireless-owner@vger.kernel.org

On Wednesday 14 March 2007 07:41, Stefano Brivio wrote:
> > > -	   aci_hw_rssi:1;
> > > +	   aci_hw_rssi:1,
> > > +	   aci_delay:5;
> > Should this be a bit field? Why not just make it a u8?
> 
> We only need 5 bits, as aci_delay will never be > 20.

I actually try to move away from the foo:x notation, as it generates bad code.
There's no benefit in saving 3 bits here and adding 100byte of additional
code, which is generated by gcc to mask and shift the bits. :)

> > > +	u8 aci_start;
> > >  };
> > >  
> > >  /* Data structures for DMA transmission, per 80211 core. */
> > > @@ -699,6 +708,18 @@ struct bcm43xx_noise_calculation {
> > >  	s8 samples[8][4];
> > >  };
> > >  
> > > +/* Statscounter data (currently ACI only). */
> > > +struct bcm43xx_statscounter_saved {
> > > +	u16 aci;
> > > +};
> > > +
> > > +/* Values for ACI moving average calculation. */
> > > +struct bcm43xx_aci_saved {
> > > +	u16 value[8];
> > > +	u8 next:3,
> > > +	   set:3;
> > As I will explain later, I don't think 'set' is needed. I also recommend
> > having 'next' be a plain u8.
> 
> Both values will never be > 7.

Please do u8 nevertheless to get better code.

-- 
Greetings Michael.