linux-kernel - Re: [PATCH] staging: media: atomisp: Use BIT macro instead of left shifting

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]

Date:   Sat, 29 Jan 2022 08:36:59 -0800
From:   Joe Perches <joe@...ches.com>
To:     Moses Christopher Bollavarapu <mosescb.dev@...il.com>,
        linux-media@...r.kernel.org, linux-staging@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Yizhuo <yzhai003@....edu>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
        Colin Ian King <colin.king@...el.com>,
        Kaixu Xia <kaixuxia@...cent.com>,
        Baokun Li <libaokun1@...wei.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Aditya Srivastava <yashsri421@...il.com>,
        Aline Santana Cordeiro <alinesantanacordeiro@...il.com>,
        Tsuchiya Yuto <kitakar@...il.com>,
        Yang Yingliang <yangyingliang@...wei.com>,
        Alan <alan@...ux.intel.com>,
        Souptick Joarder <jrdr.linux@...il.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Ard Biesheuvel <ardb@...nel.org>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH] staging: media: atomisp: Use BIT macro instead of left
 shifting

On Sat, 2022-01-29 at 12:38 +0100, Moses Christopher Bollavarapu wrote:
> There is a BIT(nr) macro available in Linux Kernel,
> which does the same thing.
> 
> Example: BIT(7) = (1UL << 7)
[]
> diff --git a/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c b/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c
[]
> @@ -548,7 +548,7 @@ static long __ov5693_set_exposure(struct v4l2_subdev *sd, int coarse_itg,
>  	 * The way is to write coarse_itg/2 to the reg, meanwhile write 2*hts
>  	 * to the reg.
>  	 */
> -	if (coarse_itg > (1 << 15)) {
> +	if (coarse_itg > BIT(15)) {

Not all uses of 1 left shift should be converted to BIT
Especially when used with a non-bit value comparison test.

This is a size and not a bit position so this is likely not appropriate.

It'd probably be better as
	if (coarse_itg > 0x8000)
or
	if (coarse_itg > 32768)
or
	if (coarse_itg > SOME_CONSTANT_DEFINE)

> diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
[]
> @@ -1913,11 +1913,11 @@ void atomisp_css_input_set_mode(struct atomisp_sub_device *asd,
>  			    &asd->stream_env[ATOMISP_INPUT_STREAM_GENERAL].stream_config;
>  		s_config->mode = IA_CSS_INPUT_MODE_TPG;
>  		s_config->source.tpg.mode = IA_CSS_TPG_MODE_CHECKERBOARD;
> -		s_config->source.tpg.x_mask = (1 << 4) - 1;
> +		s_config->source.tpg.x_mask = BIT(4) - 1;

These should probably use GENMASK

> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
[]
> @@ -626,11 +626,11 @@ static int atomisp_mrfld_pre_power_down(struct atomisp_device *isp)
>  	 * IRQ, if so, waiting for it to be served
>  	 */
>  	pci_read_config_dword(pdev, PCI_INTERRUPT_CTRL, &irq);
> -	irq = irq & 1 << INTR_IIR;
> +	irq = irq & BIT(INTR_IIR);

The rest seems sensible.