| Message ID | 1459861855-1975-1-git-send-email-linus.walleij@linaro.org |
|---|---|
| State | Accepted |
| Commit | cee1b40d96f1b49e9a1b38e2d57d37a2c20ced31 |
| Headers | show |
On Tue 05 Apr 06:10 PDT 2016, Linus Walleij wrote: > This switch to use BIT(n) instead of (1 << n) which is less > to the point. Most GPIO drivers do this to avoid mistakes. > Also switch from using <linux/gpio.h> to the apropriate > <linux/gpio/driver.h> include. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/gpio/gpio-tc3589x.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpio/gpio-tc3589x.c b/drivers/gpio/gpio-tc3589x.c [..] > @@ -55,7 +56,7 @@ static void tc3589x_gpio_set(struct gpio_chip *chip, unsigned offset, int val) > struct tc3589x *tc3589x = tc3589x_gpio->tc3589x; > u8 reg = TC3589x_GPIODATA0 + (offset / 8) * 2; > unsigned pos = offset % 8; > - u8 data[] = {!!val << pos, 1 << pos}; > + u8 data[] = {!!val << pos, BIT(pos)}; ^ | The first part of this is also a bitmask, so even though this is slightly messier I think for consistency you should go with: u8 data[] = {val ? BIT(pos) : 0, BIT(pos)}; > > tc3589x_block_write(tc3589x, reg, ARRAY_SIZE(data), data); > } The rest looks good, Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 5, 2016 at 3:52 PM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > On Tue 05 Apr 06:10 PDT 2016, Linus Walleij wrote: >> - u8 data[] = {!!val << pos, 1 << pos}; >> + u8 data[] = {!!val << pos, BIT(pos)}; > ^ > | > The first part of this is also a bitmask, so even though this is > slightly messier I think for consistency you should go with: > > u8 data[] = {val ? BIT(pos) : 0, BIT(pos)}; > >> >> tc3589x_block_write(tc3589x, reg, ARRAY_SIZE(data), data); >> } > > The rest looks good, > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> OK thanks, edited like so and applied with your review tag. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-tc3589x.c b/drivers/gpio/gpio-tc3589x.c index 4f566e6b81f1..3d84bae89067 100644 --- a/drivers/gpio/gpio-tc3589x.c +++ b/drivers/gpio/gpio-tc3589x.c @@ -10,10 +10,11 @@ #include <linux/init.h> #include <linux/platform_device.h> #include <linux/slab.h> -#include <linux/gpio.h> +#include <linux/gpio/driver.h> #include <linux/of.h> #include <linux/interrupt.h> #include <linux/mfd/tc3589x.h> +#include <linux/bitops.h> /* * These registers are modified under the irq bus lock and cached to avoid @@ -39,7 +40,7 @@ static int tc3589x_gpio_get(struct gpio_chip *chip, unsigned offset) struct tc3589x_gpio *tc3589x_gpio = gpiochip_get_data(chip); struct tc3589x *tc3589x = tc3589x_gpio->tc3589x; u8 reg = TC3589x_GPIODATA0 + (offset / 8) * 2; - u8 mask = 1 << (offset % 8); + u8 mask = BIT(offset % 8); int ret; ret = tc3589x_reg_read(tc3589x, reg); @@ -55,7 +56,7 @@ static void tc3589x_gpio_set(struct gpio_chip *chip, unsigned offset, int val) struct tc3589x *tc3589x = tc3589x_gpio->tc3589x; u8 reg = TC3589x_GPIODATA0 + (offset / 8) * 2; unsigned pos = offset % 8; - u8 data[] = {!!val << pos, 1 << pos}; + u8 data[] = {!!val << pos, BIT(pos)}; tc3589x_block_write(tc3589x, reg, ARRAY_SIZE(data), data); } @@ -70,7 +71,7 @@ static int tc3589x_gpio_direction_output(struct gpio_chip *chip, tc3589x_gpio_set(chip, offset, val); - return tc3589x_set_bits(tc3589x, reg, 1 << pos, 1 << pos); + return tc3589x_set_bits(tc3589x, reg, BIT(pos), BIT(pos)); } static int tc3589x_gpio_direction_input(struct gpio_chip *chip, @@ -100,7 +101,7 @@ static int tc3589x_gpio_irq_set_type(struct irq_data *d, unsigned int type) struct tc3589x_gpio *tc3589x_gpio = gpiochip_get_data(gc); int offset = d->hwirq; int regoffset = offset / 8; - int mask = 1 << (offset % 8); + int mask = BIT(offset % 8); if (type == IRQ_TYPE_EDGE_BOTH) { tc3589x_gpio->regs[REG_IBE][regoffset] |= mask; @@ -165,7 +166,7 @@ static void tc3589x_gpio_irq_mask(struct irq_data *d) struct tc3589x_gpio *tc3589x_gpio = gpiochip_get_data(gc); int offset = d->hwirq; int regoffset = offset / 8; - int mask = 1 << (offset % 8); + int mask = BIT(offset % 8); tc3589x_gpio->regs[REG_IE][regoffset] &= ~mask; } @@ -176,7 +177,7 @@ static void tc3589x_gpio_irq_unmask(struct irq_data *d) struct tc3589x_gpio *tc3589x_gpio = gpiochip_get_data(gc); int offset = d->hwirq; int regoffset = offset / 8; - int mask = 1 << (offset % 8); + int mask = BIT(offset % 8); tc3589x_gpio->regs[REG_IE][regoffset] |= mask; }
This switch to use BIT(n) instead of (1 << n) which is less to the point. Most GPIO drivers do this to avoid mistakes. Also switch from using <linux/gpio.h> to the apropriate <linux/gpio/driver.h> include. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/gpio/gpio-tc3589x.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html