[PATCH] drm/imx/dcss: Fix clock leak on blkctl init failure

[PATCH] drm/imx/dcss: Fix clock leak on blkctl init failure

[laixiangwu]

dcss_submodules_init() enables the DCSS clocks before initializing the
submodules, but if the first submodule (blkctl) fails to initialize the
function returns directly, without disabling the clocks again. All the
later error paths already jump to ctxld_err, which calls
dcss_clocks_disable().

Since the clocks are only devm-managed for their reference (devm_clk_get),
a blkctl init failure leaves all five DCSS clocks prepared and enabled.

Jump to ctxld_err so the clocks are disabled like on the other error
paths.

Fixes: 9021c317b770 ("drm/imx: Add initial support for DCSS on iMX8MQ")
Signed-off-by: laixiangwu <laixiangwu@cmss.chinamobile.com>
---
 drivers/gpu/drm/imx/dcss/dcss-dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/dcss/dcss-dev.c b/drivers/gpu/drm/imx/dcss/dcss-dev.c
index 7fd0c4c14..551786060 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-dev.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-dev.c
@@ -70,7 +70,7 @@ static int dcss_submodules_init(struct dcss_dev *dcss)
 
 	ret = dcss_blkctl_init(dcss, base_addr + devtype->blkctl_ofs);
 	if (ret)
-		return ret;
+		goto ctxld_err;
 
 	ret = dcss_ctxld_init(dcss, base_addr + devtype->ctxld_ofs);
 	if (ret)

Re: [PATCH] drm/imx/dcss: Fix clock leak on blkctl init failure

[Laurentiu Palcu]

Hi,

Thank you for the fix.

On Fri, Oct 30, 2026 at 04:33:42PM +0800, laixiangwu wrote:
> dcss_submodules_init() enables the DCSS clocks before initializing the
> submodules, but if the first submodule (blkctl) fails to initialize the
> function returns directly, without disabling the clocks again. All the
> later error paths already jump to ctxld_err, which calls
> dcss_clocks_disable().
> 
> Since the clocks are only devm-managed for their reference (devm_clk_get),
> a blkctl init failure leaves all five DCSS clocks prepared and enabled.
> 
> Jump to ctxld_err so the clocks are disabled like on the other error
> paths.
> 
> Fixes: 9021c317b770 ("drm/imx: Add initial support for DCSS on iMX8MQ")
> Signed-off-by: laixiangwu <laixiangwu@cmss.chinamobile.com>
> ---
>  drivers/gpu/drm/imx/dcss/dcss-dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/imx/dcss/dcss-dev.c b/drivers/gpu/drm/imx/dcss/dcss-dev.c
> index 7fd0c4c14..551786060 100644
> --- a/drivers/gpu/drm/imx/dcss/dcss-dev.c
> +++ b/drivers/gpu/drm/imx/dcss/dcss-dev.c
> @@ -70,7 +70,7 @@ static int dcss_submodules_init(struct dcss_dev *dcss)
> 
>         ret = dcss_blkctl_init(dcss, base_addr + devtype->blkctl_ofs);
>         if (ret)
> -               return ret;
> +               goto ctxld_err;

With this change, now the label names I chose when I first developed this
driver don't feel right. Could you please send a v2 and rename those labels
below as well?

I'm thinking:
ctxld_err -> err_clocks
dtg_err -> err_ctxld
ss_err -> err_dtg
dpr_err -> err_ss
scaler_err -> err_dpr

Update those 'goto' lines accordingly.

-- 
Thanks,
Laurentiu

[PATCH v2] drm/imx/dcss: Fix clock leak on blkctl init failure

[laixiangwu]

dcss_submodules_init() enables the DCSS clocks before initializing the
submodules, but if the first submodule (blkctl) fails to initialize the
function returns directly, without disabling the clocks again. All the
later error paths already jump to err_clocks, which calls
dcss_clocks_disable().

Since the clocks are only devm-managed for their reference (devm_clk_get),
a blkctl init failure leaves all five DCSS clocks prepared and enabled.

Jump to err_clocks so the clocks are disabled like on the other error
paths.

Also rename the error labels of dcss_submodules_init() to the more
common err_<x> style, which describes what is cleaned up from each
label on, and update the gotos accordingly.

Fixes: 9021c317b770 ("drm/imx: Add initial support for DCSS on iMX8MQ")
Suggested-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Signed-off-by: laixiangwu <laixiangwu@cmss.chinamobile.com>

---
V1 -> V2: Also rename the error labels of dcss_submodules_init() and
update the gotos accordingly, as suggested in review.
---
 drivers/gpu/drm/imx/dcss/dcss-dev.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/imx/dcss/dcss-dev.c b/drivers/gpu/drm/imx/dcss/dcss-dev.c
index 7fd0c4c14..206679d52 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-dev.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-dev.c
@@ -70,45 +70,45 @@ static int dcss_submodules_init(struct dcss_dev *dcss)
 
 	ret = dcss_blkctl_init(dcss, base_addr + devtype->blkctl_ofs);
 	if (ret)
-		return ret;
+		goto err_clocks;
 
 	ret = dcss_ctxld_init(dcss, base_addr + devtype->ctxld_ofs);
 	if (ret)
-		goto ctxld_err;
+		goto err_clocks;
 
 	ret = dcss_dtg_init(dcss, base_addr + devtype->dtg_ofs);
 	if (ret)
-		goto dtg_err;
+		goto err_ctxld;
 
 	ret = dcss_ss_init(dcss, base_addr + devtype->ss_ofs);
 	if (ret)
-		goto ss_err;
+		goto err_dtg;
 
 	ret = dcss_dpr_init(dcss, base_addr + devtype->dpr_ofs);
 	if (ret)
-		goto dpr_err;
+		goto err_ss;
 
 	ret = dcss_scaler_init(dcss, base_addr + devtype->scaler_ofs);
 	if (ret)
-		goto scaler_err;
+		goto err_dpr;
 
 	dcss_clocks_disable(dcss);
 
 	return 0;
 
-scaler_err:
+err_dpr:
 	dcss_dpr_exit(dcss->dpr);
 
-dpr_err:
+err_ss:
 	dcss_ss_exit(dcss->ss);
 
-ss_err:
+err_dtg:
 	dcss_dtg_exit(dcss->dtg);
 
-dtg_err:
+err_ctxld:
 	dcss_ctxld_exit(dcss->ctxld);
 
-ctxld_err:
+err_clocks:
 	dcss_clocks_disable(dcss);
 
 	return ret;

Re: [PATCH] drm/imx/dcss: Fix clock leak on blkctl init failure

[laixiangwu]

Hi Laurentiu,

Thank you for the review!

I've renamed the error labels as you suggested and sent a v2, which
threads below this mail. Please take a look.

Best regards,
laixiangwu

Re: [PATCH v2] drm/imx/dcss: Fix clock leak on blkctl init failure

[Laurentiu Palcu]

On Fri, Oct 30, 2026 at 07:13:03PM +0800, laixiangwu wrote:
> dcss_submodules_init() enables the DCSS clocks before initializing the
> submodules, but if the first submodule (blkctl) fails to initialize the
> function returns directly, without disabling the clocks again. All the
> later error paths already jump to err_clocks, which calls
> dcss_clocks_disable().
> 
> Since the clocks are only devm-managed for their reference (devm_clk_get),
> a blkctl init failure leaves all five DCSS clocks prepared and enabled.
> 
> Jump to err_clocks so the clocks are disabled like on the other error
> paths.
> 
> Also rename the error labels of dcss_submodules_init() to the more
> common err_<x> style, which describes what is cleaned up from each
> label on, and update the gotos accordingly.
> 
> Fixes: 9021c317b770 ("drm/imx: Add initial support for DCSS on iMX8MQ")
> Suggested-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

Thanks, but the fix is yours. I merely suggested some cosmetic changes. :)

> Signed-off-by: laixiangwu <laixiangwu@cmss.chinamobile.com>

Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

-- 
Thanks,
Laurentiu