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@xxxxxxxxxx):
>
> > 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@xxxxxxxxxx>
> > ---
> > 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