Re: [PATCH v3 2/3] media: i2c: imx471: Add Sony IMX471 image sensor driver

[Tarang Raval]

Hi Kate,

> On Wed, May 27, 2026 at 7:42 PM Sakari Ailus
> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> >
> > Hi Kate,
> >
> > On Wed, May 27, 2026 at 02:14:24PM +0800, Kate Hsuan wrote:
> > > > > +struct imx471 {
> > > > > +     struct v4l2_subdev sd;
> > > > > +     struct media_pad pad;
> > > > > +
> > > > > +     struct v4l2_ctrl_handler ctrl_handler;
> > > > > +     /* V4L2 Controls */
> > > > > +     struct v4l2_ctrl *link_freq;
> > > > > +     struct v4l2_ctrl *pixel_rate;
> > > > > +     struct v4l2_ctrl *vblank;
> > > > > +     struct v4l2_ctrl *hblank;
> > > > > +     struct v4l2_ctrl *vflip;
> > > > > +     struct v4l2_ctrl *hflip;
> > > > > +     struct v4l2_ctrl *exposure;
> > > >
> > > > Do you need all these? At least link_freq remains effectively unused.
> > > I'll tweak these ctrl based on
> > > https://libcamera.org/sensor_driver_requirements.html.
> >
> > I rather meant that you're assigning all of these fields but then not using
> > them. You could thus remove the fields and the assignments. But I can't say
> > which ones, apart from link_freq.
>
> I dropped link_freq from the driver but ipu7 complains of errors
> regarding link_freq, shown as follows.
>
> [ 3628.195722] intel_ipu7_isys.isys intel_ipu7.isys.40: bind imx471
> 0-0010 nlanes is 4 port is 0
> [ 3628.196207] intel_ipu7_isys.isys intel_ipu7.isys.40: All sensor
> registration completed.
> [ 3631.754550] intel_ipu7_isys.isys intel_ipu7.isys.40: get link freq
> failed (-2)
> [ 3631.754562] intel_ipu7_isys.isys intel_ipu7.isys.40: CSI-0 PHY
> power up failed -2
> [ 3631.754565] intel_ipu7_isys.isys intel_ipu7.isys.40: enable streams
> Intel IPU7 CSI2 0 failed with -2
>
> So, I'll keep link_freq. :)

I believe Sakari's suggestion was to remove only the struct v4l2_ctrl *link_freq member.

Since the driver does not use the control pointer later, there is no need
to store the returned pointer in struct imx471.

In other words, the suggestion was to remove the unused link_freq field, 
rather than the V4L2_CID_LINK_FREQ control itself.

Best Regards,
Tarang