| Message ID | 20220610110846.8307-7-LinoSanfilippo@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TPM IRQ fixes | expand |
On Fri, Jun 10, 2022 at 01:08:42PM +0200, LinoSanfilippo@gmx.de wrote: > From: Lino Sanfilippo <l.sanfilippo@kunbus.com> > > According to the TPM Interface Specification (TIS) interrupts for > "stsValid" and "commandReady" might not be supported. > > Take this into account and only wait for interrupts which are actually in > use in wait_for_tpm_stat(). After that process all the remaining status > changes by polling the status register. > > Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com> > --- > drivers/char/tpm/tpm_tis_core.c | 46 ++++++++++++++++++++++++--------- > 1 file changed, 34 insertions(+), 12 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > index 2f03fefa1706..028bec44362d 100644 > --- a/drivers/char/tpm/tpm_tis_core.c > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -53,41 +53,63 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, > long rc; > u8 status; > bool canceled = false; > + u8 active_irqs = 0; Something like sts_mask would be a better name. Calling it 'active_irqs' is a bit confusing. > + int ret = 0; > > /* check current status */ > status = chip->ops->status(chip); > if ((status & mask) == mask) > return 0; > > - stop = jiffies + timeout; > + /* check what status changes can be handled by irqs */ > + if (priv->irqs_in_use & TPM_INTF_STS_VALID_INT) > + active_irqs |= TPM_STS_VALID; > > - if (chip->flags & TPM_CHIP_FLAG_IRQ) { > + if (priv->irqs_in_use & TPM_INTF_DATA_AVAIL_INT) > + active_irqs |= TPM_STS_DATA_AVAIL; > + > + if (priv->irqs_in_use & TPM_INTF_CMD_READY_INT) > + active_irqs |= TPM_STS_COMMAND_READY; > + > + active_irqs &= mask; > + > + stop = jiffies + timeout; > + /* process status changes with irq support */ > + if (active_irqs) { > + ret = -ETIME; > again: > timeout = stop - jiffies; > if ((long)timeout <= 0) > return -ETIME; > rc = wait_event_interruptible_timeout(*queue, > - wait_for_tpm_stat_cond(chip, mask, check_cancel, > + wait_for_tpm_stat_cond(chip, active_irqs, check_cancel, > &canceled), > timeout); > if (rc > 0) { > if (canceled) > return -ECANCELED; > - return 0; > + ret = 0; > } > if (rc == -ERESTARTSYS && freezing(current)) { > clear_thread_flag(TIF_SIGPENDING); > goto again; > } > - } else { > - do { > - usleep_range(priv->timeout_min, > - priv->timeout_max); > - status = chip->ops->status(chip); > - if ((status & mask) == mask) > - return 0; > - } while (time_before(jiffies, stop)); > } > + > + if (ret) > + return ret; > + > + mask &= ~active_irqs; > + if (!mask) /* all done */ > + return 0; > + /* process status changes without irq support */ > + do { > + status = chip->ops->status(chip); > + if ((status & mask) == mask) > + return 0; > + usleep_range(priv->timeout_min, > + priv->timeout_max); > + } while (time_before(jiffies, stop)); > return -ETIME; > } > > -- > 2.36.1 > BR, Jarkko
On 15.06.22 at 20:21, Jarkko Sakkinen wrote: > On Fri, Jun 10, 2022 at 01:08:42PM +0200, LinoSanfilippo@gmx.de wrote: >> From: Lino Sanfilippo <l.sanfilippo@kunbus.com> >> >> According to the TPM Interface Specification (TIS) interrupts for >> "stsValid" and "commandReady" might not be supported. >> >> Take this into account and only wait for interrupts which are actually in >> use in wait_for_tpm_stat(). After that process all the remaining status >> changes by polling the status register. >> >> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com> >> --- >> drivers/char/tpm/tpm_tis_core.c | 46 ++++++++++++++++++++++++--------- >> 1 file changed, 34 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c >> index 2f03fefa1706..028bec44362d 100644 >> --- a/drivers/char/tpm/tpm_tis_core.c >> +++ b/drivers/char/tpm/tpm_tis_core.c >> @@ -53,41 +53,63 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, >> long rc; >> u8 status; >> bool canceled = false; >> + u8 active_irqs = 0; > > Something like sts_mask would be a better name. Calling it > 'active_irqs' is a bit confusing. Ok. Regards, Lino
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index 2f03fefa1706..028bec44362d 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -53,41 +53,63 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, long rc; u8 status; bool canceled = false; + u8 active_irqs = 0; + int ret = 0; /* check current status */ status = chip->ops->status(chip); if ((status & mask) == mask) return 0; - stop = jiffies + timeout; + /* check what status changes can be handled by irqs */ + if (priv->irqs_in_use & TPM_INTF_STS_VALID_INT) + active_irqs |= TPM_STS_VALID; - if (chip->flags & TPM_CHIP_FLAG_IRQ) { + if (priv->irqs_in_use & TPM_INTF_DATA_AVAIL_INT) + active_irqs |= TPM_STS_DATA_AVAIL; + + if (priv->irqs_in_use & TPM_INTF_CMD_READY_INT) + active_irqs |= TPM_STS_COMMAND_READY; + + active_irqs &= mask; + + stop = jiffies + timeout; + /* process status changes with irq support */ + if (active_irqs) { + ret = -ETIME; again: timeout = stop - jiffies; if ((long)timeout <= 0) return -ETIME; rc = wait_event_interruptible_timeout(*queue, - wait_for_tpm_stat_cond(chip, mask, check_cancel, + wait_for_tpm_stat_cond(chip, active_irqs, check_cancel, &canceled), timeout); if (rc > 0) { if (canceled) return -ECANCELED; - return 0; + ret = 0; } if (rc == -ERESTARTSYS && freezing(current)) { clear_thread_flag(TIF_SIGPENDING); goto again; } - } else { - do { - usleep_range(priv->timeout_min, - priv->timeout_max); - status = chip->ops->status(chip); - if ((status & mask) == mask) - return 0; - } while (time_before(jiffies, stop)); } + + if (ret) + return ret; + + mask &= ~active_irqs; + if (!mask) /* all done */ + return 0; + /* process status changes without irq support */ + do { + status = chip->ops->status(chip); + if ((status & mask) == mask) + return 0; + usleep_range(priv->timeout_min, + priv->timeout_max); + } while (time_before(jiffies, stop)); return -ETIME; }