Re: [PATCH v4 20/21] media: imx355: Switch to using the subdev state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] v4l2_subdev_get_fmt returns a stale format code after HFLIP/VFLIP changes, breaking frame size enumeration.
--

--- Patch [20]: [PATCH v4 20/21] media: imx355: Switch to using the subdev state ---
commit 742fe3da9be2474160e24d38a786d4c4d792ff1f
Author: Dave Stevenson <dave.stevenson@raspberrypi.com>

media: imx355: Switch to using the subdev state

This commit switches the imx355 driver to use the V4L2 subdev state
for managing crop and format rather than handling them within the
driver state.

> diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> index 0462e0feee362..97f4594f7ed9a 100644
> --- a/drivers/media/i2c/imx355.c
> +++ b/drivers/media/i2c/imx355.c

[ ... ]

> @@ -1034,7 +960,7 @@ static const struct v4l2_subdev_video_ops imx355_video_ops = {
>  
>  static const struct v4l2_subdev_pad_ops imx355_pad_ops = {
>  	.enum_mbus_code = imx355_enum_mbus_code,
> -	.get_fmt = imx355_get_pad_format,
> +	.get_fmt = v4l2_subdev_get_fmt,

[Severity: High]
Does switching to v4l2_subdev_get_fmt return a stale format code after
HFLIP/VFLIP changes?

In imx355_set_ctrl(), the hardware orientation and logically the Bayer
format change when HFLIP or VFLIP are updated:

imx355_set_ctrl()
    ...
    case V4L2_CID_HFLIP:
    case V4L2_CID_VFLIP:
        ret = cci_write(imx355->regmap, IMX355_REG_ORIENTATION,
                        imx355->hflip->val | imx355->vflip->val << 1,
                        NULL);
        break;

However, the active state's format code is not updated here. Because
get_fmt now relies on v4l2_subdev_get_fmt instead of the dynamically
evaluated imx355_get_pad_format(), VIDIOC_SUBDEV_G_FMT returns the
stale format.

This appears to break frame size enumeration, as
VIDIOC_SUBDEV_ENUM_FRAME_SIZE will evaluate the correct format code and
reject the stale code with -EINVAL:

imx355_enum_frame_size()
    if (fse->code != imx355_get_format_code(imx355)) {
        return -EINVAL;
    }

Should the active subdev state format be updated when the orientation
controls change?

>  	.set_fmt = imx355_set_pad_format,
>  	.enum_frame_size = imx355_enum_frame_size,
>  	.get_selection = imx355_get_selection,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-media-imx355-v4-0-f7f966fb9ffd@raspberrypi.com?part=20