[PATCH 01/23] staging: comedi: cb_pcidas: don't calc ai pacer divisors twice

[Ian Abbott]

On 29/04/14 18:56, Hartley Sweeten wrote:
> On Tuesday, April 29, 2014 7:52 AM, Ian Abbott wrote:
>> On 2014-04-28 19:53, H Hartley Sweeten wrote:
>>> -	/* Write the values of ctr1 and ctr2 into counters 1 and 2 */
>>> -	i8254_load(devpriv->pacer_counter_dio + ADC8254, 0, 1,
>>> -		   devpriv->divisor1, 2);
>>> -	i8254_load(devpriv->pacer_counter_dio + ADC8254, 0, 2,
>>> -		   devpriv->divisor2, 2);
>>> +	i8254_load(timer_base, 0,
>>> +		   1, devpriv->divisor1, I8254_MODE2 | I8254_BINARY);
>>> +	i8254_load(timer_base, 0,
>>> +		   2, devpriv->divisor2, I8254_MODE2 | I8254_BINARY);
>>>    }
>>
>> i8254_load() doesn't take that sort of mode value; it expects a mode
>> value in the range 0 to 5 and shifts it left by one.  The mode values
>> you are passing are pre-shifted, so it the 8254 channels will actually
>> be set to mode 4 with those values.
>
> Hmm.. I don't think I ever noticed that before. Seems kind of odd.
>
> i8254_load()		"mode" is a raw number, I8254_BINARY is enforced
> i8254_set_mode()	"mode" is the enum i8254_mode from comedi.h
>
> To me it seems they should both use the enum i8254_mode to provide
> clearer documentation since the i8254_load() is basically a i8254_set_mode()
> followed by a i8254_write().

Indeed.  It's for historical reasons.  i8254_load() already existed when 
I submitted a patch that included i8254_set_mode(), and I wanted to make 
the mode more generic in case someone really wanted to count in BCD for 
some bizarre reason!  (This predates "staging" by a number of years.)

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