[PATCH] pmdomain: imx: gpc: remove all initialized power domains

[PATCH] pmdomain: imx: gpc: remove all initialized power domains

[Guangshuo Li]

imx_gpc_old_dt_init() initializes all power domains described by
of_id_data->num_domains. Its probe failure path removes all of these
domains, but imx_gpc_remove() only removes the PU and ARM domains.

On SoCs with more than two domains, such as i.MX6SL and i.MX6SX, the
additional DISPLAY and PCI generic power domains can remain registered
after the GPC driver is removed.

Remove all initialized power domains in reverse order. Keep releasing
the PU clocks immediately after the PU domain has been successfully
removed.

This issue was found by manual code inspection.

Fixes: 5a42d1198901 ("soc: imx: gpc: fix imx6sl gpc power domain regression")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/pmdomain/imx/gpc.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/pmdomain/imx/gpc.c b/drivers/pmdomain/imx/gpc.c
index abca5f449a22..5766e3b40470 100644
--- a/drivers/pmdomain/imx/gpc.c
+++ b/drivers/pmdomain/imx/gpc.c
@@ -502,8 +502,10 @@ static int imx_gpc_probe(struct platform_device *pdev)
 
 static void imx_gpc_remove(struct platform_device *pdev)
 {
+	const struct imx_gpc_dt_data *of_id_data =
+		device_get_match_data(&pdev->dev);
 	struct device_node *pgc_node;
-	int ret;
+	int i, ret;
 
 	pgc_node = of_get_child_by_name(pdev->dev.of_node, "pgc");
 
@@ -519,19 +521,18 @@ static void imx_gpc_remove(struct platform_device *pdev)
 	if (!pgc_node) {
 		of_genpd_del_provider(pdev->dev.of_node);
 
-		ret = pm_genpd_remove(&imx_gpc_domains[GPC_PGC_DOMAIN_PU].base);
-		if (ret) {
-			dev_err(&pdev->dev, "Failed to remove PU power domain (%pe)\n",
-				ERR_PTR(ret));
-			return;
-		}
-		imx_pgc_put_clocks(&imx_gpc_domains[GPC_PGC_DOMAIN_PU]);
+		for (i = of_id_data->num_domains - 1; i >= 0; i--) {
+			ret = pm_genpd_remove(&imx_gpc_domains[i].base);
+			if (ret) {
+				dev_err(&pdev->dev,
+					"Failed to remove %s power domain (%pe)\n",
+					imx_gpc_domains[i].base.name,
+					ERR_PTR(ret));
+				return;
+			}
 
-		ret = pm_genpd_remove(&imx_gpc_domains[GPC_PGC_DOMAIN_ARM].base);
-		if (ret) {
-			dev_err(&pdev->dev, "Failed to remove ARM power domain (%pe)\n",
-				ERR_PTR(ret));
-			return;
+			if (i == GPC_PGC_DOMAIN_PU)
+				imx_pgc_put_clocks(&imx_gpc_domains[i]);
 		}
 	}
 
-- 
2.43.0

Re: [PATCH] pmdomain: imx: gpc: remove all initialized power domains

[Andy Shevchenko]

On Fri, Sep 18, 2026 at 11:13:22AM +0800, Guangshuo Li wrote:
> imx_gpc_old_dt_init() initializes all power domains described by
> of_id_data->num_domains. Its probe failure path removes all of these
> domains, but imx_gpc_remove() only removes the PU and ARM domains.
> 
> On SoCs with more than two domains, such as i.MX6SL and i.MX6SX, the
> additional DISPLAY and PCI generic power domains can remain registered
> after the GPC driver is removed.
> 
> Remove all initialized power domains in reverse order. Keep releasing
> the PU clocks immediately after the PU domain has been successfully
> removed.
> 
> This issue was found by manual code inspection.

...

>  static void imx_gpc_remove(struct platform_device *pdev)
>  {
> +	const struct imx_gpc_dt_data *of_id_data =
> +		device_get_match_data(&pdev->dev);

No...

>  	struct device_node *pgc_node;
> -	int ret;
> +	int i, ret;

...and no.

The result of the device_get_match_data() better to be checked, in some cases
it might lead to NULL dereference.

It's better to split semantically different variables, also naming 'i' is not
good, num_domains is better. Besides that, why is it signed?

...

> +		for (i = of_id_data->num_domains - 1; i >= 0; i--) {

What you are doing here can be written in a shorter form

		num_domains = of_id_data->num_domains;
		while (num_domains--) {

> +			ret = pm_genpd_remove(&imx_gpc_domains[i].base);
> +			if (ret) {
> +				dev_err(&pdev->dev,
> +					"Failed to remove %s power domain (%pe)\n",
> +					imx_gpc_domains[i].base.name,
> +					ERR_PTR(ret));
> +				return;
> +			}
>  
> +			if (i == GPC_PGC_DOMAIN_PU)
> +				imx_pgc_put_clocks(&imx_gpc_domains[i]);
>  		}

-- 
With Best Regards,
Andy Shevchenko