On Wed, 24 Jul 2024 at 02:56, Ilias Apalodimas <[email protected]> wrote: > > Thanks Lukas, > > On Wed, 24 Jul 2024 at 10:35, <[email protected]> wrote: > > > > From: Lukas Funke <[email protected]> > > > > 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 <[email protected]> > > Acked-by: Miquel Raynal <[email protected]> > > --- > > > > Changes in v2: > > - Call tpm_tis_wait_init() from tpm_tis_init() > > - Use phy_ops for bus access in tpm_tis_wait_init() > > > > drivers/tpm/tpm2_tis_core.c | 28 ++++++++++++++++++++++++++++ > > drivers/tpm/tpm2_tis_spi.c | 29 ----------------------------- > > 2 files changed, 28 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/tpm/tpm2_tis_core.c b/drivers/tpm/tpm2_tis_core.c > > index 680a6409433..1fdf8cfa319 100644 > > --- a/drivers/tpm/tpm2_tis_core.c > > +++ b/drivers/tpm/tpm2_tis_core.c > > @@ -419,6 +419,28 @@ static bool tis_check_ops(struct tpm_tis_phy_ops > > *phy_ops) > > return true; > > } > > > > +static int tpm_tis_wait_init(struct udevice *dev, int loc) > > +{ > > + struct tpm_chip *chip = dev_get_priv(dev); > > + unsigned long start, stop; > > + u8 status; > > + int ret; > > + > > + start = get_timer(0); > > + stop = chip->timeout_b; > > + do { > > + mdelay(TPM_TIMEOUT_MS); > > + ret = chip->phy_ops->read_bytes(dev, TPM_ACCESS(loc), 1, > > &status); > > + if (ret) > > + break; > > + > > + if (status & TPM_ACCESS_VALID) > > + return 0; > > + } while (get_timer(start) < stop); > > + > > + return -EIO; > > +} > > + > > int tpm_tis_init(struct udevice *dev) > > { > > struct tpm_chip *chip = dev_get_priv(dev); > > @@ -436,6 +458,12 @@ int tpm_tis_init(struct udevice *dev) > > chip->timeout_c = TIS_SHORT_TIMEOUT_MS; > > chip->timeout_d = TIS_SHORT_TIMEOUT_MS; > > > > + ret = tpm_tis_wait_init(dev, chip->locality); > > + if (ret) { > > + log(LOGC_DM, LOGL_ERR, "%s: no device found\n", __func__); > > + return ret; > > + } > > + > > ret = tpm_tis_request_locality(dev, 0); > > if (ret) > > return ret; > > diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c > > index b0fe97ab1d0..7909a147c2d 100644 > > --- a/drivers/tpm/tpm2_tis_spi.c > > +++ b/drivers/tpm/tpm2_tis_spi.c > > @@ -188,29 +188,6 @@ static int tpm_tis_spi_write32(struct udevice *dev, > > u32 addr, u32 value) > > return tpm_tis_spi_write(dev, addr, sizeof(value), (u8 *)&value_le); > > } > > > > -static int tpm_tis_wait_init(struct udevice *dev, int loc) > > -{ > > - struct tpm_chip *chip = dev_get_priv(dev); > > - unsigned long start, stop; > > - u8 status; > > - int ret; > > - > > - start = get_timer(0); > > - stop = chip->timeout_b; > > - do { > > - mdelay(TPM_TIMEOUT_MS); > > - > > - ret = tpm_tis_spi_read(dev, TPM_ACCESS(loc), 1, &status); > > - if (ret) > > - break; > > - > > - if (status & TPM_ACCESS_VALID) > > - return 0; > > - } while (get_timer(start) < stop); > > - > > - return -EIO; > > -} > > - > > static struct tpm_tis_phy_ops phy_ops = { > > .read_bytes = tpm_tis_spi_read, > > .write_bytes = tpm_tis_spi_write, > > @@ -256,12 +233,6 @@ 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); > > > > - 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) > > -- > > 2.30.2 > > > > I'll manually add the fixes tag on merge. Other than that > Reviewed-by: Ilias Apalodimas <[email protected]>
Reviewed-by: Simon Glass <[email protected]> Fixes: a5c30c26b28 ("tpm: Use the new API on tpm2 spi driver")
