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
