Re: [PATCH v5 3/4] media: i2c: og0ve1b: Introduce per-sensor data structure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but incorrect handling of pm_runtime_get_if_active() error returns causes an unbalanced pm_runtime_put() and usage counter underflow.
--

--- Patch [3]: [PATCH v5 3/4] media: i2c: og0ve1b: Introduce per-sensor data structure ---
commit 4767769879b1fbb36a5857ed0679efc6bcd34ead
Author: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>

media: i2c: og0ve1b: Introduce per-sensor data structure

This refactors the driver to separate sensor-specific data from the common
driver code. It introduces a sensor_data structure populated through
i2c_get_match_data() to hold the chip ID, MCLK frequency, link frequencies,
and supported modes.

> diff --git a/drivers/media/i2c/og0ve1b.c b/drivers/media/i2c/og0ve1b.c
> index da5c8e49d4a81..f560d5d90fda9 100644
> --- a/drivers/media/i2c/og0ve1b.c
> +++ b/drivers/media/i2c/og0ve1b.c

[ ... ]

> @@ -314,7 +339,7 @@ static int og0ve1b_set_ctrl(struct v4l2_ctrl *ctrl)
>  				ctrl->val + mode->height, NULL);
>  		break;
>  	case V4L2_CID_TEST_PATTERN:
> -		ret = og0ve1b_enable_test_pattern(og0ve1b, ctrl->val);
> +		ret = og0ve1b->data->enable_test_pattern(og0ve1b, ctrl->val);
>  		break;
>  	default:
>  		ret = -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing
og0ve1b_set_ctrl(), I noticed a potential PM usage counter underflow.

Earlier in this function, there is a check:

	if (!pm_runtime_get_if_active(og0ve1b->dev))
		return 0;

If pm_runtime_get_if_active() returns a negative error code (like -EINVAL
when Runtime PM is disabled), the negation !(-EINVAL) evaluates to false.
This bypasses the early return.

Execution then proceeds through this switch statement and unconditionally
calls pm_runtime_put(og0ve1b->dev) at the end of the function. 

Since the initial get failed and never incremented the usage counter,
wouldn't this unconditional put operation cause an unbalanced decrement and
underflow the counter?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-og0va1b-v5-0-c2b90b601241@oss.qualcomm.com?part=3