Re: [PATCH v2 07/15] media: i2c: os05b10: Add test pattern options

[Mehdi Djait <mehdi.djait@linux.intel.com>]

Hi Tarang,

On Wed, Mar 25, 2026 at 05:13:53PM +0530, Tarang Raval wrote:
> Add V4L2_CID_TEST_PATTERN support with multiple sensor test-pattern modes
> and program them via register 0x5080. Drop the fixed 0x5080 setting from
> the common register sequence so the pattern is selected only through the
> control.
> 
> Signed-off-by: Tarang Raval <tarang.raval@siliconsignals.io>
> ---
>  drivers/media/i2c/os05b10.c | 55 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/os05b10.c b/drivers/media/i2c/os05b10.c
> index bf848eb9ba52..c8de7f5601bf 100644
> --- a/drivers/media/i2c/os05b10.c
> +++ b/drivers/media/i2c/os05b10.c
> @@ -103,6 +103,17 @@
>  #define OS05B10_REG_FORMAT2		CCI_REG8(0x3821)
>  #define OS05B10_HDR_ENABLE		0x04
>  
> +#define OS05B10_REG_PRE_ISP_20_0	CCI_REG8(0x5080)
> +#define OS05B10_DISABLED		0x00
> +#define OS05B10_COLOR_BAR_1		0x80
> +#define OS05B10_COLOR_BAR_2		0x84
> +#define OS05B10_COLOR_BAR_3		0x88
> +#define OS05B10_COLOR_BAR_4		0x8c
> +#define OS05B10_COLOR_SQUARE		0x82
> +#define OS05B10_BW_SQUARE		0x92
> +#define OS05B10_TRANSPARENT_EFFECT	0xa0
> +#define OS05B10_ROLLING_BAR_EFFECT	0xc0
> +
>  #define OS05B10_LINK_FREQ_600MHZ	(600 * HZ_PER_MHZ)
>  
>  static const struct v4l2_rect os05b10_native_area = {
> @@ -396,7 +407,6 @@ static const struct cci_reg_sequence os05b10_common_regs[] = {
>  	{ CCI_REG8(0x5004), 0x00 },
>  	{ CCI_REG8(0x5005), 0x0e },
>  	{ CCI_REG8(0x5036), 0x00 },
> -	{ CCI_REG8(0x5080), 0x04 },

I suppose not writing 0x04 like before does not change anything ? Maybe
mention this value in the commit message ?

>  	{ CCI_REG8(0x5082), 0x00 },
>  	{ CCI_REG8(0x5180), 0x00 },
>  	{ CCI_REG8(0x5181), 0x10 },
> @@ -514,6 +524,30 @@ static const u32 os05b10_mbus_codes[] = {
>  	MEDIA_BUS_FMT_SBGGR10_1X10,
>  };
>  

two nits here:

> +static const char * const os05b10_test_pattern_menu[] = {
> +	"Disabled",

Uppercase

> +	"colour bar type 1",

or lowercase ?

> +	"colour bar type 2",
> +	"colour bar type 3",
> +	"colour bar type 4",

so colour

> +	"color square",

or color ?

> +	"black-white square",
> +	"transparent effect",
> +	"rolling bar effect",
> +};

--
Kind Regards
Mehdi Djait