Re: [PATCH v2] media: i2c: imx214: Exit early on control init errors

[Hans Verkuil <hverkuil+cisco@kernel.org>]

On 14/10/2025 13:54, Ricardo Ribalda wrote:
> On Tue, 14 Oct 2025 at 13:52, Hans Verkuil <hverkuil+cisco@kernel.org> wrote:
>>
>> On 14/10/2025 13:00, Ricardo Ribalda wrote:
>>> Now we try to initialize all the controls and at the very end check
>>> ctrl_hdlr->error to check if one of them has failed.
>>>
>>> This confuses smatch, who do not know how to track the state of
>>> imx214->link_freq.
>>>
>>> drivers/media/i2c/imx214.c:1109 imx214_ctrls_init() error: we previously assumed 'imx214->link_freq' could be null (see line 1017)
>>>
>>> Fix this by exiting early on control initialization errors.
>>>
>>> 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.
>>
>> OK, will you keep track of this? This patch is delegated to me, so if you tell me when
>> it should be merged, then I can do that. And if it is fixed in smatch, then you can just
>> drop this patch in patchwork, of course.
> 
> Which is the latest rc that you will feel comfortable merging this? rc5? rc6?

Late rc5.

Regards,

	Hans

> 
> I can ping you then if smatch is not ready by then.
> 
> 
> Thanks :)
> 
>>
>> Until then it just stays in my TODO list.
>>
>> Regards,
>>
>>         Hans
>>
>>> ---
>>> 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 | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
>>> index 94ebe625c9e6ee0fb67fe1d89b48b2f1bf58ffc6..c66f0e18726c3fc15df91c37888a797bcea82134 100644
>>> --- a/drivers/media/i2c/imx214.c
>>> +++ b/drivers/media/i2c/imx214.c
>>> @@ -1014,8 +1014,10 @@ 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;
>>> +     if (!imx214->link_freq)
>>> +             goto err_init_ctrl;
>>> +
>>> +     imx214->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>>
>>>       /*
>>>        * WARNING!
>>> @@ -1099,6 +1101,7 @@ static int imx214_ctrls_init(struct imx214 *imx214)
>>>
>>>       v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &imx214_ctrl_ops, &props);
>>>
>>> +err_init_ctrl:
>>>       ret = ctrl_hdlr->error;
>>>       if (ret) {
>>>               v4l2_ctrl_handler_free(ctrl_hdlr);
>>>
>>> ---
>>> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
>>> change-id: 20250829-imx214-smatch-c4d4d47428d5
>>>
>>> Best regards,
>>
> 
>