[PATCH 00/32] staging: comedi: continue async command cleanup

Hartley Sweeten HartleyS at visionengravers.com
Thu May 1 16:31:11 UTC 2014


On Thursday, May 01, 2014 2:46 AM, Ian Abbott wrote:
> On 2014-04-30 17:46, Hartley Sweeten wrote:
>> On Wednesday, April 30, 2014 2:13 AM, Ian Abbott wrote:
>>> As a side node, I wonder if it's worth stripping out those `|
>>> I8254_BINARY` bits as it's basically 'OR'ing with zero anyway.
>>
>> I like the I8254_BINARY, it documents the mode that the counter
>> is used in. But, if you want to strip them out...
>
> Fair enough.
>
>> BTW, I noticed that the i8254_set_mode() helpers have a slight "bug".
>>
>> 	if (mode > (I8254_MODE5 | I8254_BINARY))
>> 		return -1;
>>
>> This test will not allow the mode (I8254_MODE5 | I8254_BCD). Nothing
>> uses it right now, and it's a bit silly to use BCD counting anyway. But...
>
> It does get exposed to user-space by a few drivers that expose the 8254 
> as a counter subdevice.  The "adv_pci_dio" and "das08" drivers expose it 
> when handling the INSN_CONFIG_SET_COUNTER_MODE instruction (also, 
> "me4000" driver handling it as a GPCT_SET_OPERATION instruction whatever 
> that is - it should probably be deprecated in favour of 
> INSN_CONFIG_SET_COUNTER_MODE).  Also, the "amplc_dio200_common" module 
> has the same bug in its handling of INSN_CONFIG_SET_COUNTER_MODE but 
> uses its own function to set the mode instead of calling i8254_set_mode().
>
> I'll post some patches to fix it later.  Probably not worth backporting 
> them to "stable".

Great!

BTW, what's with all the NI_GPCT_* stuff in the main comedi.h header?

These all seem pretty driver specific to ni_tio.c and ni_tiocmd.c. I don't
understand why they are exposed in the user space header.

Thanks,
Hartley


More information about the devel mailing list