Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1606162 > unrolled thread

[PATCH] staging: fbtft: Replace a bit shift by a use of BIT.

Started byArushi Singhal <arushisinghal19971997@gmail.com>
First post2017-03-22 03:40 +0100
Last post2017-03-22 17:40 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] staging: fbtft: Replace a bit shift by a use of BIT. Arushi Singhal <arushisinghal19971997@gmail.com> - 2017-03-22 03:40 +0100
    Re: [PATCH] staging: fbtft: Replace a bit shift by a use of BIT. Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-03-22 17:20 +0100
      Re: [PATCH] staging: fbtft: Replace a bit shift by a use of BIT. Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-03-22 17:40 +0100

#1606162 — [PATCH] staging: fbtft: Replace a bit shift by a use of BIT.

FromArushi Singhal <arushisinghal19971997@gmail.com>
Date2017-03-22 03:40 +0100
Subject[PATCH] staging: fbtft: Replace a bit shift by a use of BIT.
Message-ID<tnElP-3TW-7@gated-at.bofh.it>
This patch replaces bit shifting on 1 with the BIT(x) macro.
This was done with coccinelle:
@@
constant c;
@@

-1 << c
+BIT(c)

Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
---
 drivers/staging/fbtft/fb_agm1264k-fl.c | 4 ++--
 drivers/staging/fbtft/fb_ili9163.c     | 2 +-
 drivers/staging/fbtft/fb_ili9325.c     | 2 +-
 drivers/staging/fbtft/fb_ssd1289.c     | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c b/drivers/staging/fbtft/fb_agm1264k-fl.c
index 4ee76dbd30b5..d31deeab69b2 100644
--- a/drivers/staging/fbtft/fb_agm1264k-fl.c
+++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
@@ -369,7 +369,7 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 			/* select left side (sc0)
 			 * set addr
 			 */
-			write_reg(par, 0x00, (1 << 6) | (u8)addr_win.xs);
+			write_reg(par, 0x00, (BIT(6)) | (u8)addr_win.xs);
 			write_reg(par, 0x00, (0x17 << 3) | (u8)y);
 
 			/* write bitmap */
@@ -392,7 +392,7 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 			/* select right side (sc1)
 			 * set addr
 			 */
-			write_reg(par, 0x01, 1 << 6);
+			write_reg(par, 0x01, BIT(6));
 			write_reg(par, 0x01, (0x17 << 3) | (u8)y);
 
 			/* write bitmap */
diff --git a/drivers/staging/fbtft/fb_ili9163.c b/drivers/staging/fbtft/fb_ili9163.c
index 579e17734612..06a5a5f6e33e 100644
--- a/drivers/staging/fbtft/fb_ili9163.c
+++ b/drivers/staging/fbtft/fb_ili9163.c
@@ -194,7 +194,7 @@ static int set_var(struct fbtft_par *par)
 
 	/* Colorspcae */
 	if (par->bgr)
-		mactrl_data |= (1 << 2);
+		mactrl_data |= (BIT(2));
 	write_reg(par, MIPI_DCS_SET_ADDRESS_MODE, mactrl_data);
 	write_reg(par, MIPI_DCS_WRITE_MEMORY_START);
 	return 0;
diff --git a/drivers/staging/fbtft/fb_ili9325.c b/drivers/staging/fbtft/fb_ili9325.c
index 7189de5ae4b3..ab1267a9cd11 100644
--- a/drivers/staging/fbtft/fb_ili9325.c
+++ b/drivers/staging/fbtft/fb_ili9325.c
@@ -126,7 +126,7 @@ static int init_display(struct fbtft_par *par)
 	write_reg(par, 0x0013, 0x0000); /* VDV[4:0] for VCOM amplitude */
 	mdelay(200); /* Dis-charge capacitor power voltage */
 	write_reg(par, 0x0010, /* SAP, BT[3:0], AP, DSTB, SLP, STB */
-		(1 << 12) | (bt << 8) | (1 << 7) | (0x01 << 4));
+		(BIT(12)) | (bt << 8) | (BIT(7)) | (BIT(4)));
 	write_reg(par, 0x0011, 0x220 | vc); /* DC1[2:0], DC0[2:0], VC[2:0] */
 	mdelay(50); /* Delay 50ms */
 	write_reg(par, 0x0012, vrh); /* Internal reference voltage= Vci; */
diff --git a/drivers/staging/fbtft/fb_ssd1289.c b/drivers/staging/fbtft/fb_ssd1289.c
index c603e1516e64..b22a07d79b34 100644
--- a/drivers/staging/fbtft/fb_ssd1289.c
+++ b/drivers/staging/fbtft/fb_ssd1289.c
@@ -47,7 +47,7 @@ static int init_display(struct fbtft_par *par)
 	write_reg(par, 0x0E, 0x2B00);
 	write_reg(par, 0x1E, 0x00B7);
 	write_reg(par, 0x01,
-		(1 << 13) | (par->bgr << 11) | (1 << 9) | (HEIGHT - 1));
+		(BIT(13)) | (par->bgr << 11) | (BIT(9)) | (HEIGHT - 1));
 	write_reg(par, 0x02, 0x0600);
 	write_reg(par, 0x10, 0x0000);
 	write_reg(par, 0x05, 0x0000);
-- 
2.11.0

[toc] | [next] | [standalone]

#1606711

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2017-03-22 17:20 +0100
Message-ID<tnR9n-5eZ-1@gated-at.bofh.it>
In reply to#1606162
On Wed, Mar 22, 2017 at 4:37 AM, Arushi Singhal
<arushisinghal19971997@gmail.com> wrote:
> This patch replaces bit shifting on 1 with the BIT(x) macro.
> This was done with coccinelle:
> @@
> constant c;
> @@
>
> -1 << c
> +BIT(c)
>

While using BIT() macro is a good idea, you make it inconsistent here.
There are at least two options:
- do nothing
- define _MASK:s with GENMASK() and reuse in the code (looking at the
code they are masks, not just separate bits).


> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> ---
>  drivers/staging/fbtft/fb_agm1264k-fl.c | 4 ++--
>  drivers/staging/fbtft/fb_ili9163.c     | 2 +-
>  drivers/staging/fbtft/fb_ili9325.c     | 2 +-
>  drivers/staging/fbtft/fb_ssd1289.c     | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c b/drivers/staging/fbtft/fb_agm1264k-fl.c
> index 4ee76dbd30b5..d31deeab69b2 100644
> --- a/drivers/staging/fbtft/fb_agm1264k-fl.c
> +++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
> @@ -369,7 +369,7 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
>                         /* select left side (sc0)
>                          * set addr
>                          */
> -                       write_reg(par, 0x00, (1 << 6) | (u8)addr_win.xs);
> +                       write_reg(par, 0x00, (BIT(6)) | (u8)addr_win.xs);
>                         write_reg(par, 0x00, (0x17 << 3) | (u8)y);
>
>                         /* write bitmap */
> @@ -392,7 +392,7 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
>                         /* select right side (sc1)
>                          * set addr
>                          */
> -                       write_reg(par, 0x01, 1 << 6);
> +                       write_reg(par, 0x01, BIT(6));
>                         write_reg(par, 0x01, (0x17 << 3) | (u8)y);
>
>                         /* write bitmap */
> diff --git a/drivers/staging/fbtft/fb_ili9163.c b/drivers/staging/fbtft/fb_ili9163.c
> index 579e17734612..06a5a5f6e33e 100644
> --- a/drivers/staging/fbtft/fb_ili9163.c
> +++ b/drivers/staging/fbtft/fb_ili9163.c
> @@ -194,7 +194,7 @@ static int set_var(struct fbtft_par *par)
>
>         /* Colorspcae */
>         if (par->bgr)
> -               mactrl_data |= (1 << 2);
> +               mactrl_data |= (BIT(2));
>         write_reg(par, MIPI_DCS_SET_ADDRESS_MODE, mactrl_data);
>         write_reg(par, MIPI_DCS_WRITE_MEMORY_START);
>         return 0;
> diff --git a/drivers/staging/fbtft/fb_ili9325.c b/drivers/staging/fbtft/fb_ili9325.c
> index 7189de5ae4b3..ab1267a9cd11 100644
> --- a/drivers/staging/fbtft/fb_ili9325.c
> +++ b/drivers/staging/fbtft/fb_ili9325.c
> @@ -126,7 +126,7 @@ static int init_display(struct fbtft_par *par)
>         write_reg(par, 0x0013, 0x0000); /* VDV[4:0] for VCOM amplitude */
>         mdelay(200); /* Dis-charge capacitor power voltage */
>         write_reg(par, 0x0010, /* SAP, BT[3:0], AP, DSTB, SLP, STB */
> -               (1 << 12) | (bt << 8) | (1 << 7) | (0x01 << 4));
> +               (BIT(12)) | (bt << 8) | (BIT(7)) | (BIT(4)));
>         write_reg(par, 0x0011, 0x220 | vc); /* DC1[2:0], DC0[2:0], VC[2:0] */
>         mdelay(50); /* Delay 50ms */
>         write_reg(par, 0x0012, vrh); /* Internal reference voltage= Vci; */
> diff --git a/drivers/staging/fbtft/fb_ssd1289.c b/drivers/staging/fbtft/fb_ssd1289.c
> index c603e1516e64..b22a07d79b34 100644
> --- a/drivers/staging/fbtft/fb_ssd1289.c
> +++ b/drivers/staging/fbtft/fb_ssd1289.c
> @@ -47,7 +47,7 @@ static int init_display(struct fbtft_par *par)
>         write_reg(par, 0x0E, 0x2B00);
>         write_reg(par, 0x1E, 0x00B7);
>         write_reg(par, 0x01,
> -               (1 << 13) | (par->bgr << 11) | (1 << 9) | (HEIGHT - 1));
> +               (BIT(13)) | (par->bgr << 11) | (BIT(9)) | (HEIGHT - 1));
>         write_reg(par, 0x02, 0x0600);
>         write_reg(par, 0x10, 0x0000);
>         write_reg(par, 0x05, 0x0000);
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1606729

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2017-03-22 17:40 +0100
Message-ID<tnRsJ-5on-15@gated-at.bofh.it>
In reply to#1606711
On Wed, Mar 22, 2017 at 6:18 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Mar 22, 2017 at 4:37 AM, Arushi Singhal
> <arushisinghal19971997@gmail.com> wrote:
>> This patch replaces bit shifting on 1 with the BIT(x) macro.
>> This was done with coccinelle:
>> @@
>> constant c;
>> @@
>>
>> -1 << c
>> +BIT(c)
>>
>
> While using BIT() macro is a good idea, you make it inconsistent here.
> There are at least two options:
> - do nothing
> - define _MASK:s with GENMASK() and reuse in the code (looking at the
> code they are masks, not just separate bits).

Looking a bit more, not masks, but values.
So, best would be define for bits and values like 0x17.

One more thing, you don't need extra parens surround BIT() macro.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web