Re: [PATCH v6 1/5] media: hi846: Fix hi846_write_reg_16 handling

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unbalanced pm_runtime_put() underflow and skipped I2C hardware writes result from unhandled negative error codes (e.g., -EINVAL) returned by pm_runtime_get_if_in_use().
--

commit baf92f1dfb7e9ab68d0b8a703aedfddfef46030d
Author: Pengyu Luo <mitltlatltl@gmail.com>

media: hi846: Fix hi846_write_reg_16 handling

hi846_write_reg_16() does not clear a positive *err value on success.
pm_runtime_get_if_in_use() returns a positive value when the device
is already in use. Fix this by resetting *err to 0 only when it is positive.

> diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c
> index a3f77b8434ca7..7f069aca0fce5 100644
> --- a/drivers/media/i2c/hi846.c
> +++ b/drivers/media/i2c/hi846.c
> @@ -1271,6 +1271,8 @@ static void hi846_write_reg_16(struct hi846 *hi846, u16 reg, u16 val, int *err)
>  	if (*err < 0)
>  		return;
>  
> +	*err = 0;
> +
>  	put_unaligned_be16(reg, buf);
>  	put_unaligned_be16(val, buf + 2);
>  	ret = i2c_master_send(client, buf, sizeof(buf));

[Severity: High]
This is a pre-existing issue, but does the error handling in hi846_set_ctrl()
correctly handle negative error codes returned by pm_runtime_get_if_in_use()?

If pm_runtime_get_if_in_use() returns a negative error code (like -EINVAL),
it bypasses the early return condition:

drivers/media/i2c/hi846.c:hi846_set_ctrl() {
    ...
	ret = pm_runtime_get_if_in_use(&client->dev);
	if (!ret || ret == -EAGAIN)
		return 0;

	switch (ctrl->id) {
    ...
}

This causes the negative error code to propagate into the switch statement.
If the V4L2_CID_EXPOSURE or V4L2_CID_VBLANK case is hit, the negative value
is passed to hi846_write_reg_16(), which will immediately abort and skip all
I2C register initialization because of the `if (*err < 0)` check.

Furthermore, execution then unconditionally falls through to the end of
hi846_set_ctrl():

    ...
	pm_runtime_put(&client->dev);

	return ret;
}

Since a negative return from pm_runtime_get_if_in_use() means the reference
counter was not incremented, could this unconditional pm_runtime_put()
underflow the power management reference counter?

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