Message ID | 20210501135727.17747-5-LinoSanfilippo@gmx.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Fixes for TPM interrupt handling | expand |
On 5/1/21 9:57 AM, Lino Sanfilippo wrote: > Do not set interrupts which are not supported by the hardware. Instead use > the information from the capability query and activate only the reported > interrupts. > > Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
On 01.05.21 at 21:09, Stefan Berger wrote: > > On 5/1/21 9:57 AM, Lino Sanfilippo wrote: >> Do not set interrupts which are not supported by the hardware. Instead use >> the information from the capability query and activate only the reported >> interrupts. >> >> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de> > > > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> > > Ah sorry, I forgot to add your Reviewed-By for this patch. Will do so in the next version. Thx, Lino
On Sat, May 01, 2021 at 03:57:27PM +0200, Lino Sanfilippo wrote: > Do not set interrupts which are not supported by the hardware. Instead use > the information from the capability query and activate only the reported > interrupts. > > Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de> Zero reasoning again, why. > --- > drivers/char/tpm/tpm_tis_core.c | 68 ++++++++++++++++++--------------- > drivers/char/tpm/tpm_tis_core.h | 1 + > 2 files changed, 38 insertions(+), 31 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > index 9615234054aa..757498a63f57 100644 > --- a/drivers/char/tpm/tpm_tis_core.c > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -936,13 +936,47 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, > if (rc) > goto out_err; > > + /* Figure out the capabilities */ > + rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps); > + if (rc < 0) > + goto out_err; > + > + dev_dbg(dev, "TPM interface capabilities (0x%x):\n", > + intfcaps); > + if (intfcaps & TPM_INTF_BURST_COUNT_STATIC) > + dev_dbg(dev, "\tBurst Count Static\n"); > + if (intfcaps & TPM_INTF_CMD_READY_INT) { > + priv->supported_irqs |= TPM_INTF_CMD_READY_INT; > + dev_dbg(dev, "\tCommand Ready Int Support\n"); > + } > + if (intfcaps & TPM_INTF_INT_EDGE_FALLING) > + dev_dbg(dev, "\tInterrupt Edge Falling\n"); > + if (intfcaps & TPM_INTF_INT_EDGE_RISING) > + dev_dbg(dev, "\tInterrupt Edge Rising\n"); > + if (intfcaps & TPM_INTF_INT_LEVEL_LOW) > + dev_dbg(dev, "\tInterrupt Level Low\n"); > + if (intfcaps & TPM_INTF_INT_LEVEL_HIGH) > + dev_dbg(dev, "\tInterrupt Level High\n"); > + if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT) { > + priv->supported_irqs |= TPM_INTF_LOCALITY_CHANGE_INT; > + dev_dbg(dev, "\tLocality Change Int Support\n"); > + } > + if (intfcaps & TPM_INTF_STS_VALID_INT) { > + priv->supported_irqs |= TPM_INTF_STS_VALID_INT; > + dev_dbg(dev, "\tSts Valid Int Support\n"); > + } > + if (intfcaps & TPM_INTF_DATA_AVAIL_INT) { > + priv->supported_irqs |= TPM_INTF_DATA_AVAIL_INT; > + dev_dbg(dev, "\tData Avail Int Support\n"); > + } > + > /* Take control of the TPM's interrupt hardware and shut it off */ > rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask); > if (rc < 0) > goto out_err; > > - intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT | > - TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT; > + intmask |= priv->supported_irqs; > + > intmask &= ~TPM_GLOBAL_INT_ENABLE; > tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask); > > @@ -971,32 +1005,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, > goto out_err; > } > > - /* Figure out the capabilities */ > - rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps); > - if (rc < 0) > - goto out_err; > - > - dev_dbg(dev, "TPM interface capabilities (0x%x):\n", > - intfcaps); > - if (intfcaps & TPM_INTF_BURST_COUNT_STATIC) > - dev_dbg(dev, "\tBurst Count Static\n"); > - if (intfcaps & TPM_INTF_CMD_READY_INT) > - dev_dbg(dev, "\tCommand Ready Int Support\n"); > - if (intfcaps & TPM_INTF_INT_EDGE_FALLING) > - dev_dbg(dev, "\tInterrupt Edge Falling\n"); > - if (intfcaps & TPM_INTF_INT_EDGE_RISING) > - dev_dbg(dev, "\tInterrupt Edge Rising\n"); > - if (intfcaps & TPM_INTF_INT_LEVEL_LOW) > - dev_dbg(dev, "\tInterrupt Level Low\n"); > - if (intfcaps & TPM_INTF_INT_LEVEL_HIGH) > - dev_dbg(dev, "\tInterrupt Level High\n"); > - if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT) > - dev_dbg(dev, "\tLocality Change Int Support\n"); > - if (intfcaps & TPM_INTF_STS_VALID_INT) > - dev_dbg(dev, "\tSts Valid Int Support\n"); > - if (intfcaps & TPM_INTF_DATA_AVAIL_INT) > - dev_dbg(dev, "\tData Avail Int Support\n"); > - > /* INTERRUPT Setup */ > init_waitqueue_head(&priv->read_queue); > init_waitqueue_head(&priv->int_queue); > @@ -1066,9 +1074,7 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip) > if (rc < 0) > goto out; > > - intmask |= TPM_INTF_CMD_READY_INT > - | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_DATA_AVAIL_INT > - | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE; > + intmask |= priv->supported_irqs | TPM_GLOBAL_INT_ENABLE; > > tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask); > > diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h > index dc5f92b18dca..8ff62213d8fc 100644 > --- a/drivers/char/tpm/tpm_tis_core.h > +++ b/drivers/char/tpm/tpm_tis_core.h > @@ -89,6 +89,7 @@ struct tpm_tis_data { > u16 manufacturer_id; > int locality; > int irq; > + unsigned int supported_irqs; > unsigned int flags; > void __iomem *ilb_base_addr; > u16 clkrun_enabled; > -- > 2.31.1 /Jarkko >
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index 9615234054aa..757498a63f57 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -936,13 +936,47 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, if (rc) goto out_err; + /* Figure out the capabilities */ + rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps); + if (rc < 0) + goto out_err; + + dev_dbg(dev, "TPM interface capabilities (0x%x):\n", + intfcaps); + if (intfcaps & TPM_INTF_BURST_COUNT_STATIC) + dev_dbg(dev, "\tBurst Count Static\n"); + if (intfcaps & TPM_INTF_CMD_READY_INT) { + priv->supported_irqs |= TPM_INTF_CMD_READY_INT; + dev_dbg(dev, "\tCommand Ready Int Support\n"); + } + if (intfcaps & TPM_INTF_INT_EDGE_FALLING) + dev_dbg(dev, "\tInterrupt Edge Falling\n"); + if (intfcaps & TPM_INTF_INT_EDGE_RISING) + dev_dbg(dev, "\tInterrupt Edge Rising\n"); + if (intfcaps & TPM_INTF_INT_LEVEL_LOW) + dev_dbg(dev, "\tInterrupt Level Low\n"); + if (intfcaps & TPM_INTF_INT_LEVEL_HIGH) + dev_dbg(dev, "\tInterrupt Level High\n"); + if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT) { + priv->supported_irqs |= TPM_INTF_LOCALITY_CHANGE_INT; + dev_dbg(dev, "\tLocality Change Int Support\n"); + } + if (intfcaps & TPM_INTF_STS_VALID_INT) { + priv->supported_irqs |= TPM_INTF_STS_VALID_INT; + dev_dbg(dev, "\tSts Valid Int Support\n"); + } + if (intfcaps & TPM_INTF_DATA_AVAIL_INT) { + priv->supported_irqs |= TPM_INTF_DATA_AVAIL_INT; + dev_dbg(dev, "\tData Avail Int Support\n"); + } + /* Take control of the TPM's interrupt hardware and shut it off */ rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask); if (rc < 0) goto out_err; - intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT | - TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT; + intmask |= priv->supported_irqs; + intmask &= ~TPM_GLOBAL_INT_ENABLE; tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask); @@ -971,32 +1005,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, goto out_err; } - /* Figure out the capabilities */ - rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps); - if (rc < 0) - goto out_err; - - dev_dbg(dev, "TPM interface capabilities (0x%x):\n", - intfcaps); - if (intfcaps & TPM_INTF_BURST_COUNT_STATIC) - dev_dbg(dev, "\tBurst Count Static\n"); - if (intfcaps & TPM_INTF_CMD_READY_INT) - dev_dbg(dev, "\tCommand Ready Int Support\n"); - if (intfcaps & TPM_INTF_INT_EDGE_FALLING) - dev_dbg(dev, "\tInterrupt Edge Falling\n"); - if (intfcaps & TPM_INTF_INT_EDGE_RISING) - dev_dbg(dev, "\tInterrupt Edge Rising\n"); - if (intfcaps & TPM_INTF_INT_LEVEL_LOW) - dev_dbg(dev, "\tInterrupt Level Low\n"); - if (intfcaps & TPM_INTF_INT_LEVEL_HIGH) - dev_dbg(dev, "\tInterrupt Level High\n"); - if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT) - dev_dbg(dev, "\tLocality Change Int Support\n"); - if (intfcaps & TPM_INTF_STS_VALID_INT) - dev_dbg(dev, "\tSts Valid Int Support\n"); - if (intfcaps & TPM_INTF_DATA_AVAIL_INT) - dev_dbg(dev, "\tData Avail Int Support\n"); - /* INTERRUPT Setup */ init_waitqueue_head(&priv->read_queue); init_waitqueue_head(&priv->int_queue); @@ -1066,9 +1074,7 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip) if (rc < 0) goto out; - intmask |= TPM_INTF_CMD_READY_INT - | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_DATA_AVAIL_INT - | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE; + intmask |= priv->supported_irqs | TPM_GLOBAL_INT_ENABLE; tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask); diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h index dc5f92b18dca..8ff62213d8fc 100644 --- a/drivers/char/tpm/tpm_tis_core.h +++ b/drivers/char/tpm/tpm_tis_core.h @@ -89,6 +89,7 @@ struct tpm_tis_data { u16 manufacturer_id; int locality; int irq; + unsigned int supported_irqs; unsigned int flags; void __iomem *ilb_base_addr; u16 clkrun_enabled;
Do not set interrupts which are not supported by the hardware. Instead use the information from the capability query and activate only the reported interrupts. Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de> --- drivers/char/tpm/tpm_tis_core.c | 68 ++++++++++++++++++--------------- drivers/char/tpm/tpm_tis_core.h | 1 + 2 files changed, 38 insertions(+), 31 deletions(-)