Re: [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues

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

 



On Thu, Apr 12, 2018 at 05:50:31PM +0200, Sergio Paracuellos wrote:
> This commit replace current custom implementation of some circular
> buffer head and tail logic in favour of the use of macros defined
> in linux circ_buf.h header. It also review internal names and adds
> a new CIRC_INC macro to make code more readable. Note also that
> CIRC_INC does not need to go inside do-while(0) block because its
> use is only located in the four functions that make use of it 
> so it won't expand into invalid code at all.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> ---
>  drivers/staging/ks7010/ks7010_sdio.c | 59 ++++++++++++++++++++++--------------
>  1 file changed, 36 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> index 9c591e0..9676902 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -10,6 +10,7 @@
>   *   published by the Free Software Foundation.
>   */
>  
> +#include <linux/circ_buf.h>
>  #include <linux/firmware.h>
>  #include <linux/mmc/card.h>
>  #include <linux/mmc/sdio_func.h>
> @@ -101,38 +102,50 @@ enum gen_com_reg_b {
>  
>  #define KS7010_IO_BLOCK_SIZE 512
>  
> +#define CIRC_INC(a, b) if (++a >= b) a = 0

I don't like this macro.  If we're going to call it CIRC_INC() then it
needs to be included in include/linux/circ_buf.h and not here.  I don't
like that the argument is  "b" instead of "size" or something.  It
should be wrapped in a do { } while(0).  There should be parens around
"a" and "b" so I don't have to think about precedence bugs.

regards,
dan carpenter

_______________________________________________
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