Re: [PATCH v3 2/3] media: i2c: Add driver for OmniVision OV32C4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incorrect error handling of pm_runtime_get_if_active() leads to PM usage counter underflow.
- [Medium] Missing colorspace and format field initializations in ov32c4_update_pad_format().
- [Low] Missing stubs for ACPI resource functions when CONFIG_ACPI is disabled causes build errors.
--

commit cc0f9470db4c3422b5043e7cf05c1cf8e2c925dd
Author: Robert Bozik <robertbozik@gmail.com>
Date:   Sat Aug 29 11:58:31 2026 +0000

media: i2c: Add driver for OmniVision OV32C4

This commit adds a Video4Linux2 driver for the OmniVision OV32C4 CMOS image 
sensor, connected over MIPI CSI-2. It supports 3264x1840 at 30 fps 
enumerated through ACPI and paired with an Intel IPU.

> diff --git a/drivers/media/i2c/ov32c4.c b/drivers/media/i2c/ov32c4.c
> new file mode 100644
> index 0000000000000..cfeeafcfbc56f
> --- /dev/null
> +++ b/drivers/media/i2c/ov32c4.c
[ ... ]
> +static u16 ov32c4_acpi_i2c_addr(struct device *dev, int index)
> +{
> +	struct ov32c4_i2c_res_ctx ctx = { .want = index };
> +	struct acpi_device *adev = ACPI_COMPANION(dev);
> +	LIST_HEAD(resources);
> +	int ret;
> +
> +	if (!adev)
> +		return 0;
> +
> +	ret = acpi_dev_get_resources(adev, &resources, ov32c4_i2c_res_cb, &ctx);
> +	acpi_dev_free_resource_list(&resources);

[Severity: Low]
When CONFIG_ACPI is disabled, are acpi_dev_get_resources() and
acpi_dev_free_resource_list() available?

Since the depends on ACPI constraint was dropped from Kconfig, this might
cause build failures on non-ACPI configurations where these functions lack
empty stub implementations.

[ ... ]
> +static int ov32c4_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct ov32c4 *ov32c4 = container_of(ctrl->handler,
> +					     struct ov32c4, ctrl_handler);
> +	const u32 height = supported_modes[0].height;
> +	s64 exposure_max;
> +	int ret = 0;
> +
> +	if (ctrl->id == V4L2_CID_VBLANK) {
[ ... ]
> +	}
> +
> +	if (!pm_runtime_get_if_active(ov32c4->dev))
> +		return 0;

[Severity: High]
Does this properly handle the case where Runtime PM is disabled?

If pm_runtime_get_if_active() returns -EINVAL (which occurs when Runtime PM
is disabled), the !-EINVAL condition evaluates to false. This bypasses
the early return and allows execution to continue down to pm_runtime_put(),
leading to a usage counter underflow.

[ ... ]
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	pm_runtime_put(ov32c4->dev);
> +
> +	return ret;
> +}
[ ... ]
> +static void ov32c4_update_pad_format(const struct ov32c4_mode *mode,
> +				     struct v4l2_mbus_framefmt *fmt)
> +{
> +	fmt->width = mode->width;
> +	fmt->height = mode->height;
> +	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> +	fmt->field = V4L2_FIELD_NONE;
> +}

[Severity: Medium]
Are the remaining format fields intentionally left uninitialized here?

Fields like colorspace, ycbcr_enc, quantization, and xfer_func are not
set. If a user application calls VIDIOC_SUBDEV_S_FMT with arbitrary values
for these fields, they will bypass this function and be stored in the
subdev state, leading to incorrect colorspace reporting to userspace.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829115832.8749-1-robertbozik@gmail.com?part=2