Re: [PATCH v2 01/18] staging: comedi: comedi_buf: introduce comedi_buf_read_samples()

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

 



On 23/10/14 10:50, Ian Abbott wrote:
On 22/10/14 22:36, H Hartley Sweeten wrote:
Introduce a generic method to read samples from the async buffer.

The number of requested samples is clampled to the number of samples that
would fill the async buffer. The size of each sample is determined using
the bytes_per_sample() helper. The number of bytes need are then read
from the async buffer using comedi_read_array_from_buffer().

This will allow converting all the comedi drivers to use a common method
to read data from the async buffer.

Since comedi_read_array_from_buffer() sets the COMEDI_CB_BLOCK event
after
reading the data, those events can be removed from the drivers.

In addition, comedi_inc_scan_progress() will automatically detect the
end of
scan and set the COMEDI_CB_EOS event. Those events can also be removed
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/comedi_buf.c | 27 +++++++++++++++++++++++++++
  drivers/staging/comedi/comedidev.h  |  2 ++
  2 files changed, 29 insertions(+)

diff --git a/drivers/staging/comedi/comedi_buf.c
b/drivers/staging/comedi/comedi_buf.c
index c60a45ad..88a7cae 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -575,3 +575,30 @@ unsigned int comedi_read_array_from_buffer(struct
comedi_subdevice *s,
      return num_bytes;
  }
  EXPORT_SYMBOL_GPL(comedi_read_array_from_buffer);
+
+/**
+ * comedi_buf_read_samples - read sample data from comedi buffer
+ * @s: comedi_subdevice struct
+ * @data: destination
+ * @nsamples: maximum number of samples to read
+ *
+ * Reads up to nsamples from the comedi buffer associated with the
subdevice,
+ * marks it as read and updates the acquisition scan progress.
+ *
+ * Returns the amount of data read in bytes.
+ */
+unsigned int comedi_buf_read_samples(struct comedi_subdevice *s,
+                     void *data, unsigned int nsamples)
+{
+    unsigned int max_samples;
+    unsigned int nbytes;
+
+    max_samples = s->async->prealloc_bufsz / bytes_per_sample(s);
+    if (nsamples > max_samples)
+        nsamples = max_samples;
+
+    nbytes = nsamples * bytes_per_sample(s);
+
+    return comedi_read_array_from_buffer(s, data, nbytes);
+}
+EXPORT_SYMBOL_GPL(comedi_buf_read_samples);

There is a bit of a problem if the buffer contains a partial sample at
the end (because the file read/write deals with bytes, but the drivers
deal with with 2-or-4-byte samples).  In that case,
comedi_read_array_from_buffer() could return a value that is not a
multiple of the sample width.

For example, cb_pcidas_ao_load_fifo() in your patch 03 could read an odd
number of bytes, round that down to a whole number of samples and ignore
the last byte.  But that last byte comes back to haunt you on the next
call because the next call to comedi_buf_read_samples() would start
reading from the buffer in the middle of a sample.

A simple solution is to change the max_samples assignment to:

     max_samples = comedi_buf_read_n_available(s) / bytes_per_sample(s);

This works because comedi_read_array_from_buffer() should return the
number of bytes you asked for if you've already checked they are there.
  The only time it wouldn't do so is if the buffer is reset inbetween
the calls, and that should only occur at end of acquisition or when the
acquisition is cancelled.

Actually, I see you did exactly the above change to max_samples in your patch 09, so the fact that it is slightly broken between patch 01 and 09 exclusive is only a minor issue.

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