Re: [PATCH] tpm_tis_core: Disable broken IRQ handling code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Apr 11, 2020 at 12:09:36AM +0300, Jarkko Sakkinen wrote:
> On Sat, Apr 11, 2020 at 12:07:02AM +0300, Jarkko Sakkinen wrote:
> > On Thu, Apr 09, 2020 at 11:10:44PM +0200, Hans de Goede wrote:
> > > Since commit dda8b2af395b ("tpm: Revert "tpm_tis_core: Set
> > > TPM_CHIP_FLAG_IRQ before probing for interrupts"") we no longer set
> > > the TPM_CHIP_FLAG_IRQ ever.
> > > 
> > > So the whole IRQ probing code is not useful, worse we rely on the
> > > IRQ-test path of tpm_tis_send() to call disable_interrupts() if
> > > interrupts do not work, but that path never gets entered because we
> > > never set the TPM_CHIP_FLAG_IRQ.
> > > 
> > > So the remaining IRQ probe code calls request_irq() and never calls
> > > free_irq() even when the interrupt is not working.
> > > 
> > > On some systems, e.g. the Lenovo X1 8th gen,  the interrupt we try
> > > to use and never free creates an interrupt storm followed by
> > > an "irq XX: nobody cared" oops.
> > > 
> > > Since it is non-functional at the moment anyways, lets just completely
> > > disable the IRQ code in tpm_tis_core for now.
> > > 
> > > Fixes: dda8b2af395b ("tpm: Revert "tpm_tis_core: Set TPM_CHIP_FLAG_IRQ before probing for interrupts"")
> > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > > ---
> > > Note I'm working with Lenovo to try and get to the bottom of this.
> > > ---
> > 
> > OK if I recall correctly the reason for reverting was that the fixes
> > Stefan was sending were broken and no access to hardware were the
> > issues would be visible. The reason for not doing anything til this
> > day is that we don't have T490 available.
> > 
> > The lack of devm_free_irq() is an by itself, so please instead send
> > a fix that adds that to the code instead of "#if 0" crap.
> 
> If we were to go with your proposal, then you'd have to do "if 0" to
> bunch of other places. Otherwise, the change would be incomplete. Thus,
> it is now more sane just to free that IRQ.

Also, already the commit that has the fixes tag disables IRQ handling
and that was for the reason that some hardware that we (basically me
and Jerry) did not have access to. That's why there is no proper fix.

/Jarkko



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux