Messages in this thread | | | Date | Sat, 20 May 2023 04:20:20 +0300 | Subject | Re: [PATCH 2/3] drm/msm/dp: Clean up logs dp_power module | From | Dmitry Baryshkov <> |
| |
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);
-- With best wishes Dmitry
| |