| Message ID | 20201001180925.13808-5-James.Bottomley@HansenPartnership.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jarkko Sakkinen |
| Headers | show |
| Series | tpm_tis: fix interrupts (again) | expand |
On Thu, Oct 01, 2020 at 11:09:24AM -0700, James Bottomley wrote: > There are two problems with our current interrupt probing: firstly the > TPM_CHIP_FLAG_IRQ never gets set initially, so a check for interrupts > is never done. Fix this by setting the flag before we generate and > interrupt for probing. Secondly our IRQ setup may be ineffective on a > TPM without legacy access cycles becuase according to the TPM > Interface Specification the interrupt registers are only writeable in > the current locality, so issue a request_locality before setting up > the interrupts. > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > --- > > v2: improved description Ditto. /Jarkko
James Bottomley @ 2020-10-01 11:09 MST: > There are two problems with our current interrupt probing: firstly the > TPM_CHIP_FLAG_IRQ never gets set initially, so a check for interrupts > is never done. Fix this by setting the flag before we generate and > interrupt for probing. Secondly our IRQ setup may be ineffective on a > TPM without legacy access cycles becuase according to the TPM > Interface Specification the interrupt registers are only writeable in > the current locality, so issue a request_locality before setting up > the interrupts. > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > --- > > v2: improved description > --- > drivers/char/tpm/tpm_tis_core.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > index 0c07da8cd680..12b657ed3a39 100644 > --- a/drivers/char/tpm/tpm_tis_core.c > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -809,6 +809,19 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, > } > priv->irq = irq; > > + /* > + * note writes to the interrupt registers are only effective > + * when the TPM is in the active locality, so we have to > + * request the locality here to get the interrupt set up. > + * This request has no corresponding release, because the > + * locality will be relinquished at the end of the tpm command > + * that probes the interrupts > + */ > + if (request_locality(chip, 0) != 0) { > + dev_err(&chip->dev, "failed to gain locality for irq probe\n"); > + return -EBUSY; > + } > + > rc = tpm_tis_read8(priv, TPM_INT_VECTOR(priv->locality), > &original_int_vec); > if (rc < 0) > @@ -836,6 +849,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, > return rc; > > priv->irq_tested = false; > + chip->flags |= TPM_CHIP_FLAG_IRQ; > > /* Generate an interrupt by having the core call through to > * tpm_tis_send Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
On Mon, Oct 19, 2020 at 04:41:35PM -0700, Jerry Snitselaar wrote: > > James Bottomley @ 2020-10-01 11:09 MST: > > > There are two problems with our current interrupt probing: firstly the > > TPM_CHIP_FLAG_IRQ never gets set initially, so a check for interrupts > > is never done. Fix this by setting the flag before we generate and > > interrupt for probing. Secondly our IRQ setup may be ineffective on a > > TPM without legacy access cycles becuase according to the TPM > > Interface Specification the interrupt registers are only writeable in > > the current locality, so issue a request_locality before setting up > > the interrupts. > > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > > > --- > > > > v2: improved description > > --- > > drivers/char/tpm/tpm_tis_core.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > > index 0c07da8cd680..12b657ed3a39 100644 > > --- a/drivers/char/tpm/tpm_tis_core.c > > +++ b/drivers/char/tpm/tpm_tis_core.c > > @@ -809,6 +809,19 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, > > } > > priv->irq = irq; > > > > + /* > > + * note writes to the interrupt registers are only effective > > + * when the TPM is in the active locality, so we have to > > + * request the locality here to get the interrupt set up. > > + * This request has no corresponding release, because the > > + * locality will be relinquished at the end of the tpm command > > + * that probes the interrupts > > + */ > > + if (request_locality(chip, 0) != 0) { > > + dev_err(&chip->dev, "failed to gain locality for irq probe\n"); > > + return -EBUSY; > > + } Appreciate the comment a lot, but s/note/Note/ /Jarkko
On Sat, Oct 24, 2020 at 03:17:18PM +0300, Jarkko Sakkinen wrote: > On Mon, Oct 19, 2020 at 04:41:35PM -0700, Jerry Snitselaar wrote: > > > > James Bottomley @ 2020-10-01 11:09 MST: > > > > > There are two problems with our current interrupt probing: firstly the > > > TPM_CHIP_FLAG_IRQ never gets set initially, so a check for interrupts > > > is never done. Fix this by setting the flag before we generate and > > > interrupt for probing. Secondly our IRQ setup may be ineffective on a > > > TPM without legacy access cycles becuase according to the TPM > > > Interface Specification the interrupt registers are only writeable in > > > the current locality, so issue a request_locality before setting up > > > the interrupts. > > > > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > > > > > --- > > > > > > v2: improved description > > > --- > > > drivers/char/tpm/tpm_tis_core.c | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > > > index 0c07da8cd680..12b657ed3a39 100644 > > > --- a/drivers/char/tpm/tpm_tis_core.c > > > +++ b/drivers/char/tpm/tpm_tis_core.c > > > @@ -809,6 +809,19 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, > > > } > > > priv->irq = irq; > > > > > > + /* > > > + * note writes to the interrupt registers are only effective > > > + * when the TPM is in the active locality, so we have to > > > + * request the locality here to get the interrupt set up. > > > + * This request has no corresponding release, because the > > > + * locality will be relinquished at the end of the tpm command > > > + * that probes the interrupts > > > + */ > > > + if (request_locality(chip, 0) != 0) { > > > + dev_err(&chip->dev, "failed to gain locality for irq probe\n"); > > > + return -EBUSY; > > > + } > > Appreciate the comment a lot, but s/note/Note/ I tested this with: - https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html dTPM 1.2 - https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html dTPM 2.0 I did not get "TPM interrupt not working, polling instead" to klog. But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? /Jarkko
On Fri, 2020-10-30 at 14:43 +0200, Jarkko Sakkinen wrote: [...] > I tested this with: > > - > https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html > dTPM 1.2 > - > https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html > dTPM 2.0 > > I did not get "TPM interrupt not working, polling instead" to klog. > But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? That's usually what you get when ACPI specifies the interrupt isn't connected (we don't try to probe it). James
James Bottomley @ 2020-10-30 08:49 MST: > On Fri, 2020-10-30 at 14:43 +0200, Jarkko Sakkinen wrote: > [...] >> I tested this with: >> >> - >> https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html >> dTPM 1.2 >> - >> https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html >> dTPM 2.0 >> >> I did not get "TPM interrupt not working, polling instead" to klog. >> But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? > > That's usually what you get when ACPI specifies the interrupt isn't > connected (we don't try to probe it). > > James That is the problem I've been running into. When I do find a system with a tpm and using tpm_tis, it usually seems to not have the interrupt connected. Should this commit have: Fixes: 570a36097f30 ("tpm: drop 'irq' from struct tpm_vendor_specific") That is where TPM_CHIP_FLAG_IRQ was added and not set for tpm_tis.
On Fri, Oct 30, 2020 at 08:49:27AM -0700, James Bottomley wrote: > On Fri, 2020-10-30 at 14:43 +0200, Jarkko Sakkinen wrote: > [...] > > I tested this with: > > > > - > > https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html > > dTPM 1.2 > > - > > https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html > > dTPM 2.0 > > > > I did not get "TPM interrupt not working, polling instead" to klog. > > But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? > > That's usually what you get when ACPI specifies the interrupt isn't > connected (we don't try to probe it). > > James Right, I'll test today with the same NUC's with "force=1". /Jarkko
On Fri, Oct 30, 2020 at 09:11:30AM -0700, Jerry Snitselaar wrote: > > James Bottomley @ 2020-10-30 08:49 MST: > > > On Fri, 2020-10-30 at 14:43 +0200, Jarkko Sakkinen wrote: > > [...] > >> I tested this with: > >> > >> - > >> https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html > >> dTPM 1.2 > >> - > >> https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html > >> dTPM 2.0 > >> > >> I did not get "TPM interrupt not working, polling instead" to klog. > >> But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? > > > > That's usually what you get when ACPI specifies the interrupt isn't > > connected (we don't try to probe it). > > > > James > > That is the problem I've been running into. When I do find a system > with a tpm and using tpm_tis, it usually seems to not have the interrupt > connected. > > Should this commit have: > > Fixes: 570a36097f30 ("tpm: drop 'irq' from struct tpm_vendor_specific") > > That is where TPM_CHIP_FLAG_IRQ was added and not set for tpm_tis. Have you tested 4eea703caaac? /Jarkko
Jarkko Sakkinen @ 2020-11-02 21:43 MST: > On Fri, Oct 30, 2020 at 09:11:30AM -0700, Jerry Snitselaar wrote: >> >> James Bottomley @ 2020-10-30 08:49 MST: >> >> > On Fri, 2020-10-30 at 14:43 +0200, Jarkko Sakkinen wrote: >> > [...] >> >> I tested this with: >> >> >> >> - >> >> https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html >> >> dTPM 1.2 >> >> - >> >> https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html >> >> dTPM 2.0 >> >> >> >> I did not get "TPM interrupt not working, polling instead" to klog. >> >> But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? >> > >> > That's usually what you get when ACPI specifies the interrupt isn't >> > connected (we don't try to probe it). >> > >> > James >> >> That is the problem I've been running into. When I do find a system >> with a tpm and using tpm_tis, it usually seems to not have the interrupt >> connected. >> >> Should this commit have: >> >> Fixes: 570a36097f30 ("tpm: drop 'irq' from struct tpm_vendor_specific") >> >> That is where TPM_CHIP_FLAG_IRQ was added and not set for tpm_tis. > > Have you tested 4eea703caaac? > > /Jarkko Is that the right commit id? 4eea703caaac tpm: drop 'iobase' from struct tpm_vendor_specific | 2016-06-25 | (Christophe Ricard)
On Tue, Nov 03, 2020 at 04:00:29PM -0700, Jerry Snitselaar wrote: > > Jarkko Sakkinen @ 2020-11-02 21:43 MST: > > > On Fri, Oct 30, 2020 at 09:11:30AM -0700, Jerry Snitselaar wrote: > >> > >> James Bottomley @ 2020-10-30 08:49 MST: > >> > >> > On Fri, 2020-10-30 at 14:43 +0200, Jarkko Sakkinen wrote: > >> > [...] > >> >> I tested this with: > >> >> > >> >> - > >> >> https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html > >> >> dTPM 1.2 > >> >> - > >> >> https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html > >> >> dTPM 2.0 > >> >> > >> >> I did not get "TPM interrupt not working, polling instead" to klog. > >> >> But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? > >> > > >> > That's usually what you get when ACPI specifies the interrupt isn't > >> > connected (we don't try to probe it). > >> > > >> > James > >> > >> That is the problem I've been running into. When I do find a system > >> with a tpm and using tpm_tis, it usually seems to not have the interrupt > >> connected. > >> > >> Should this commit have: > >> > >> Fixes: 570a36097f30 ("tpm: drop 'irq' from struct tpm_vendor_specific") > >> > >> That is where TPM_CHIP_FLAG_IRQ was added and not set for tpm_tis. > > > > Have you tested 4eea703caaac? > > > > /Jarkko > > Is that the right commit id? > > 4eea703caaac tpm: drop 'iobase' from struct tpm_vendor_specific | 2016-06-25 | (Christophe Ricard) Yeah. /Jarkko
On Fri, Oct 30, 2020 at 02:43:35PM +0200, Jarkko Sakkinen wrote: > On Sat, Oct 24, 2020 at 03:17:18PM +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 19, 2020 at 04:41:35PM -0700, Jerry Snitselaar wrote: > > > > > > James Bottomley @ 2020-10-01 11:09 MST: > > > > > > > There are two problems with our current interrupt probing: firstly the > > > > TPM_CHIP_FLAG_IRQ never gets set initially, so a check for interrupts > > > > is never done. Fix this by setting the flag before we generate and > > > > interrupt for probing. Secondly our IRQ setup may be ineffective on a > > > > TPM without legacy access cycles becuase according to the TPM > > > > Interface Specification the interrupt registers are only writeable in > > > > the current locality, so issue a request_locality before setting up > > > > the interrupts. > > > > > > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > > > > > > > --- > > > > > > > > v2: improved description > > > > --- > > > > drivers/char/tpm/tpm_tis_core.c | 14 ++++++++++++++ > > > > 1 file changed, 14 insertions(+) > > > > > > > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > > > > index 0c07da8cd680..12b657ed3a39 100644 > > > > --- a/drivers/char/tpm/tpm_tis_core.c > > > > +++ b/drivers/char/tpm/tpm_tis_core.c > > > > @@ -809,6 +809,19 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, > > > > } > > > > priv->irq = irq; > > > > > > > > + /* > > > > + * note writes to the interrupt registers are only effective > > > > + * when the TPM is in the active locality, so we have to > > > > + * request the locality here to get the interrupt set up. > > > > + * This request has no corresponding release, because the > > > > + * locality will be relinquished at the end of the tpm command > > > > + * that probes the interrupts > > > > + */ > > > > + if (request_locality(chip, 0) != 0) { > > > > + dev_err(&chip->dev, "failed to gain locality for irq probe\n"); > > > > + return -EBUSY; > > > > + } > > > > Appreciate the comment a lot, but s/note/Note/ > > I tested this with: > > - https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html > dTPM 1.2 > - https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html > dTPM 2.0 > > I did not get "TPM interrupt not working, polling instead" to klog. > But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? With dTPM2 NUC, I ended up get this when I just pass 'irq=0' to tpm_tis_core_init(): [ 0.584421] tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1A, rev-id 16) [ 0.680417] genirq: Flags mismatch irq 7. 00000008 (tpm0) vs. 00040088 (INT3432:00) [ 0.680448] tpm tpm0: Unable to request irq: 7 for probe [ 0.704416] genirq: Flags mismatch irq 9. 00000000 (tpm0) vs. 00000080 (acpi) [ 0.704444] tpm tpm0: Unable to request irq: 9 for probe /Jarkko
On Fri, 2020-11-06 at 17:32 +0200, Jarkko Sakkinen wrote: > On Fri, Oct 30, 2020 at 02:43:35PM +0200, Jarkko Sakkinen wrote: > > On Sat, Oct 24, 2020 at 03:17:18PM +0300, Jarkko Sakkinen wrote: > > > On Mon, Oct 19, 2020 at 04:41:35PM -0700, Jerry Snitselaar wrote: > > > > James Bottomley @ 2020-10-01 11:09 MST: > > > > > > > > > There are two problems with our current interrupt probing: > > > > > firstly the TPM_CHIP_FLAG_IRQ never gets set initially, so a > > > > > check for interrupts is never done. Fix this by setting the > > > > > flag before we generate and interrupt for probing. Secondly > > > > > our IRQ setup may be ineffective on a TPM without legacy > > > > > access cycles becuase according to the TPM Interface > > > > > Specification the interrupt registers are only > > > > > writeable in the current locality, so issue a > > > > > request_locality before setting up the interrupts. > > > > > > > > > > Signed-off-by: James Bottomley < > > > > > James.Bottomley@HansenPartnership.com> > > > > > > > > > > --- > > > > > > > > > > v2: improved description > > > > > --- > > > > > drivers/char/tpm/tpm_tis_core.c | 14 ++++++++++++++ > > > > > 1 file changed, 14 insertions(+) > > > > > > > > > > diff --git a/drivers/char/tpm/tpm_tis_core.c > > > > > b/drivers/char/tpm/tpm_tis_core.c > > > > > index 0c07da8cd680..12b657ed3a39 100644 > > > > > --- a/drivers/char/tpm/tpm_tis_core.c > > > > > +++ b/drivers/char/tpm/tpm_tis_core.c > > > > > @@ -809,6 +809,19 @@ static int > > > > > tpm_tis_probe_irq_single(struct tpm_chip *chip, > > > > > } > > > > > priv->irq = irq; > > > > > > > > > > + /* > > > > > + * note writes to the interrupt registers are only > > > > > effective > > > > > + * when the TPM is in the active locality, so we have > > > > > to > > > > > + * request the locality here to get the interrupt set > > > > > up. > > > > > + * This request has no corresponding release, because > > > > > the > > > > > + * locality will be relinquished at the end of the tpm > > > > > command > > > > > + * that probes the interrupts > > > > > + */ > > > > > + if (request_locality(chip, 0) != 0) { > > > > > + dev_err(&chip->dev, "failed to gain locality > > > > > for irq probe\n"); > > > > > + return -EBUSY; > > > > > + } > > > > > > Appreciate the comment a lot, but s/note/Note/ > > > > I tested this with: > > > > - > > https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html > > dTPM 1.2 > > - > > https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html > > dTPM 2.0 > > > > I did not get "TPM interrupt not working, polling instead" to klog. > > But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? > > With dTPM2 NUC, I ended up get this when I just pass 'irq=0' to > tpm_tis_core_init(): > > [ 0.584421] tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1A, rev-id > 16) > [ 0.680417] genirq: Flags mismatch irq 7. 00000008 (tpm0) vs. > 00040088 (INT3432:00) > [ 0.680448] tpm tpm0: Unable to request irq: 7 for probe > [ 0.704416] genirq: Flags mismatch irq 9. 00000000 (tpm0) vs. > 00000080 (acpi) > [ 0.704444] tpm tpm0: Unable to request irq: 9 for probe Well this looks normal: you forced the tis subsystem to probe for an IRQ even though ACPI says there isn't one and it didn't find one ... so ACPI was actually right (for once). James
On Fri, Nov 06, 2020 at 08:21:44AM -0800, James Bottomley wrote: > On Fri, 2020-11-06 at 17:32 +0200, Jarkko Sakkinen wrote: > > On Fri, Oct 30, 2020 at 02:43:35PM +0200, Jarkko Sakkinen wrote: > > > On Sat, Oct 24, 2020 at 03:17:18PM +0300, Jarkko Sakkinen wrote: > > > > On Mon, Oct 19, 2020 at 04:41:35PM -0700, Jerry Snitselaar wrote: > > > > > James Bottomley @ 2020-10-01 11:09 MST: > > > > > > > > > > > There are two problems with our current interrupt probing: > > > > > > firstly the TPM_CHIP_FLAG_IRQ never gets set initially, so a > > > > > > check for interrupts is never done. Fix this by setting the > > > > > > flag before we generate and interrupt for probing. Secondly > > > > > > our IRQ setup may be ineffective on a TPM without legacy > > > > > > access cycles becuase according to the TPM Interface > > > > > > Specification the interrupt registers are only > > > > > > writeable in the current locality, so issue a > > > > > > request_locality before setting up the interrupts. > > > > > > > > > > > > Signed-off-by: James Bottomley < > > > > > > James.Bottomley@HansenPartnership.com> > > > > > > > > > > > > --- > > > > > > > > > > > > v2: improved description > > > > > > --- > > > > > > drivers/char/tpm/tpm_tis_core.c | 14 ++++++++++++++ > > > > > > 1 file changed, 14 insertions(+) > > > > > > > > > > > > diff --git a/drivers/char/tpm/tpm_tis_core.c > > > > > > b/drivers/char/tpm/tpm_tis_core.c > > > > > > index 0c07da8cd680..12b657ed3a39 100644 > > > > > > --- a/drivers/char/tpm/tpm_tis_core.c > > > > > > +++ b/drivers/char/tpm/tpm_tis_core.c > > > > > > @@ -809,6 +809,19 @@ static int > > > > > > tpm_tis_probe_irq_single(struct tpm_chip *chip, > > > > > > } > > > > > > priv->irq = irq; > > > > > > > > > > > > + /* > > > > > > + * note writes to the interrupt registers are only > > > > > > effective > > > > > > + * when the TPM is in the active locality, so we have > > > > > > to > > > > > > + * request the locality here to get the interrupt set > > > > > > up. > > > > > > + * This request has no corresponding release, because > > > > > > the > > > > > > + * locality will be relinquished at the end of the tpm > > > > > > command > > > > > > + * that probes the interrupts > > > > > > + */ > > > > > > + if (request_locality(chip, 0) != 0) { > > > > > > + dev_err(&chip->dev, "failed to gain locality > > > > > > for irq probe\n"); > > > > > > + return -EBUSY; > > > > > > + } > > > > > > > > Appreciate the comment a lot, but s/note/Note/ > > > > > > I tested this with: > > > > > > - > > > https://ark.intel.com/content/www/us/en/ark/products/84861/intel-nuc-kit-nuc5i5myhe.html > > > dTPM 1.2 > > > - > > > https://ark.intel.com/content/www/us/en/ark/products/74483/intel-nuc-kit-dc53427hye.html > > > dTPM 2.0 > > > > > > I did not get "TPM interrupt not working, polling instead" to klog. > > > But I neither see tpm0 in /proc/interrupts. What I'm doing wrong? > > > > With dTPM2 NUC, I ended up get this when I just pass 'irq=0' to > > tpm_tis_core_init(): > > > > [ 0.584421] tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1A, rev-id > > 16) > > [ 0.680417] genirq: Flags mismatch irq 7. 00000008 (tpm0) vs. > > 00040088 (INT3432:00) > > [ 0.680448] tpm tpm0: Unable to request irq: 7 for probe > > [ 0.704416] genirq: Flags mismatch irq 9. 00000000 (tpm0) vs. > > 00000080 (acpi) > > [ 0.704444] tpm tpm0: Unable to request irq: 9 for probe > > Well this looks normal: you forced the tis subsystem to probe for an > IRQ even though ACPI says there isn't one and it didn't find one ... so > ACPI was actually right (for once). > > James True. I wanted to give quick feedback with the results, did not have time to analyze at that point. Can you do one more round with remarks fixed that I mentioned (if I recall right they were only about comments and commit messages, code itself is in great condition), and you can add my tested-by to each commit? I'll put this then to my next rc request so that we get this to 5.10. /Jarkko
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index 0c07da8cd680..12b657ed3a39 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -809,6 +809,19 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, } priv->irq = irq; + /* + * note writes to the interrupt registers are only effective + * when the TPM is in the active locality, so we have to + * request the locality here to get the interrupt set up. + * This request has no corresponding release, because the + * locality will be relinquished at the end of the tpm command + * that probes the interrupts + */ + if (request_locality(chip, 0) != 0) { + dev_err(&chip->dev, "failed to gain locality for irq probe\n"); + return -EBUSY; + } + rc = tpm_tis_read8(priv, TPM_INT_VECTOR(priv->locality), &original_int_vec); if (rc < 0) @@ -836,6 +849,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, return rc; priv->irq_tested = false; + chip->flags |= TPM_CHIP_FLAG_IRQ; /* Generate an interrupt by having the core call through to * tpm_tis_send
There are two problems with our current interrupt probing: firstly the TPM_CHIP_FLAG_IRQ never gets set initially, so a check for interrupts is never done. Fix this by setting the flag before we generate and interrupt for probing. Secondly our IRQ setup may be ineffective on a TPM without legacy access cycles becuase according to the TPM Interface Specification the interrupt registers are only writeable in the current locality, so issue a request_locality before setting up the interrupts. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- v2: improved description --- drivers/char/tpm/tpm_tis_core.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)