Re: [PATCH v2 09/20] media: imx355: pixel_rate never changes, so don't recompute

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] In the probe error path, `imx355_power_off()` can be called twice if `v4l2_async_register_subdev_sensor()` fails, leading to unbalanced clock and regulator disables.
--

--- Patch [9]: [PATCH v2 09/20] media: imx355: pixel_rate never changes, so don't recompute ---
commit b383e35f1be32e2228da9d8a41ed9f15cb5ff8e5
Author: Dave Stevenson <dave.stevenson@raspberrypi.com>

media: imx355: pixel_rate never changes, so don't recompute

The pixel rate is always the same, so there is no need to try and
recompute it in imx355_set_pad_format, and then no need to have the
pointer to it stored.

> diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> index 20cf6afee43ee..9d977bac23b6c 100644
> --- a/drivers/media/i2c/imx355.c
> +++ b/drivers/media/i2c/imx355.c

[ ... ]

This is a pre-existing issue, but I noticed a potential double power-off
in the probe error handling path while reviewing this file.

drivers/media/i2c/imx355.c:imx355_probe() {
        ...
        pm_runtime_set_active(imx355->dev);
        pm_runtime_enable(imx355->dev);
        pm_runtime_idle(imx355->dev);
        pm_runtime_set_autosuspend_delay(imx355->dev, 1000);
        pm_runtime_use_autosuspend(imx355->dev);

        ret = v4l2_async_register_subdev_sensor(&imx355->sd);
        if (ret < 0)
                goto error_media_entity_runtime_pm;

        return 0;

error_media_entity_runtime_pm:
        pm_runtime_disable(imx355->dev);
        pm_runtime_set_suspended(imx355->dev);
        v4l2_subdev_cleanup(&imx355->sd);
        media_entity_cleanup(&imx355->sd.entity);

error_handler_free:
        v4l2_ctrl_handler_free(imx355->sd.ctrl_handler);

error_power_off:
        imx355_power_off(imx355->dev);

        return ret;
}

[Severity: High]
If v4l2_async_register_subdev_sensor() fails after the device was already
suspended by pm_runtime_idle(), won't the code fall through to
error_power_off and call imx355_power_off() a second time?

Calling regulator_bulk_disable() and clk_disable_unprepare() on already
disabled resources might cause unbalanced disable warnings and refcount
underflows. If these resources are shared with other active devices, could
this unexpected force-disable break functionality for those other devices?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-media-imx355-v2-0-1683ec07b897@raspberrypi.com?part=9