From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755450AbaIIDQN (ORCPT ); Mon, 8 Sep 2014 23:16:13 -0400 Received: from mx1.scotdoyle.com ([23.226.141.211]:33217 "EHLO mx1.scotdoyle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755176AbaIIDQM (ORCPT ); Mon, 8 Sep 2014 23:16:12 -0400 Date: Tue, 9 Sep 2014 03:12:51 +0000 (UTC) From: Scot Doyle To: tpmdd-devel@lists.sourceforge.net cc: Peter Huewe , Ashley Lai , Marcel Selhorst , Jason Gunthorpe , Michael Mullin , Stefan Berger , Luigi Semenzato , linux-kernel@vger.kernel.org Subject: Re: [PATCH v7] tpm_tis: verify interrupt during init In-Reply-To: Message-ID: References: <20140827173142.GA11183@obsidianresearch.com> <20140827214743.GC11183@obsidianresearch.com> <20140828165348.GE11183@obsidianresearch.com> <20140830174920.GA26218@obsidianresearch.com> <20140902172015.GD13956@obsidianresearch.com> <20140908220238.GB6204@obsidianresearch.com> User-Agent: Alpine 2.11 (LNX 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 9 Sep 2014, Scot Doyle wrote: > Some machines, such as the Acer C720 and Toshiba CB35, have TPMs that do > not send IRQs while also having an ACPI TPM entry indicating that they > will be sent. These machines freeze on resume while the tpm_tis module > waits for an IRQ, eventually timing out. > > When in interrupt mode, the tpm_tis module should receive an IRQ during > module init. Fall back to polling mode if none is received when expected. > > Signed-off-by: Scot Doyle > Tested-by: Michael Mullin > Reviewed-By: Jason Gunthorpe Would anyone be willing to test this patch on a system that uses the tpm_tis module with interrupts? > --- > drivers/char/tpm/tpm_tis.c | 75 +++++++++++++++++++++++++++++++++++----------- > 1 file changed, 57 insertions(+), 18 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index 2c46734..43aeb6961 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -75,6 +75,10 @@ enum tis_defaults { > #define TPM_DID_VID(l) (0x0F00 | ((l) << 12)) > #define TPM_RID(l) (0x0F04 | ((l) << 12)) > > +struct priv_data { > + bool irq_tested; > +}; > + > static LIST_HEAD(tis_chips); > static DEFINE_MUTEX(tis_lock); > > @@ -338,6 +342,21 @@ out_err: > return rc; > } > > +static void disable_interrupts(struct tpm_chip *chip) > +{ > + u32 intmask; > + intmask = > + ioread32(chip->vendor.iobase + > + TPM_INT_ENABLE(chip->vendor.locality)); > + intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT | > + TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT; > + iowrite32(intmask, > + chip->vendor.iobase + > + TPM_INT_ENABLE(chip->vendor.locality)); > + free_irq(chip->vendor.irq, chip); > + chip->vendor.irq = 0; > +} > + > /* > * If interrupts are used (signaled by an irq set in the vendor structure) > * tpm.c can skip polling for the data to be available as the interrupt is > @@ -345,8 +364,10 @@ out_err: > */ > static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len) > { > - int rc; > + int rc, irq; > u32 ordinal; > + bool test_irq; > + struct priv_data *priv = chip->vendor.priv; > > rc = tpm_tis_send_data(chip, buf, len); > if (rc < 0) > @@ -358,13 +379,27 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len) > > if (chip->vendor.irq) { > ordinal = be32_to_cpu(*((__be32 *) (buf + 6))); > - if (wait_for_tpm_stat > - (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID, > - tpm_calc_ordinal_duration(chip, ordinal), > - &chip->vendor.read_queue, false) < 0) { > + test_irq = !priv->irq_tested; > + if (test_irq) { > + irq = chip->vendor.irq; > + chip->vendor.irq = 0; > + } > + rc = wait_for_tpm_stat > + (chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID, > + tpm_calc_ordinal_duration(chip, ordinal), > + &chip->vendor.read_queue, false); > + if (test_irq) > + chip->vendor.irq = irq; > + if (rc < 0) { > rc = -ETIME; > goto out_err; > } > + if (test_irq && !priv->irq_tested) { > + priv->irq_tested = true; > + disable_interrupts(chip); > + dev_err(chip->dev, > + FW_BUG "TPM interrupt not working, polling instead\n"); > + } > } > return len; > out_err: > @@ -505,6 +540,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id) > if (interrupt == 0) > return IRQ_NONE; > > + ((struct priv_data*)chip->vendor.priv)->irq_tested = true; > if (interrupt & TPM_INTF_DATA_AVAIL_INT) > wake_up_interruptible(&chip->vendor.read_queue); > if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT) > @@ -534,10 +570,14 @@ static int tpm_tis_init(struct device *dev, resource_size_t start, > u32 vendor, intfcaps, intmask; > int rc, i, irq_s, irq_e, probe; > struct tpm_chip *chip; > + struct priv_data *priv; > > if (!(chip = tpm_register_hardware(dev, &tpm_tis))) > return -ENODEV; > > + priv = devm_kzalloc(dev, sizeof(struct priv_data), GFP_KERNEL); > + chip->vendor.priv = priv; > + > chip->vendor.iobase = ioremap(start, len); > if (!chip->vendor.iobase) { > rc = -EIO; > @@ -605,19 +645,6 @@ static int tpm_tis_init(struct device *dev, resource_size_t start, > if (intfcaps & TPM_INTF_DATA_AVAIL_INT) > dev_dbg(dev, "\tData Avail Int Support\n"); > > - /* get the timeouts before testing for irqs */ > - if (tpm_get_timeouts(chip)) { > - dev_err(dev, "Could not get TPM timeouts and durations\n"); > - rc = -ENODEV; > - goto out_err; > - } > - > - if (tpm_do_selftest(chip)) { > - dev_err(dev, "TPM self test failed\n"); > - rc = -ENODEV; > - goto out_err; > - } > - > /* INTERRUPT Setup */ > init_waitqueue_head(&chip->vendor.read_queue); > init_waitqueue_head(&chip->vendor.int_queue); > @@ -719,6 +746,18 @@ static int tpm_tis_init(struct device *dev, resource_size_t start, > } > } > > + if (tpm_get_timeouts(chip)) { > + dev_err(dev, "Could not get TPM timeouts and durations\n"); > + rc = -ENODEV; > + goto out_err; > + } > + > + if (tpm_do_selftest(chip)) { > + dev_err(dev, "TPM self test failed\n"); > + rc = -ENODEV; > + goto out_err; > + } > + > INIT_LIST_HEAD(&chip->vendor.list); > mutex_lock(&tis_lock); > list_add(&chip->vendor.list, &tis_chips); > -- > 2.0.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >