Re: [PATCH v8 11/18] media: iris: Add framework support for AR50_LITE video core

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Failed power domain enables or already-active domains leak the PM runtime usage counter and inappropriately abort initialization due to improper handling of `pm_runtime_get_sync()` return values.
--

--- Patch [11]: [PATCH v8 11/18] media: iris: Add framework support for AR50_LITE video core ---
commit cfd8ed53c04ddb4ea2cea6f16627a62b8400f868
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>

media: iris: Add framework support for AR50_LITE video core

Add power sequence for ar5lt core.
Add register handling for ar50lt by hooking up vpu op with ar50lt
specific implemtation or resue from earlier generation wherever
feasible.

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c b/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c
> new file mode 100644
> index 0000000000000..e084a5b49f2e3
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_ar50lt.c
[ ... ]
> +static int iris_vpu_ar50lt_power_on_controller(struct iris_core *core)
> +{
> +	int ret;
> +
> +	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
> +	if (ret)
> +		return ret;

[Severity: High]
This isn't a bug introduced by this patch, but since
iris_enable_power_domains() returns the raw value of pm_runtime_get_sync(),
does this incorrectly abort initialization if the domain is already active?

pm_runtime_get_sync() returns 1 when the device is already active. Treating
any non-zero return value as a failure causes this to return 1 and abort
the power-on sequence.

Also, if pm_runtime_get_sync() fails (returns < 0) or returns 1, the usage
counter is left incremented. Could this leak the PM runtime reference since
the error path doesn't call pm_runtime_put_noidle()? Using
pm_runtime_resume_and_get() instead of pm_runtime_get_sync() in the wrapper
would avoid both issues.

[ ... ]
> +static int iris_vpu_ar50lt_power_on_hw(struct iris_core *core)
> +{
> +	int ret;
> +
> +	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
> +	if (ret)
> +		return ret;

[Severity: High]
This isn't a bug introduced by this patch, but does this share the same
pm_runtime_get_sync() return value mismatch and reference leak as seen in
iris_vpu_ar50lt_power_on_controller() above?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-iris-ar50lt-v8-0-6597c08cea6f@oss.qualcomm.com?part=11