Re: [PATCH] media: i2c: ov5647: handle V4L2_CID_LINK_FREQ in s_ctrl

[Dave Stevenson]

Hi Suraj

On Sun, 22 Mar 2026 at 13:59, Suraj Sonawane
<surajsonawane0215@xxxxxxxxx> wrote:
>
> Handle V4L2_CID_LINK_FREQ in ov5647_s_ctrl().
>
> Currently this control is defined but not handled in s_ctrl(),
> so V4L2 falls back to estimating link frequency from pixel rate
> and prints warning like:
>
>   v4l2_get_link_freq: Link frequency estimated using pixel rate:
>   result might be inaccurate
>   v4l2_get_link_freq: Consider implementing support for V4L2_CID_LINK_FREQ
>   in the transmitter driver
>
> Handle it as no-op since link frequency is fixed per mode and
> not meant to be changed at runtime.

I'm confused by this description compared to the patch.

v4l2_get_link_freq searches for the V4L2_CID_LINK_FREQ control, and if
found then it calls g_ctrl (not s_ctrl).
If it can't find the control then it searches for V4L2_CID_PIXEL_RATE
and will log the error message quoted.

The control is registered by the ov5647 driver, therefore it should
never go into that second clause, so how have you got that error
message logged?

AFAIK no part of that code path will result in a call to ov5647_s_ctrl
that you're patching.
I've just run with the ov5647 driver on a Pi5 (which uses
v4l2_get_link_freq) running 7.0.0-rc5, and I can't get an error
logged. v4l2_get_link_freq finds V4L2_CID_LINK_FREQ and uses the value
it reports.


You are right that ov5647_s_ctrl doesn't handle V4L2_CID_LINK_FREQ,
which could mean that an error is returned from the __v4l2_ctrl_s_ctrl
calls from within the driver to change the link frequency and lead to
the dev_info in the driver being logged iff the sensor was powered up
at the time (otherwise pm_runtime_get_if_in_use will fail). Generally
the sensor won't be powered on as the pad format is set before
enable_streams, and it shouldn't be possible to change it whilst
streaming.

However, as I understand it, the current preferred way to handle this
case of read only controls where the value is changed by the driver is
to pass NULL as the ops for the ctrl when registering. That is already
the case looking at c6e115144b50 ("media: i2c: ov5647: Add
V4L2_CID_LINK_FREQUENCY control"). So how are you managing to get
ov5647_s_ctrl called for control V4L2_CID_LINK_FREQ at all when it has
no s_ctrl op?

Have I totally missed something here?

  Dave

> Avoid these warnings when control is queried.
>
> Signed-off-by: Suraj Sonawane <surajsonawane0215@xxxxxxxxx>
> ---
>  drivers/media/i2c/ov5647.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> index 6a46ef723..a5a9cff5a 100644
> --- a/drivers/media/i2c/ov5647.c
> +++ b/drivers/media/i2c/ov5647.c
> @@ -999,6 +999,9 @@ static int ov5647_s_ctrl(struct v4l2_ctrl *ctrl)
>                 ret = cci_write(sensor->regmap, OV5647_REG_HTS,
>                                 sensor->mode->format.width + ctrl->val, &ret);
>                 break;
> +       case V4L2_CID_LINK_FREQ:
> +               ret = 0;
> +               break;
>         case V4L2_CID_TEST_PATTERN:
>                 ret = cci_write(sensor->regmap, OV5647_REG_ISPCTRL3D,
>                                 ov5647_test_pattern_val[ctrl->val], NULL);
> --
> 2.34.1
>