Re: [PATCH] tpm_tis: Check for an error after exhausting send retires

[Jarkko Sakkinen <jarkko@kernel.org>]

On Fri, Apr 10, 2026 at 09:13:50PM +0000, Jacqueline Wong wrote:
> From: Jordan Hand <jhand@google.com>
> 
> tpm_tis_send_main() will attempt to retry sending data TPM_RETRY times.
> Currently, if those retries are exhausted, the driver will attempt to
> call execute. The TPM will be in the wrong state, leading to the
> operation simply timing out.
> 
> Instead, if there is still an error after retries are exhausted, return
> that error immediately.
> 
> Additionally, add logging to more easily determine reason for transmit
> failure.
> 
> Fixes: 280db21e153d8 ("tpm_tis: Resend command to recover from data transfer errors")
> Signed-off-by: Jordan Hand <jhand@google.com>
> ---
>  drivers/char/tpm/tpm_tis_core.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index e2a1769081b1..b78937841879 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -471,6 +471,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
>  		status = tpm_tis_status(chip);
>  		if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
>  			rc = -EIO;
> +			dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be set. sts = 0x%08x\n",
> +					status);
>  			goto out_err;
>  		}
>  	}
> @@ -491,6 +493,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
>  	status = tpm_tis_status(chip);
>  	if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) {
>  		rc = -EIO;
> +		dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be unset. sts = 0x%08x\n",
> +				status);
>  		goto out_err;
>  	}
>  
> @@ -552,11 +556,16 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
>  			break;
>  		else if (rc != -EAGAIN && rc != -EIO)
>  			/* Data transfer failed, not recoverable */
> -			return rc;
> +			goto out_err;
>  
>  		usleep_range(priv->timeout_min, priv->timeout_max);
>  	}
>  
> +	if (rc == -EAGAIN || rc == -EIO) {
> +		dev_err(&chip->dev, "Exhausted tpm_tis_send_data retries\n");
> +		goto out_err;
> +	}
> +
>  	/* go and do it */
>  	rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);
>  	if (rc < 0)
> -- 
> 2.53.0.1213.gd9a14994de-goog
> 

CHECK: Alignment should match open parenthesis
#35: FILE: drivers/char/tpm/tpm_tis_core.c:475:
+                       dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be set. sts = 0x%08x\n",
+                                       status);

CHECK: Alignment should match open parenthesis
#44: FILE: drivers/char/tpm/tpm_tis_core.c:497:
+               dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be unset. sts = 0x%08x\n",
+                               status);


I fixed these manually.

Good catch, thanks:

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Applied.

BR, Jarkko