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]>
