Re: [PATCH] tpm_crb: Use the common ACPI definition of struct acpi_tpm2

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Wed, Dec 02, 2015 at 12:19:00PM -0700, Jason Gunthorpe wrote:
> include/acpi/actbl2.h is the proper place for these definitions
> and the needed TPM2 ones have been there since
> commit 413d4a6defe0 ("ACPICA: Update TPM2 ACPI table")
> 
> This also drops the le32_to_cpu for members of this table,
> consistent with other ACPI stuff.

Why le32_to_cpu() is not needed?

/Jarkko

> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/char/tpm/tpm.h     |  7 -------
>  drivers/char/tpm/tpm_crb.c | 24 ++++++++----------------
>  drivers/char/tpm/tpm_tis.c |  2 +-
>  3 files changed, 9 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 542a80cbfd9c..28b477e8da6a 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -128,13 +128,6 @@ enum tpm2_startup_types {
>  	TPM2_SU_STATE	= 0x0001,
>  };
>  
> -enum tpm2_start_method {
> -	TPM2_START_ACPI = 2,
> -	TPM2_START_FIFO = 6,
> -	TPM2_START_CRB = 7,
> -	TPM2_START_CRB_WITH_ACPI = 8,
> -};
> -
>  struct tpm_chip;
>  
>  struct tpm_vendor_specific {
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 8342cf51ffdc..5b74ad8ca7fd 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -34,14 +34,6 @@ enum crb_defaults {
>  	CRB_ACPI_START_INDEX = 1,
>  };
>  
> -struct acpi_tpm2 {
> -	struct acpi_table_header hdr;
> -	u16 platform_class;
> -	u16 reserved;
> -	u64 control_area_pa;
> -	u32 start_method;
> -} __packed;
> -
>  enum crb_ca_request {
>  	CRB_CA_REQ_GO_IDLE	= BIT(0),
>  	CRB_CA_REQ_CMD_READY	= BIT(1),
> @@ -207,7 +199,7 @@ static const struct tpm_class_ops tpm_crb = {
>  static int crb_acpi_add(struct acpi_device *device)
>  {
>  	struct tpm_chip *chip;
> -	struct acpi_tpm2 *buf;
> +	struct acpi_table_tpm2 *buf;
>  	struct crb_priv *priv;
>  	struct device *dev = &device->dev;
>  	acpi_status status;
> @@ -223,7 +215,8 @@ static int crb_acpi_add(struct acpi_device *device)
>  	}
>  
>  	/* Should the FIFO driver handle this? */
> -	if (buf->start_method == TPM2_START_FIFO)
> +	sm = buf->start_method;
> +	if (sm == ACPI_TPM2_MEMORY_MAPPED)
>  		return -ENODEV;
>  
>  	chip = tpmm_chip_alloc(dev, &tpm_crb);
> @@ -232,7 +225,7 @@ static int crb_acpi_add(struct acpi_device *device)
>  
>  	chip->flags = TPM_CHIP_FLAG_TPM2;
>  
> -	if (buf->hdr.length < sizeof(struct acpi_tpm2)) {
> +	if (buf->header.length < sizeof(*buf)) {
>  		dev_err(dev, "TPM2 ACPI table has wrong size");
>  		return -EINVAL;
>  	}
> @@ -244,21 +237,20 @@ static int crb_acpi_add(struct acpi_device *device)
>  		return -ENOMEM;
>  	}
>  
> -	sm = le32_to_cpu(buf->start_method);
> -
>  	/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
>  	 * report only ACPI start but in practice seems to require both
>  	 * ACPI start and CRB start.
>  	 */
> -	if (sm == TPM2_START_CRB || sm == TPM2_START_FIFO ||
> +	if (sm == ACPI_TPM2_COMMAND_BUFFER || sm == ACPI_TPM2_MEMORY_MAPPED ||
>  	    !strcmp(acpi_device_hid(device), "MSFT0101"))
>  		priv->flags |= CRB_FL_CRB_START;
>  
> -	if (sm == TPM2_START_ACPI || sm == TPM2_START_CRB_WITH_ACPI)
> +	if (sm == ACPI_TPM2_START_METHOD ||
> +	    sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
>  		priv->flags |= CRB_FL_ACPI_START;
>  
>  	priv->cca = (struct crb_control_area __iomem *)
> -		devm_ioremap_nocache(dev, buf->control_area_pa, 0x1000);
> +		devm_ioremap_nocache(dev, buf->control_address, 0x1000);
>  	if (!priv->cca) {
>  		dev_err(dev, "ioremap of the control area failed\n");
>  		return -ENOMEM;
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 8a3509cb10da..304323bdcaaa 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -135,7 +135,7 @@ static inline int is_fifo(struct acpi_device *dev)
>  		return 0;
>  	}
>  
> -	if (le32_to_cpu(tbl->start_method) != TPM2_START_FIFO)
> +	if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
>  		return 0;
>  
>  	/* TPM 2.0 FIFO */
> -- 
> 2.1.4
>