On Thu Sep 14, 2023 at 5:28 PM EEST, Jan Beulich wrote:
> tpm_tis_core_init() may fail before tpm_tis_probe_irq_single() is
> called, in which case tpm_tis_remove() unconditionally calling
> flush_work() is triggering a warning for .func still being NULL.
>
> Fixes: 481c2d14627d ("tpm,tpm_tis: Disable interrupts after 1000 unhandled IRQs")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> An alternative would be to move INIT_WORK(), but where to put it is far
> more difficult to tell for an outsider than simply making the flush call
> conditional.
>
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -1022,7 +1022,8 @@ void tpm_tis_remove(struct tpm_chip *chi
> interrupt = 0;
>
> tpm_tis_write32(priv, reg, ~TPM_GLOBAL_INT_ENABLE & interrupt);
> - flush_work(&priv->free_irq_work);
> + if (priv->free_irq_work.func)
> + flush_work(&priv->free_irq_work);
>
> tpm_tis_clkrun_enable(chip, false);
>
Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
Jan, I'm about to leave to vacation but will be back after next week.
Do you think that the fix can hold up unti that?
The feature is opt-in as I documented to kernel-parameters.txt:
tpm_tis.interrupts= [HW,TPM]
Enable interrupts for the MMIO based physical layer
for the FIFO interface. By default it is set to false
(0). For more information about TPM hardware interfaces
defined by Trusted Computing Group (TCG) see
https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
I'd prefer to apply this right after I'm back and not on rush but
will listen to other reasoning :-) Thanks.
BR, Jarkko
