RE: [PATCH 1/6] staging: comedi: addi_apci_1564: remove send_sig() use

[Hartley Sweeten]

On Thursday, May 29, 2014 9:42 PM, Chase Southwood wrote:
> The addi-data drivers use send_sig() to let the user know when an
> interrupt has occurred. The "standard" way to do this in the comedi
> subsystem is to have a subdevice that supports asynchronous commands
> and use comedi_event() to signal the user.
>
> Remove the send_sig() usage in this driver.
>
> Signed-off-by: Chase Southwood <chase.southwood@xxxxxxxxx>
> Cc: Ian Abbott <abbotti@xxxxxxxxx>
> Cc: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
> ---
>  .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 23 ----------------------
>  1 file changed, 23 deletions(-)

Chase,

Normally this patch might be a problem.

With this patch you remove the mechanism that the driver uses to signal the
user that an event has occurred without providing a new mechanism. In theory
this "breaks" the driver.

In reality, the driver is already "broke" since the interrupts are currently enabled
using the digital input subdevice (*insn_config) callback and that callback does not
follow the comedi API. Currently the apci1564_di_config() function is using the
data[0] value as a flag to enable/disable the interrupt instead of using it as the
comedi configuration instruction. It is expecting data[0] to be 1 or 0, when it is 1
it expects the data to include 3 additional parameters but the core will sanity check
the instruction length and return -EINVAL if insn->n != 1. You fix this in patch 5/6.

Because the driver is already "broke" I guess this patch is ok.

Reviewed-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/