On Thu Sep 14, 2023 at 6:29 PM EEST, Jan Beulich wrote:
> On 14.09.2023 17:19, Jarkko Sakkinen wrote:
> > 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?
>
> There's no rush from my pov, as I have helped myself. Nevertheless ...
>
> > 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'm not doing anything non-default. The issue here is that part of
> interrupt cleanup occurs without interrupt setup having happened. So
> I'm inclined to think that the warning is independent of (and perhaps
> more likely to observe without) use of that optional functionality.
>
> Jan
Ah, you're absolutely right!
I'll apply and test this properly after I'm back. Thanks for finding
the bug and fixing it.
BR, Jrakko
