[PATCH v2 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths

Krzysztof Kozlowski posted 4 patches 10 months, 1 week ago
[PATCH v2 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths
Posted by Krzysztof Kozlowski 10 months, 1 week ago
dsi_clk_init(), which gets the clocks, is called only through platform
driver probe and its failure is a failure of the probe.  Therefore
NULL-ifying specific clocks is pointless and redundant - the PTR_ERR
value stored there won't be used/dereferenced afterwards.  What's more,
variant-specific clock init calls like dsi_clk_init_6g_v2() are not
doing this cleanup.  Dropping redundant code allows later to make this a
bit simpler.

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 007311c21fdaa0462b05d53cd8a2aad0269b1727..397c9f1f588558b2081d6400d2cbae746c900697 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -292,7 +292,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
 		ret = PTR_ERR(msm_host->byte_clk);
 		pr_err("%s: can't find dsi_byte clock. ret=%d\n",
 			__func__, ret);
-		msm_host->byte_clk = NULL;
 		goto exit;
 	}
 
@@ -301,7 +300,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
 		ret = PTR_ERR(msm_host->pixel_clk);
 		pr_err("%s: can't find dsi_pixel clock. ret=%d\n",
 			__func__, ret);
-		msm_host->pixel_clk = NULL;
 		goto exit;
 	}
 
@@ -310,7 +308,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
 		ret = PTR_ERR(msm_host->esc_clk);
 		pr_err("%s: can't find dsi_esc clock. ret=%d\n",
 			__func__, ret);
-		msm_host->esc_clk = NULL;
 		goto exit;
 	}
 

-- 
2.43.0
Re: [PATCH v2 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths
Posted by Dmitry Baryshkov 10 months, 1 week ago
On Fri, Feb 14, 2025 at 02:17:44PM +0100, Krzysztof Kozlowski wrote:
> dsi_clk_init(), which gets the clocks, is called only through platform
> driver probe and its failure is a failure of the probe.  Therefore
> NULL-ifying specific clocks is pointless and redundant - the PTR_ERR
> value stored there won't be used/dereferenced afterwards.  What's more,
> variant-specific clock init calls like dsi_clk_init_6g_v2() are not
> doing this cleanup.  Dropping redundant code allows later to make this a
> bit simpler.
> 
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ---
>  1 file changed, 3 deletions(-)
> 

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

-- 
With best wishes
Dmitry
Re: [PATCH v2 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths
Posted by Krzysztof Kozlowski 10 months, 1 week ago
On 14/02/2025 16:22, Dmitry Baryshkov wrote:
> On Fri, Feb 14, 2025 at 02:17:44PM +0100, Krzysztof Kozlowski wrote:
>> dsi_clk_init(), which gets the clocks, is called only through platform
>> driver probe and its failure is a failure of the probe.  Therefore
>> NULL-ifying specific clocks is pointless and redundant - the PTR_ERR
>> value stored there won't be used/dereferenced afterwards.  What's more,
>> variant-specific clock init calls like dsi_clk_init_6g_v2() are not
>> doing this cleanup.  Dropping redundant code allows later to make this a
>> bit simpler.
>>
>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 


Thanks for reviews. Both you and Abhinav reviewed, so I assume this will
be applied by Rob?


Best regards,
Krzysztof
Re: [PATCH v2 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths
Posted by Dmitry Baryshkov 10 months, 1 week ago
On Fri, Feb 14, 2025 at 04:28:45PM +0100, Krzysztof Kozlowski wrote:
> On 14/02/2025 16:22, Dmitry Baryshkov wrote:
> > On Fri, Feb 14, 2025 at 02:17:44PM +0100, Krzysztof Kozlowski wrote:
> >> dsi_clk_init(), which gets the clocks, is called only through platform
> >> driver probe and its failure is a failure of the probe.  Therefore
> >> NULL-ifying specific clocks is pointless and redundant - the PTR_ERR
> >> value stored there won't be used/dereferenced afterwards.  What's more,
> >> variant-specific clock init calls like dsi_clk_init_6g_v2() are not
> >> doing this cleanup.  Dropping redundant code allows later to make this a
> >> bit simpler.
> >>
> >> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >>  drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ---
> >>  1 file changed, 3 deletions(-)
> >>
> > 
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > 
> 
> 
> Thanks for reviews. Both you and Abhinav reviewed, so I assume this will
> be applied by Rob?

I will pick it up once Abhinav posts msm-fixes

-- 
With best wishes
Dmitry