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

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hello, one tiny addition

On Fri, Aug 07, 2026 at 12:23:02PM +0530, Jai Luthra wrote:
> Hi Lachlan,
>
> Thank you for the patch!
>
> The driver is mostly in good shape, except a few small things and requests
> for information that Sony is in the best position to help us with.
>
> Quoting Lachlan Michael (2026-08-06 12:39:34)
> > The Sony IMX908 is an 8.39 megapixel (3856x2176) CMOS image sensor
> > with a MIPI CSI-2 output interface, configurable as either 2 or 4
> > data lanes.
> >
> > Add a V4L2 sub-device driver for the sensor. The driver supports
> > RAW10 and RAW12 output formats, exposure and analogue gain controls,
> > horizontal and vertical flipping, horizontal and vertical blanking
> > controls, window cropping and test pattern generation.
> >
> > HDR modes and RAW16 output are not currently supported.
> >
> > Signed-off-by: Lachlan Michael <lachlan.michael@sony.com>
> > ---
> > Changes in v2:
> > - Treat the pixel rate as a fixed sensor property (594 MHz, 8 px/clock),

[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")

Thanks
  j