| Message ID | 1412712189-1234-16-git-send-email-christophe-h.ricard@st.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, Oct 07, 2014 at 10:03:08PM +0200, Christophe Ricard wrote: > In order to manage irq, locality must be active. As Status Ready > interrupt is activated, when going back into ready state with the > cancel function, we need to add a little delay to make sure the irq > is going to be serviced before the release_locality is hit. Using a delay for this seems pretty sketchy, is this supposed to be waiting for a level sensitive IRQ to desassert? or a queued IRQ to be delivered? What is the harm without the udelay? Jason ------------------------------------------------------------------------------ Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
Hi Jason, This delay is inserted at those place because: - An interrupt can be cleared only when the locality is active. - The current sequence is triggering an irq (most likely a |TPM_INTF_CMD_READY_INT) Therefore If the locality is released before the irq is cleared. The irq line will remain active for ever. The small udelay is the most accurate fix i had in mind to guarantee that the irq will get serviced before the locality is released. I believe requesting the locality inside the isr is not accurate and the release_locality is still accurate at the end of those send/recv functions. However one drawback here is that the small udelay will still be there in case we are running the driver in polling mode. Is that really important/bad ? Best Regards Christophe |On 07/10/2014 23:56, Jason Gunthorpe wrote: > On Tue, Oct 07, 2014 at 10:03:08PM +0200, Christophe Ricard wrote: >> In order to manage irq, locality must be active. As Status Ready >> interrupt is activated, when going back into ready state with the >> cancel function, we need to add a little delay to make sure the irq >> is going to be serviced before the release_locality is hit. > Using a delay for this seems pretty sketchy, is this supposed to be > waiting for a level sensitive IRQ to desassert? or a queued IRQ to be > delivered? > > What is the harm without the udelay? > > Jason ------------------------------------------------------------------------------ Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c index de9f12e..4c78845 100644 --- a/drivers/char/tpm/tpm_i2c_stm_st33.c +++ b/drivers/char/tpm/tpm_i2c_stm_st33.c @@ -509,6 +509,7 @@ static int tpm_stm_i2c_send(struct tpm_chip *chip, unsigned char *buf, return len; out_err: tpm_stm_i2c_cancel(chip); + usleep_range(100, 250); release_locality(chip); return r; } @@ -557,6 +558,7 @@ static int tpm_stm_i2c_recv(struct tpm_chip *chip, unsigned char *buf, out: chip->ops->cancel(chip); + usleep_range(100, 250); release_locality(chip); return size; }
In order to manage irq, locality must be active. As Status Ready interrupt is activated, when going back into ready state with the cancel function, we need to add a little delay to make sure the irq is going to be serviced before the release_locality is hit. Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com> --- drivers/char/tpm/tpm_i2c_stm_st33.c | 2 ++ 1 file changed, 2 insertions(+)