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

[Nathan Chancellor <nathan@kernel.org>]

Hi Umang,

On Tue, Apr 02, 2024 at 03:37:51PM +0530, Umang Jain wrote:
> From: Kieran Bingham <kieran.bingham@ideasonboard.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@ideasonboard.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.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