diff mbox series

[2/3] drm/msm/dp: Clean up logs dp_power module

Message ID	20230515030256.300104-2-quic_bjorande@quicinc.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Bjorn Andersson <quic_bjorande@quicinc.com> To: Rob Clark <robdclark@gmail.com>, Abhinav Kumar <quic_abhinavk@quicinc.com>, Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Kuogee Hsieh <quic_khsieh@quicinc.com> CC: Johan Hovold <johan+linaro@kernel.org>, Vinod Polimera <quic_vpolimer@quicinc.com>, Sankeerth Billakanti <quic_sbillaka@quicinc.com>, <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>, <dri-devel@lists.freedesktop.org>, <freedreno@lists.freedesktop.org> Subject: [PATCH 2/3] drm/msm/dp: Clean up logs dp_power module Date: Sun, 14 May 2023 20:02:55 -0700 Message-ID: <20230515030256.300104-2-quic_bjorande@quicinc.com> In-Reply-To: <20230515030256.300104-1-quic_bjorande@quicinc.com> References: <20230515030256.300104-1-quic_bjorande@quicinc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain Precedence: bulk
Series	[1/3] drm/msm/dp: Drop now unused dp_hpd module \| [1/3] drm/msm/dp: Drop now unused dp_hpd module [2/3] drm/msm/dp: Clean up logs dp_power module [3/3] drm/msm/dp: Clean up pdev/dev duplication in dp_power

Commit Message

Bjorn Andersson May 15, 2023, 3:02 a.m. UTC

  The clk_bulk API already provides error messages indicating which
specific clock in the request for which the operation failed, further
more these errors are associated with the specific DisplayPort
controller (rather than the shared drm_device). The additional error
messages int he dp_power module does thereby not provide any benefit.

While at it, none of the dp_power handles passed to these functions are
dynamic in nature, so there should not be any need for runtime checking
them. Drop these as well.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_power.c | 62 +++++--------------------------
 1 file changed, 9 insertions(+), 53 deletions(-)

Comments

Dmitry Baryshkov May 20, 2023, 1:20 a.m. UTC | #1

On 15/05/2023 06:02, Bjorn Andersson wrote:
> The clk_bulk API already provides error messages indicating which
> specific clock in the request for which the operation failed, further
> more these errors are associated with the specific DisplayPort
> controller (rather than the shared drm_device). The additional error
> messages int he dp_power module does thereby not provide any benefit.
> 
> While at it, none of the dp_power handles passed to these functions are
> dynamic in nature, so there should not be any need for runtime checking
> them. Drop these as well.

It might have been slightly better to have this split into two patches, 
but as this is a debug/logs only, it's fine with me.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dp/dp_power.c | 62 +++++--------------------------
>   1 file changed, 9 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
> index c0aaabb03389..031d2eefef07 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.c
> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
> @@ -35,25 +35,16 @@ static int dp_power_clk_init(struct dp_power_private *power)
>   	stream = &power->parser->mp[DP_STREAM_PM];
>   
>   	rc = devm_clk_bulk_get(dev, core->num_clk, core->clocks);
> -	if (rc) {
> -		DRM_ERROR("failed to get %s clk. err=%d\n",
> -			dp_parser_pm_name(DP_CORE_PM), rc);
> +	if (rc)
>   		return rc;
> -	}
>   
>   	rc = devm_clk_bulk_get(dev, ctrl->num_clk, ctrl->clocks);
> -	if (rc) {
> -		DRM_ERROR("failed to get %s clk. err=%d\n",
> -			dp_parser_pm_name(DP_CTRL_PM), rc);
> +	if (rc)
>   		return -ENODEV;
> -	}
>   
>   	rc = devm_clk_bulk_get(dev, stream->num_clk, stream->clocks);
> -	if (rc) {
> -		DRM_ERROR("failed to get %s clk. err=%d\n",
> -			dp_parser_pm_name(DP_CTRL_PM), rc);
> +	if (rc)
>   		return -ENODEV;
> -	}
>   
>   	return 0;
>   }
> @@ -121,11 +112,9 @@ int dp_power_clk_enable(struct dp_power *dp_power,
>   			mp = &power->parser->mp[DP_CORE_PM];
>   
>   			rc = clk_bulk_prepare_enable(mp->num_clk, mp->clocks);
> -			if (rc) {
> -				DRM_ERROR("fail to enable clks: %s. err=%d\n",
> -					dp_parser_pm_name(DP_CORE_PM), rc);
> +			if (rc)
>   				return rc;
> -			}
> +
>   			dp_power->core_clks_on = true;
>   		}
>   	}
> @@ -133,10 +122,8 @@ int dp_power_clk_enable(struct dp_power *dp_power,
>   	mp = &power->parser->mp[pm_type];
>   	if (enable) {
>   		rc = clk_bulk_prepare_enable(mp->num_clk, mp->clocks);
> -		if (rc) {
> -			DRM_ERROR("failed to enable clks, err: %d\n", rc);
> +		if (rc)
>   			return rc;
> -		}
>   	} else {
>   		clk_bulk_disable_unprepare(mp->num_clk, mp->clocks);
>   	}
> @@ -162,34 +149,19 @@ int dp_power_clk_enable(struct dp_power *dp_power,
>   
>   int dp_power_client_init(struct dp_power *dp_power)
>   {
> -	int rc = 0;
>   	struct dp_power_private *power;
>   
> -	if (!dp_power) {
> -		DRM_ERROR("invalid power data\n");
> -		return -EINVAL;
> -	}
> -
>   	power = container_of(dp_power, struct dp_power_private, dp_power);
>   
>   	pm_runtime_enable(&power->pdev->dev);
>   
> -	rc = dp_power_clk_init(power);
> -	if (rc)
> -		DRM_ERROR("failed to init clocks %d\n", rc);
> -
> -	return rc;
> +	return dp_power_clk_init(power);
>   }
>   
>   void dp_power_client_deinit(struct dp_power *dp_power)
>   {
>   	struct dp_power_private *power;
>   
> -	if (!dp_power) {
> -		DRM_ERROR("invalid power data\n");
> -		return;
> -	}
> -
>   	power = container_of(dp_power, struct dp_power_private, dp_power);
>   
>   	pm_runtime_disable(&power->pdev->dev);
> @@ -200,25 +172,14 @@ int dp_power_init(struct dp_power *dp_power, bool flip)
>   	int rc = 0;
>   	struct dp_power_private *power = NULL;
>   
> -	if (!dp_power) {
> -		DRM_ERROR("invalid power data\n");
> -		return -EINVAL;
> -	}
> -
>   	power = container_of(dp_power, struct dp_power_private, dp_power);
>   
>   	pm_runtime_get_sync(&power->pdev->dev);
>   
>   	rc = dp_power_clk_enable(dp_power, DP_CORE_PM, true);
> -	if (rc) {
> -		DRM_ERROR("failed to enable DP core clocks, %d\n", rc);
> -		goto exit;
> -	}
> -
> -	return 0;
> +	if (rc)
> +		pm_runtime_put_sync(&power->pdev->dev);
>   
> -exit:
> -	pm_runtime_put_sync(&power->pdev->dev);
>   	return rc;
>   }
>   
> @@ -238,11 +199,6 @@ struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser)
>   	struct dp_power_private *power;
>   	struct dp_power *dp_power;
>   
> -	if (!parser) {
> -		DRM_ERROR("invalid input\n");
> -		return ERR_PTR(-EINVAL);
> -	}
> -
>   	power = devm_kzalloc(&parser->pdev->dev, sizeof(*power), GFP_KERNEL);
>   	if (!power)
>   		return ERR_PTR(-ENOMEM);

diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
index c0aaabb03389..031d2eefef07 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -35,25 +35,16 @@  static int dp_power_clk_init(struct dp_power_private *power)
 	stream = &power->parser->mp[DP_STREAM_PM];
 
 	rc = devm_clk_bulk_get(dev, core->num_clk, core->clocks);
-	if (rc) {
-		DRM_ERROR("failed to get %s clk. err=%d\n",
-			dp_parser_pm_name(DP_CORE_PM), rc);
+	if (rc)
 		return rc;
-	}
 
 	rc = devm_clk_bulk_get(dev, ctrl->num_clk, ctrl->clocks);
-	if (rc) {
-		DRM_ERROR("failed to get %s clk. err=%d\n",
-			dp_parser_pm_name(DP_CTRL_PM), rc);
+	if (rc)
 		return -ENODEV;
-	}
 
 	rc = devm_clk_bulk_get(dev, stream->num_clk, stream->clocks);
-	if (rc) {
-		DRM_ERROR("failed to get %s clk. err=%d\n",
-			dp_parser_pm_name(DP_CTRL_PM), rc);
+	if (rc)
 		return -ENODEV;
-	}
 
 	return 0;
 }
@@ -121,11 +112,9 @@  int dp_power_clk_enable(struct dp_power *dp_power,
 			mp = &power->parser->mp[DP_CORE_PM];
 
 			rc = clk_bulk_prepare_enable(mp->num_clk, mp->clocks);
-			if (rc) {
-				DRM_ERROR("fail to enable clks: %s. err=%d\n",
-					dp_parser_pm_name(DP_CORE_PM), rc);
+			if (rc)
 				return rc;
-			}
+
 			dp_power->core_clks_on = true;
 		}
 	}
@@ -133,10 +122,8 @@  int dp_power_clk_enable(struct dp_power *dp_power,
 	mp = &power->parser->mp[pm_type];
 	if (enable) {
 		rc = clk_bulk_prepare_enable(mp->num_clk, mp->clocks);
-		if (rc) {
-			DRM_ERROR("failed to enable clks, err: %d\n", rc);
+		if (rc)
 			return rc;
-		}
 	} else {
 		clk_bulk_disable_unprepare(mp->num_clk, mp->clocks);
 	}
@@ -162,34 +149,19 @@  int dp_power_clk_enable(struct dp_power *dp_power,
 
 int dp_power_client_init(struct dp_power *dp_power)
 {
-	int rc = 0;
 	struct dp_power_private *power;
 
-	if (!dp_power) {
-		DRM_ERROR("invalid power data\n");
-		return -EINVAL;
-	}
-
 	power = container_of(dp_power, struct dp_power_private, dp_power);
 
 	pm_runtime_enable(&power->pdev->dev);
 
-	rc = dp_power_clk_init(power);
-	if (rc)
-		DRM_ERROR("failed to init clocks %d\n", rc);
-
-	return rc;
+	return dp_power_clk_init(power);
 }
 
 void dp_power_client_deinit(struct dp_power *dp_power)
 {
 	struct dp_power_private *power;
 
-	if (!dp_power) {
-		DRM_ERROR("invalid power data\n");
-		return;
-	}
-
 	power = container_of(dp_power, struct dp_power_private, dp_power);
 
 	pm_runtime_disable(&power->pdev->dev);
@@ -200,25 +172,14 @@  int dp_power_init(struct dp_power *dp_power, bool flip)
 	int rc = 0;
 	struct dp_power_private *power = NULL;
 
-	if (!dp_power) {
-		DRM_ERROR("invalid power data\n");
-		return -EINVAL;
-	}
-
 	power = container_of(dp_power, struct dp_power_private, dp_power);
 
 	pm_runtime_get_sync(&power->pdev->dev);
 
 	rc = dp_power_clk_enable(dp_power, DP_CORE_PM, true);
-	if (rc) {
-		DRM_ERROR("failed to enable DP core clocks, %d\n", rc);
-		goto exit;
-	}
-
-	return 0;
+	if (rc)
+		pm_runtime_put_sync(&power->pdev->dev);
 
-exit:
-	pm_runtime_put_sync(&power->pdev->dev);
 	return rc;
 }
 
@@ -238,11 +199,6 @@  struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser)
 	struct dp_power_private *power;
 	struct dp_power *dp_power;
 
-	if (!parser) {
-		DRM_ERROR("invalid input\n");
-		return ERR_PTR(-EINVAL);
-	}
-
 	power = devm_kzalloc(&parser->pdev->dev, sizeof(*power), GFP_KERNEL);
 	if (!power)
 		return ERR_PTR(-ENOMEM);