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

[Ian Abbott <abbotti@mev.co.uk>]

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@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-