| Message ID | 20221120133134.28926-5-LinoSanfilippo@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TPM IRQ fixes | expand |
On Sun, Nov 20, 2022 at 02:31:24PM +0100, Lino Sanfilippo wrote: > From: Lino Sanfilippo <l.sanfilippo@kunbus.com> > > If in tpm_tis_probe_irq_single() an error occurs after the original > interrupt vector has been read, restore the interrupts before the error is > returned. > > Since the caller does not check the error value, return -1 in any case that > the TPM_CHIP_FLAG_IRQ flag is not set. Since the return value of function > tpm_tis_gen_interrupt() is not longer used, make it a void function. > > Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access") > Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com> > --- > drivers/char/tpm/tpm_tis_core.c | 33 +++++++++++++-------------------- > 1 file changed, 13 insertions(+), 20 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > index 603b82ca56da..1eac1279594d 100644 > --- a/drivers/char/tpm/tpm_tis_core.c > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -729,7 +729,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id) > return IRQ_HANDLED; > } > > -static int tpm_tis_gen_interrupt(struct tpm_chip *chip) > +static void tpm_tis_gen_interrupt(struct tpm_chip *chip) > { > const char *desc = "attempting to generate an interrupt"; > u32 cap2; > @@ -738,16 +738,14 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip) > > ret = request_locality(chip, 0); > if (ret < 0) > - return ret; > + return; > > if (chip->flags & TPM_CHIP_FLAG_TPM2) > - ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); > + tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); In a successive patch: - tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); + ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); ` BR, Jarkko
On 24.11.22 at 07:58, Jarkko Sakkinen wrote: > On Sun, Nov 20, 2022 at 02:31:24PM +0100, Lino Sanfilippo wrote: >> From: Lino Sanfilippo <l.sanfilippo@kunbus.com> >> >> If in tpm_tis_probe_irq_single() an error occurs after the original >> interrupt vector has been read, restore the interrupts before the error is >> returned. >> >> Since the caller does not check the error value, return -1 in any case that >> the TPM_CHIP_FLAG_IRQ flag is not set. Since the return value of function >> tpm_tis_gen_interrupt() is not longer used, make it a void function. >> >> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access") >> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com> >> --- >> drivers/char/tpm/tpm_tis_core.c | 33 +++++++++++++-------------------- >> 1 file changed, 13 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c >> index 603b82ca56da..1eac1279594d 100644 >> --- a/drivers/char/tpm/tpm_tis_core.c >> +++ b/drivers/char/tpm/tpm_tis_core.c >> @@ -729,7 +729,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id) >> return IRQ_HANDLED; >> } >> >> -static int tpm_tis_gen_interrupt(struct tpm_chip *chip) >> +static void tpm_tis_gen_interrupt(struct tpm_chip *chip) >> { >> const char *desc = "attempting to generate an interrupt"; >> u32 cap2; >> @@ -738,16 +738,14 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip) >> >> ret = request_locality(chip, 0); >> if (ret < 0) >> - return ret; >> + return; >> >> if (chip->flags & TPM_CHIP_FLAG_TPM2) >> - ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); >> + tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); > > In a successive patch: > > - tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); > + ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); > ` > > BR, Jarkko > If it was a single patch it would be IMHO correct to remove ret, since at this point it is not needed any more. But as part of a series it is admittedly a bit odd to remove the value only to re-add it in a later patch of the same series. I will fix this and send a v11. Regards, Lino
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index 603b82ca56da..1eac1279594d 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -729,7 +729,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id) return IRQ_HANDLED; } -static int tpm_tis_gen_interrupt(struct tpm_chip *chip) +static void tpm_tis_gen_interrupt(struct tpm_chip *chip) { const char *desc = "attempting to generate an interrupt"; u32 cap2; @@ -738,16 +738,14 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip) ret = request_locality(chip, 0); if (ret < 0) - return ret; + return; if (chip->flags & TPM_CHIP_FLAG_TPM2) - ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); + tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); else - ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0); + tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0); release_locality(chip, 0); - - return ret; } /* Register the IRQ and issue a command that will cause an interrupt. If an @@ -777,42 +775,37 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask, rc = tpm_tis_write8(priv, TPM_INT_VECTOR(priv->locality), irq); if (rc < 0) - return rc; + goto restore_irqs; rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &int_status); if (rc < 0) - return rc; + goto restore_irqs; /* Clear all existing */ rc = tpm_tis_write32(priv, TPM_INT_STATUS(priv->locality), int_status); if (rc < 0) - return rc; - + goto restore_irqs; /* Turn on */ rc = tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask | TPM_GLOBAL_INT_ENABLE); if (rc < 0) - return rc; + goto restore_irqs; clear_bit(TPM_TIS_IRQ_TESTED, &priv->flags); /* Generate an interrupt by having the core call through to * tpm_tis_send */ - rc = tpm_tis_gen_interrupt(chip); - if (rc < 0) - return rc; + tpm_tis_gen_interrupt(chip); +restore_irqs: /* tpm_tis_send will either confirm the interrupt is working or it * will call disable_irq which undoes all of the above. */ if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) { - rc = tpm_tis_write8(priv, original_int_vec, - TPM_INT_VECTOR(priv->locality)); - if (rc < 0) - return rc; - - return 1; + tpm_tis_write8(priv, original_int_vec, + TPM_INT_VECTOR(priv->locality)); + return -1; } return 0;