Re: [PATCH v2 2/2] media: i2c: Add Sony IMX908 image sensor driver

[Lachlan Michael]

Dear Jacopi,

Thanks for the suggestion and reference.

On 8/19/2026 6:34 PM, Jacopo Mondi wrote:

> [snip]
>
> > > +static int imx908_init_controls(struct imx908 *imx)
> > > +{
> > > +       struct v4l2_ctrl_handler *hdl = &imx->ctrls.handler;
> > > +       struct v4l2_fwnode_device_properties props;
> > > +       struct v4l2_ctrl *link_freq_ctl;
> > > +       int ret;
> > > +
> > > +       ret = v4l2_ctrl_handler_init(hdl, 11);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > > +       imx->ctrls.pixel_rate = v4l2_ctrl_new_std(hdl, &imx908_ctrl_ops,
> > > +                                                 V4L2_CID_PIXEL_RATE,
> > > +                                                 IMX908_PIXEL_RATE,
> > > +                                                 IMX908_PIXEL_RATE, 1,
> > > +                                                 IMX908_PIXEL_RATE);
> > > +       if (imx->ctrls.pixel_rate)
> > > +               imx->ctrls.pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> >
> > Not needed, PIXEL_RATE is marked as RO by the control framework
> >
> > > +
> > > +       link_freq_ctl = v4l2_ctrl_new_int_menu(hdl, &imx908_ctrl_ops,
> > > +                                              V4L2_CID_LINK_FREQ,
> > > +                                              ARRAY_SIZE(imx908_link_freqs) - 1,
> > > +                                              imx->link_freq_idx,
> > > +                                              imx908_link_freqs);
> > > +
> > > +       if (link_freq_ctl)
> > > +               link_freq_ctl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> Read-only controls should have their control ops set to NULL, so that
> .s_ctrl is never called for them.
>
> See as an example:
> 892ac5e6cd1f ("media: imx355: Use NULL ctrl_ops for HBLANK as it is a read-only control")

Updated accordingly. Both V4L2_CID_PIXEL_RATE and V4L2_CID_LINK_FREQ are 
now created with NULL control ops, as they are read-only controls and do 
not require s_ctrl() handling.

Regards, Lachlan