linux-kernel - Re: [PATCH v3 21/25] drivers: media: i2c: imx258: Use macros

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: Thu, 4 Apr 2024 06:46:39 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Luigi311 <git@...gi311.com>
Cc: linux-media@...r.kernel.org, dave.stevenson@...pberrypi.com,
	jacopo.mondi@...asonboard.com, mchehab@...nel.org, robh@...nel.org,
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
	shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
	festevam@...il.com, devicetree@...r.kernel.org, imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	pavel@....cz, phone-devel@...r.kernel.org,
	Ondrej Jirman <megi@....cz>
Subject: Re: [PATCH v3 21/25] drivers: media: i2c: imx258: Use macros

On Wed, Apr 03, 2024 at 01:17:26PM -0600, Luigi311 wrote:
> On 4/3/24 10:23, Sakari Ailus wrote:
> > Hi Luis,
> > 
> > On Wed, Apr 03, 2024 at 09:03:50AM -0600, git@...gi311.com wrote:
> >> From: Luis Garcia <git@...gi311.com>
> >>
> >> Use understandable macros instead of raw values.
> >>
> >> Signed-off-by: Ondrej Jirman <megi@....cz>
> >> Signed-off-by: Luis Garcia <git@...gi311.com>
> >> ---
> >>  drivers/media/i2c/imx258.c | 434 ++++++++++++++++++-------------------
> >>  1 file changed, 207 insertions(+), 227 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> >> index e2ecf6109516..30352c33f63c 100644
> >> --- a/drivers/media/i2c/imx258.c
> >> +++ b/drivers/media/i2c/imx258.c
> >> @@ -33,8 +33,6 @@
> >>  #define IMX258_VTS_30FPS_VGA		0x034c
> >>  #define IMX258_VTS_MAX			65525
> >>  
> >> -#define IMX258_REG_VTS			0x0340
> >> -
> >>  /* HBLANK control - read only */
> >>  #define IMX258_PPL_DEFAULT		5352
> >>  
> >> @@ -90,6 +88,53 @@
> >>  #define IMX258_PIXEL_ARRAY_WIDTH	4208U
> >>  #define IMX258_PIXEL_ARRAY_HEIGHT	3120U
> >>  
> >> +/* regs */
> >> +#define IMX258_REG_PLL_MULT_DRIV                  0x0310
> >> +#define IMX258_REG_IVTPXCK_DIV                    0x0301
> >> +#define IMX258_REG_IVTSYCK_DIV                    0x0303
> >> +#define IMX258_REG_PREPLLCK_VT_DIV                0x0305
> >> +#define IMX258_REG_IOPPXCK_DIV                    0x0309
> >> +#define IMX258_REG_IOPSYCK_DIV                    0x030b
> >> +#define IMX258_REG_PREPLLCK_OP_DIV                0x030d
> >> +#define IMX258_REG_PHASE_PIX_OUTEN                0x3030
> >> +#define IMX258_REG_PDPIX_DATA_RATE                0x3032
> >> +#define IMX258_REG_SCALE_MODE                     0x0401
> >> +#define IMX258_REG_SCALE_MODE_EXT                 0x3038
> >> +#define IMX258_REG_AF_WINDOW_MODE                 0x7bcd
> >> +#define IMX258_REG_FRM_LENGTH_CTL                 0x0350
> >> +#define IMX258_REG_CSI_LANE_MODE                  0x0114
> >> +#define IMX258_REG_X_EVN_INC                      0x0381
> >> +#define IMX258_REG_X_ODD_INC                      0x0383
> >> +#define IMX258_REG_Y_EVN_INC                      0x0385
> >> +#define IMX258_REG_Y_ODD_INC                      0x0387
> >> +#define IMX258_REG_BINNING_MODE                   0x0900
> >> +#define IMX258_REG_BINNING_TYPE_V                 0x0901
> >> +#define IMX258_REG_FORCE_FD_SUM                   0x300d
> >> +#define IMX258_REG_DIG_CROP_X_OFFSET              0x0408
> >> +#define IMX258_REG_DIG_CROP_Y_OFFSET              0x040a
> >> +#define IMX258_REG_DIG_CROP_IMAGE_WIDTH           0x040c
> >> +#define IMX258_REG_DIG_CROP_IMAGE_HEIGHT          0x040e
> >> +#define IMX258_REG_SCALE_M                        0x0404
> >> +#define IMX258_REG_X_OUT_SIZE                     0x034c
> >> +#define IMX258_REG_Y_OUT_SIZE                     0x034e
> >> +#define IMX258_REG_X_ADD_STA                      0x0344
> >> +#define IMX258_REG_Y_ADD_STA                      0x0346
> >> +#define IMX258_REG_X_ADD_END                      0x0348
> >> +#define IMX258_REG_Y_ADD_END                      0x034a
> >> +#define IMX258_REG_EXCK_FREQ                      0x0136
> >> +#define IMX258_REG_CSI_DT_FMT                     0x0112
> >> +#define IMX258_REG_LINE_LENGTH_PCK                0x0342
> >> +#define IMX258_REG_SCALE_M_EXT                    0x303a
> >> +#define IMX258_REG_FRM_LENGTH_LINES               0x0340
> >> +#define IMX258_REG_FINE_INTEG_TIME                0x0200
> >> +#define IMX258_REG_PLL_IVT_MPY                    0x0306
> >> +#define IMX258_REG_PLL_IOP_MPY                    0x030e
> >> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H       0x0820
> >> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L       0x0822
> >> +
> >> +#define REG8(a, v) { a, v }
> >> +#define REG16(a, v) { a, ((v) >> 8) & 0xff }, { (a) + 1, (v) & 0xff }
> > 
> > The patch is nice but these macros are better replaced by the V4L2 CCI
> > helper that also offers register access functions. Could you add a patch to
> > convert the driver to use it (maybe after this one)?
> > 
> 
> Ohh perfect, using something else would be great. Ill go ahead and see
> if I can get that working.

Thanks. It may be easier to just do it in this one actually. Up to you.

-- 
Sakari Ailus