[Originally posted 2013-06-20. Reposting to new driverdev-devel list.]
On 2013/06/19 09:43 PM, H Hartley Sweeten wrote:
On Wednesday, June 19, 2013 10:28 AM, Ian Abbott wrote:
On 2013-06-19 17:22, H Hartley Sweeten wrote:
On Wednesday, June 19, 2013 9:09 AM, Ian Abbott wrote:
On 2013-06-18 21:20, H Hartley Sweeten wrote:
Clean up and simplify this legacy driver.
The async command support (interrupts) still needs a bit of work.
The command support is a bit goofy and does not really follow the
comedi API. This will be addressed.
Command support for reading DIO subdevices is a bit goofy. All of them
pack the single-bit channels somehow. Some drivers just return the data
from all the channels, where bit 'n' corresponds to the data from
channel 'n'. Other drivers (like this one) set bit 'n' to the data read
from channel 'chanlist[n]'. That's probably closer to the spirit of the
handling of the scan data for AI subdevices in that the samples are in
the order specified by the chanlist.
Actually, on closer inspection, this driver is setting bit 'n' according
to whether the desired edge was detected for 'chanlist[n]', not
according to the actual data value of channel 'chanlist[n]'.
But, the logic of setting the desired edge doesn't work.
do_cmd_ioctl() calls comedi_check_chanlist() before calling the
s->do_cmdtest() function. Then, as long as everything is still ok,
the s->do_cmd() function will get called.
comedi_check_chanlist() will check the chanlist[] to make sure all
the channel, range, and aref values are valid for the subdevice. The
range_table for all the subdevices in this driver is range_digital. So:
CR_CHAN() must be < s->n_chan (< 24)
Ok, channels 0-23 are valid.
CR_RANGE() must be < s->range_table->length (< 1)
Not ok, only a range of 0 is valid so only falling-edge detection
can be selected.
CR_AREF() must be part of the aref s->subdev_flags
Not ok, no aref subdev_flags are set so again only falling-edge
detection can be selected.
So, if the user tries to pass a non-zero CR_RANGE() of CR_AREF()
comedi_check_chanlist() will fail with -EINVAL and the command
function is never called.
That's true, so it won't work for falling edge detection. It could be
changed to check for CR_INVERT for falling edge detection instead, maybe
with a bit of sanity checking on the whole chanlist to make sure that it
does not contain both edges of the same channel.
--
-=( 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/driverdev-devel
