linux-kernel - Re: [PATCH] staging: fbtft: Replace a bit shift by a use of BIT.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75VfD=4BcMV==zKejEPaJm=S6MqxJWrP2PnBhgNH0EQue9g@mail.gmail.com>
Date:   Wed, 22 Mar 2017 18:18:07 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Arushi Singhal <arushisinghal19971997@...il.com>
Cc:     Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        outreachy-kernel@...glegroups.com
Subject: Re: [PATCH] staging: fbtft: Replace a bit shift by a use of BIT.

On Wed, Mar 22, 2017 at 4:37 AM, Arushi Singhal
<arushisinghal19971997@...il.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@...il.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