vop2_enable() returns void, so callers cannot tell whether powering up
the VOP2 succeeded.

Additionally, if rockchip_drm_dma_attach_device() fails, vop2_enable()
returns without disabling the core clocks or dropping the runtime PM
reference taken earlier, leaking both.

Make vop2_enable() return int and unwind all resources on every failure
path.  As disabling the core clocks is also needed in vop2_disable(),
factor it out into a local helper used by both.

Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
Reported-by: Sashiko <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 39 +++++++++++++++++++---------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 4cce3e336f5b..e11dc8c7f78b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -860,6 +860,13 @@ static int vop2_core_clks_prepare_enable(struct vop2 *vop2)
        return ret;
 }
 
+static void vop2_core_clks_disable_unprepare(struct vop2 *vop2)
+{
+       clk_disable_unprepare(vop2->pclk);
+       clk_disable_unprepare(vop2->aclk);
+       clk_disable_unprepare(vop2->hclk);
+}
+
 static void rk3588_vop2_power_domain_enable_all(struct vop2 *vop2)
 {
        u32 pd;
@@ -871,7 +878,7 @@ static void rk3588_vop2_power_domain_enable_all(struct vop2 
*vop2)
        vop2_writel(vop2, RK3588_SYS_PD_CTRL, pd);
 }
 
-static void vop2_enable(struct vop2 *vop2)
+static int vop2_enable(struct vop2 *vop2)
 {
        int ret;
        u32 version;
@@ -879,25 +886,24 @@ static void vop2_enable(struct vop2 *vop2)
        ret = pm_runtime_resume_and_get(vop2->dev);
        if (ret < 0) {
                drm_err(vop2->drm, "failed to get pm runtime: %d\n", ret);
-               return;
+               return ret;
        }
 
        ret = vop2_core_clks_prepare_enable(vop2);
-       if (ret) {
-               pm_runtime_put_sync(vop2->dev);
-               return;
-       }
+       if (ret)
+               goto err_put_pm;
 
        ret = rockchip_drm_dma_attach_device(vop2->drm, vop2->dev);
        if (ret) {
-               drm_err(vop2->drm, "failed to attach dma mapping, %d\n", ret);
-               return;
+               drm_err(vop2->drm, "failed to attach dma mapping: %d\n", ret);
+               goto err_disable_clks;
        }
 
        version = vop2_readl(vop2, RK3568_VERSION_INFO);
        if (version != vop2->version) {
                drm_err(vop2->drm, "Hardware version(0x%08x) mismatch\n", 
version);
-               return;
+               ret = -EINVAL;
+               goto err_detach_dma;
        }
 
        /*
@@ -932,6 +938,17 @@ static void vop2_enable(struct vop2 *vop2)
                    VOP2_INT_BUS_ERRPR << 16 | VOP2_INT_BUS_ERRPR);
        vop2_writel(vop2, RK3568_SYS1_INT_EN,
                    VOP2_INT_BUS_ERRPR << 16 | VOP2_INT_BUS_ERRPR);
+
+       return 0;
+
+err_detach_dma:
+       rockchip_drm_dma_detach_device(vop2->drm, vop2->dev);
+err_disable_clks:
+       vop2_core_clks_disable_unprepare(vop2);
+err_put_pm:
+       pm_runtime_put_sync(vop2->dev);
+
+       return ret;
 }
 
 static void vop2_disable(struct vop2 *vop2)
@@ -942,9 +959,7 @@ static void vop2_disable(struct vop2 *vop2)
 
        regcache_drop_region(vop2->map, 0, vop2_regmap_config.max_register);
 
-       clk_disable_unprepare(vop2->pclk);
-       clk_disable_unprepare(vop2->aclk);
-       clk_disable_unprepare(vop2->hclk);
+       vop2_core_clks_disable_unprepare(vop2);
 }
 
 static bool vop2_vp_dsp_lut_is_enabled(struct vop2_video_port *vp)

-- 
2.54.0

Reply via email to