On Fri, Apr 13, 2018 at 2:14 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> 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.
Ok, I'll send v2 using other macros included in
include/linux/circ_buf.h but avoiding
the use of this new one.
Thanks, Dan.
>
> regards,
> dan carpenter
Best regards,
Sergio Paracuellos
>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
