linux-kernel - Re: [PATCH v4 2/2] media: i2c: Add imx283 camera sensor driver

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] [day] [month] [year] [list]
Message-ID: <20240611172602.GA2226028@thelio-3990X>
Date: Tue, 11 Jun 2024 10:26:02 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Umang Jain <umang.jain@...asonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	NXP Linux Team <linux-imx@....com>,
	Sakari Ailus <sakari.ailus@...ux.intel.com>,
	Kieran Bingham <kieran.bingham@...asonboard.com>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Andy Shevchenko <andy.shevchenko@...il.com>,
	linux-media@...r.kernel.org, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	llvm@...ts.linux.dev
Subject: Re: [PATCH v4 2/2] media: i2c: Add imx283 camera sensor driver

Hi Umang,

On Tue, Apr 02, 2024 at 03:37:51PM +0530, Umang Jain wrote:
> From: Kieran Bingham <kieran.bingham@...asonboard.com>
> 
> Add a v4l2 subdevice driver for the Sony IMX283 image sensor.
> 
> The IMX283 is a 20MP Diagonal 15.86 mm (Type 1) CMOS Image Sensor with
> Square Pixel for Color Cameras.
> 
> The following features are supported:
> - Manual exposure an gain control support
> - vblank/hblank/link freq control support
> - Test pattern support control
> - Arbitrary horizontal and vertical cropping
> - Supported resolution:
>   - 5472x3648 @ 20fps (SRGGB12)
>   - 5472x3648 @ 25fps (SRGGB10)
>   - 2736x1824 @ 50fps (SRGGB12)
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@...asonboard.com>
> Signed-off-by: Umang Jain <umang.jain@...asonboard.com>

This change is now in -next as commit ccb4eb4496fa ("media: i2c: Add
imx283 camera sensor driver").

> +++ b/drivers/media/i2c/imx283.c
...
> +/* IMX283 native and active pixel array size. */
> +static const struct v4l2_rect imx283_native_area = {
> +	.top = 0,
> +	.left = 0,
> +	.width = 5592,
> +	.height = 3710,
> +};
> +
> +static const struct v4l2_rect imx283_active_area = {
> +	.top = 40,
> +	.left = 108,
> +	.width = 5472,
> +	.height = 3648,
> +};
...
> +#define CENTERED_RECTANGLE(rect, _width, _height)			\
> +	{								\
> +		.left = rect.left + ((rect.width - (_width)) / 2),	\
> +		.top = rect.top + ((rect.height - (_height)) / 2),	\
> +		.width = (_width),					\
> +		.height = (_height),					\
> +	}
...
> +		.crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),

This construct does not work with GCC prior to 7 and Clang prior to 17
(where certain const structures and variables will be considered
constant expressions for the sake of initializers and such), resulting
in:

  drivers/media/i2c/imx283.c:443:30: error: initializer element is not constant
     .crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
                                ^
  drivers/media/i2c/imx283.c:412:11: note: in definition of macro 'CENTERED_RECTANGLE'
     .left = rect.left + ((rect.width - (_width)) / 2), \
             ^~~~
  drivers/media/i2c/imx283.c:443:30: note: (near initialization for 'supported_modes_12bit[0].crop.left')
     .crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
                                ^
  drivers/media/i2c/imx283.c:412:11: note: in definition of macro 'CENTERED_RECTANGLE'
     .left = rect.left + ((rect.width - (_width)) / 2), \
             ^~~~
  drivers/media/i2c/imx283.c:443:30: error: initializer element is not constant
     .crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
                                ^
  drivers/media/i2c/imx283.c:413:10: note: in definition of macro 'CENTERED_RECTANGLE'
     .top = rect.top + ((rect.height - (_height)) / 2), \
            ^~~~
  drivers/media/i2c/imx283.c:443:30: note: (near initialization for 'supported_modes_12bit[0].crop.top')
     .crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
                                ^
  drivers/media/i2c/imx283.c:413:10: note: in definition of macro 'CENTERED_RECTANGLE'
     .top = rect.top + ((rect.height - (_height)) / 2), \
            ^~~~

  drivers/media/i2c/imx283.c:443:30: error: initializer element is not a compile-time constant
                  .crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
                          ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/media/i2c/imx283.c:412:11: note: expanded from macro 'CENTERED_RECTANGLE'
                  .left = rect.left + ((rect.width - (_width)) / 2),      \
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/media/i2c/imx283.c:492:30: error: initializer element is not a compile-time constant
                  .crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
                          ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/media/i2c/imx283.c:412:11: note: expanded from macro 'CENTERED_RECTANGLE'
                  .left = rect.left + ((rect.width - (_width)) / 2),      \
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  2 errors generated.

with these compiler versions. Usually, the values are just refactored
with #define macros.

Cheers,
Nathan