Re: [PATCH v3 4/4] media: ox05b1s: Add support for Omnivision OS08A20 raw sensor

[Christophe JAILLET]

Le 24/01/2025 à 01:12, Mirela Rabulea a écrit :
> This is an 8 megapixel raw10/raw12 sensor with HDR capabilities.
> HDR mode control is supported, with one HDR mode: staggered HDR
> with 2 exposures on separate virtual channels. However, for now,
> only one exposure (VC 0) is accessible via get_frame_desc.
>
> Supported resolutions:
>     - 1920 x 1080 @ 60fps (SBGGR10, no HDR)
>     - 1920 x 1080 @ 30fps (SBGGR10, HDR)
>     - 3840 x 2160 @ 30fps (SBGGR12, no HDR)
>     - 3840 x 2160 @ 15fps (SBGGR12, HDR)
>     - 3840 x 2160 @ 30fps (SBGGR10, no HDR)
>     - 3840 x 2160 @ 15fps (SBGGR10, HDR)
>
> Signed-off-by: Mirela Rabulea <mirela.rabulea-3arQi8VN3Tc@xxxxxxxxxxxxxxxx>

...

> +/* Common register configuration for Omnivision OS08A20 raw camera */
> +struct ox05b1s_reg os08a20_init_setting_common[] = {
> +	{ 0x3605, 0x50 },
> +	{ 0x3610, 0x39 },
> +	{ 0x375e, 0x0b },
> +	{ 0x5001, 0x42 },
> +	{ 0x5005, 0x00 },
> +	{ /* sentinel*/ },

No need for ending comma.
The struct could maybe be const.

> +};
> +
> +/* Common register configuration for Omnivision OS08A20 10 bit */
> +struct ox05b1s_reg os08a20_init_setting_10bit[] = {
> +	{ 0x031e, 0x09 },
> +	{ 0x3609, 0xb5 },
> +	{ 0x3660, 0x43 },
> +	{ 0x3706, 0x35 },
> +	{ 0x370a, 0x00 },
> +	{ 0x370b, 0x98 },
> +	{ 0x3709, 0x49 },
> +	{ /* sentinel*/ },

No need for ending comma.
The struct could maybe be const.

> +};
> +
> +/* Common register configuration for Omnivision OS08A20 12 bit */
> +struct ox05b1s_reg os08a20_init_setting_12bit[] = {
> +	{ 0x031e, 0x0a },
> +	{ 0x3609, 0xdb },
> +	{ 0x3660, 0xd3 },
> +	{ 0x3706, 0x6a },
> +	{ 0x370a, 0x01 },
> +	{ 0x370b, 0x30 },
> +	{ 0x3709, 0x48 },
> +	{ /* sentinel*/ },

No need for ending comma.
The struct could maybe be const.

> +};
> +
> +/* Mode specific register configurations for Omnivision OS08A20 raw camera */
> +
> +/* OS08A20 3840 x 2160 @30fps BGGR10 no more HDR */
> +struct ox05b1s_reg os08a20_init_setting_4k_10b[] = {
> +	{ 0x3821, 0x04 }, /* mirror */
> +	{ 0x4837, 0x10 }, /* PCLK PERIOD */
> +	{ /* sentinel*/ },

No need for ending comma.
The struct could maybe be const.

> +};
> +
> +/* OS08A20 3840 x 2160 @30fps BGGR12 */
> +struct ox05b1s_reg os08a20_init_setting_4k_12b[] = {
> +	{ 0x3821, 0x04 }, /* mirror */
> +	{ 0x4837, 0x10 }, /* PCLK PERIOD */
> +	{ /* sentinel*/ },

No need for ending comma.
The struct could maybe be const.

> +};
> +
> +/* OS08A20 1920 x 1080 @60fps BGGR10 */
> +struct ox05b1s_reg os08a20_init_setting_1080p_10b[] = {
> +	{ 0x3814, 0x03 }, /* X INC ODD */
> +	{ 0x3816, 0x03 }, /* Y INC ODD */
> +	{ 0x3820, 0x01 }, /* vertical bining */
> +	{ 0x3821, 0x05 }, /* mirror, horizontal bining */
> +	{ 0x4837, 0x16 }, /* PCLK PERIOD */
> +	{ /* sentinel*/ },

No need for ending comma.
The struct could maybe be const.

> +};
> +
> +struct ox05b1s_reglist os08a20_reglist_4k_12b[] = {

The struct could maybe be const.

> +	{
> +		.regs = os08a20_init_setting_common
> +	}, {
> +		.regs = os08a20_init_setting_12bit
> +	}, {
> +		.regs = os08a20_init_setting_4k_12b
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +
> +struct ox05b1s_reglist os08a20_reglist_1080p_10b[] = {

The struct could maybe be const.

> +	{
> +		.regs = os08a20_init_setting_common
> +	}, {
> +		.regs = os08a20_init_setting_10bit
> +	}, {
> +		.regs = os08a20_init_setting_1080p_10b
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +
>   /*
>    * Register configuration for Omnivision OX05B1S raw camera
>    * 2592X1944_30FPS_FULL_RGBIr 2592 1944

CJ