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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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