Hi,

On Mon, 15 Jul 2024 at 12:23, <lukas.funke-...@weidmueller.com> wrote:
>
> From: Lukas Funke <lukas.fu...@weidmueller.com>
>
> tpm_tis_wait_init() is using the 'chip->timeout_b' field which is
> initialized in tpm_tis_init(). However, the init-function is called
> *after* tpm_tis_wait_init() introducing an uninitalized field access.
>
> This commit switches both routines.
>
> Signed-off-by: Lukas Funke <lukas.fu...@weidmueller.com>
> ---
>
>  drivers/tpm/tpm2_tis_spi.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c
> index b0fe97ab1d0..5a4dbfd3ccb 100644
> --- a/drivers/tpm/tpm2_tis_spi.c
> +++ b/drivers/tpm/tpm2_tis_spi.c
> @@ -256,17 +256,17 @@ static int tpm_tis_spi_probe(struct udevice *dev)
>         /* Ensure a minimum amount of time elapsed since reset of the TPM */
>         mdelay(drv_data->time_before_first_cmd_ms);
>
> +       tpm_tis_ops_register(dev, &phy_ops);
> +       ret = tpm_tis_init(dev);
> +       if (ret)
> +               goto err;
> +
>         ret = tpm_tis_wait_init(dev, chip->locality);
>         if (ret) {
>                 log(LOGC_DM, LOGL_ERR, "%s: no device found\n", __func__);
>                 return ret;
>         }
>
> -       tpm_tis_ops_register(dev, &phy_ops);
> -       ret = tpm_tis_init(dev);
> -       if (ret)
> -               goto err;
> -
>         priv->pcr_count = drv_data->pcr_count;
>         priv->pcr_select_min = drv_data->pcr_select_min;
>         priv->version = TPM_V2;
> --
> 2.30.2
>

This needs a Fixes tag for a5c30c26b28 (HEAD) tpm: Use the new API on
tpm2 spi driver

The old code set up the timeouts first, then did the wait_init.
Presumably the point of wait_init is to wait before doing the init, so
we should try to keep that behaviour, unless it is actually wrong.

So my thought would be to move the setup of the required timeout into
tpm_tis_ops_register(), instead.

Regards,
Simon

Reply via email to