Re: [PATCH] hwrng: omap - Unregister hwrng before disabling the device on remove

[Andreas Kemnade <andreas@kemnade.info>]

On Sun, 15 Nov 2026 21:50:50 +0700
Nhat Tran <nhat.tranminhk2@gmail.com> wrote:

> Commit 3e75241be808 ("hwrng: drivers - Use device-managed registration
> API") switched omap-rng to devm_hwrng_register() and dropped the
> hwrng_unregister() call from the start of omap_rng_remove(). Since then
> the hwrng is unregistered by devres only after omap_rng_remove() has
> returned, i.e. after the device has been runtime suspended and its
> runtime PM disabled.
> 
> When the omap-rng is the current hwrng, hwrng_unregister() drops the
> last reference and schedules cleanup_rng_work(), which calls
> omap_rng_cleanup() and in turn omap4_rng_cleanup(). That reads and
> writes RNG_CONTROL_REG while the module clock is gated. On a
> BeagleBone Black (AM335x) unbinding the driver triggers:
> 
>   # echo 48310000.rng > /sys/bus/platform/drivers/omap_rng/unbind
>   Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa310014
>   Internal error: : 1028 [#1] SMP ARM
>   Workqueue: events cleanup_rng_work
>   PC is at omap4_rng_cleanup+0x14/0x24
>   LR is at cleanup_rng_work+0x30/0x48
>   Call trace:
>    omap4_rng_cleanup from cleanup_rng_work+0x30/0x48
>    cleanup_rng_work from process_scheduled_works+0xf0/0x518
>    process_scheduled_works from worker_thread+0x140/0x2f0
>    worker_thread from kthread+0xfc/0x134
>    kthread from ret_from_fork+0x14/0x28
> 
> The worker dies before it completes rng->cleanup_done, so the unbind
> also never returns because hwrng_unregister() waits for that
> completion. The same window lets the hwrng fill thread call
> omap_rng_do_read() on the suspended device.
> 
> Restore the original teardown order: register with hwrng_register()
> and call hwrng_unregister() first in omap_rng_remove(), while the
> device is still powered.
> 
> Fixes: 3e75241be808 ("hwrng: drivers - Use device-managed registration API")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nhat Tran <nhat.tranminhk2@gmail.com>
> ---
>  drivers/char/hw_random/omap-rng.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
> index 49d7f021fd31..e5b14ab1b7e4 100644
> --- a/drivers/char/hw_random/omap-rng.c
> +++ b/drivers/char/hw_random/omap-rng.c
> @@ -493,7 +493,7 @@ static int omap_rng_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_register;
>  
> -	ret = devm_hwrng_register(&pdev->dev, &priv->rng);
> +	ret = hwrng_register(&priv->rng);
>  	if (ret)
>  		goto err_register;
>  
> @@ -520,6 +520,7 @@ static void omap_rng_remove(struct platform_device *pdev)
>  {
>  	struct omap_rng_dev *priv = platform_get_drvdata(pdev);
>  
> +	hwrng_unregister(&priv->rng);
>  
>  	priv->pdata->cleanup(priv);
>  
what about going forward add add a devm_add_action_or_reset() for this instead.
And use some devm_runtime for enabling runtime?

I guess we are done then.

Regards,
Andreas