From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Wayne Porter <wporter82@gmail.com>
Cc: mchehab@kernel.org, gregkh@linuxfoundation.org,
linux-media@vger.kernel.org, devel@driverdev.osuosl.org
Subject: Re: [PATCH] [media] v4l: omap4iss: Fix using BIT macro
Date: Mon, 03 Oct 2016 12:44:50 +0300 [thread overview]
Message-ID: <2520306.7rv5ZlfFad@avalon> (raw)
In-Reply-To: <20161001233746.nowzbvhimd3jutbz@Chronos>
Hi Wayne,
Thank you for the patch.
On Saturday 01 Oct 2016 16:37:46 Wayne Porter wrote:
> Checks found by checkpatch
>
> Signed-off-by: Wayne Porter <wporter82@gmail.com>
> ---
> drivers/staging/media/omap4iss/iss_regs.h | 76 +++++++++++++---------------
> 1 file changed, 38 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/staging/media/omap4iss/iss_regs.h
> b/drivers/staging/media/omap4iss/iss_regs.h index cb415e8..c675212 100644
> --- a/drivers/staging/media/omap4iss/iss_regs.h
> +++ b/drivers/staging/media/omap4iss/iss_regs.h
> @@ -42,7 +42,7 @@
> #define ISS_CTRL_CLK_DIV_MASK (3 << 4)
> #define ISS_CTRL_INPUT_SEL_MASK (3 << 2)
> #define ISS_CTRL_INPUT_SEL_CSI2A (0 << 2)
> -#define ISS_CTRL_INPUT_SEL_CSI2B (1 << 2)
> +#define ISS_CTRL_INPUT_SEL_CSI2B BIT(2)
There's a reason why the driver doesn't use the BIT() macro here (and in
several locations below). Looking at the surrounding code, can you find it out
? (Hint: see how other macros for the same register have the same shift value,
and how they have a corresponding _MASK macro)
> #define ISS_CTRL_SYNC_DETECT_VS_RAISING (3 << 0)
>
> #define ISS_CLKCTRL 0x84
> @@ -97,10 +97,10 @@
> #define CSI2_SYSCONFIG 0x10
> #define CSI2_SYSCONFIG_MSTANDBY_MODE_MASK (3 << 12)
> #define CSI2_SYSCONFIG_MSTANDBY_MODE_FORCE (0 << 12)
> -#define CSI2_SYSCONFIG_MSTANDBY_MODE_NO (1 << 12)
> +#define CSI2_SYSCONFIG_MSTANDBY_MODE_NO BIT(12)
> #define CSI2_SYSCONFIG_MSTANDBY_MODE_SMART (2 << 12)
> -#define CSI2_SYSCONFIG_SOFT_RESET (1 << 1)
> -#define CSI2_SYSCONFIG_AUTO_IDLE (1 << 0)
> +#define CSI2_SYSCONFIG_SOFT_RESET BIT(1)
> +#define CSI2_SYSCONFIG_AUTO_IDLE BIT(0)
>
> #define CSI2_SYSSTATUS 0x14
> #define CSI2_SYSSTATUS_RESET_DONE BIT(0)
> @@ -123,37 +123,37 @@
> #define CSI2_CTRL_MFLAG_LEVH_SHIFT 20
> #define CSI2_CTRL_MFLAG_LEVL_MASK (7 << 17)
> #define CSI2_CTRL_MFLAG_LEVL_SHIFT 17
> -#define CSI2_CTRL_BURST_SIZE_EXPAND (1 << 16)
> -#define CSI2_CTRL_VP_CLK_EN (1 << 15)
> -#define CSI2_CTRL_NON_POSTED_WRITE (1 << 13)
> -#define CSI2_CTRL_VP_ONLY_EN (1 << 11)
> +#define CSI2_CTRL_BURST_SIZE_EXPAND BIT(16)
> +#define CSI2_CTRL_VP_CLK_EN BIT(15)
> +#define CSI2_CTRL_NON_POSTED_WRITE BIT(13)
> +#define CSI2_CTRL_VP_ONLY_EN BIT(11)
> #define CSI2_CTRL_VP_OUT_CTRL_MASK (3 << 8)
> #define CSI2_CTRL_VP_OUT_CTRL_SHIFT 8
> -#define CSI2_CTRL_DBG_EN (1 << 7)
> +#define CSI2_CTRL_DBG_EN BIT(7)
> #define CSI2_CTRL_BURST_SIZE_MASK (3 << 5)
> -#define CSI2_CTRL_ENDIANNESS (1 << 4)
> -#define CSI2_CTRL_FRAME (1 << 3)
> -#define CSI2_CTRL_ECC_EN (1 << 2)
> -#define CSI2_CTRL_IF_EN (1 << 0)
> +#define CSI2_CTRL_ENDIANNESS BIT(4)
> +#define CSI2_CTRL_FRAME BIT(3)
> +#define CSI2_CTRL_ECC_EN BIT(2)
> +#define CSI2_CTRL_IF_EN BIT(0)
>
> #define CSI2_DBG_H 0x44
>
> #define CSI2_COMPLEXIO_CFG 0x50
> -#define CSI2_COMPLEXIO_CFG_RESET_CTRL (1 << 30)
> -#define CSI2_COMPLEXIO_CFG_RESET_DONE (1 << 29)
> +#define CSI2_COMPLEXIO_CFG_RESET_CTRL BIT(30)
> +#define CSI2_COMPLEXIO_CFG_RESET_DONE BIT(29)
> #define CSI2_COMPLEXIO_CFG_PWD_CMD_MASK (3 << 27)
> #define CSI2_COMPLEXIO_CFG_PWD_CMD_OFF (0 << 27)
> -#define CSI2_COMPLEXIO_CFG_PWD_CMD_ON (1 << 27)
> +#define CSI2_COMPLEXIO_CFG_PWD_CMD_ON BIT(27)
> #define CSI2_COMPLEXIO_CFG_PWD_CMD_ULP (2 << 27)
> #define CSI2_COMPLEXIO_CFG_PWD_STATUS_MASK (3 << 25)
> #define CSI2_COMPLEXIO_CFG_PWD_STATUS_OFF (0 << 25)
> -#define CSI2_COMPLEXIO_CFG_PWD_STATUS_ON (1 << 25)
> +#define CSI2_COMPLEXIO_CFG_PWD_STATUS_ON BIT(25)
> #define CSI2_COMPLEXIO_CFG_PWD_STATUS_ULP (2 << 25)
> -#define CSI2_COMPLEXIO_CFG_PWR_AUTO (1 << 24)
> +#define CSI2_COMPLEXIO_CFG_PWR_AUTO BIT(24)
> #define CSI2_COMPLEXIO_CFG_DATA_POL(i) (1 << (((i) *
4) + 3))
> #define CSI2_COMPLEXIO_CFG_DATA_POSITION_MASK(i) (7 << ((i) * 4))
> #define CSI2_COMPLEXIO_CFG_DATA_POSITION_SHIFT(i) ((i) * 4)
> -#define CSI2_COMPLEXIO_CFG_CLOCK_POL (1 << 3)
> +#define CSI2_COMPLEXIO_CFG_CLOCK_POL BIT(3)
> #define CSI2_COMPLEXIO_CFG_CLOCK_POSITION_MASK (7 << 0)
> #define CSI2_COMPLEXIO_CFG_CLOCK_POSITION_SHIFT 0
>
> @@ -222,7 +222,7 @@
> (0x3 << CSI2_CTX_CTRL2_USER_DEF_MAP_SHIFT)
> #define CSI2_CTX_CTRL2_VIRTUAL_ID_MASK (3 << 11)
> #define CSI2_CTX_CTRL2_VIRTUAL_ID_SHIFT 11
> -#define CSI2_CTX_CTRL2_DPCM_PRED (1 << 10)
> +#define CSI2_CTX_CTRL2_DPCM_PRED BIT(10)
> #define CSI2_CTX_CTRL2_FORMAT_MASK (0x3ff << 0)
> #define CSI2_CTX_CTRL2_FORMAT_SHIFT 0
>
> @@ -263,9 +263,9 @@
> #define ISP5_SYSCONFIG (0x0010)
> #define ISP5_SYSCONFIG_STANDBYMODE_MASK (3 << 4)
> #define ISP5_SYSCONFIG_STANDBYMODE_FORCE (0 << 4)
> -#define ISP5_SYSCONFIG_STANDBYMODE_NO (1 << 4)
> +#define ISP5_SYSCONFIG_STANDBYMODE_NO BIT(4)
> #define ISP5_SYSCONFIG_STANDBYMODE_SMART (2 << 4)
> -#define ISP5_SYSCONFIG_SOFTRESET (1 << 1)
> +#define ISP5_SYSCONFIG_SOFTRESET BIT(1)
>
> #define ISP5_IRQSTATUS(i) (0x0028 + (0x10 *
(i)))
> #define ISP5_IRQENABLE_SET(i) (0x002c +
(0x10 * (i)))
> @@ -319,14 +319,14 @@
> #define ISIF_MODESET (0x0004)
> #define ISIF_MODESET_INPMOD_MASK (3 << 12)
> #define ISIF_MODESET_INPMOD_RAW (0 << 12)
> -#define ISIF_MODESET_INPMOD_YCBCR16 (1 << 12)
> +#define ISIF_MODESET_INPMOD_YCBCR16 BIT(12)
> #define ISIF_MODESET_INPMOD_YCBCR8 (2 << 12)
> #define ISIF_MODESET_CCDW_MASK (7 << 8)
> #define ISIF_MODESET_CCDW_2BIT (2 << 8)
> -#define ISIF_MODESET_CCDMD (1 << 7)
> -#define ISIF_MODESET_SWEN (1 << 5)
> -#define ISIF_MODESET_HDPOL (1 << 3)
> -#define ISIF_MODESET_VDPOL (1 << 2)
> +#define ISIF_MODESET_CCDMD BIT(7)
> +#define ISIF_MODESET_SWEN BIT(5)
> +#define ISIF_MODESET_HDPOL BIT(3)
> +#define ISIF_MODESET_VDPOL BIT(2)
>
> #define ISIF_SPH (0x0018)
> #define ISIF_SPH_MASK (0x7fff)
> @@ -349,19 +349,19 @@
>
> #define ISIF_CCOLP (0x004c)
> #define ISIF_CCOLP_CP0_F0_R (0 << 6)
> -#define ISIF_CCOLP_CP0_F0_GR (1 << 6)
> +#define ISIF_CCOLP_CP0_F0_GR BIT(6)
> #define ISIF_CCOLP_CP0_F0_B (3 << 6)
> #define ISIF_CCOLP_CP0_F0_GB (2 << 6)
> #define ISIF_CCOLP_CP1_F0_R (0 << 4)
> -#define ISIF_CCOLP_CP1_F0_GR (1 << 4)
> +#define ISIF_CCOLP_CP1_F0_GR BIT(4)
> #define ISIF_CCOLP_CP1_F0_B (3 << 4)
> #define ISIF_CCOLP_CP1_F0_GB (2 << 4)
> #define ISIF_CCOLP_CP2_F0_R (0 << 2)
> -#define ISIF_CCOLP_CP2_F0_GR (1 << 2)
> +#define ISIF_CCOLP_CP2_F0_GR BIT(2)
> #define ISIF_CCOLP_CP2_F0_B (3 << 2)
> #define ISIF_CCOLP_CP2_F0_GB (2 << 2)
> #define ISIF_CCOLP_CP3_F0_R (0 << 0)
> -#define ISIF_CCOLP_CP3_F0_GR (1 << 0)
> +#define ISIF_CCOLP_CP3_F0_GR BIT(0)
> #define ISIF_CCOLP_CP3_F0_B (3 << 0)
> #define ISIF_CCOLP_CP3_F0_GB (2 << 0)
>
> @@ -381,12 +381,12 @@
> #define IPIPEIF_CFG1 (0x0004)
> #define IPIPEIF_CFG1_INPSRC1_MASK (3 << 14)
> #define IPIPEIF_CFG1_INPSRC1_VPORT_RAW (0 << 14)
> -#define IPIPEIF_CFG1_INPSRC1_SDRAM_RAW (1 << 14)
> +#define IPIPEIF_CFG1_INPSRC1_SDRAM_RAW BIT(14)
> #define IPIPEIF_CFG1_INPSRC1_ISIF_DARKFM (2 << 14)
> #define IPIPEIF_CFG1_INPSRC1_SDRAM_YUV (3 << 14)
> #define IPIPEIF_CFG1_INPSRC2_MASK (3 << 2)
> #define IPIPEIF_CFG1_INPSRC2_ISIF (0 << 2)
> -#define IPIPEIF_CFG1_INPSRC2_SDRAM_RAW (1 << 2)
> +#define IPIPEIF_CFG1_INPSRC2_SDRAM_RAW BIT(2)
> #define IPIPEIF_CFG1_INPSRC2_ISIF_DARKFM (2 << 2)
> #define IPIPEIF_CFG1_INPSRC2_SDRAM_YUV (3 << 2)
>
> @@ -410,25 +410,25 @@
>
> #define IPIPE_SRC_FMT (0x0008)
> #define IPIPE_SRC_FMT_RAW2YUV (0 << 0)
> -#define IPIPE_SRC_FMT_RAW2RAW (1 << 0)
> +#define IPIPE_SRC_FMT_RAW2RAW BIT(0)
> #define IPIPE_SRC_FMT_RAW2STATS (2 << 0)
> #define IPIPE_SRC_FMT_YUV2YUV (3 << 0)
>
> #define IPIPE_SRC_COL (0x000c)
> #define IPIPE_SRC_COL_OO_R (0 << 6)
> -#define IPIPE_SRC_COL_OO_GR (1 << 6)
> +#define IPIPE_SRC_COL_OO_GR BIT(6)
> #define IPIPE_SRC_COL_OO_B (3 << 6)
> #define IPIPE_SRC_COL_OO_GB (2 << 6)
> #define IPIPE_SRC_COL_OE_R (0 << 4)
> -#define IPIPE_SRC_COL_OE_GR (1 << 4)
> +#define IPIPE_SRC_COL_OE_GR BIT(4)
> #define IPIPE_SRC_COL_OE_B (3 << 4)
> #define IPIPE_SRC_COL_OE_GB (2 << 4)
> #define IPIPE_SRC_COL_EO_R (0 << 2)
> -#define IPIPE_SRC_COL_EO_GR (1 << 2)
> +#define IPIPE_SRC_COL_EO_GR BIT(2)
> #define IPIPE_SRC_COL_EO_B (3 << 2)
> #define IPIPE_SRC_COL_EO_GB (2 << 2)
> #define IPIPE_SRC_COL_EE_R (0 << 0)
> -#define IPIPE_SRC_COL_EE_GR (1 << 0)
> +#define IPIPE_SRC_COL_EE_GR BIT(0)
> #define IPIPE_SRC_COL_EE_B (3 << 0)
> #define IPIPE_SRC_COL_EE_GB (2 << 0)
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2016-10-03 9:44 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-01 23:37 [PATCH] [media] v4l: omap4iss: Fix using BIT macro Wayne Porter
2016-10-03 9:44 ` Laurent Pinchart [this message]
2016-10-03 9:58 ` Mauro Carvalho Chehab
2016-10-03 10:01 ` Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2520306.7rv5ZlfFad@avalon \
--to=laurent.pinchart@ideasonboard.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=wporter82@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox