On Thu, Feb 13, 2025 at 02:28:01PM +0300, Vitalii Mordan wrote:
> If the clock mhdp->clk was not enabled in cdns_mhdp_probe(), it should not
> be disabled in any path.
>
> Use the devm_clk_get_enabled() helper function to ensure proper call
> balance for mhdp->clk.
>
> Found by Linux Verification Center (linuxtesting.org) with Klever.
>
> Fixes: fb43aa0acdfd ("drm: bridge: Add support for Cadence MHDP8546 DPI/DP bridge")
Please describe, what is the actual issue that is being fixed. In other
words, which path leads to unbalanced enable or disable call. Otherwise
it seems to be a valid patch, not qualified as a fix.
> Signed-off-by: Vitalii Mordan <mordan@xxxxxxxxx>
> ---
> v2: Use devm_clk_get_enabled() helper function, as per Dmitry Baryshkov's
> request.
>
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index d081850e3c03..d4e4f484cbe5 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -2463,9 +2463,9 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
> if (!mhdp)
> return -ENOMEM;
>
> - clk = devm_clk_get(dev, NULL);
> + clk = devm_clk_get_enabled(dev, NULL);
> if (IS_ERR(clk)) {
> - dev_err(dev, "couldn't get clk: %ld\n", PTR_ERR(clk));
> + dev_err(dev, "couldn't get and enable clk: %ld\n", PTR_ERR(clk));
> return PTR_ERR(clk);
> }
>
> @@ -2504,14 +2504,12 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
>
> mhdp->info = of_device_get_match_data(dev);
>
> - clk_prepare_enable(clk);
> -
> pm_runtime_enable(dev);
> ret = pm_runtime_resume_and_get(dev);
> if (ret < 0) {
> dev_err(dev, "pm_runtime_resume_and_get failed\n");
> pm_runtime_disable(dev);
> - goto clk_disable;
> + return ret;
> }
>
> if (mhdp->info && mhdp->info->ops && mhdp->info->ops->init) {
> @@ -2590,8 +2588,6 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
> runtime_put:
> pm_runtime_put_sync(dev);
> pm_runtime_disable(dev);
> -clk_disable:
> - clk_disable_unprepare(mhdp->clk);
>
> return ret;
> }
> @@ -2632,8 +2628,6 @@ static void cdns_mhdp_remove(struct platform_device *pdev)
> cancel_work_sync(&mhdp->modeset_retry_work);
> flush_work(&mhdp->hpd_work);
> /* Ignoring mhdp->hdcp.check_work and mhdp->hdcp.prop_work here. */
> -
> - clk_disable_unprepare(mhdp->clk);
> }
>
> static const struct of_device_id mhdp_ids[] = {
> --
> 2.25.1
>
--
With best wishes
Dmitry
