[PATCH v3] media: i2c: imx214: Rearrange control initialization

[PATCH v3] media: i2c: imx214: Rearrange control initialization

[Ricardo Ribalda]

Smatch is confused by the control initialization logic. It assumes that
there can be a case where imx214->link_freq can be NULL and ctrls_init
returns 0.

Re-arrage the function to make smatch happy.

This patch fixes this smatch error:
drivers/media/i2c/imx214.c:1109 imx214_ctrls_init() error: we previously assumed 'imx214->link_freq' could be null (see line 1017)

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Right now we are handling this with a quirk in media-ci, if Dan cannot
fix smatch in a kernel cycle we should merge this patch.
---
Changes in v3:
- Rearrange init function. Thanks Sakari
- Link to v2: https://lore.kernel.org/r/20251014-imx214-smatch-v2-1-04218043086d@chromium.org

Changes in v2:
- Fix typo in commit message commit
- Move error tag where it belongs (Thanks Hans!)
- Link to v1: https://lore.kernel.org/r/20250829-imx214-smatch-v1-1-f3d1653b48e4@chromium.org
---
 drivers/media/i2c/imx214.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
index 94ebe625c9e6ee0fb67fe1d89b48b2f1bf58ffc6..d4945b192776bc76f4645ec9ddf047088934d3c5 100644
--- a/drivers/media/i2c/imx214.c
+++ b/drivers/media/i2c/imx214.c
@@ -1014,8 +1014,6 @@ static int imx214_ctrls_init(struct imx214 *imx214)
 						   V4L2_CID_LINK_FREQ,
 						   imx214->bus_cfg.nr_of_link_frequencies - 1,
 						   0, imx214->bus_cfg.link_frequencies);
-	if (imx214->link_freq)
-		imx214->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
 	/*
 	 * WARNING!
@@ -1038,9 +1036,6 @@ static int imx214_ctrls_init(struct imx214 *imx214)
 	imx214->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx214_ctrl_ops,
 					   V4L2_CID_HBLANK, hblank, hblank,
 					   1, hblank);
-	if (imx214->hblank)
-		imx214->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
-
 	exposure_max = mode->vts_def - IMX214_EXPOSURE_OFFSET;
 	exposure_def = min(exposure_max, IMX214_EXPOSURE_DEFAULT);
 	imx214->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx214_ctrl_ops,
@@ -1060,13 +1055,9 @@ static int imx214_ctrls_init(struct imx214 *imx214)
 
 	imx214->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx214_ctrl_ops,
 					  V4L2_CID_HFLIP, 0, 1, 1, 0);
-	if (imx214->hflip)
-		imx214->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
 
 	imx214->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx214_ctrl_ops,
 					  V4L2_CID_VFLIP, 0, 1, 1, 0);
-	if (imx214->vflip)
-		imx214->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
 
 	v4l2_ctrl_cluster(2, &imx214->hflip);
 
@@ -1106,6 +1097,12 @@ static int imx214_ctrls_init(struct imx214 *imx214)
 		return ret;
 	}
 
+	/* Now that the controls have been properly created, set their flags. */
+	imx214->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	imx214->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	imx214->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+	imx214->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+
 	ret = imx214_pll_update(imx214);
 	if (ret < 0) {
 		v4l2_ctrl_handler_free(ctrl_hdlr);

---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20250829-imx214-smatch-c4d4d47428d5

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

Re: [PATCH v3] media: i2c: imx214: Rearrange control initialization

[Sakari Ailus]

Hi Ricardo,

Thank you for the update.

On Mon, Oct 20, 2025 at 08:18:13PM +0000, Ricardo Ribalda wrote:
> Smatch is confused by the control initialization logic. It assumes that
> there can be a case where imx214->link_freq can be NULL and ctrls_init
> returns 0.
> 
> Re-arrage the function to make smatch happy.
> 
> This patch fixes this smatch error:
> drivers/media/i2c/imx214.c:1109 imx214_ctrls_init() error: we previously assumed 'imx214->link_freq' could be null (see line 1017)
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus

Re: [PATCH v3] media: i2c: imx214: Rearrange control initialization

[Ricardo Ribalda]

Hi Sakari

On Tue, 21 Oct 2025 at 11:18, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the update.
Thanks for the prompt review


>
> On Mon, Oct 20, 2025 at 08:18:13PM +0000, Ricardo Ribalda wrote:
> > Smatch is confused by the control initialization logic. It assumes that
> > there can be a case where imx214->link_freq can be NULL and ctrls_init
> > returns 0.
> >
> > Re-arrage the function to make smatch happy.
Re-arrange the function to make Sakari and Smatch happy :P

Best regards!

> xhci_hcd 0000:09:00.0: HC died; cleaning up
> usb 13-2: USB disconnect, device number 2
> >
> > This patch fixes this smatch error:
> > drivers/media/i2c/imx214.c:1109 imx214_ctrls_init() error: we previously assumed 'imx214->link_freq' could be null (see line 1017)
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> --
> Sakari Ailus



-- 
Ricardo Ribalda