Re: [PATCH 00/11] staging: comedi: cleanup asynchronous buffer functions

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

 



On 2013-01-08 23:22, H Hartley Sweeten wrote:
On Thursday, January 03, 2013 5:26 AM, Ian Abbott wrote:
On 2012-12-21 16:36, H Hartley Sweeten wrote:
The functions used to interact with the asynchronous buffer currently are
located in the drivers.c source file. The only function drivers.c uses is
comedi_buf_alloc().

For aesthetic reasons and to and help maintainability, separate all the
asynchronous buffer functions out to a new comedi_buf.c source file.

Do a bit of cleanup of the comedi_buf_* functions to clarify them.

H Hartley Sweeten (11):
    staging: comedi: separate out comedi_buf_* functions
    staging: comedi: comedi_buf: factor out common comedi_buf_write_alloc_* code
    staging: comedi: comedi_buf: factor out common code to free the buf_page_list
    staging: comedi: comedi_buf: factor out the new buffer allocation code
    staging: comedi: comedi_buf: consolidate the buffer deallocation code
    staging: comedi: comedi_buf: rename comedi_reset_async_buf()
    staging: comedi: comedi_buf: clarify comedi_buf_write_free()
    staging: comedi: comedi_buf: clarify comedi_buf_read_free()
    staging: comedi: comedi_buf: clarify comedi_buf_read_alloc()
    staging: comedi: comedi_buf: clarify __comedi_buf_write_alloc()
    staging: comedi: comedi_buf: remove comedi_buf_write_alloc_strict()

   drivers/staging/comedi/Makefile          |   3 +-
   drivers/staging/comedi/comedi_buf.c      | 402 +++++++++++++++++++++++++++++++
   drivers/staging/comedi/comedi_fops.c     |   4 +-
   drivers/staging/comedi/comedi_internal.h |   3 +-
   drivers/staging/comedi/comedidev.h       |   2 -
   drivers/staging/comedi/drivers.c         | 398 ------------------------------
   6 files changed, 408 insertions(+), 404 deletions(-)
   create mode 100644 drivers/staging/comedi/comedi_buf.c

I need to check more carefully, but my current view is that patches 01
to 06 and 11 are okay, but 07 to 10 are wrong as they've effectively
replaced signed comparisons with unsigned comparisons leading to
problems when the unsigned integers wrap around modulo 2^32.

Ian,

I just reposted this series based on next-20130108. I think I addressed
all your previous concerns with the int/unsigned int usage.

There could still be a modulo 2^32 issue but I think that problem also
exists with the signed comparisons in the existing code. I think the
changes in the new series (patches 8, 10, and 11) at least make the code
a bit more understandable. If you still have issues with them we can drop
patches 8-11 for now.

I think they're mostly okay apart from the barrier removal in patch 3 (which I should have spotted the first time around).

(Also, being really nit-picky, I guess the comments should should really use the verb "ensure" instead of "insure", but that was a problem with the original comments too!)

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