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

Ian Abbott abbotti at mev.co.uk
Thu Oct 23 09:56:18 UTC 2014


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 at visionengravers.com>
>> Cc: Ian Abbott <abbotti at mev.co.uk>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> ---
>>   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 at mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-


More information about the devel mailing list