[PATCH] hwrng: cctrng - Fix an error handling path in cctrng_probe

[PATCH] hwrng: cctrng - Fix an error handling path in cctrng_probe

[Christophe JAILLET]

If an error occurs after a successful cc_trng_pm_init() call, it must be
undone by a corresponding cc_trng_pm_fini() call.

The commit in Fixes has erroneously updated the error handling path if
cc_trng_pm_get() fails.

Fixes: 8bcd96893848 ("hwrng: cctrng - let devres enable the clock")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only.

This patch also undo the clean-up done by commit a7ea3d63e897 ("hwrng:
cctrng - simplify cctrng_probe()"). Keeping an error path makes sense with
2 users. But if preferred, the missing cc_trng_pm_fini() could also be
added after the test for errors in cc_trng_pm_get().
---
 drivers/char/hw_random/cctrng.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/char/hw_random/cctrng.c b/drivers/char/hw_random/cctrng.c
index 03401ee59a8e..608b3321c05a 100644
--- a/drivers/char/hw_random/cctrng.c
+++ b/drivers/char/hw_random/cctrng.c
@@ -524,8 +524,10 @@ static int cctrng_probe(struct platform_device *pdev)
 
 	/* increment device's usage counter */
 	rc = cc_trng_pm_get(dev);
-	if (rc)
-		return dev_err_probe(dev, rc, "cc_trng_pm_get returned %x\n", rc);
+	if (rc) {
+		dev_err_probe(dev, rc, "cc_trng_pm_get returned %x\n", rc);
+		goto post_pm_err;
+	}
 
 	/* set pending_hw to verify that HW won't be triggered from read */
 	atomic_set(&drvdata->pending_hw, 1);
@@ -534,8 +536,7 @@ static int cctrng_probe(struct platform_device *pdev)
 	rc = devm_hwrng_register(dev, &drvdata->rng);
 	if (rc) {
 		dev_err(dev, "Could not register hwrng device.\n");
-		cc_trng_pm_fini(drvdata);
-		return rc;
+		goto post_pm_err;
 	}
 
 	/* trigger HW to start generate data */
@@ -548,6 +549,11 @@ static int cctrng_probe(struct platform_device *pdev)
 	dev_info(dev, "ARM cctrng device initialized\n");
 
 	return 0;
+
+post_pm_err:
+	cc_trng_pm_fini(drvdata);
+
+	return rc;
 }
 
 static void cctrng_remove(struct platform_device *pdev)
-- 
2.56.0

Re: [PATCH] hwrng: cctrng - Fix an error handling path in cctrng_probe

[Martin Kaiser]

The sashiko review makes sense to me. We should probably call
pm_runtime_put_noidle after post_pm_err.

Best regards,
Martin

Thus wrote Christophe JAILLET (christophe.jaillet@wanadoo.fr):

> If an error occurs after a successful cc_trng_pm_init() call, it must be
> undone by a corresponding cc_trng_pm_fini() call.

> The commit in Fixes has erroneously updated the error handling path if
> cc_trng_pm_get() fails.

> Fixes: 8bcd96893848 ("hwrng: cctrng - let devres enable the clock")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Compile tested only.

> This patch also undo the clean-up done by commit a7ea3d63e897 ("hwrng:
> cctrng - simplify cctrng_probe()"). Keeping an error path makes sense with
> 2 users. But if preferred, the missing cc_trng_pm_fini() could also be
> added after the test for errors in cc_trng_pm_get().
> ---
>  drivers/char/hw_random/cctrng.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)

> diff --git a/drivers/char/hw_random/cctrng.c b/drivers/char/hw_random/cctrng.c
> index 03401ee59a8e..608b3321c05a 100644
> --- a/drivers/char/hw_random/cctrng.c
> +++ b/drivers/char/hw_random/cctrng.c
> @@ -524,8 +524,10 @@ static int cctrng_probe(struct platform_device *pdev)

>  	/* increment device's usage counter */
>  	rc = cc_trng_pm_get(dev);
> -	if (rc)
> -		return dev_err_probe(dev, rc, "cc_trng_pm_get returned %x\n", rc);
> +	if (rc) {
> +		dev_err_probe(dev, rc, "cc_trng_pm_get returned %x\n", rc);
> +		goto post_pm_err;
> +	}

>  	/* set pending_hw to verify that HW won't be triggered from read */
>  	atomic_set(&drvdata->pending_hw, 1);
> @@ -534,8 +536,7 @@ static int cctrng_probe(struct platform_device *pdev)
>  	rc = devm_hwrng_register(dev, &drvdata->rng);
>  	if (rc) {
>  		dev_err(dev, "Could not register hwrng device.\n");
> -		cc_trng_pm_fini(drvdata);
> -		return rc;
> +		goto post_pm_err;
>  	}

>  	/* trigger HW to start generate data */
> @@ -548,6 +549,11 @@ static int cctrng_probe(struct platform_device *pdev)
>  	dev_info(dev, "ARM cctrng device initialized\n");

>  	return 0;
> +
> +post_pm_err:
> +	cc_trng_pm_fini(drvdata);
> +
> +	return rc;
>  }

>  static void cctrng_remove(struct platform_device *pdev)
> -- 
> 2.56.0

Re: [PATCH] hwrng: cctrng - Fix an error handling path in cctrng_probe

[Christophe JAILLET]

Le 27/10/2026 à 09:22, Martin Kaiser a écrit :
> The sashiko review makes sense to me. We should probably call
> pm_runtime_put_noidle after post_pm_err.
> 
> Best regards,
> Martin
> 

I agree that my patch looks broken.

I've not followed all sashiko explanation, but usually I try to stay 
away from pm code, which most of the time looks too tricky for me.

So should something be fixed here, I'll leave it to you.

CJ

> Thus wrote Christophe JAILLET (christophe.jaillet@wanadoo.fr):
> 
>> If an error occurs after a successful cc_trng_pm_init() call, it must be
>> undone by a corresponding cc_trng_pm_fini() call.
> 
>> The commit in Fixes has erroneously updated the error handling path if
>> cc_trng_pm_get() fails.
> 
>> Fixes: 8bcd96893848 ("hwrng: cctrng - let devres enable the clock")
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> Compile tested only.
> 
>> This patch also undo the clean-up done by commit a7ea3d63e897 ("hwrng:
>> cctrng - simplify cctrng_probe()"). Keeping an error path makes sense with
>> 2 users. But if preferred, the missing cc_trng_pm_fini() could also be
>> added after the test for errors in cc_trng_pm_get().
>> ---
>>   drivers/char/hw_random/cctrng.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
>> diff --git a/drivers/char/hw_random/cctrng.c b/drivers/char/hw_random/cctrng.c
>> index 03401ee59a8e..608b3321c05a 100644
>> --- a/drivers/char/hw_random/cctrng.c
>> +++ b/drivers/char/hw_random/cctrng.c
>> @@ -524,8 +524,10 @@ static int cctrng_probe(struct platform_device *pdev)
> 
>>   	/* increment device's usage counter */
>>   	rc = cc_trng_pm_get(dev);
>> -	if (rc)
>> -		return dev_err_probe(dev, rc, "cc_trng_pm_get returned %x\n", rc);
>> +	if (rc) {
>> +		dev_err_probe(dev, rc, "cc_trng_pm_get returned %x\n", rc);
>> +		goto post_pm_err;
>> +	}
> 
>>   	/* set pending_hw to verify that HW won't be triggered from read */
>>   	atomic_set(&drvdata->pending_hw, 1);
>> @@ -534,8 +536,7 @@ static int cctrng_probe(struct platform_device *pdev)
>>   	rc = devm_hwrng_register(dev, &drvdata->rng);
>>   	if (rc) {
>>   		dev_err(dev, "Could not register hwrng device.\n");
>> -		cc_trng_pm_fini(drvdata);
>> -		return rc;
>> +		goto post_pm_err;
>>   	}
> 
>>   	/* trigger HW to start generate data */
>> @@ -548,6 +549,11 @@ static int cctrng_probe(struct platform_device *pdev)
>>   	dev_info(dev, "ARM cctrng device initialized\n");
> 
>>   	return 0;
>> +
>> +post_pm_err:
>> +	cc_trng_pm_fini(drvdata);
>> +
>> +	return rc;
>>   }
> 
>>   static void cctrng_remove(struct platform_device *pdev)
>> -- 
>> 2.56.0
>