Re: [PATCH 2/3] drm/msm/mdss: Enable INT2_GDSC alongside CORE_GDSC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing cleanup for IRQ domain and chained handler on power domain attach failure.
--

--- Patch [2]: [PATCH 2/3] drm/msm/mdss: Enable INT2_GDSC alongside CORE_GDSC ---
commit 9d271e868146fa30f583363d709a200283bfe76e
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>

drm/msm/mdss: Enable INT2_GDSC alongside CORE_GDSC

This patch explicitly attaches and manages the secondary INT2_GDSC power
domain for platforms that use it. It also ensures both power domains are
enabled during MDSS runtime PM activation.

> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 9087c4b290dba..63ece55c4f2ec 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c

[ ... ]

> @@ -421,6 +451,27 @@ static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5
>  	irq_set_chained_handler_and_data(irq, msm_mdss_irq,
>  					 msm_mdss);
>  
> +	/*
> +	 * When two power domains are listed in DT, the core framework does not
> +	 * auto-attach either of them.  Attach and manage both explicitly so
> +	 * that INT2_GDSC (needed for VIG2/VIG3) is enabled alongside the main
> +	 * CORE_GDSC.  Single-domain platforms are unaffected.
> +	 */
> +	if (of_count_phandle_with_args(pdev->dev.of_node, "power-domains",
> +				       "#power-domain-cells") > 1) {
> +		static const char * const mdss_pd_names[] = { "core", "int2" };
> +		static const struct dev_pm_domain_attach_data pd_data = {
> +			.pd_names = mdss_pd_names,
> +			.num_pd_names = ARRAY_SIZE(mdss_pd_names),
> +			.pd_flags = PD_FLAG_NO_DEV_LINK,
> +		};
> +
> +		ret = devm_pm_domain_attach_list(&pdev->dev, &pd_data,
> +						 &msm_mdss->pd_list);
> +		if (ret < 0)
> +			return ERR_PTR(ret);

[Severity: High]
If devm_pm_domain_attach_list() fails here, we return directly with an error
pointer. Since the msm_mdss structure is allocated with devm_kzalloc(), it
will be automatically freed by the devres subsystem when the probe fails.

However, irq_set_chained_handler_and_data() was already called just above to
register msm_mdss_irq() with msm_mdss as its data pointer.

Could this leave a dangling pointer in the active chained handler, resulting
in a use-after-free if an interrupt fires? Also, do we need to clean up the
IRQ domain created by _msm_mdss_irq_domain_add() before returning on this
error path?

> +	}
> +
>  	pm_runtime_enable(&pdev->dev);
>  
>  	return msm_mdss;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-msm_gdsc2-v1-0-4687866d6cb0@oss.qualcomm.com?part=2