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

[Ian Abbott]

On 20/10/14 19:52, H Hartley Sweeten wrote:
> Introduce a generic method to read samples from the async buffer.
>
> The size of each sample is detected automatically using the bytes_per_sample()
> helper. The unsigned long long is used to avoid any possible integer overflow
> when calculating the 'nbytes' for all the requested samples. This value is
> then clampled to the actual size of the async buffer. The samples are read
> from the async buffer using comedi_read_array_from_buffer().
>
> This will allow converting all the comedi drivers to use a common method when
> reading 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 | 23 +++++++++++++++++++++++
>   drivers/staging/comedi/comedidev.h  |  2 ++
>   2 files changed, 25 insertions(+)
>
> diff --git a/drivers/staging/comedi/comedi_buf.c b/drivers/staging/comedi/comedi_buf.c
> index c60a45ad..03b475a 100644
> --- a/drivers/staging/comedi/comedi_buf.c
> +++ b/drivers/staging/comedi/comedi_buf.c
> @@ -575,3 +575,26 @@ 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 long long nbytes = nsamples * bytes_per_sample(s);

That doesn't do what you think.  One of the operands of '*' would have 
to be cast to '(unsigned long long)' first.

The other option is to clamp 'nsamples' to 's->async->prealloc_bufsz / 
bytes_per_sample(s)'.  If there was a log2 version of 
bytes_per_sample(), the division could be replaced with a bit-shift (the 
compiler probably isn't clever enough to do that optimization by 
itself).  On a side-note, 'bytes_per_sample()' ought to be renamed to 
'comedi_bytes_per_sample()' at some point.

> +
> +	if (nbytes > s->async->prealloc_bufsz)
> +		nbytes = s->async->prealloc_bufsz;
> +
> +	return comedi_read_array_from_buffer(s, data, nbytes);
> +}
> +EXPORT_SYMBOL_GPL(comedi_buf_read_samples);
> diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
> index fb8ff84..ba4084b 100644
> --- a/drivers/staging/comedi/comedidev.h
> +++ b/drivers/staging/comedi/comedidev.h
> @@ -453,6 +453,8 @@ unsigned int comedi_write_array_to_buffer(struct comedi_subdevice *s,
>   					  unsigned int num_bytes);
>   unsigned int comedi_read_array_from_buffer(struct comedi_subdevice *s,
>   					   void *data, unsigned int num_bytes);
> +unsigned int comedi_buf_read_samples(struct comedi_subdevice *s,
> +				     void *data, unsigned int nsamples);
>
>   /* drivers.c - general comedi driver functions */
>
>


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-