[PATCH] staging: comedi: fix subdev_flags for all digital subdevices

[Ian Abbott]

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
> flag.

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 
by comedi).

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         )=-