Re: [PATCH] staging: comedi: comedi_buf: introduce comedi_buf_n_bytes_ready()

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

 



On 2014-06-04 19:30, Hartley Sweeten wrote:
On Wednesday, June 04, 2014 11:17 AM, Ian Abbott wrote:
On 2014-05-29 18:38, H Hartley Sweeten wrote:
Introduce a helper function to return the number of bytes that are
ready to read/write from/to the comedi_async buffer. The "write to"
use doesn't really make much sense but is handled for completeness.

Use the helper function in the comedi drivers that currently do the
calculation as part of the (*poll) operation.

Also, use the helper in comedi_fops where the calculation is used as
part of the subdevice going nonbusy.

Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregk@xxxxxxxxxxxxxxxxxxx>
---
   drivers/staging/comedi/comedi_buf.c            | 18 ++++++++++++++++++
   drivers/staging/comedi/comedi_fops.c           |  5 ++---
   drivers/staging/comedi/comedidev.h             |  2 ++
   drivers/staging/comedi/drivers/das16m1.c       |  2 +-
   drivers/staging/comedi/drivers/das1800.c       |  2 +-
   drivers/staging/comedi/drivers/ni_mio_common.c |  2 +-
   drivers/staging/comedi/drivers/ni_pcidio.c     |  2 +-
   drivers/staging/comedi/drivers/pcl812.c        |  2 +-
   drivers/staging/comedi/drivers/pcl816.c        |  2 +-
   9 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/comedi_buf.c b/drivers/staging/comedi/comedi_buf.c
index df4a9c4..52fc634 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -243,6 +243,24 @@ void comedi_buf_reset(struct comedi_subdevice *s)
   	async->events = 0;
   }

+/**
+ * comedi_buf_n_bytes_ready() - Returns the number of bytes ready to read/write
+ * @s: comedi_subdevice struct
+ */
+unsigned int comedi_buf_n_bytes_ready(struct comedi_subdevice *s)
+{
+	struct comedi_async *async = s->async;
+
+	if (async->buf_write_count == async->buf_read_count)
+		return 0;
+
+	if (async->buf_write_count > async->buf_read_count)
+		return async->buf_write_count - async->buf_read_count;
+	else
+		return async->buf_read_count - async->buf_write_count;
+}
+EXPORT_SYMBOL_GPL(comedi_buf_n_bytes_ready);
+

This will actually break things.  The buf_write_count, buf_read_count,
buf_write_alloc_count, buf_read_alloc_count, and munge_count members of
struct comedi_async are all supposed to work modulo 2^32 (UINT_MAX+1).
Logically (with big-ints):

buf_read_count <= buf_read_alloc_count <= munge_count <= buf_write_count
<= buf_write_alloc_count <= (buf_read_count + prealloc_bufsz)

and logically they only increase in value, but in practice are all
Reduced modulo 2^32.  The upshot is that this function should always return:

	return async->buf_write_count - async->buf_read_count;

which could easily be inlined to avoid an external function call.

I don't see how it "breaks" anything since the two if cases will return the
same result as your proposal above. And if the logic of your "count" values
is correct, and I assume it is, then the 'else' case could never happen.

It breaks when buf_write_count has wrapped around more often than buf_read_count. For example if buf_read_count is near the 2^32 boundary and buf_write_count has already gone past the 2^32 boundary then your code would hit the 'else' (because of the modulo arithmetic) and return buf_read_count - buf_write_count instead of buf_write_count - buf_read_count.


Regardless, I like your inline proposal better than the exported function.

I'll rebase this as suggested.

Thanks,
Hartley



Cheers,
Ian.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
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