Re: [PATCH v10 07/14] media: iris: Replace enum-indexed clock and power domain tables with per-block structures

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unchecked dereference of optional `iris_power_domain` pointers (`pd`) in resource helpers (`iris_enable_power_domain_and_clocks`, etc.) leads to kernel oops.
--

--- Patch [7]: [PATCH v10 07/14] media: iris: Replace enum-indexed clock and power domain tables with per-block structures ---
commit a990e7238dd52b7ecf138859cc143e38431554ef
Author: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

media: iris: Replace enum-indexed clock and power domain tables with per-block structures

This commit replaces the flat tables and enum-based lookups for clocks
and power domains with per-hardware-block structures. It introduces the
iris_power_domain structure to bundle devices and clocks without runtime
index lookups.

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu4x.c b/drivers/media/platform/qcom/iris/iris_vpu4x.c
> --- a/drivers/media/platform/qcom/iris/iris_vpu4x.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu4x.c

[ ... ]

>  static int iris_vpu4x_power_on_hardware(struct iris_core *core)
>  {
>  	u32 efuse_value = readl(core->reg_base + WRAPPER_EFUSE_MONITOR);
>  	int ret;
>  
> -	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
> +	ret = iris_enable_power_domain_and_clocks(core, core->vcodec);
>  	if (ret)
>  		return ret;
>  
>  	if (!(efuse_value & DISABLE_VIDEO_VPP0_BIT)) {
> -		ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs
> -						[IRIS_VPP0_HW_POWER_DOMAIN]);
> +		ret = iris_enable_power_domain_and_clocks(core, core->vcodec_vpp0);

[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer 
dereference if the platform data doesn't provide configuration for this 
block?

If the hardware efuse indicates VPP0 is present, but the software 
platform data omitted the VPP0 configuration, core->vcodec_vpp0 will be 
NULL. It is then passed to iris_enable_power_domain_and_clocks(), which 
dereferences it immediately:

iris_enable_power_domain_and_clocks(core, pd)
    for (i = 0; i < pd->pd_cnt; i++) {
        ...
    }

The same pattern appears in iris_disable_power_domain_and_clocks() and 
iris_genpd_set_hwmode() for the core->vcodec_vpp1 and core->apv pointers. 
Should there be a NULL check for these pointers in the resource helpers, 
or should the efuse check be combined with a software configuration check?

>  		if (ret)
>  			goto disable_hw_power_domain;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-glymur-v10-0-de451559e88b@oss.qualcomm.com?part=7