Re: [PATCH 28/33] staging: comedi: cb_das16_cs: cleanup das16cs_ai_rinsn()

[Ian Abbott <abbotti@xxxxxxxxx>]

On 2012-06-26 10:09, Ian Abbott wrote:
> On 2012-06-26 01:17, H Hartley Sweeten wrote:
> > @@ -129,7 +124,22 @@ static int das16cs_ai_rinsn(struct comedi_device *dev,
> >    	outw(devpriv->status1, dev->iobase + 4);
> >
> >    	devpriv->status2 &= ~0xff00;
> > -	devpriv->status2 |= range_bits[range];
> > +	switch (range) {
> > +	case 0:
> > +		devpriv->status2 |= 0x800;
> > +		break;
> > +	case 1:
> > +		devpriv->status2 |= 0x000;
> > +		break;
> > +	case 2:
> > +		devpriv->status2 |= 0x100;
> > +		break;
> > +	case 3:
> > +		devpriv->status2 |= 0x200;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
>
> Is that really an improvement?  The 'range' variable value will be in
> range anyway (the comedi core checks beforehand in
> comedi_check_chanlist()), and looking up the constant to OR with
> devpriv->status2 is probably less object code (and certainly less source
> code).

I meant looking up the constant in the static array of course.  It 
doesn't really matter if you want to do it this way though, and the 
`static int range_bits[]` you removed should have been `static const int 
range_bits[]` anyway.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-


_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel