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.
Regardless, I like your inline proposal better than the exported function.
I'll rebase this as suggested.
Thanks,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
