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

[Christophe JAILLET <christophe.jaillet@wanadoo.fr>]

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
>