[PATCH] st33zp24 fix when using irq mode

[PATCH] st33zp24 fix when using irq mode

[Christophe Ricard]

Hi,

Please find in this patch a fix that fix an improper command completion wait.
This patch apply on top of for-james branch 
tpm/tpm_infineon: Use struct dev_pm_ops for power management 6b37729bd184fdd44f144c6cc4951b06b55bcf4b

*** Note: This bug is also present in kernel 4.0 but will not apply as the driver structure changed between
kernel 4.0 and kernel 4.1.

A backported version is also going to be sent to <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> ?

Best Regards
Christophe

Christophe Ricard (1):
  tpm/st33zp24: Add proper wait for ordinal duration in case of irq mode

 drivers/char/tpm/st33zp24/st33zp24.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] tpm/st33zp24: Add proper wait for ordinal duration in case of irq mode

[Christophe Ricard]

In case the driver is configured to use irq, we are not waiting the command
command for a duration period to see the DATA_AVAIL status bit to raise but
at maximum timeout_c. This may result in critical failure as we will
not wait long enough for command completion.

Signed-off-by: Christophe Ricard <christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
---
 drivers/char/tpm/st33zp24/st33zp24.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
index 03f2543..976ccb5 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -393,7 +393,7 @@ static irqreturn_t tpm_ioserirq_handler(int irq, void *dev_id)
 static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
 			 size_t len)
 {
-	u32 status, i, size;
+	u32 status, i, size, ordinal;
 	int burstcnt = 0;
 	int ret;
 	u8 data;
@@ -456,6 +456,17 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
 	if (ret < 0)
 		goto out_err;
 
+	if (chip->vendor.irq) {
+		ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
+
+		if (wait_for_stat(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+				  tpm_calc_ordinal_duration(chip, ordinal),
+				  &chip->vendor.read_queue, true) < 0) {
+			ret = -ETIME;
+			goto out_err;
+		}
+	}
+
 	return len;
 out_err:
 	st33zp24_cancel(chip);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] st33zp24 fix when using irq mode

[Peter Hüwe]

Hi,

great timing :) I just sent the pull request for 4.1 out yesterday :)
But thanks for spotting and fixing it. 
Found bugs are good bugs, and fixed ones are even better.

Am Donnerstag, 19. März 2015, 22:15:08 schrieb Christophe Ricard:
> Hi,
> 
> Please find in this patch a fix that fix an improper command completion
> wait. This patch apply on top of for-james branch
> tpm/tpm_infineon: Use struct dev_pm_ops for power management
> 6b37729bd184fdd44f144c6cc4951b06b55bcf4b
> 
> *** Note: This bug is also present in kernel 4.0 but will not apply as the
> driver structure changed between kernel 4.0 and kernel 4.1.
> 
> A backported version is also going to be sent to <stable-u79uwXL29TasMV2rI37PzA@public.gmane.orgorg> ?

Are kernels < 4.0 also affected or was this code introduced in 4.0 ?

How critical is the mentioned 'critical failure' ? What will happen?
How often does it happen?  

If this thing is big enough and kernels < 4.0 are not affected we can try to 
get it into 4.0 still, so no stable. (but I still need the backport :) )


If it won't hit 4.0 then backport to 4.0.1 is needed,
which will be sent to stable@...
with the upstream commit mentioned.


See 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/stable_kernel_rules.txt

Thanks,
Peter


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] st33zp24 fix when using irq mode

[christophe.ricard]

Hi Peter,

Basically if running a TPM command with a long enough duration (> 
timeout_c), the answer will never be read correctly.
This only appear if using irq mode.

I am actually doing the backport to james tree at the moment and will 
send it later on tonight.

I will send this directly without including stable@...

Best Regards
Christophe

On 19/03/2015 22:34, Peter Hüwe wrote:
> Hi,
>
> great timing :) I just sent the pull request for 4.1 out yesterday :)
> But thanks for spotting and fixing it.
> Found bugs are good bugs, and fixed ones are even better.
>
> Am Donnerstag, 19. März 2015, 22:15:08 schrieb Christophe Ricard:
>> Hi,
>>
>> Please find in this patch a fix that fix an improper command completion
>> wait. This patch apply on top of for-james branch
>> tpm/tpm_infineon: Use struct dev_pm_ops for power management
>> 6b37729bd184fdd44f144c6cc4951b06b55bcf4b
>>
>> *** Note: This bug is also present in kernel 4.0 but will not apply as the
>> driver structure changed between kernel 4.0 and kernel 4.1.
>>
>> A backported version is also going to be sent to <stable-u79uwXL29TZUIDd8j+nm9g@public.gmane.org.org> ?
> Are kernels < 4.0 also affected or was this code introduced in 4.0 ?
>
> How critical is the mentioned 'critical failure' ? What will happen?
> How often does it happen?
>
> If this thing is big enough and kernels < 4.0 are not affected we can try to
> get it into 4.0 still, so no stable. (but I still need the backport :) )
>
>
> If it won't hit 4.0 then backport to 4.0.1 is needed,
> which will be sent to stable@...
> with the upstream commit mentioned.
>
>
> See
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/stable_kernel_rules.txt
>
> Thanks,
> Peter
>
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html