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

Jacopo Mondi <[email protected]>
Newsgroups org.kernel.vger.linux-media,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel
Message-ID <aoV3zV1jDB2j9b5j@zed>
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 <[email protected]>
> > ---
> > 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.