Re: [PATCH] Fix Atmel TPM crash caused by too frequent queries

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Mon, Sep 28, 2020 at 03:11:39PM -0700, James Bottomley wrote:
> On Sun, 2020-09-27 at 22:59 -0700, Hao Wu wrote:
> [...]
> > > However, there is another possibility: it's something to do with
> > > the byte read; I notice you don't require the same slowdown for the
> > > burst count read, which actually reads the status register and
> > > burst count as a read32.  If that really is the case, for the atmel
> > > would substituting a read32 and just throwing the upper bytes away
> > > in tpm_tis_status() allow us to keep the current timings?  I can
> > > actually try doing this and see if it fixes my nuvoton.
> > 
> > If would be helpful if you can find the solution without reducing
> > performance. I think it is a separate problem to address though.
> > Maybe not worth to mix them in the same fix.
> 
> Well, if it works, no other fix is needed.
> 
> This is what I'm currently trying out on my nuvoton with the timings
> reverted to being those in the vanilla kernel.  So far it hasn't
> crashed, but I haven't run it for long enough to be sure yet.
> 
> James

OK, so the bus does not like one byte reads but prefers full (32-bit)
word reads? I.e. what's the context?

> ---
> 
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 6b884badabe7..c4dbac8edc9b 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -233,9 +233,9 @@ static u8 tpm_tis_status(struct tpm_chip *chip)
>  {
>  	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>  	int rc;
> -	u8 status;
> +	u32 status;
>  
> -	rc = tpm_tis_read8(priv, TPM_STS(priv->locality), &status);
> +	rc = tpm_tis_read32(priv, TPM_STS(priv->locality), &status);
>  	if (rc < 0)
>  		return 0;
>  
> 

/Jarkko