Re: [PATCH v2] tpm: Add flag to use default cancellation policy

From: Joel Stanley
Date: Sun Nov 13 2022 - 22:18:18 EST


On Mon, 7 Nov 2022 at 17:14, Eddie James <eajames@xxxxxxxxxxxxx> wrote:
>
> The check for cancelled request depends on the VID of the chip, but
> some chips share VID which shouldn't share their cancellation
> behavior. This is the case for the Nuvoton NPCT75X, which should use
> the default cancellation check, not the Winbond one.
> To avoid changing the existing behavior, add a new flag to indicate
> that the chip should use the default cancellation check and set it
> for the I2C TPM2 TIS driver.
>
> Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>

Tested-by: Joel Stanley <joel@xxxxxxxxx>
Fixes: bbc23a07b072 ("tpm: Add tpm_tis_i2c backend for tpm_tis_core")

With this patch, and "tpm: tis_i2c: Fix sanity check interrupt enable
mask", and the extra compatible strings the driver correctly probes on
boot with the Nuvoton part.


> ---
> Changes since v1:
> - Update switch statement with default: break;
>
> drivers/char/tpm/tpm_tis_core.c | 20 ++++++++++++--------
> drivers/char/tpm/tpm_tis_core.h | 1 +
> drivers/char/tpm/tpm_tis_i2c.c | 1 +
> 3 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 757623bacfd5..3f98e587b3e8 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -682,15 +682,19 @@ static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
> {
> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>
> - switch (priv->manufacturer_id) {
> - case TPM_VID_WINBOND:
> - return ((status == TPM_STS_VALID) ||
> - (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
> - case TPM_VID_STM:
> - return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY));
> - default:
> - return (status == TPM_STS_COMMAND_READY);
> + if (!test_bit(TPM_TIS_DEFAULT_CANCELLATION, &priv->flags)) {
> + switch (priv->manufacturer_id) {
> + case TPM_VID_WINBOND:
> + return ((status == TPM_STS_VALID) ||
> + (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
> + case TPM_VID_STM:
> + return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY));
> + default:
> + break;
> + }
> }
> +
> + return status == TPM_STS_COMMAND_READY;
> }
>
> static irqreturn_t tis_int_handler(int dummy, void *dev_id)
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 66a5a13cd1df..b68479e0de10 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -86,6 +86,7 @@ enum tis_defaults {
> enum tpm_tis_flags {
> TPM_TIS_ITPM_WORKAROUND = BIT(0),
> TPM_TIS_INVALID_STATUS = BIT(1),
> + TPM_TIS_DEFAULT_CANCELLATION = BIT(2),
> };
>
> struct tpm_tis_data {
> diff --git a/drivers/char/tpm/tpm_tis_i2c.c b/drivers/char/tpm/tpm_tis_i2c.c
> index 45f388127f4b..91451ee1ef8d 100644
> --- a/drivers/char/tpm/tpm_tis_i2c.c
> +++ b/drivers/char/tpm/tpm_tis_i2c.c
> @@ -329,6 +329,7 @@ static int tpm_tis_i2c_probe(struct i2c_client *dev,
> if (!phy->io_buf)
> return -ENOMEM;
>
> + set_bit(TPM_TIS_DEFAULT_CANCELLATION, &phy->priv.flags);
> phy->i2c_client = dev;
>
> /* must precede all communication with the tpm */
> --
> 2.31.1
>