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
