On Thursday, October 23, 2014 4:28 AM, Ian Abbott wrote: > On 22/10/14 23:37, H Hartley Sweeten wrote: >> Subdevices that set the SDF_PACKED flag return all the scan data in a single >> sample. The cmd->chanlist_len is used to pass the DIO channel information to >> the command and does not indicate the length (cmd->scan_end_arg) of the scan. > > In general, the scan data could be packed into more than one "sample", > but you can fit 8*bytes_per_sample() 1-bit samples in each "sample". In > practice, currently only the ni_pcidio driver sets the SDF_PACKED flag > (although some other drivers ought to set it) and the whole scan will > fit into a single "sample" in that case. Patches 47, 48, 50, and 51 add the SDF_PACKED flag to the addi_apci_2032, amplc_dio200_common, pcmmio, and pcmuio drivers. >> Currently this flag is not handled in the comedi core files. Modify >> comedi_inc_scan_progress() to automatically set the COMEDI_CB_EOS event if >> the SDF_PACKED flag is set. This makes the flag work and we can remove the >> events from the drivers. >> >> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx> >> Cc: Ian Abbott <abbotti@xxxxxxxxx> >> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >> --- >> drivers/staging/comedi/drivers.c | 19 +++++++++++++++---- >> 1 file changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c >> index ff2df85..dd8f5db 100644 >> --- a/drivers/staging/comedi/drivers.c >> +++ b/drivers/staging/comedi/drivers.c >> @@ -341,12 +341,23 @@ void comedi_inc_scan_progress(struct comedi_subdevice *s, >> unsigned int num_bytes) >> { >> struct comedi_async *async = s->async; >> - unsigned int scan_length = comedi_bytes_per_scan(s); >> >> - async->scan_progress += num_bytes; >> - if (async->scan_progress >= scan_length) { >> - async->scan_progress %= scan_length; >> + /* >> + * The SDF_PACKED flag indicates that the subdevice does "packed DIO". >> + * >> + * This means that all the scan data is returned in a single sample >> + * and an "end of scan" occurs for every sample. >> + */ >> + if (s->subdev_flags & SDF_PACKED) { >> async->events |= COMEDI_CB_EOS; >> + } else { >> + unsigned int scan_length = comedi_bytes_per_scan(s); >> + >> + async->scan_progress += num_bytes; >> + if (async->scan_progress >= scan_length) { >> + async->scan_progress %= scan_length; >> + async->events |= COMEDI_CB_EOS; >> + } >> } >> } >> EXPORT_SYMBOL_GPL(comedi_inc_scan_progress); >> > > Is this patch even necessary? comedi_bytes_per_scan() assumes digital > samples will be packed into a whole number of "samples" and calculates > the scan length in bytes accordingly. I guess it's not really "necessary" but it adds clarification to what the SDF_PACKED flag does. Right now the only documentation for that flag is: #define SDF_PACKED 0x20000000 /* subdevice can do packed DIO */ But nothing in the core uses it. I don't think comedilib references the flag either. It took me a bit of head scratching to figure out why the ni_pcidio driver would be setting the flag. This seemed like the best way to make the core use the flag for something. As you pointed out, this will not work if a driver packs a scan into more than one "sample" (32-bits if the SDF_LSAMPL flag is set, 16-bits if not). But, it works for all the current comedi drivers. If necessary, a separate patch could be made to make it work if a driver does use more than one "sample" to pack the scan. Regards, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel