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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
