Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 2 Jul 2021 16:13:43 +0300
From:   Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To:     Stefan Riedmueller <s.riedmueller@phytec.de>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Sakari Ailus <sakari.ailus@linux.intel.com>,
        linux-media@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 6/6] media: mt9p031: Use BIT macro
Message-ID: <YN8RB6yaI7BFNk+O@pendragon.ideasonboard.com>
References: <20210702095922.118614-1-s.riedmueller@phytec.de>
 <20210702095922.118614-7-s.riedmueller@phytec.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20210702095922.118614-7-s.riedmueller@phytec.de>
Precedence: bulk

Hi Stefan,

Thank you for the patch.

On Fri, Jul 02, 2021 at 11:59:22AM +0200, Stefan Riedmueller wrote:
> Make use of the BIT macro for setting individual bits. This improves
> readability and safety with respect to shifts.
> 
> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> ---
>  drivers/media/i2c/mt9p031.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index 3511c4ff350d..0a5bcbebe55f 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -76,39 +76,39 @@
>  #define	MT9P031_PLL_CONFIG_1				0x11
>  #define	MT9P031_PLL_CONFIG_2				0x12
>  #define MT9P031_PIXEL_CLOCK_CONTROL			0x0a
> -#define		MT9P031_PIXEL_CLOCK_INVERT		(1 << 15)
> +#define		MT9P031_PIXEL_CLOCK_INVERT		BIT(15)
>  #define		MT9P031_PIXEL_CLOCK_SHIFT(n)		((n) << 8)
>  #define		MT9P031_PIXEL_CLOCK_DIVIDE(n)		((n) << 0)
>  #define MT9P031_RESTART					0x0b
> -#define		MT9P031_FRAME_PAUSE_RESTART		(1 << 1)
> -#define		MT9P031_FRAME_RESTART			(1 << 0)
> +#define		MT9P031_FRAME_PAUSE_RESTART		BIT(1)
> +#define		MT9P031_FRAME_RESTART			BIT(0)
>  #define MT9P031_SHUTTER_DELAY				0x0c
>  #define MT9P031_RST					0x0d
>  #define		MT9P031_RST_ENABLE			1

This could also be turned into BIT(0).

>  #define		MT9P031_RST_DISABLE			0

This should then be dropped.

>  #define MT9P031_READ_MODE_1				0x1e
>  #define MT9P031_READ_MODE_2				0x20
> -#define		MT9P031_READ_MODE_2_ROW_MIR		(1 << 15)
> -#define		MT9P031_READ_MODE_2_COL_MIR		(1 << 14)
> -#define		MT9P031_READ_MODE_2_ROW_BLC		(1 << 6)
> +#define		MT9P031_READ_MODE_2_ROW_MIR		BIT(15)
> +#define		MT9P031_READ_MODE_2_COL_MIR		BIT(14)
> +#define		MT9P031_READ_MODE_2_ROW_BLC		BIT(6)
>  #define MT9P031_ROW_ADDRESS_MODE			0x22
>  #define MT9P031_COLUMN_ADDRESS_MODE			0x23
>  #define MT9P031_GLOBAL_GAIN				0x35
>  #define		MT9P031_GLOBAL_GAIN_MIN			8
>  #define		MT9P031_GLOBAL_GAIN_MAX			1024
>  #define		MT9P031_GLOBAL_GAIN_DEF			8
> -#define		MT9P031_GLOBAL_GAIN_MULT		(1 << 6)
> +#define		MT9P031_GLOBAL_GAIN_MULT		BIT(6)
>  #define MT9P031_ROW_BLACK_TARGET			0x49
>  #define MT9P031_ROW_BLACK_DEF_OFFSET			0x4b
>  #define MT9P031_GREEN1_OFFSET				0x60
>  #define MT9P031_GREEN2_OFFSET				0x61
>  #define MT9P031_BLACK_LEVEL_CALIBRATION			0x62
> -#define		MT9P031_BLC_MANUAL_BLC			(1 << 0)
> +#define		MT9P031_BLC_MANUAL_BLC			BIT(0)
>  #define MT9P031_RED_OFFSET				0x63
>  #define MT9P031_BLUE_OFFSET				0x64
>  #define MT9P031_TEST_PATTERN				0xa0
>  #define		MT9P031_TEST_PATTERN_SHIFT		3
> -#define		MT9P031_TEST_PATTERN_ENABLE		(1 << 0)
> +#define		MT9P031_TEST_PATTERN_ENABLE		BIT(0)
>  #define		MT9P031_TEST_PATTERN_DISABLE		(0 << 0)

Similarly here, MT9P031_TEST_PATTERN_DISABLE should be dropped.

>  #define MT9P031_TEST_PATTERN_GREEN			0xa1
>  #define MT9P031_TEST_PATTERN_RED			0xa2

-- 
Regards,

Laurent Pinchart