On Thu, Mar 27, 2025 at 03:23:39PM +0200, Jarkko Sakkinen wrote:
On Thu, Mar 27, 2025 at 10:58:00AM +0100, Stefano Garzarella wrote:
On Wed, Mar 26, 2025 at 06:18:38PM +0200, Jarkko Sakkinen wrote:
> From: Jarkko Sakkinen <jarkko.sakki...@opinsys.com>
>
> tpm_ftpm_tee does not require chip->status, chip->cancel and
> chip->req_canceled. Make them optional.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakki...@opinsys.com>
> ---
> drivers/char/tpm/tpm-interface.c | 31 ++++++++++++++++++++++++++++---
> drivers/char/tpm/tpm_ftpm_tee.c | 20 --------------------
> 2 files changed, 28 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c
b/drivers/char/tpm/tpm-interface.c
> index f62f7871edbd..10ba47a882d8 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -58,6 +58,30 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip
*chip, u32 ordinal)
> }
> EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
>
> +static void tpm_chip_cancel(struct tpm_chip *chip)
> +{
> + if (!chip->ops->cancel)
> + return;
> +
> + chip->ops->cancel(chip);
> +}
> +
> +static u8 tpm_chip_status(struct tpm_chip *chip)
> +{
> + if (!chip->ops->status)
> + return 0;
> +
> + return chip->ops->status(chip);
> +}
> +
> +static bool tpm_chip_req_canceled(struct tpm_chip *chip, u8 status)
> +{
> + if (!chip->ops->req_canceled)
> + return false;
> +
> + return chip->ops->req_canceled(chip, status);
> +}
> +
> static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t
bufsiz)
> {
> struct tpm_header *header = buf;
> @@ -65,6 +89,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void
*buf, size_t bufsiz)
> ssize_t len = 0;
> u32 count, ordinal;
> unsigned long stop;
> + u8 status;
Why move `status` out of the do/while block?
I'm not a huge fan of stack allocations inside blocks, unless there is
a particular reason to do so.
>
> if (bufsiz < TPM_HEADER_SIZE)
> return -EINVAL;
> @@ -104,12 +129,12 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
void *buf, size_t bufsiz)
>
What about doing an early return avoiding also calling
tpm_calc_ordinal_duration()?
I mean something like this:
rc = 0;
}
- if (chip->flags & TPM_CHIP_FLAG_IRQ)
+ if (!chip->ops->status || chip->flags & TPM_CHIP_FLAG_IRQ)
goto out_recv;
Anyway, those are small things, the patch LGTM and it's a great cleanup
for ftpm and the svsm driver I'm developing!
If you refined send() and had that the sync flag, this would become:
if (chip->flags & (TPM_CHIP_FLAG_IRQ | TPM_CHIP_FLAG_SYNC))
goto out_recv;
Yep, good point!
Reviewed-by: Stefano Garzarella <sgarz...@redhat.com>
Thank you.
You're welcome!
Stefano
