[PATCH 0/4] use BIT() macro instead manually shifting bits

[PATCH 0/4] use BIT() macro instead manually shifting bits

[Javier Martinez Canillas]

Hello Linus and Alexandre,

While learning coccinelle towards doing the big gpio_chip/gpio_chip_ops
split refactoring I wrote this trivial semantic patch that replaces a
manual bit shift by using the BIT macro from <linux/bitops.h>

    @hasbitops@
    @@

    #include <linux/bitops.h>

    @depends on hasbitops@
    expression E;
    @@

    - 1 << E
    + BIT(E)

    @depends on hasbitops@
    expression E;
    @@

    - BIT((E))
    + BIT(E)

When applying to the drivers/gpio subdirectory I got
the following clean up patches for some GPIO drivers.

Javier Martinez Canillas (4):
  gpio: em: use BIT() macro instead manually shifting bits
  gpio: kempld: use BIT() macro instead manually shifting bits
  gpio: omap: use BIT() macro instead manually shifting bits
  gpio: pl061: use BIT() macro instead manually shifting bits

 drivers/gpio/gpio-em.c     |  2 +-
 drivers/gpio/gpio-kempld.c |  2 +-
 drivers/gpio/gpio-omap.c   | 50 +++++++++++++++++++++++-----------------------
 drivers/gpio/gpio-pl061.c  | 26 ++++++++++++------------
 4 files changed, 40 insertions(+), 40 deletions(-)

I've only tested the changes on a OMAP3 board but the changes are very
straightforward and that's the advantage of using coccinelle after all.

Best regards,
Javier

-- 
1.9.1

[PATCH 1/4] gpio: em: use BIT() macro instead manually shifting bits

[Javier Martinez Canillas]

Using the BIT() macro instead of manually shifting bits
makes the code less error prone and also more readable.

Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
---
 drivers/gpio/gpio-em.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-em.c b/drivers/gpio/gpio-em.c
index 8765bd6..1da1aa4 100644
--- a/drivers/gpio/gpio-em.c
+++ b/drivers/gpio/gpio-em.c
@@ -212,7 +212,7 @@ static void __em_gio_set(struct gpio_chip *chip, unsigned int reg,
 {
 	/* upper 16 bits contains mask and lower 16 actual value */
 	em_gio_write(gpio_to_priv(chip), reg,
-		     (1 << (shift + 16)) | (value << shift));
+		     (BIT(shift + 16)) | (value << shift));
 }
 
 static void em_gio_set(struct gpio_chip *chip, unsigned offset, int value)
-- 
1.9.1

[PATCH 2/4] gpio: kempld: use BIT() macro instead manually shifting bits

[Javier Martinez Canillas]

Using the BIT() macro instead of manually shifting bits
makes the code less error prone and also more readable.

Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
---
 drivers/gpio/gpio-kempld.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-kempld.c b/drivers/gpio/gpio-kempld.c
index ebe1023..1e5e519 100644
--- a/drivers/gpio/gpio-kempld.c
+++ b/drivers/gpio/gpio-kempld.c
@@ -24,7 +24,7 @@
 #include <linux/mfd/kempld.h>
 
 #define KEMPLD_GPIO_MAX_NUM		16
-#define KEMPLD_GPIO_MASK(x)		(1 << ((x) % 8))
+#define KEMPLD_GPIO_MASK(x)		(BIT((x) % 8))
 #define KEMPLD_GPIO_DIR_NUM(x)		(0x40 + (x) / 8)
 #define KEMPLD_GPIO_LVL_NUM(x)		(0x42 + (x) / 8)
 #define KEMPLD_GPIO_EVT_LVL_EDGE	0x46
-- 
1.9.1

[PATCH 3/4] gpio: omap: use BIT() macro instead manually shifting bits

[Javier Martinez Canillas]

Using the BIT() macro instead of manually shifting bits
makes the code less error prone and also more readable.

Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
---
 drivers/gpio/gpio-omap.c | 50 ++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 47c6056..01d50a0 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -83,11 +83,11 @@ struct gpio_bank {
 };
 
 #define GPIO_INDEX(bank, gpio) (gpio % bank->width)
-#define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
+#define GPIO_BIT(bank, gpio) (BIT(GPIO_INDEX(bank, gpio)))
 #define GPIO_MOD_CTRL_BIT	BIT(0)
 
 #define BANK_USED(bank) (bank->mod_usage || bank->irq_usage)
-#define LINE_USED(line, offset) (line & (1 << offset))
+#define LINE_USED(line, offset) (line & (BIT(offset)))
 
 static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
 {
@@ -108,9 +108,9 @@ static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input)
 	reg += bank->regs->direction;
 	l = readl_relaxed(reg);
 	if (is_input)
-		l |= 1 << gpio;
+		l |= BIT(gpio);
 	else
-		l &= ~(1 << gpio);
+		l &= ~(BIT(gpio));
 	writel_relaxed(l, reg);
 	bank->context.oe = l;
 }
@@ -153,14 +153,14 @@ static int _get_gpio_datain(struct gpio_bank *bank, int offset)
 {
 	void __iomem *reg = bank->base + bank->regs->datain;
 
-	return (readl_relaxed(reg) & (1 << offset)) != 0;
+	return (readl_relaxed(reg) & (BIT(offset))) != 0;
 }
 
 static int _get_gpio_dataout(struct gpio_bank *bank, int offset)
 {
 	void __iomem *reg = bank->base + bank->regs->dataout;
 
-	return (readl_relaxed(reg) & (1 << offset)) != 0;
+	return (readl_relaxed(reg) & (BIT(offset))) != 0;
 }
 
 static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)
@@ -297,7 +297,7 @@ static inline void set_gpio_trigger(struct gpio_bank *bank, int gpio,
 						unsigned trigger)
 {
 	void __iomem *base = bank->base;
-	u32 gpio_bit = 1 << gpio;
+	u32 gpio_bit = BIT(gpio);
 
 	_gpio_rmw(base, bank->regs->leveldetect0, gpio_bit,
 		  trigger & IRQ_TYPE_LEVEL_LOW);
@@ -366,9 +366,9 @@ static void _toggle_gpio_edge_triggering(struct gpio_bank *bank, int gpio)
 
 	l = readl_relaxed(reg);
 	if ((l >> gpio) & 1)
-		l &= ~(1 << gpio);
+		l &= ~(BIT(gpio));
 	else
-		l |= 1 << gpio;
+		l |= BIT(gpio);
 
 	writel_relaxed(l, reg);
 }
@@ -390,11 +390,11 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int gpio,
 
 		l = readl_relaxed(reg);
 		if ((trigger & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH)
-			bank->toggle_mask |= 1 << gpio;
+			bank->toggle_mask |= BIT(gpio);
 		if (trigger & IRQ_TYPE_EDGE_RISING)
-			l |= 1 << gpio;
+			l |= BIT(gpio);
 		else if (trigger & IRQ_TYPE_EDGE_FALLING)
-			l &= ~(1 << gpio);
+			l &= ~(BIT(gpio));
 		else
 			return -EINVAL;
 
@@ -411,10 +411,10 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int gpio,
 		if (trigger & IRQ_TYPE_EDGE_RISING)
 			l |= 2 << (gpio << 1);
 		if (trigger & IRQ_TYPE_EDGE_FALLING)
-			l |= 1 << (gpio << 1);
+			l |= BIT(gpio << 1);
 
 		/* Enable wake-up during idle for dynamic tick */
-		_gpio_rmw(base, bank->regs->wkup_en, 1 << gpio, trigger);
+		_gpio_rmw(base, bank->regs->wkup_en, BIT(gpio), trigger);
 		bank->context.wake_en =
 			readl_relaxed(bank->base + bank->regs->wkup_en);
 		writel_relaxed(l, reg);
@@ -428,7 +428,7 @@ static void _enable_gpio_module(struct gpio_bank *bank, unsigned offset)
 		void __iomem *reg = bank->base + bank->regs->pinctrl;
 
 		/* Claim the pin for MPU */
-		writel_relaxed(readl_relaxed(reg) | (1 << offset), reg);
+		writel_relaxed(readl_relaxed(reg) | (BIT(offset)), reg);
 	}
 
 	if (bank->regs->ctrl && !BANK_USED(bank)) {
@@ -451,7 +451,7 @@ static void _disable_gpio_module(struct gpio_bank *bank, unsigned offset)
 	    !LINE_USED(bank->mod_usage, offset) &&
 	    !LINE_USED(bank->irq_usage, offset)) {
 		/* Disable wake-up during idle for dynamic tick */
-		_gpio_rmw(base, bank->regs->wkup_en, 1 << offset, 0);
+		_gpio_rmw(base, bank->regs->wkup_en, BIT(offset), 0);
 		bank->context.wake_en =
 			readl_relaxed(bank->base + bank->regs->wkup_en);
 	}
@@ -507,12 +507,12 @@ static int gpio_irq_type(struct irq_data *d, unsigned type)
 	if (!LINE_USED(bank->mod_usage, offset)) {
 		_enable_gpio_module(bank, offset);
 		_set_gpio_direction(bank, offset, 1);
-	} else if (!gpio_is_input(bank, 1 << offset)) {
+	} else if (!gpio_is_input(bank, BIT(offset))) {
 		spin_unlock_irqrestore(&bank->lock, flags);
 		return -EINVAL;
 	}
 
-	bank->irq_usage |= 1 << GPIO_INDEX(bank, gpio);
+	bank->irq_usage |= BIT(GPIO_INDEX(bank, gpio));
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))
@@ -549,7 +549,7 @@ static u32 _get_gpio_irqbank_mask(struct gpio_bank *bank)
 {
 	void __iomem *reg = bank->base;
 	u32 l;
-	u32 mask = (1 << bank->width) - 1;
+	u32 mask = (BIT(bank->width)) - 1;
 
 	reg += bank->regs->irqenable;
 	l = readl_relaxed(reg);
@@ -681,7 +681,7 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
 		_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
 		_enable_gpio_module(bank, offset);
 	}
-	bank->mod_usage |= 1 << offset;
+	bank->mod_usage |= BIT(offset);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -693,7 +693,7 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
-	bank->mod_usage &= ~(1 << offset);
+	bank->mod_usage &= ~(BIT(offset));
 	_disable_gpio_module(bank, offset);
 	_reset_gpio(bank, bank->chip.base + offset);
 	spin_unlock_irqrestore(&bank->lock, flags);
@@ -763,7 +763,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 
 		while (isr) {
 			bit = __ffs(isr);
-			isr &= ~(1 << bit);
+			isr &= ~(BIT(bit));
 
 			/*
 			 * Some chips can't respond to both rising and falling
@@ -772,7 +772,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 			 * to respond to the IRQ for the opposite direction.
 			 * This will be indicated in the bank toggle_mask.
 			 */
-			if (bank->toggle_mask & (1 << bit))
+			if (bank->toggle_mask & (BIT(bit)))
 				_toggle_gpio_edge_triggering(bank, bit);
 
 			generic_handle_irq(irq_find_mapping(bank->chip.irqdomain,
@@ -798,7 +798,7 @@ static void gpio_irq_shutdown(struct irq_data *d)
 
 	spin_lock_irqsave(&bank->lock, flags);
 	gpio_unlock_as_irq(&bank->chip, offset);
-	bank->irq_usage &= ~(1 << offset);
+	bank->irq_usage &= ~(BIT(offset));
 	_disable_gpio_module(bank, offset);
 	_reset_gpio(bank, gpio);
 	spin_unlock_irqrestore(&bank->lock, flags);
@@ -961,7 +961,7 @@ static int gpio_get(struct gpio_chip *chip, unsigned offset)
 	u32 mask;
 
 	bank = container_of(chip, struct gpio_bank, chip);
-	mask = (1 << offset);
+	mask = (BIT(offset));
 
 	if (gpio_is_input(bank, mask))
 		return _get_gpio_datain(bank, offset);
-- 
1.9.1

[PATCH 4/4] gpio: pl061: use BIT() macro instead manually shifting bits

[Javier Martinez Canillas]

Using the BIT() macro instead of manually shifting bits
makes the code less error prone and also more readable.

Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
---
 drivers/gpio/gpio-pl061.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 84553d3..84b49cf 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -87,7 +87,7 @@ static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
 
 	spin_lock_irqsave(&chip->lock, flags);
 	gpiodir = readb(chip->base + GPIODIR);
-	gpiodir &= ~(1 << offset);
+	gpiodir &= ~(BIT(offset));
 	writeb(gpiodir, chip->base + GPIODIR);
 	spin_unlock_irqrestore(&chip->lock, flags);
 
@@ -105,16 +105,16 @@ static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
 		return -EINVAL;
 
 	spin_lock_irqsave(&chip->lock, flags);
-	writeb(!!value << offset, chip->base + (1 << (offset + 2)));
+	writeb(!!value << offset, chip->base + (BIT(offset + 2)));
 	gpiodir = readb(chip->base + GPIODIR);
-	gpiodir |= 1 << offset;
+	gpiodir |= BIT(offset);
 	writeb(gpiodir, chip->base + GPIODIR);
 
 	/*
 	 * gpio value is set again, because pl061 doesn't allow to set value of
 	 * a gpio pin before configuring it in OUT mode.
 	 */
-	writeb(!!value << offset, chip->base + (1 << (offset + 2)));
+	writeb(!!value << offset, chip->base + (BIT(offset + 2)));
 	spin_unlock_irqrestore(&chip->lock, flags);
 
 	return 0;
@@ -124,14 +124,14 @@ static int pl061_get_value(struct gpio_chip *gc, unsigned offset)
 {
 	struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
 
-	return !!readb(chip->base + (1 << (offset + 2)));
+	return !!readb(chip->base + (BIT(offset + 2)));
 }
 
 static void pl061_set_value(struct gpio_chip *gc, unsigned offset, int value)
 {
 	struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
 
-	writeb(!!value << offset, chip->base + (1 << (offset + 2)));
+	writeb(!!value << offset, chip->base + (BIT(offset + 2)));
 }
 
 static int pl061_irq_type(struct irq_data *d, unsigned trigger)
@@ -206,7 +206,7 @@ static void pl061_irq_mask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
-	u8 mask = 1 << (irqd_to_hwirq(d) % PL061_GPIO_NR);
+	u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR);
 	u8 gpioie;
 
 	spin_lock(&chip->lock);
@@ -219,7 +219,7 @@ static void pl061_irq_unmask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
-	u8 mask = 1 << (irqd_to_hwirq(d) % PL061_GPIO_NR);
+	u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR);
 	u8 gpioie;
 
 	spin_lock(&chip->lock);
@@ -301,9 +301,9 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
 
 	for (i = 0; i < PL061_GPIO_NR; i++) {
 		if (pdata) {
-			if (pdata->directions & (1 << i))
+			if (pdata->directions & (BIT(i)))
 				pl061_direction_output(&chip->gc, i,
-						pdata->values & (1 << i));
+						pdata->values & (BIT(i)));
 			else
 				pl061_direction_input(&chip->gc, i);
 		}
@@ -330,7 +330,7 @@ static int pl061_suspend(struct device *dev)
 	chip->csave_regs.gpio_ie = readb(chip->base + GPIOIE);
 
 	for (offset = 0; offset < PL061_GPIO_NR; offset++) {
-		if (chip->csave_regs.gpio_dir & (1 << offset))
+		if (chip->csave_regs.gpio_dir & (BIT(offset)))
 			chip->csave_regs.gpio_data |=
 				pl061_get_value(&chip->gc, offset) << offset;
 	}
@@ -344,10 +344,10 @@ static int pl061_resume(struct device *dev)
 	int offset;
 
 	for (offset = 0; offset < PL061_GPIO_NR; offset++) {
-		if (chip->csave_regs.gpio_dir & (1 << offset))
+		if (chip->csave_regs.gpio_dir & (BIT(offset)))
 			pl061_direction_output(&chip->gc, offset,
 					chip->csave_regs.gpio_data &
-					(1 << offset));
+					(BIT(offset)));
 		else
 			pl061_direction_input(&chip->gc, offset);
 	}
-- 
1.9.1

Re: [PATCH 0/4] use BIT() macro instead manually shifting bits

[Alexandre Courbot]

On Sun, Apr 27, 2014 at 9:00 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Linus and Alexandre,
>
> While learning coccinelle towards doing the big gpio_chip/gpio_chip_ops
> split refactoring I wrote this trivial semantic patch that replaces a
> manual bit shift by using the BIT macro from <linux/bitops.h>
>
>     @hasbitops@
>     @@
>
>     #include <linux/bitops.h>
>
>     @depends on hasbitops@
>     expression E;
>     @@
>
>     - 1 << E
>     + BIT(E)
>
>     @depends on hasbitops@
>     expression E;
>     @@
>
>     - BIT((E))
>     + BIT(E)
>
> When applying to the drivers/gpio subdirectory I got
> the following clean up patches for some GPIO drivers.

I personally find "1 << n" easier to read than a macro, but you are
right that the macro is less error-prone. Nice use of Coccinelle btw,
I should really spend the time to learn it.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH 0/4] use BIT() macro instead manually shifting bits

[Javier Martinez Canillas]

Hello Alexandre,

Thanks a lot for your feedback.

On Sun, Apr 27, 2014 at 12:16 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sun, Apr 27, 2014 at 9:00 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
>> Hello Linus and Alexandre,
>>
>> While learning coccinelle towards doing the big gpio_chip/gpio_chip_ops
>> split refactoring I wrote this trivial semantic patch that replaces a
>> manual bit shift by using the BIT macro from <linux/bitops.h>
>>
>>     @hasbitops@
>>     @@
>>
>>     #include <linux/bitops.h>
>>
>>     @depends on hasbitops@
>>     expression E;
>>     @@
>>
>>     - 1 << E
>>     + BIT(E)
>>
>>     @depends on hasbitops@
>>     expression E;
>>     @@
>>
>>     - BIT((E))
>>     + BIT(E)
>>
>> When applying to the drivers/gpio subdirectory I got
>> the following clean up patches for some GPIO drivers.
>
> I personally find "1 << n" easier to read than a macro, but you are
> right that the macro is less error-prone. Nice use of Coccinelle btw,
> I should really spend the time to learn it.
>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Yes I agree that the readability is a matter of personal taste. Do you
want me to send a v2 with a better wording on the commit message?

Best regards,
Javier

Re: [PATCH 0/4] use BIT() macro instead manually shifting bits

[Alexandre Courbot]

On Sun, Apr 27, 2014 at 7:46 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Alexandre,
>
> Thanks a lot for your feedback.
>
> On Sun, Apr 27, 2014 at 12:16 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Sun, Apr 27, 2014 at 9:00 AM, Javier Martinez Canillas
>> <javier@dowhile0.org> wrote:
>>> Hello Linus and Alexandre,
>>>
>>> While learning coccinelle towards doing the big gpio_chip/gpio_chip_ops
>>> split refactoring I wrote this trivial semantic patch that replaces a
>>> manual bit shift by using the BIT macro from <linux/bitops.h>
>>>
>>>     @hasbitops@
>>>     @@
>>>
>>>     #include <linux/bitops.h>
>>>
>>>     @depends on hasbitops@
>>>     expression E;
>>>     @@
>>>
>>>     - 1 << E
>>>     + BIT(E)
>>>
>>>     @depends on hasbitops@
>>>     expression E;
>>>     @@
>>>
>>>     - BIT((E))
>>>     + BIT(E)
>>>
>>> When applying to the drivers/gpio subdirectory I got
>>> the following clean up patches for some GPIO drivers.
>>
>> I personally find "1 << n" easier to read than a macro, but you are
>> right that the macro is less error-prone. Nice use of Coccinelle btw,
>> I should really spend the time to learn it.
>>
>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Yes I agree that the readability is a matter of personal taste. Do you
> want me to send a v2 with a better wording on the commit message?

No, I think it's good as it is. Nice usage of Coccinelle btw. I really
should spend some time learning how to use it.

Re: [PATCH 0/4] use BIT() macro instead manually shifting bits

[Javier Martinez Canillas]

On Sun, Apr 27, 2014 at 1:16 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sun, Apr 27, 2014 at 7:46 PM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
>> Hello Alexandre,
>>
>> Thanks a lot for your feedback.
>>
>> On Sun, Apr 27, 2014 at 12:16 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>> On Sun, Apr 27, 2014 at 9:00 AM, Javier Martinez Canillas
>>> <javier@dowhile0.org> wrote:
>>>> Hello Linus and Alexandre,
>>>>
>>>> While learning coccinelle towards doing the big gpio_chip/gpio_chip_ops
>>>> split refactoring I wrote this trivial semantic patch that replaces a
>>>> manual bit shift by using the BIT macro from <linux/bitops.h>
>>>>
>>>>     @hasbitops@
>>>>     @@
>>>>
>>>>     #include <linux/bitops.h>
>>>>
>>>>     @depends on hasbitops@
>>>>     expression E;
>>>>     @@
>>>>
>>>>     - 1 << E
>>>>     + BIT(E)
>>>>
>>>>     @depends on hasbitops@
>>>>     expression E;
>>>>     @@
>>>>
>>>>     - BIT((E))
>>>>     + BIT(E)
>>>>
>>>> When applying to the drivers/gpio subdirectory I got
>>>> the following clean up patches for some GPIO drivers.
>>>
>>> I personally find "1 << n" easier to read than a macro, but you are
>>> right that the macro is less error-prone. Nice use of Coccinelle btw,
>>> I should really spend the time to learn it.
>>>
>>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>>
>> Yes I agree that the readability is a matter of personal taste. Do you
>> want me to send a v2 with a better wording on the commit message?
>
> No, I think it's good as it is. Nice usage of Coccinelle btw. I really
> should spend some time learning how to use it.

Thanks, now that I understand the basics I will try to write the
semantic patch for the struct gpio_chip refactoring. I expect that to
be a little trickier :-)

Best regards,
Javier

Re: [PATCH 0/4] use BIT() macro instead manually shifting bits

[Linus Walleij]

On Sat, Apr 26, 2014 at 5:00 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:

> Hello Linus and Alexandre,
>
> While learning coccinelle towards doing the big gpio_chip/gpio_chip_ops
> split refactoring I wrote this trivial semantic patch that replaces a
> manual bit shift by using the BIT macro from <linux/bitops.h>

Nice semantic patch!

And all patches applied with Alex's ACK.

However I will edit the commit messages to remove the word
"manual". This just breaks my heart everytime I see that word
used in code, manual means a person is typing the keys or something
I bet it's still a computer doing the heavy lifting here.

Yours,
Linus Walleij

Re: [PATCH 0/4] use BIT() macro instead manually shifting bits

[Javier Martinez Canillas]

Hello Linus,

On Sat, May 3, 2014 at 9:15 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Apr 26, 2014 at 5:00 PM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
>
>> Hello Linus and Alexandre,
>>
>> While learning coccinelle towards doing the big gpio_chip/gpio_chip_ops
>> split refactoring I wrote this trivial semantic patch that replaces a
>> manual bit shift by using the BIT macro from <linux/bitops.h>
>
> Nice semantic patch!
>

Thanks

> And all patches applied with Alex's ACK.
>
> However I will edit the commit messages to remove the word
> "manual". This just breaks my heart everytime I see that word
> used in code, manual means a person is typing the keys or something
> I bet it's still a computer doing the heavy lifting here.
>

Yes, I agree that the wording was not the best. Thanks a lot for
fixing the commit messages.

> Yours,
> Linus Walleij

Best regards,
Javier