On Tue, Sep 08, 2026 at 05:58:02PM +0800, Richard Lyu wrote:
> The comment above the wmb() in tpm_pm_resume() states that the barrier
> guarantees TPM_CHIP_FLAG_SUSPENDED is written last, so that hwrng does
> not activate before the chip has been fully resumed. It cannot do so:
> it is placed after the store that clears the flag, and therefore does
> not order the preceding resume work before that store. It also has
> nothing to pair with, as tpm_try_get_ops() contains no matching read
> barrier.
>
> Drop the barrier along with the comment rather than leave a no-op
> behind. Should such ordering turn out to be needed, it would require
> paired barriers or locking.
>
> Fixes: 99d464506255 ("tpm: Prevent hwrng from activating during resume")
> Link:
> https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Richard Lyu <[email protected]>
I agree that wmb() is necessary but what is the bug here?
Agree with the change but I'm not convinced about fixes tag here.
> ---
> drivers/char/tpm/tpm-interface.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c
> b/drivers/char/tpm/tpm-interface.c
> index b4e749e70b02..0bab78c8767c 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -497,12 +497,6 @@ int tpm_pm_resume(struct device *dev)
>
> chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
>
> - /*
> - * Guarantee that SUSPENDED is written last, so that hwrng does not
> - * activate before the chip has been fully resumed.
> - */
> - wmb();
> -
> return 0;
> }
> EXPORT_SYMBOL_GPL(tpm_pm_resume);
> --
> 2.51.0
>
BR, Jarkko
