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

[Tarang Raval]

Hi Mehdi,

> 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@xxxxxxxxxxxxxxxxx>
> > ---
> >  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 ?

The previous value (0x04) keeps test_enable (bit 7) cleared, so the sensor
remains in normal operation. It only presets the color_bar_style field,
which is ignored unless the test pattern is enabled.

> >       { 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 ?

Thanks for spotting that.

I'll fix the capitalization and use consistent spelling throughout

Best Regards,
Tarang

> > +     "black-white square",
> > +     "transparent effect",
> > +     "rolling bar effect",
> > +};
>
> --
> Kind Regards
> Mehdi Djait