[PATCH] nvmem: imx-ocotp: Use helper function devm_clk_get_enabled()

[PATCH] nvmem: imx-ocotp: Use helper function devm_clk_get_enabled()

[Huan Tang]

Use devm_clk_get_enabled() helper to simplify code.

Signed-off-by: Huan Tang <tanghuan@vivo.com>
---
 drivers/nvmem/imx-ocotp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 79dd4fda0329..ce5970ba4033 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -605,7 +605,7 @@ static int imx_ocotp_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	priv->clk = devm_clk_get(dev, NULL);
+	priv->clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(priv->clk))
 		return PTR_ERR(priv->clk);
 
@@ -618,7 +618,6 @@ static int imx_ocotp_probe(struct platform_device *pdev)
 
 	priv->config = &imx_ocotp_nvmem_config;
 
-	clk_prepare_enable(priv->clk);
 	imx_ocotp_clr_err_if_set(priv);
 	clk_disable_unprepare(priv->clk);
 
-- 
2.48.1

Re: [PATCH] nvmem: imx-ocotp: Use helper function devm_clk_get_enabled()

[Krzysztof Kozlowski]

On 24/06/2025 05:23, Huan Tang wrote:
> Use devm_clk_get_enabled() helper to simplify code.
> 
> Signed-off-by: Huan Tang <tanghuan@vivo.com>
> ---
>  drivers/nvmem/imx-ocotp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
> index 79dd4fda0329..ce5970ba4033 100644
> --- a/drivers/nvmem/imx-ocotp.c
> +++ b/drivers/nvmem/imx-ocotp.c
> @@ -605,7 +605,7 @@ static int imx_ocotp_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->base))
>  		return PTR_ERR(priv->base);
>  
> -	priv->clk = devm_clk_get(dev, NULL);
> +	priv->clk = devm_clk_get_enabled(dev, NULL);

This is just confusing or even wrong. You do not understand the code,
just blindly do some changes pointed by scripting.

NAK.


>  


Best regards,
Krzysztof

Re: [PATCH] nvmem: imx-ocotp: Use helper function devm_clk_get_enabled()

[Krzysztof Kozlowski]

On 24/06/2025 08:50, Krzysztof Kozlowski wrote:
> On 24/06/2025 05:23, Huan Tang wrote:
>> Use devm_clk_get_enabled() helper to simplify code.
>>
>> Signed-off-by: Huan Tang <tanghuan@vivo.com>
>> ---
>>  drivers/nvmem/imx-ocotp.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
>> index 79dd4fda0329..ce5970ba4033 100644
>> --- a/drivers/nvmem/imx-ocotp.c
>> +++ b/drivers/nvmem/imx-ocotp.c
>> @@ -605,7 +605,7 @@ static int imx_ocotp_probe(struct platform_device *pdev)
>>  	if (IS_ERR(priv->base))
>>  		return PTR_ERR(priv->base);
>>  
>> -	priv->clk = devm_clk_get(dev, NULL);
>> +	priv->clk = devm_clk_get_enabled(dev, NULL);
> 
> This is just confusing or even wrong. You do not understand the code,
> just blindly do some changes pointed by scripting.
> 
> NAK.


I spotted error path further, so let's correct above: it is not only
confusing, but you introduce actual bugs!

No for another round of terrible vivo.com scripted bugs.

Best regards,
Krzysztof

Re: [PATCH] nvmem: imx-ocotp: Use helper function devm_clk_get_enabled()

[Huan Tang]

>>> diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
>>> index 79dd4fda0329..ce5970ba4033 100644
>>> --- a/drivers/nvmem/imx-ocotp.c
>>> +++ b/drivers/nvmem/imx-ocotp.c
>>> @@ -605,7 +605,7 @@ static int imx_ocotp_probe(struct platform_device *pdev)
>>>  	if (IS_ERR(priv->base))
>>>  		return PTR_ERR(priv->base);
>>>  
>>> -	priv->clk = devm_clk_get(dev, NULL);
>>> +	priv->clk = devm_clk_get_enabled(dev, NULL);
>> 
>> This is just confusing or even wrong. You do not understand the code,
>> just blindly do some changes pointed by scripting.
>> 
>> NAK.
>
>
> I spotted error path further, so let's correct above: it is not only
> confusing, but you introduce actual bugs!
>

Hi  krzk sir,

Thank you for your reply and guidance.

This patch does have the problem of double clk_unprepare_disable.
The usage scenarios of devm_clk_get_enabled are limited, as described below:

Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared
and enabled clocks"), devm_clk_get() and clk_prepare_enable() can now be
replaced by devm_clk_get_enabled() when driver enables (and possibly
prepares) the clocks for the whole lifetime of the device.

I will learn from you and improve the standards of patch upstream in the future.
such as this patch:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0fa8ce76b713a31f6aef2f88d08eee74d7d3d8a7

>
> No for another round of terrible vivo.com scripted bugs.

To clarify: 
This is my personal mistake. Hope you have a good impression of "vivo.com".

Thanks
Huan

Re: [PATCH] nvmem: imx-ocotp: Use helper function devm_clk_get_enabled()

[Krzysztof Kozlowski]

On 24/06/2025 10:33, Huan Tang wrote:
>> No for another round of terrible vivo.com scripted bugs.
> 
> To clarify: 
> This is my personal mistake. Hope you have a good impression of "vivo.com".

I have terrible impression of vivo.com, because you were sending dozen
or hundred of poorly crafted patches, created by some automation,
repeating same issues, putting strain on maintainer resources instead of
approaching this slowly and learning while doing the task.

One of the things I requested was to perform internal review. Who
reviewed this patch internally?

Best regards,
Krzysztof