[PATCH] tpm: display message when using gpio-reset instead of when missing it

[Tim Harvey]

On Wed, Mar 27, 2024 at 7:44 AM Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Tim,
>
> On Thu, 21 Mar 2024 at 20:02, Tim Harvey <tharvey at gateworks.com> wrote:
> >
> > Instead of displaying what looks like an error message if a
> > gpio-reset dt prop is missing for a TPM dipslay a more
> > informative message about resetting the TPM if the gpio is found:
> >
> > before:
> > tpm_tis_spi_probe: missing reset GPIO
> >
> > after:
> > tpm at 0: performing 1ms reset on gpio at 30210000:12
> >
> > Note that the reset dt binding prop used in this driver is not
> > dt-compliant; it does not exist in the Linux dt-bindings documentation
> > and the reset is not done by the Linux driver.
>
> Probably for a good reason. You aren't supposed to be able to reset a
> TPM without resetting the CPUI as well no?

Hi Ilias,

Could you clarify what you know about TPM reset? We use the ATTPM20P
[1] which states in the datasheet under the reset pin: "To be
compliant with TCG requirements, this pin needs to be tied to system
reset. TPM_Init is indicated by asserting this pin."

Our boards have a resistor loading option which routes the TPM RST# to
an SoC GPIO or alternately to a POR# (hardware power on reset provided
by power supply and/or PMIC). Could you point me to where in the spec
it explains what the TPM reset should be connected to?

> That being said, printing that the TPM was reset is pointless imho.
> OTOH the existing error message at least points out a potential
> problem in the DT.
>

I'm not sure if you are NAK'ing this patch or asking me to change it.

Displaying a 'missing GPIO' message is not helpful when there is no
GPIO in the dt bindings to begin with.

Best Regards,

Tim
[1] https://ww1.microchip.com/downloads/en/DeviceDoc/ATTPM20P-Trusted-Platform-Module-TPM-2.0-SPI-Interface-Summary-Data-Sheet-DS40002082A.pdf


> Thanks
> /Ilias
> >
> > Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> > ---
> >  drivers/tpm/tpm2_tis_spi.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c
> > index de9cf8f21e07..944540f7a711 100644
> > --- a/drivers/tpm/tpm2_tis_spi.c
> > +++ b/drivers/tpm/tpm2_tis_spi.c
> > @@ -237,14 +237,14 @@ static int tpm_tis_spi_probe(struct udevice *dev)
> >                         /* legacy reset */
> >                         ret = gpio_request_by_name(dev, "gpio-reset", 0,
> >                                                    &reset_gpio, GPIOD_IS_OUT);
> > -                       if (ret) {
> > -                               log(LOGC_NONE, LOGL_NOTICE,
> > -                                   "%s: missing reset GPIO\n", __func__);
> > +                       if (ret)
> >                                 goto init;
> > -                       }
> >                         log(LOGC_NONE, LOGL_NOTICE,
> >                             "%s: gpio-reset is deprecated\n", __func__);
> >                 }
> > +               log(LOGC_NONE, LOGL_NOTICE,
> > +                   "%s: performing 1ms reset on %s:%d\n", dev->name,
> > +                   reset_gpio.dev->name, reset_gpio.offset);
> >                 dm_gpio_set_value(&reset_gpio, 1);
> >                 mdelay(1);
> >                 dm_gpio_set_value(&reset_gpio, 0);
> > --
> > 2.25.1
> >