[PATCH] staging: comedi: fix subdev_flags for all digital subdevices
abbotti at mev.co.uk
Thu Jul 25 12:05:30 UTC 2013
On 2013-07-24 21:14, H Hartley Sweeten wrote:
> The SDF_GROUND and SDF_COMMON flags only apply to analog subdevices.
> It makes no sense to set these flags for digital subdevices.
Possibly. It depends on the device. I suppose the reported flags only
matter if the grounding is configurable.
> Digital input (COMEDI_SUBD_DI) subdevices only need the SDF_READABLE
> flag and optionally SDF_CMD_READ it the subdevice supports commands.
> Digital output (COMEDI_SUBD_DO) subdevices only need the SDF_WRITABLE
I think the rule should be that if the INSN_WRITE instruction works it
should have the SDF_WRITABLE flag set and ditto for INSN_READ and
SDF_READABLE. Digital output subdevices generally support INSN_READ
even if its emulated by the insn_bits handler, so ought to have the
SDF_READABLE flag set in that case.
In some drivers the data read back from an digital output subdevice is
just a software shadow of what was last written, although in other
drivers it reads back the data from actual hardware registers. For some
hardware, it's even possible for the data read back to differ from the
data written (for example if the outputs are open collector (or open
drain) outputs, but those should probably be modelled as DIO subdevices
Perhaps __comedi_device_postconfig() in drivers/staging/comedi/drivers.c
should just overwrite the SDF_READABLE and SDF_WRITABLE flags according
to which insn_read and insn_write handlers are set (after they may have
been set to insn_rw_emulate_bits()).
> Digital In/Out (COMEDI_SUBD_DIO) subdevices need both flags.
> The aio_iiro_16 driver sets the type of the subdevices incorrectly.
> Subdevice 0 is actually a COMEDI_SUBD_DO and subdevice 1 is a
> COMEDI_SUBD_DI. Fix them and rename the (*insn_bits) functions.
> The addi_apci_1032 driver incorrectly sets the SDF_CMD_READ subdev_flag
> in the subdevice type. Fix it.
> The adv_pci_dio driver sets the SDF_LSAMPL subdev_flag if the
> number of channels is > 16. This flag has nothing to do with the
> number of channels and is not needed. Remove it.
The SDF_LSAMPL flag might be meaningful if it supported asynchronous
commands, but it doesn't in this case.
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti at mev.co.uk> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
More information about the devel