Re: [PATCH v2] media: i2c: ov9282: Add test pattern control

[xiaolei wang <xiaolei.wang@windriver.com>]

Hi Sakari,

Thanks for the review.

On 3/18/26 15:36, Sakari Ailus wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> Hi Xiaolei,
>
> Thanks for the update.
>
> On Wed, Mar 18, 2026 at 01:16:18PM +0800, Xiaolei Wang wrote:
>> Add V4L2_CID_TEST_PATTERN control with bar and solid white patterns.
>> Since the OV9282 is a monochrome sensor, white is more useful than
>> black for solid pattern testing.
>>
>> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
>> ---
>>
>> Changes in v2:
>>   - Rename to "Bar" / "Solid White" since OV9282 is a monochrome
>>     sensor (Dave)
>>   - Use cci_write instead of cci_update_bits (Dave)
>>   - Default solid pattern to white (Dave)
>>   Link to v1: https://patchwork.linuxtv.org/project/linux-media/patch/20260316090558.1537823-1-xiaolei.wang@windriver.com/
>>
>>   drivers/media/i2c/ov9282.c | 61 +++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
>> index 2167fb73ea41..85f612e7ab28 100644
>> --- a/drivers/media/i2c/ov9282.c
>> +++ b/drivers/media/i2c/ov9282.c
>> @@ -104,6 +104,17 @@
>>   #define OV9282_REG_STROBE_FRAME_SPAN         CCI_REG32(0x3925)
>>   #define OV9282_STROBE_FRAME_SPAN_DEFAULT     0x0000001a
>>
>> +/* Test Pattern registers */
>> +#define OV9282_REG_TEST_PATTERN_BAR  CCI_REG8(0x5e00)
>> +#define OV9282_TEST_PATTERN_BAR_EN   BIT(7)
>> +#define OV9282_REG_TEST_PATTERN_SOLID        CCI_REG8(0x4320)
>> +#define OV9282_TEST_PATTERN_SOLID_EN BIT(1)
>> +#define OV9282_TEST_PATTERN_SOLID_DEFAULT    0x80
>> +#define OV9282_REG_SOLID_P1          CCI_REG16(0x4322)
>> +#define OV9282_REG_SOLID_P2          CCI_REG16(0x4324)
>> +#define OV9282_REG_SOLID_P3          CCI_REG16(0x4328)
>> +#define OV9282_REG_SOLID_P4          CCI_REG16(0x4326)
>> +
>>   /* Input clock rate */
>>   #define OV9282_INCLK_RATE    24000000
>>
>> @@ -462,6 +473,18 @@ static const struct ov9282_mode supported_modes[] = {
>>        },
>>   };
>>
>> +enum {
>> +     OV9282_TEST_PATTERN_DISABLED,
>> +     OV9282_TEST_PATTERN_BAR,
>> +     OV9282_TEST_PATTERN_SOLID_WHITE,
>> +};
>> +
>> +static const char * const ov9282_test_pattern_menu[] = {
>> +     "Disabled",
>> +     "Bar",
>> +     "Solid White",
>> +};
>> +
>>   /**
>>    * to_ov9282() - ov9282 V4L2 sub-device to ov9282 device.
>>    * @subdev: pointer to ov9282 V4L2 sub-device
>> @@ -586,6 +609,32 @@ static u32 ov9282_flash_duration_to_us(struct ov9282 *ov9282, u32 value)
>>        return DIV_ROUND_UP(value * frame_width, OV9282_STROBE_SPAN_FACTOR);
>>   }
>>
>> +static int ov9282_set_ctrl_test_pattern(struct ov9282 *ov9282, int pattern)
>> +{
>> +     int ret;
>> +
>> +     ret = cci_write(ov9282->regmap, OV9282_REG_TEST_PATTERN_BAR,
>> +                     pattern == OV9282_TEST_PATTERN_BAR ?
>> +                     OV9282_TEST_PATTERN_BAR_EN : 0, NULL);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (pattern == OV9282_TEST_PATTERN_SOLID_WHITE) {
>> +             /* Set all four pixel values to 0x3ff (white) */
>> +             cci_write(ov9282->regmap, OV9282_REG_SOLID_P1, 0x3ff, &ret);
>> +             cci_write(ov9282->regmap, OV9282_REG_SOLID_P2, 0x3ff, &ret);
>> +             cci_write(ov9282->regmap, OV9282_REG_SOLID_P3, 0x3ff, &ret);
>> +             cci_write(ov9282->regmap, OV9282_REG_SOLID_P4, 0x3ff, &ret);
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>> +     return cci_write(ov9282->regmap, OV9282_REG_TEST_PATTERN_SOLID,
>> +                      pattern == OV9282_TEST_PATTERN_SOLID_WHITE ?
>> +                      OV9282_TEST_PATTERN_SOLID_DEFAULT | OV9282_TEST_PATTERN_SOLID_EN :
>> +                      OV9282_TEST_PATTERN_SOLID_DEFAULT, NULL);
> As OV9282_TEST_PATTERN_SOLID_DEFAULT isn't conditional, you can keep it out
> of the ternary operator.

My mistake, I will update it in the next version.

thanks

xiaolei

>
>> +}
>> +
>>   /**
>>    * ov9282_set_ctrl() - Set subdevice control
>>    * @ctrl: pointer to v4l2_ctrl structure
>> @@ -662,6 +711,11 @@ static int ov9282_set_ctrl(struct v4l2_ctrl *ctrl)
>>        case V4L2_CID_FLASH_DURATION:
>>                ret = cci_write(ov9282->regmap, OV9282_REG_STROBE_FRAME_SPAN, ctrl->val, NULL);
>>                break;
>> +
>> +     case V4L2_CID_TEST_PATTERN:
>> +             ret = ov9282_set_ctrl_test_pattern(ov9282, ctrl->val);
>> +             break;
>> +
>>        default:
>>                dev_err(ov9282->dev, "Invalid control %d", ctrl->id);
>>                ret = -EINVAL;
>> @@ -1242,7 +1296,7 @@ static int ov9282_init_controls(struct ov9282 *ov9282)
>>        u32 lpfr;
>>        int ret;
>>
>> -     ret = v4l2_ctrl_handler_init(ctrl_hdlr, 12);
>> +     ret = v4l2_ctrl_handler_init(ctrl_hdlr, 13);
>>        if (ret)
>>                return ret;
>>
>> @@ -1314,6 +1368,11 @@ static int ov9282_init_controls(struct ov9282 *ov9282)
>>                                  V4L2_CID_FLASH_DURATION, 0, exposure_us, 1,
>>                                  OV9282_STROBE_FRAME_SPAN_DEFAULT);
>>
>> +     v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &ov9282_ctrl_ops,
>> +                                  V4L2_CID_TEST_PATTERN,
>> +                                  ARRAY_SIZE(ov9282_test_pattern_menu) - 1,
>> +                                  0, 0, ov9282_test_pattern_menu);
>> +
>>        ret = v4l2_fwnode_device_parse(ov9282->dev, &props);
>>        if (!ret) {
>>                /* Failure sets ctrl_hdlr->error, which we check afterwards anyway */
> --
> Regards,
>
> Sakari Ailus