* [PATCH] staging: fbtft: Replace a bit shift by a use of BIT.
@ 2017-03-22 2:37 Arushi Singhal
2017-03-22 16:18 ` Andy Shevchenko
0 siblings, 1 reply; 4+ messages in thread
From: Arushi Singhal @ 2017-03-22 2:37 UTC (permalink / raw)
To: thomas.petazzoni
Cc: Greg Kroah-Hartman, devel, linux-kernel, outreachy-kernel
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: fbtft: Replace a bit shift by a use of BIT.
2017-03-22 2:37 [PATCH] staging: fbtft: Replace a bit shift by a use of BIT Arushi Singhal
@ 2017-03-22 16:18 ` Andy Shevchenko
2017-03-22 16:22 ` Andy Shevchenko
2017-03-25 6:47 ` Arushi Singhal
0 siblings, 2 replies; 4+ messages in thread
From: Andy Shevchenko @ 2017-03-22 16:18 UTC (permalink / raw)
To: Arushi Singhal
Cc: Thomas Petazzoni, Greg Kroah-Hartman, devel, linux-kernel,
outreachy-kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: fbtft: Replace a bit shift by a use of BIT.
2017-03-22 16:18 ` Andy Shevchenko
@ 2017-03-22 16:22 ` Andy Shevchenko
2017-03-25 6:47 ` Arushi Singhal
1 sibling, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2017-03-22 16:22 UTC (permalink / raw)
To: Arushi Singhal
Cc: Thomas Petazzoni, Greg Kroah-Hartman, devel, linux-kernel,
outreachy-kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: fbtft: Replace a bit shift by a use of BIT.
2017-03-22 16:18 ` Andy Shevchenko
2017-03-22 16:22 ` Andy Shevchenko
@ 2017-03-25 6:47 ` Arushi Singhal
1 sibling, 0 replies; 4+ messages in thread
From: Arushi Singhal @ 2017-03-25 6:47 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Thomas Petazzoni, Greg Kroah-Hartman, driverdevel, linux-kernel,
outreachy-kernel
[-- Attachment #1: Type: text/plain, Size: 4502 bytes --]
On Wed, Mar 22, 2017 at 9:48 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).
>
Hi Andy
I somewhat understood my mistake and your views.
But It would be great if you can highlight more for my better understanding.
Thanks
Arushi Singhal
>
>
> > 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
>
[-- Attachment #2: Type: text/html, Size: 6306 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-25 6:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-22 2:37 [PATCH] staging: fbtft: Replace a bit shift by a use of BIT Arushi Singhal
2017-03-22 16:18 ` Andy Shevchenko
2017-03-22 16:22 ` Andy Shevchenko
2017-03-25 6:47 ` Arushi Singhal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.