Re: [PATCH 46/55] staging: comedi: drivers: handle SDF_PACKED in comedi_inc_scan_progress()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23/10/14 17:35, Hartley Sweeten wrote:
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.

I appreciate that. It was something I was hoping to get around to, especially as pcmmio and pcmuio had endian issues.

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.

Applications could see it, though they would probably ignore it as well, as any application using async commands for digital I/O probably needs to be hardware specific as the order of data in the scan data doesn't match the order of channels in the chanlist for all drivers. (Some just return all channels in channel number order.)

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.

But the original already works in that case!

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux