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 <[email protected]> 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/[email protected]?part=2
