* [PATCH v4 1/7] tpm_crb: Use the common ACPI definition of struct acpi_tpm2
2016-01-08 0:36 ` Jason Gunthorpe
@ 2016-01-08 0:36 ` Jason Gunthorpe
-1 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
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 a couple of le32_to_cpu's for members of this table,
the existing swapping was not done consistently, and the standard
used by other Linux callers of acpi_get_table is unswapped.
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Tested-by: Wilck, Martin <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/char/tpm/tpm.h | 7 -------
drivers/char/tpm/tpm_crb.c | 31 +++++++++----------------------
drivers/char/tpm/tpm_tis.c | 2 +-
3 files changed, 10 insertions(+), 30 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..8dd70696ebe8 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;
@@ -217,13 +209,14 @@ static int crb_acpi_add(struct acpi_device *device)
status = acpi_get_table(ACPI_SIG_TPM2, 1,
(struct acpi_table_header **) &buf);
- if (ACPI_FAILURE(status)) {
- dev_err(dev, "failed to get TPM2 ACPI table\n");
+ if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
+ dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
return -ENODEV;
}
/* 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,11 +225,6 @@ static int crb_acpi_add(struct acpi_device *device)
chip->flags = TPM_CHIP_FLAG_TPM2;
- if (buf->hdr.length < sizeof(struct acpi_tpm2)) {
- dev_err(dev, "TPM2 ACPI table has wrong size");
- return -EINVAL;
- }
-
priv = (struct crb_priv *) devm_kzalloc(dev, sizeof(struct crb_priv),
GFP_KERNEL);
if (!priv) {
@@ -244,21 +232,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
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 1/7] tpm_crb: Use the common ACPI definition of struct acpi_tpm2
@ 2016-01-08 0:36 ` Jason Gunthorpe
0 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel, linux-kernel; +Cc: Peter Huewe
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 a couple of le32_to_cpu's for members of this table,
the existing swapping was not done consistently, and the standard
used by other Linux callers of acpi_get_table is unswapped.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Tested-by: Wilck, Martin <martin.wilck@ts.fujitsu.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm.h | 7 -------
drivers/char/tpm/tpm_crb.c | 31 +++++++++----------------------
drivers/char/tpm/tpm_tis.c | 2 +-
3 files changed, 10 insertions(+), 30 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..8dd70696ebe8 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;
@@ -217,13 +209,14 @@ static int crb_acpi_add(struct acpi_device *device)
status = acpi_get_table(ACPI_SIG_TPM2, 1,
(struct acpi_table_header **) &buf);
- if (ACPI_FAILURE(status)) {
- dev_err(dev, "failed to get TPM2 ACPI table\n");
+ if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
+ dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
return -ENODEV;
}
/* 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,11 +225,6 @@ static int crb_acpi_add(struct acpi_device *device)
chip->flags = TPM_CHIP_FLAG_TPM2;
- if (buf->hdr.length < sizeof(struct acpi_tpm2)) {
- dev_err(dev, "TPM2 ACPI table has wrong size");
- return -EINVAL;
- }
-
priv = (struct crb_priv *) devm_kzalloc(dev, sizeof(struct crb_priv),
GFP_KERNEL);
if (!priv) {
@@ -244,21 +232,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
^ permalink raw reply related [flat|nested] 29+ messages in thread[parent not found: <1452213386-21460-2-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [PATCH v4 1/7] tpm_crb: Use the common ACPI definition of struct acpi_tpm2
2016-01-08 0:36 ` Jason Gunthorpe
@ 2016-01-08 12:01 ` Jarkko Sakkinen
-1 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2016-01-08 12:01 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thu, Jan 07, 2016 at 05:36:20PM -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 a couple of le32_to_cpu's for members of this table,
> the existing swapping was not done consistently, and the standard
> used by other Linux callers of acpi_get_table is unswapped.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Tested-by: Wilck, Martin <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
PS. I'll update the commit message a bit. Thanks.
/Jarkko
> ---
> drivers/char/tpm/tpm.h | 7 -------
> drivers/char/tpm/tpm_crb.c | 31 +++++++++----------------------
> drivers/char/tpm/tpm_tis.c | 2 +-
> 3 files changed, 10 insertions(+), 30 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..8dd70696ebe8 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;
> @@ -217,13 +209,14 @@ static int crb_acpi_add(struct acpi_device *device)
>
> status = acpi_get_table(ACPI_SIG_TPM2, 1,
> (struct acpi_table_header **) &buf);
> - if (ACPI_FAILURE(status)) {
> - dev_err(dev, "failed to get TPM2 ACPI table\n");
> + if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
> + dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
> return -ENODEV;
> }
>
> /* 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,11 +225,6 @@ static int crb_acpi_add(struct acpi_device *device)
>
> chip->flags = TPM_CHIP_FLAG_TPM2;
>
> - if (buf->hdr.length < sizeof(struct acpi_tpm2)) {
> - dev_err(dev, "TPM2 ACPI table has wrong size");
> - return -EINVAL;
> - }
> -
> priv = (struct crb_priv *) devm_kzalloc(dev, sizeof(struct crb_priv),
> GFP_KERNEL);
> if (!priv) {
> @@ -244,21 +232,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
>
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 1/7] tpm_crb: Use the common ACPI definition of struct acpi_tpm2
@ 2016-01-08 12:01 ` Jarkko Sakkinen
0 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2016-01-08 12:01 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: tpmdd-devel, linux-kernel, Peter Huewe
On Thu, Jan 07, 2016 at 05:36:20PM -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 a couple of le32_to_cpu's for members of this table,
> the existing swapping was not done consistently, and the standard
> used by other Linux callers of acpi_get_table is unswapped.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Tested-by: Wilck, Martin <martin.wilck@ts.fujitsu.com>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
PS. I'll update the commit message a bit. Thanks.
/Jarkko
> ---
> drivers/char/tpm/tpm.h | 7 -------
> drivers/char/tpm/tpm_crb.c | 31 +++++++++----------------------
> drivers/char/tpm/tpm_tis.c | 2 +-
> 3 files changed, 10 insertions(+), 30 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..8dd70696ebe8 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;
> @@ -217,13 +209,14 @@ static int crb_acpi_add(struct acpi_device *device)
>
> status = acpi_get_table(ACPI_SIG_TPM2, 1,
> (struct acpi_table_header **) &buf);
> - if (ACPI_FAILURE(status)) {
> - dev_err(dev, "failed to get TPM2 ACPI table\n");
> + if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
> + dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
> return -ENODEV;
> }
>
> /* 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,11 +225,6 @@ static int crb_acpi_add(struct acpi_device *device)
>
> chip->flags = TPM_CHIP_FLAG_TPM2;
>
> - if (buf->hdr.length < sizeof(struct acpi_tpm2)) {
> - dev_err(dev, "TPM2 ACPI table has wrong size");
> - return -EINVAL;
> - }
> -
> priv = (struct crb_priv *) devm_kzalloc(dev, sizeof(struct crb_priv),
> GFP_KERNEL);
> if (!priv) {
> @@ -244,21 +232,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
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 2/7] tpm_tis: Disable interrupt auto probing on a per-device basis
2016-01-08 0:36 ` Jason Gunthorpe
@ 2016-01-08 0:36 ` Jason Gunthorpe
-1 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Instead of clearing the global interrupts flag when any device
does not have an interrupt just pass -1 through tpm_info.irq.
The only thing that asks for autoprobing is the force=1 path.
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Tested-by: Wilck, Martin <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/char/tpm/tpm_tis.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 304323bdcaaa..fecd27b45fd1 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -69,7 +69,11 @@ enum tis_defaults {
struct tpm_info {
unsigned long start;
unsigned long len;
- unsigned int irq;
+ /* irq > 0 means: use irq $irq;
+ * irq = 0 means: autoprobe for an irq;
+ * irq = -1 means: no irq support
+ */
+ int irq;
};
static struct tpm_info tis_default_info = {
@@ -807,7 +811,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
/* INTERRUPT Setup */
init_waitqueue_head(&chip->vendor.read_queue);
init_waitqueue_head(&chip->vendor.int_queue);
- if (interrupts) {
+ if (interrupts && tpm_info->irq != -1) {
if (tpm_info->irq) {
tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED,
tpm_info->irq);
@@ -895,9 +899,9 @@ static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
#ifdef CONFIG_PNP
static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
- const struct pnp_device_id *pnp_id)
+ const struct pnp_device_id *pnp_id)
{
- struct tpm_info tpm_info = tis_default_info;
+ struct tpm_info tpm_info = {};
acpi_handle acpi_dev_handle = NULL;
tpm_info.start = pnp_mem_start(pnp_dev, 0);
@@ -906,7 +910,7 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
if (pnp_irq_valid(pnp_dev, 0))
tpm_info.irq = pnp_irq(pnp_dev, 0);
else
- interrupts = false;
+ tpm_info.irq = -1;
#ifdef CONFIG_ACPI
if (pnp_acpi_device(pnp_dev)) {
@@ -984,6 +988,7 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
return -ENODEV;
INIT_LIST_HEAD(&resources);
+ tpm_info.irq = -1;
ret = acpi_dev_get_resources(acpi_dev, &resources, tpm_check_resource,
&tpm_info);
if (ret < 0)
@@ -991,9 +996,6 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
acpi_dev_free_resource_list(&resources);
- if (!tpm_info.irq)
- interrupts = false;
-
if (is_itpm(acpi_dev))
itpm = true;
--
2.1.4
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 2/7] tpm_tis: Disable interrupt auto probing on a per-device basis
@ 2016-01-08 0:36 ` Jason Gunthorpe
0 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel, linux-kernel; +Cc: Peter Huewe
Instead of clearing the global interrupts flag when any device
does not have an interrupt just pass -1 through tpm_info.irq.
The only thing that asks for autoprobing is the force=1 path.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Tested-by: Wilck, Martin <martin.wilck@ts.fujitsu.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_tis.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 304323bdcaaa..fecd27b45fd1 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -69,7 +69,11 @@ enum tis_defaults {
struct tpm_info {
unsigned long start;
unsigned long len;
- unsigned int irq;
+ /* irq > 0 means: use irq $irq;
+ * irq = 0 means: autoprobe for an irq;
+ * irq = -1 means: no irq support
+ */
+ int irq;
};
static struct tpm_info tis_default_info = {
@@ -807,7 +811,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
/* INTERRUPT Setup */
init_waitqueue_head(&chip->vendor.read_queue);
init_waitqueue_head(&chip->vendor.int_queue);
- if (interrupts) {
+ if (interrupts && tpm_info->irq != -1) {
if (tpm_info->irq) {
tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED,
tpm_info->irq);
@@ -895,9 +899,9 @@ static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
#ifdef CONFIG_PNP
static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
- const struct pnp_device_id *pnp_id)
+ const struct pnp_device_id *pnp_id)
{
- struct tpm_info tpm_info = tis_default_info;
+ struct tpm_info tpm_info = {};
acpi_handle acpi_dev_handle = NULL;
tpm_info.start = pnp_mem_start(pnp_dev, 0);
@@ -906,7 +910,7 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
if (pnp_irq_valid(pnp_dev, 0))
tpm_info.irq = pnp_irq(pnp_dev, 0);
else
- interrupts = false;
+ tpm_info.irq = -1;
#ifdef CONFIG_ACPI
if (pnp_acpi_device(pnp_dev)) {
@@ -984,6 +988,7 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
return -ENODEV;
INIT_LIST_HEAD(&resources);
+ tpm_info.irq = -1;
ret = acpi_dev_get_resources(acpi_dev, &resources, tpm_check_resource,
&tpm_info);
if (ret < 0)
@@ -991,9 +996,6 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
acpi_dev_free_resource_list(&resources);
- if (!tpm_info.irq)
- interrupts = false;
-
if (is_itpm(acpi_dev))
itpm = true;
--
2.1.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 3/7] tpm_tis: Do not fall back to a hardcoded address for TPM2
2016-01-08 0:36 ` Jason Gunthorpe
@ 2016-01-08 0:36 ` Jason Gunthorpe
-1 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
If the ACPI tables do not declare a memory resource for the TPM2
then do not just fall back to the x86 default base address.
Also be stricter when checking the ancillary TPM2 ACPI data and error
out if any of this data is wrong rather than blindly assuming TPM1.
Fixes: 399235dc6e95 ("tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0")
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Tested-by: Wilck, Martin <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/char/tpm/tpm_tis.c | 48 +++++++++++++++++-----------------------------
1 file changed, 18 insertions(+), 30 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index fecd27b45fd1..b2b31f5418ca 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -122,39 +122,11 @@ static inline int is_itpm(struct acpi_device *dev)
{
return has_hid(dev, "INTC0102");
}
-
-static inline int is_fifo(struct acpi_device *dev)
-{
- struct acpi_table_tpm2 *tbl;
- acpi_status st;
-
- /* TPM 1.2 FIFO */
- if (!has_hid(dev, "MSFT0101"))
- return 1;
-
- st = acpi_get_table(ACPI_SIG_TPM2, 1,
- (struct acpi_table_header **) &tbl);
- if (ACPI_FAILURE(st)) {
- dev_err(&dev->dev, "failed to get TPM2 ACPI table\n");
- return 0;
- }
-
- if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
- return 0;
-
- /* TPM 2.0 FIFO */
- return 1;
-}
#else
static inline int is_itpm(struct acpi_device *dev)
{
return 0;
}
-
-static inline int is_fifo(struct acpi_device *dev)
-{
- return 1;
-}
#endif
/* Before we attempt to access the TPM we must see that the valid bit is set.
@@ -980,11 +952,21 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data)
static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
{
+ struct acpi_table_tpm2 *tbl;
+ acpi_status st;
struct list_head resources;
- struct tpm_info tpm_info = tis_default_info;
+ struct tpm_info tpm_info = {};
int ret;
- if (!is_fifo(acpi_dev))
+ st = acpi_get_table(ACPI_SIG_TPM2, 1,
+ (struct acpi_table_header **) &tbl);
+ if (ACPI_FAILURE(st) || tbl->header.length < sizeof(*tbl)) {
+ dev_err(&acpi_dev->dev,
+ FW_BUG "failed to get TPM2 ACPI table\n");
+ return -EINVAL;
+ }
+
+ if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
return -ENODEV;
INIT_LIST_HEAD(&resources);
@@ -996,6 +978,12 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
acpi_dev_free_resource_list(&resources);
+ if (tpm_info.start == 0 && tpm_info.len == 0) {
+ dev_err(&acpi_dev->dev,
+ FW_BUG "TPM2 ACPI table does not define a memory resource\n");
+ return -EINVAL;
+ }
+
if (is_itpm(acpi_dev))
itpm = true;
--
2.1.4
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 3/7] tpm_tis: Do not fall back to a hardcoded address for TPM2
@ 2016-01-08 0:36 ` Jason Gunthorpe
0 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel, linux-kernel; +Cc: Peter Huewe
If the ACPI tables do not declare a memory resource for the TPM2
then do not just fall back to the x86 default base address.
Also be stricter when checking the ancillary TPM2 ACPI data and error
out if any of this data is wrong rather than blindly assuming TPM1.
Fixes: 399235dc6e95 ("tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0")
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Tested-by: Wilck, Martin <martin.wilck@ts.fujitsu.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_tis.c | 48 +++++++++++++++++-----------------------------
1 file changed, 18 insertions(+), 30 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index fecd27b45fd1..b2b31f5418ca 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -122,39 +122,11 @@ static inline int is_itpm(struct acpi_device *dev)
{
return has_hid(dev, "INTC0102");
}
-
-static inline int is_fifo(struct acpi_device *dev)
-{
- struct acpi_table_tpm2 *tbl;
- acpi_status st;
-
- /* TPM 1.2 FIFO */
- if (!has_hid(dev, "MSFT0101"))
- return 1;
-
- st = acpi_get_table(ACPI_SIG_TPM2, 1,
- (struct acpi_table_header **) &tbl);
- if (ACPI_FAILURE(st)) {
- dev_err(&dev->dev, "failed to get TPM2 ACPI table\n");
- return 0;
- }
-
- if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
- return 0;
-
- /* TPM 2.0 FIFO */
- return 1;
-}
#else
static inline int is_itpm(struct acpi_device *dev)
{
return 0;
}
-
-static inline int is_fifo(struct acpi_device *dev)
-{
- return 1;
-}
#endif
/* Before we attempt to access the TPM we must see that the valid bit is set.
@@ -980,11 +952,21 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data)
static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
{
+ struct acpi_table_tpm2 *tbl;
+ acpi_status st;
struct list_head resources;
- struct tpm_info tpm_info = tis_default_info;
+ struct tpm_info tpm_info = {};
int ret;
- if (!is_fifo(acpi_dev))
+ st = acpi_get_table(ACPI_SIG_TPM2, 1,
+ (struct acpi_table_header **) &tbl);
+ if (ACPI_FAILURE(st) || tbl->header.length < sizeof(*tbl)) {
+ dev_err(&acpi_dev->dev,
+ FW_BUG "failed to get TPM2 ACPI table\n");
+ return -EINVAL;
+ }
+
+ if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
return -ENODEV;
INIT_LIST_HEAD(&resources);
@@ -996,6 +978,12 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
acpi_dev_free_resource_list(&resources);
+ if (tpm_info.start == 0 && tpm_info.len == 0) {
+ dev_err(&acpi_dev->dev,
+ FW_BUG "TPM2 ACPI table does not define a memory resource\n");
+ return -EINVAL;
+ }
+
if (is_itpm(acpi_dev))
itpm = true;
--
2.1.4
^ permalink raw reply related [flat|nested] 29+ messages in thread[parent not found: <1452213386-21460-4-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [PATCH v4 3/7] tpm_tis: Do not fall back to a hardcoded address for TPM2
2016-01-08 0:36 ` Jason Gunthorpe
@ 2016-01-08 12:01 ` Jarkko Sakkinen
-1 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2016-01-08 12:01 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thu, Jan 07, 2016 at 05:36:22PM -0700, Jason Gunthorpe wrote:
> If the ACPI tables do not declare a memory resource for the TPM2
> then do not just fall back to the x86 default base address.
>
> Also be stricter when checking the ancillary TPM2 ACPI data and error
> out if any of this data is wrong rather than blindly assuming TPM1.
>
> Fixes: 399235dc6e95 ("tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0")
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Tested-by: Wilck, Martin <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
/Jarkko
> ---
> drivers/char/tpm/tpm_tis.c | 48 +++++++++++++++++-----------------------------
> 1 file changed, 18 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index fecd27b45fd1..b2b31f5418ca 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -122,39 +122,11 @@ static inline int is_itpm(struct acpi_device *dev)
> {
> return has_hid(dev, "INTC0102");
> }
> -
> -static inline int is_fifo(struct acpi_device *dev)
> -{
> - struct acpi_table_tpm2 *tbl;
> - acpi_status st;
> -
> - /* TPM 1.2 FIFO */
> - if (!has_hid(dev, "MSFT0101"))
> - return 1;
> -
> - st = acpi_get_table(ACPI_SIG_TPM2, 1,
> - (struct acpi_table_header **) &tbl);
> - if (ACPI_FAILURE(st)) {
> - dev_err(&dev->dev, "failed to get TPM2 ACPI table\n");
> - return 0;
> - }
> -
> - if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
> - return 0;
> -
> - /* TPM 2.0 FIFO */
> - return 1;
> -}
> #else
> static inline int is_itpm(struct acpi_device *dev)
> {
> return 0;
> }
> -
> -static inline int is_fifo(struct acpi_device *dev)
> -{
> - return 1;
> -}
> #endif
>
> /* Before we attempt to access the TPM we must see that the valid bit is set.
> @@ -980,11 +952,21 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data)
>
> static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
> {
> + struct acpi_table_tpm2 *tbl;
> + acpi_status st;
> struct list_head resources;
> - struct tpm_info tpm_info = tis_default_info;
> + struct tpm_info tpm_info = {};
> int ret;
>
> - if (!is_fifo(acpi_dev))
> + st = acpi_get_table(ACPI_SIG_TPM2, 1,
> + (struct acpi_table_header **) &tbl);
> + if (ACPI_FAILURE(st) || tbl->header.length < sizeof(*tbl)) {
> + dev_err(&acpi_dev->dev,
> + FW_BUG "failed to get TPM2 ACPI table\n");
> + return -EINVAL;
> + }
> +
> + if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
> return -ENODEV;
>
> INIT_LIST_HEAD(&resources);
> @@ -996,6 +978,12 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
>
> acpi_dev_free_resource_list(&resources);
>
> + if (tpm_info.start == 0 && tpm_info.len == 0) {
> + dev_err(&acpi_dev->dev,
> + FW_BUG "TPM2 ACPI table does not define a memory resource\n");
> + return -EINVAL;
> + }
> +
> if (is_itpm(acpi_dev))
> itpm = true;
>
> --
> 2.1.4
>
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 3/7] tpm_tis: Do not fall back to a hardcoded address for TPM2
@ 2016-01-08 12:01 ` Jarkko Sakkinen
0 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2016-01-08 12:01 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: tpmdd-devel, linux-kernel, Peter Huewe
On Thu, Jan 07, 2016 at 05:36:22PM -0700, Jason Gunthorpe wrote:
> If the ACPI tables do not declare a memory resource for the TPM2
> then do not just fall back to the x86 default base address.
>
> Also be stricter when checking the ancillary TPM2 ACPI data and error
> out if any of this data is wrong rather than blindly assuming TPM1.
>
> Fixes: 399235dc6e95 ("tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0")
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Tested-by: Wilck, Martin <martin.wilck@ts.fujitsu.com>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
> ---
> drivers/char/tpm/tpm_tis.c | 48 +++++++++++++++++-----------------------------
> 1 file changed, 18 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index fecd27b45fd1..b2b31f5418ca 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -122,39 +122,11 @@ static inline int is_itpm(struct acpi_device *dev)
> {
> return has_hid(dev, "INTC0102");
> }
> -
> -static inline int is_fifo(struct acpi_device *dev)
> -{
> - struct acpi_table_tpm2 *tbl;
> - acpi_status st;
> -
> - /* TPM 1.2 FIFO */
> - if (!has_hid(dev, "MSFT0101"))
> - return 1;
> -
> - st = acpi_get_table(ACPI_SIG_TPM2, 1,
> - (struct acpi_table_header **) &tbl);
> - if (ACPI_FAILURE(st)) {
> - dev_err(&dev->dev, "failed to get TPM2 ACPI table\n");
> - return 0;
> - }
> -
> - if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
> - return 0;
> -
> - /* TPM 2.0 FIFO */
> - return 1;
> -}
> #else
> static inline int is_itpm(struct acpi_device *dev)
> {
> return 0;
> }
> -
> -static inline int is_fifo(struct acpi_device *dev)
> -{
> - return 1;
> -}
> #endif
>
> /* Before we attempt to access the TPM we must see that the valid bit is set.
> @@ -980,11 +952,21 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data)
>
> static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
> {
> + struct acpi_table_tpm2 *tbl;
> + acpi_status st;
> struct list_head resources;
> - struct tpm_info tpm_info = tis_default_info;
> + struct tpm_info tpm_info = {};
> int ret;
>
> - if (!is_fifo(acpi_dev))
> + st = acpi_get_table(ACPI_SIG_TPM2, 1,
> + (struct acpi_table_header **) &tbl);
> + if (ACPI_FAILURE(st) || tbl->header.length < sizeof(*tbl)) {
> + dev_err(&acpi_dev->dev,
> + FW_BUG "failed to get TPM2 ACPI table\n");
> + return -EINVAL;
> + }
> +
> + if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
> return -ENODEV;
>
> INIT_LIST_HEAD(&resources);
> @@ -996,6 +978,12 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
>
> acpi_dev_free_resource_list(&resources);
>
> + if (tpm_info.start == 0 && tpm_info.len == 0) {
> + dev_err(&acpi_dev->dev,
> + FW_BUG "TPM2 ACPI table does not define a memory resource\n");
> + return -EINVAL;
> + }
> +
> if (is_itpm(acpi_dev))
> itpm = true;
>
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 4/7] tpm_tis: Use devm_ioremap_resource
2016-01-08 0:36 ` Jason Gunthorpe
@ 2016-01-08 0:36 ` Jason Gunthorpe
-1 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
This does a request_resource under the covers which means tis holds a
lock on the memory range it is using so other drivers cannot grab it.
When doing probing it is important to ensure that other drivers are
not using the same range before tis starts touching it.
To do this flow the actual struct resource from the device right
through to devm_ioremap_resource. This ensures all the proper resource
meta-data is carried down.
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Tested-by: Wilck, Martin <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/char/tpm/tpm_tis.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index b2b31f5418ca..399c39e16a5c 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -67,8 +67,7 @@ enum tis_defaults {
};
struct tpm_info {
- unsigned long start;
- unsigned long len;
+ struct resource res;
/* irq > 0 means: use irq $irq;
* irq = 0 means: autoprobe for an irq;
* irq = -1 means: no irq support
@@ -77,8 +76,11 @@ struct tpm_info {
};
static struct tpm_info tis_default_info = {
- .start = TIS_MEM_BASE,
- .len = TIS_MEM_LEN,
+ .res = {
+ .start = TIS_MEM_BASE,
+ .end = TIS_MEM_BASE + TIS_MEM_LEN - 1,
+ .flags = IORESOURCE_MEM,
+ },
.irq = 0,
};
@@ -692,9 +694,9 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
chip->acpi_dev_handle = acpi_dev_handle;
#endif
- chip->vendor.iobase = devm_ioremap(dev, tpm_info->start, tpm_info->len);
- if (!chip->vendor.iobase)
- return -EIO;
+ chip->vendor.iobase = devm_ioremap_resource(dev, &tpm_info->res);
+ if (IS_ERR(chip->vendor.iobase))
+ return PTR_ERR(chip->vendor.iobase);
/* Maximum timeouts */
chip->vendor.timeout_a = TIS_TIMEOUT_A_MAX;
@@ -875,9 +877,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
{
struct tpm_info tpm_info = {};
acpi_handle acpi_dev_handle = NULL;
+ struct resource *res;
- tpm_info.start = pnp_mem_start(pnp_dev, 0);
- tpm_info.len = pnp_mem_len(pnp_dev, 0);
+ res = pnp_get_resource(pnp_dev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+ tpm_info.res = *res;
if (pnp_irq_valid(pnp_dev, 0))
tpm_info.irq = pnp_irq(pnp_dev, 0);
@@ -940,12 +945,10 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data)
struct tpm_info *tpm_info = (struct tpm_info *) data;
struct resource res;
- if (acpi_dev_resource_interrupt(ares, 0, &res)) {
+ if (acpi_dev_resource_interrupt(ares, 0, &res))
tpm_info->irq = res.start;
- } else if (acpi_dev_resource_memory(ares, &res)) {
- tpm_info->start = res.start;
- tpm_info->len = resource_size(&res);
- }
+ else if (acpi_dev_resource_memory(ares, &res))
+ tpm_info->res = res;
return 1;
}
@@ -978,7 +981,7 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
acpi_dev_free_resource_list(&resources);
- if (tpm_info.start == 0 && tpm_info.len == 0) {
+ if (resource_type(&tpm_info.res) != IORESOURCE_MEM) {
dev_err(&acpi_dev->dev,
FW_BUG "TPM2 ACPI table does not define a memory resource\n");
return -EINVAL;
--
2.1.4
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 4/7] tpm_tis: Use devm_ioremap_resource
@ 2016-01-08 0:36 ` Jason Gunthorpe
0 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel, linux-kernel; +Cc: Peter Huewe
This does a request_resource under the covers which means tis holds a
lock on the memory range it is using so other drivers cannot grab it.
When doing probing it is important to ensure that other drivers are
not using the same range before tis starts touching it.
To do this flow the actual struct resource from the device right
through to devm_ioremap_resource. This ensures all the proper resource
meta-data is carried down.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Tested-by: Wilck, Martin <martin.wilck@ts.fujitsu.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_tis.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index b2b31f5418ca..399c39e16a5c 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -67,8 +67,7 @@ enum tis_defaults {
};
struct tpm_info {
- unsigned long start;
- unsigned long len;
+ struct resource res;
/* irq > 0 means: use irq $irq;
* irq = 0 means: autoprobe for an irq;
* irq = -1 means: no irq support
@@ -77,8 +76,11 @@ struct tpm_info {
};
static struct tpm_info tis_default_info = {
- .start = TIS_MEM_BASE,
- .len = TIS_MEM_LEN,
+ .res = {
+ .start = TIS_MEM_BASE,
+ .end = TIS_MEM_BASE + TIS_MEM_LEN - 1,
+ .flags = IORESOURCE_MEM,
+ },
.irq = 0,
};
@@ -692,9 +694,9 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
chip->acpi_dev_handle = acpi_dev_handle;
#endif
- chip->vendor.iobase = devm_ioremap(dev, tpm_info->start, tpm_info->len);
- if (!chip->vendor.iobase)
- return -EIO;
+ chip->vendor.iobase = devm_ioremap_resource(dev, &tpm_info->res);
+ if (IS_ERR(chip->vendor.iobase))
+ return PTR_ERR(chip->vendor.iobase);
/* Maximum timeouts */
chip->vendor.timeout_a = TIS_TIMEOUT_A_MAX;
@@ -875,9 +877,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
{
struct tpm_info tpm_info = {};
acpi_handle acpi_dev_handle = NULL;
+ struct resource *res;
- tpm_info.start = pnp_mem_start(pnp_dev, 0);
- tpm_info.len = pnp_mem_len(pnp_dev, 0);
+ res = pnp_get_resource(pnp_dev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+ tpm_info.res = *res;
if (pnp_irq_valid(pnp_dev, 0))
tpm_info.irq = pnp_irq(pnp_dev, 0);
@@ -940,12 +945,10 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data)
struct tpm_info *tpm_info = (struct tpm_info *) data;
struct resource res;
- if (acpi_dev_resource_interrupt(ares, 0, &res)) {
+ if (acpi_dev_resource_interrupt(ares, 0, &res))
tpm_info->irq = res.start;
- } else if (acpi_dev_resource_memory(ares, &res)) {
- tpm_info->start = res.start;
- tpm_info->len = resource_size(&res);
- }
+ else if (acpi_dev_resource_memory(ares, &res))
+ tpm_info->res = res;
return 1;
}
@@ -978,7 +981,7 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
acpi_dev_free_resource_list(&resources);
- if (tpm_info.start == 0 && tpm_info.len == 0) {
+ if (resource_type(&tpm_info.res) != IORESOURCE_MEM) {
dev_err(&acpi_dev->dev,
FW_BUG "TPM2 ACPI table does not define a memory resource\n");
return -EINVAL;
--
2.1.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 5/7] tpm_tis: Clean up the force=1 module parameter
2016-01-08 0:36 ` Jason Gunthorpe
@ 2016-01-08 0:36 ` Jason Gunthorpe
-1 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
The TPM core has long assumed that every device has a driver attached,
however the force path was attaching the TPM core outside of a driver
context. This isn't generally reliable as the user could detatch the
driver using sysfs or something, but commit b8b2c7d845d5 ("base/platform:
assert that dev_pm_domain callbacks are called unconditionally")
forced the issue by leaving the driver pointer NULL if there is
no probe.
Rework the TPM setup to create a platform device with resources and
then allow the driver core to naturally bind and probe it through the
normal mechanisms. All this structure is needed anyhow to enable TPM
for OF environments.
Finally, since the entire flow is changing convert the init/exit to use
the modern ifdef-less coding style when possible
Reported-by: "Wilck, Martin" <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Tested-by: Wilck, Martin <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/char/tpm/tpm_tis.c | 169 ++++++++++++++++++++++++++++-----------------
1 file changed, 104 insertions(+), 65 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 399c39e16a5c..12aa96a69b6c 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -60,7 +60,6 @@ enum tis_int_flags {
};
enum tis_defaults {
- TIS_MEM_BASE = 0xFED40000,
TIS_MEM_LEN = 0x5000,
TIS_SHORT_TIMEOUT = 750, /* ms */
TIS_LONG_TIMEOUT = 2000, /* 2 sec */
@@ -75,15 +74,6 @@ struct tpm_info {
int irq;
};
-static struct tpm_info tis_default_info = {
- .res = {
- .start = TIS_MEM_BASE,
- .end = TIS_MEM_BASE + TIS_MEM_LEN - 1,
- .flags = IORESOURCE_MEM,
- },
- .irq = 0,
-};
-
/* Some timeout values are needed before it is known whether the chip is
* TPM 1.0 or TPM 2.0.
*/
@@ -825,7 +815,6 @@ out_err:
return rc;
}
-#ifdef CONFIG_PM_SLEEP
static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
{
u32 intmask;
@@ -867,11 +856,9 @@ static int tpm_tis_resume(struct device *dev)
return 0;
}
-#endif
static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
-#ifdef CONFIG_PNP
static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
const struct pnp_device_id *pnp_id)
{
@@ -889,14 +876,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
else
tpm_info.irq = -1;
-#ifdef CONFIG_ACPI
if (pnp_acpi_device(pnp_dev)) {
if (is_itpm(pnp_acpi_device(pnp_dev)))
itpm = true;
- acpi_dev_handle = pnp_acpi_device(pnp_dev)->handle;
+ acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
}
-#endif
return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
}
@@ -937,7 +922,6 @@ static struct pnp_driver tis_pnp_driver = {
module_param_string(hid, tpm_pnp_tbl[TIS_HID_USR_IDX].id,
sizeof(tpm_pnp_tbl[TIS_HID_USR_IDX].id), 0444);
MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to probe");
-#endif
#ifdef CONFIG_ACPI
static int tpm_check_resource(struct acpi_resource *ares, void *data)
@@ -1024,80 +1008,135 @@ static struct acpi_driver tis_acpi_driver = {
};
#endif
+static struct platform_device *force_pdev;
+
+static int tpm_tis_plat_probe(struct platform_device *pdev)
+{
+ struct tpm_info tpm_info = {};
+ struct resource *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (res == NULL) {
+ dev_err(&pdev->dev, "no memory resource defined\n");
+ return -ENODEV;
+ }
+ tpm_info.res = *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (res) {
+ tpm_info.irq = res->start;
+ } else {
+ if (pdev == force_pdev)
+ tpm_info.irq = -1;
+ else
+ /* When forcing auto probe the IRQ */
+ tpm_info.irq = 0;
+ }
+
+ return tpm_tis_init(&pdev->dev, &tpm_info, NULL);
+}
+
+static int tpm_tis_plat_remove(struct platform_device *pdev)
+{
+ struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
+
+ tpm_chip_unregister(chip);
+ tpm_tis_remove(chip);
+
+ return 0;
+}
+
static struct platform_driver tis_drv = {
+ .probe = tpm_tis_plat_probe,
+ .remove = tpm_tis_plat_remove,
.driver = {
.name = "tpm_tis",
.pm = &tpm_tis_pm,
},
};
-static struct platform_device *pdev;
-
static bool force;
+#ifdef CONFIG_X86
module_param(force, bool, 0444);
MODULE_PARM_DESC(force, "Force device probe rather than using ACPI entry");
+#endif
+
+static int tpm_tis_force_device(void)
+{
+ struct platform_device *pdev;
+ static const struct resource x86_resources[] = {
+ {
+ .start = 0xFED40000,
+ .end = 0xFED40000 + TIS_MEM_LEN - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ };
+
+ if (!force)
+ return 0;
+
+ /* The driver core will match the name tpm_tis of the device to
+ * the tpm_tis platform driver and complete the setup via
+ * tpm_tis_plat_probe
+ */
+ pdev = platform_device_register_simple("tpm_tis", -1, x86_resources,
+ ARRAY_SIZE(x86_resources));
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
+ force_pdev = pdev;
+
+ return 0;
+}
+
static int __init init_tis(void)
{
int rc;
-#ifdef CONFIG_PNP
- if (!force) {
- rc = pnp_register_driver(&tis_pnp_driver);
- if (rc)
- return rc;
- }
-#endif
+
+ rc = tpm_tis_force_device();
+ if (rc)
+ goto err_force;
+
+ rc = platform_driver_register(&tis_drv);
+ if (rc)
+ goto err_platform;
+
#ifdef CONFIG_ACPI
- if (!force) {
- rc = acpi_bus_register_driver(&tis_acpi_driver);
- if (rc) {
-#ifdef CONFIG_PNP
- pnp_unregister_driver(&tis_pnp_driver);
-#endif
- return rc;
- }
- }
+ rc = acpi_bus_register_driver(&tis_acpi_driver);
+ if (rc)
+ goto err_acpi;
#endif
- if (!force)
- return 0;
- rc = platform_driver_register(&tis_drv);
- if (rc < 0)
- return rc;
- pdev = platform_device_register_simple("tpm_tis", -1, NULL, 0);
- if (IS_ERR(pdev)) {
- rc = PTR_ERR(pdev);
- goto err_dev;
+ if (IS_ENABLED(CONFIG_PNP)) {
+ rc = pnp_register_driver(&tis_pnp_driver);
+ if (rc)
+ goto err_pnp;
}
- rc = tpm_tis_init(&pdev->dev, &tis_default_info, NULL);
- if (rc)
- goto err_init;
+
return 0;
-err_init:
- platform_device_unregister(pdev);
-err_dev:
- platform_driver_unregister(&tis_drv);
+
+err_pnp:
+#ifdef CONFIG_ACPI
+ acpi_bus_unregister_driver(&tis_acpi_driver);
+err_acpi:
+#endif
+ platform_device_unregister(force_pdev);
+err_platform:
+ if (force_pdev)
+ platform_device_unregister(force_pdev);
+err_force:
return rc;
}
static void __exit cleanup_tis(void)
{
- struct tpm_chip *chip;
-#if defined(CONFIG_PNP) || defined(CONFIG_ACPI)
- if (!force) {
+ pnp_unregister_driver(&tis_pnp_driver);
#ifdef CONFIG_ACPI
- acpi_bus_unregister_driver(&tis_acpi_driver);
-#endif
-#ifdef CONFIG_PNP
- pnp_unregister_driver(&tis_pnp_driver);
-#endif
- return;
- }
+ acpi_bus_unregister_driver(&tis_acpi_driver);
#endif
- chip = dev_get_drvdata(&pdev->dev);
- tpm_chip_unregister(chip);
- tpm_tis_remove(chip);
- platform_device_unregister(pdev);
platform_driver_unregister(&tis_drv);
+
+ if (force_pdev)
+ platform_device_unregister(force_pdev);
}
module_init(init_tis);
--
2.1.4
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 5/7] tpm_tis: Clean up the force=1 module parameter
@ 2016-01-08 0:36 ` Jason Gunthorpe
0 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel, linux-kernel; +Cc: Peter Huewe
The TPM core has long assumed that every device has a driver attached,
however the force path was attaching the TPM core outside of a driver
context. This isn't generally reliable as the user could detatch the
driver using sysfs or something, but commit b8b2c7d845d5 ("base/platform:
assert that dev_pm_domain callbacks are called unconditionally")
forced the issue by leaving the driver pointer NULL if there is
no probe.
Rework the TPM setup to create a platform device with resources and
then allow the driver core to naturally bind and probe it through the
normal mechanisms. All this structure is needed anyhow to enable TPM
for OF environments.
Finally, since the entire flow is changing convert the init/exit to use
the modern ifdef-less coding style when possible
Reported-by: "Wilck, Martin" <martin.wilck@ts.fujitsu.com>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Tested-by: Wilck, Martin <martin.wilck@ts.fujitsu.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_tis.c | 169 ++++++++++++++++++++++++++++-----------------
1 file changed, 104 insertions(+), 65 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 399c39e16a5c..12aa96a69b6c 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -60,7 +60,6 @@ enum tis_int_flags {
};
enum tis_defaults {
- TIS_MEM_BASE = 0xFED40000,
TIS_MEM_LEN = 0x5000,
TIS_SHORT_TIMEOUT = 750, /* ms */
TIS_LONG_TIMEOUT = 2000, /* 2 sec */
@@ -75,15 +74,6 @@ struct tpm_info {
int irq;
};
-static struct tpm_info tis_default_info = {
- .res = {
- .start = TIS_MEM_BASE,
- .end = TIS_MEM_BASE + TIS_MEM_LEN - 1,
- .flags = IORESOURCE_MEM,
- },
- .irq = 0,
-};
-
/* Some timeout values are needed before it is known whether the chip is
* TPM 1.0 or TPM 2.0.
*/
@@ -825,7 +815,6 @@ out_err:
return rc;
}
-#ifdef CONFIG_PM_SLEEP
static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
{
u32 intmask;
@@ -867,11 +856,9 @@ static int tpm_tis_resume(struct device *dev)
return 0;
}
-#endif
static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
-#ifdef CONFIG_PNP
static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
const struct pnp_device_id *pnp_id)
{
@@ -889,14 +876,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
else
tpm_info.irq = -1;
-#ifdef CONFIG_ACPI
if (pnp_acpi_device(pnp_dev)) {
if (is_itpm(pnp_acpi_device(pnp_dev)))
itpm = true;
- acpi_dev_handle = pnp_acpi_device(pnp_dev)->handle;
+ acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
}
-#endif
return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
}
@@ -937,7 +922,6 @@ static struct pnp_driver tis_pnp_driver = {
module_param_string(hid, tpm_pnp_tbl[TIS_HID_USR_IDX].id,
sizeof(tpm_pnp_tbl[TIS_HID_USR_IDX].id), 0444);
MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to probe");
-#endif
#ifdef CONFIG_ACPI
static int tpm_check_resource(struct acpi_resource *ares, void *data)
@@ -1024,80 +1008,135 @@ static struct acpi_driver tis_acpi_driver = {
};
#endif
+static struct platform_device *force_pdev;
+
+static int tpm_tis_plat_probe(struct platform_device *pdev)
+{
+ struct tpm_info tpm_info = {};
+ struct resource *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (res == NULL) {
+ dev_err(&pdev->dev, "no memory resource defined\n");
+ return -ENODEV;
+ }
+ tpm_info.res = *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (res) {
+ tpm_info.irq = res->start;
+ } else {
+ if (pdev == force_pdev)
+ tpm_info.irq = -1;
+ else
+ /* When forcing auto probe the IRQ */
+ tpm_info.irq = 0;
+ }
+
+ return tpm_tis_init(&pdev->dev, &tpm_info, NULL);
+}
+
+static int tpm_tis_plat_remove(struct platform_device *pdev)
+{
+ struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
+
+ tpm_chip_unregister(chip);
+ tpm_tis_remove(chip);
+
+ return 0;
+}
+
static struct platform_driver tis_drv = {
+ .probe = tpm_tis_plat_probe,
+ .remove = tpm_tis_plat_remove,
.driver = {
.name = "tpm_tis",
.pm = &tpm_tis_pm,
},
};
-static struct platform_device *pdev;
-
static bool force;
+#ifdef CONFIG_X86
module_param(force, bool, 0444);
MODULE_PARM_DESC(force, "Force device probe rather than using ACPI entry");
+#endif
+
+static int tpm_tis_force_device(void)
+{
+ struct platform_device *pdev;
+ static const struct resource x86_resources[] = {
+ {
+ .start = 0xFED40000,
+ .end = 0xFED40000 + TIS_MEM_LEN - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ };
+
+ if (!force)
+ return 0;
+
+ /* The driver core will match the name tpm_tis of the device to
+ * the tpm_tis platform driver and complete the setup via
+ * tpm_tis_plat_probe
+ */
+ pdev = platform_device_register_simple("tpm_tis", -1, x86_resources,
+ ARRAY_SIZE(x86_resources));
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
+ force_pdev = pdev;
+
+ return 0;
+}
+
static int __init init_tis(void)
{
int rc;
-#ifdef CONFIG_PNP
- if (!force) {
- rc = pnp_register_driver(&tis_pnp_driver);
- if (rc)
- return rc;
- }
-#endif
+
+ rc = tpm_tis_force_device();
+ if (rc)
+ goto err_force;
+
+ rc = platform_driver_register(&tis_drv);
+ if (rc)
+ goto err_platform;
+
#ifdef CONFIG_ACPI
- if (!force) {
- rc = acpi_bus_register_driver(&tis_acpi_driver);
- if (rc) {
-#ifdef CONFIG_PNP
- pnp_unregister_driver(&tis_pnp_driver);
-#endif
- return rc;
- }
- }
+ rc = acpi_bus_register_driver(&tis_acpi_driver);
+ if (rc)
+ goto err_acpi;
#endif
- if (!force)
- return 0;
- rc = platform_driver_register(&tis_drv);
- if (rc < 0)
- return rc;
- pdev = platform_device_register_simple("tpm_tis", -1, NULL, 0);
- if (IS_ERR(pdev)) {
- rc = PTR_ERR(pdev);
- goto err_dev;
+ if (IS_ENABLED(CONFIG_PNP)) {
+ rc = pnp_register_driver(&tis_pnp_driver);
+ if (rc)
+ goto err_pnp;
}
- rc = tpm_tis_init(&pdev->dev, &tis_default_info, NULL);
- if (rc)
- goto err_init;
+
return 0;
-err_init:
- platform_device_unregister(pdev);
-err_dev:
- platform_driver_unregister(&tis_drv);
+
+err_pnp:
+#ifdef CONFIG_ACPI
+ acpi_bus_unregister_driver(&tis_acpi_driver);
+err_acpi:
+#endif
+ platform_device_unregister(force_pdev);
+err_platform:
+ if (force_pdev)
+ platform_device_unregister(force_pdev);
+err_force:
return rc;
}
static void __exit cleanup_tis(void)
{
- struct tpm_chip *chip;
-#if defined(CONFIG_PNP) || defined(CONFIG_ACPI)
- if (!force) {
+ pnp_unregister_driver(&tis_pnp_driver);
#ifdef CONFIG_ACPI
- acpi_bus_unregister_driver(&tis_acpi_driver);
-#endif
-#ifdef CONFIG_PNP
- pnp_unregister_driver(&tis_pnp_driver);
-#endif
- return;
- }
+ acpi_bus_unregister_driver(&tis_acpi_driver);
#endif
- chip = dev_get_drvdata(&pdev->dev);
- tpm_chip_unregister(chip);
- tpm_tis_remove(chip);
- platform_device_unregister(pdev);
platform_driver_unregister(&tis_drv);
+
+ if (force_pdev)
+ platform_device_unregister(force_pdev);
}
module_init(init_tis);
--
2.1.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 5/7] tpm_tis: Clean up the force=1 module parameter
2016-01-08 0:36 ` Jason Gunthorpe
(?)
@ 2016-01-08 12:01 ` Jarkko Sakkinen
-1 siblings, 0 replies; 29+ messages in thread
From: Jarkko Sakkinen @ 2016-01-08 12:01 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: tpmdd-devel, linux-kernel, Peter Huewe
On Thu, Jan 07, 2016 at 05:36:24PM -0700, Jason Gunthorpe wrote:
> The TPM core has long assumed that every device has a driver attached,
> however the force path was attaching the TPM core outside of a driver
> context. This isn't generally reliable as the user could detatch the
> driver using sysfs or something, but commit b8b2c7d845d5 ("base/platform:
> assert that dev_pm_domain callbacks are called unconditionally")
> forced the issue by leaving the driver pointer NULL if there is
> no probe.
>
> Rework the TPM setup to create a platform device with resources and
> then allow the driver core to naturally bind and probe it through the
> normal mechanisms. All this structure is needed anyhow to enable TPM
> for OF environments.
>
> Finally, since the entire flow is changing convert the init/exit to use
> the modern ifdef-less coding style when possible
>
> Reported-by: "Wilck, Martin" <martin.wilck@ts.fujitsu.com>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Tested-by: Wilck, Martin <martin.wilck@ts.fujitsu.com>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
> ---
> drivers/char/tpm/tpm_tis.c | 169 ++++++++++++++++++++++++++++-----------------
> 1 file changed, 104 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 399c39e16a5c..12aa96a69b6c 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -60,7 +60,6 @@ enum tis_int_flags {
> };
>
> enum tis_defaults {
> - TIS_MEM_BASE = 0xFED40000,
> TIS_MEM_LEN = 0x5000,
> TIS_SHORT_TIMEOUT = 750, /* ms */
> TIS_LONG_TIMEOUT = 2000, /* 2 sec */
> @@ -75,15 +74,6 @@ struct tpm_info {
> int irq;
> };
>
> -static struct tpm_info tis_default_info = {
> - .res = {
> - .start = TIS_MEM_BASE,
> - .end = TIS_MEM_BASE + TIS_MEM_LEN - 1,
> - .flags = IORESOURCE_MEM,
> - },
> - .irq = 0,
> -};
> -
> /* Some timeout values are needed before it is known whether the chip is
> * TPM 1.0 or TPM 2.0.
> */
> @@ -825,7 +815,6 @@ out_err:
> return rc;
> }
>
> -#ifdef CONFIG_PM_SLEEP
> static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
> {
> u32 intmask;
> @@ -867,11 +856,9 @@ static int tpm_tis_resume(struct device *dev)
>
> return 0;
> }
> -#endif
>
> static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
>
> -#ifdef CONFIG_PNP
> static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
> const struct pnp_device_id *pnp_id)
> {
> @@ -889,14 +876,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
> else
> tpm_info.irq = -1;
>
> -#ifdef CONFIG_ACPI
> if (pnp_acpi_device(pnp_dev)) {
> if (is_itpm(pnp_acpi_device(pnp_dev)))
> itpm = true;
>
> - acpi_dev_handle = pnp_acpi_device(pnp_dev)->handle;
> + acpi_dev_handle = ACPI_HANDLE(&pnp_dev->dev);
> }
> -#endif
>
> return tpm_tis_init(&pnp_dev->dev, &tpm_info, acpi_dev_handle);
> }
> @@ -937,7 +922,6 @@ static struct pnp_driver tis_pnp_driver = {
> module_param_string(hid, tpm_pnp_tbl[TIS_HID_USR_IDX].id,
> sizeof(tpm_pnp_tbl[TIS_HID_USR_IDX].id), 0444);
> MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to probe");
> -#endif
>
> #ifdef CONFIG_ACPI
> static int tpm_check_resource(struct acpi_resource *ares, void *data)
> @@ -1024,80 +1008,135 @@ static struct acpi_driver tis_acpi_driver = {
> };
> #endif
>
> +static struct platform_device *force_pdev;
> +
> +static int tpm_tis_plat_probe(struct platform_device *pdev)
> +{
> + struct tpm_info tpm_info = {};
> + struct resource *res;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (res == NULL) {
> + dev_err(&pdev->dev, "no memory resource defined\n");
> + return -ENODEV;
> + }
> + tpm_info.res = *res;
> +
> + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (res) {
> + tpm_info.irq = res->start;
> + } else {
> + if (pdev == force_pdev)
> + tpm_info.irq = -1;
> + else
> + /* When forcing auto probe the IRQ */
> + tpm_info.irq = 0;
> + }
> +
> + return tpm_tis_init(&pdev->dev, &tpm_info, NULL);
> +}
> +
> +static int tpm_tis_plat_remove(struct platform_device *pdev)
> +{
> + struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
> +
> + tpm_chip_unregister(chip);
> + tpm_tis_remove(chip);
> +
> + return 0;
> +}
> +
> static struct platform_driver tis_drv = {
> + .probe = tpm_tis_plat_probe,
> + .remove = tpm_tis_plat_remove,
> .driver = {
> .name = "tpm_tis",
> .pm = &tpm_tis_pm,
> },
> };
>
> -static struct platform_device *pdev;
> -
> static bool force;
> +#ifdef CONFIG_X86
> module_param(force, bool, 0444);
> MODULE_PARM_DESC(force, "Force device probe rather than using ACPI entry");
> +#endif
> +
> +static int tpm_tis_force_device(void)
> +{
> + struct platform_device *pdev;
> + static const struct resource x86_resources[] = {
> + {
> + .start = 0xFED40000,
> + .end = 0xFED40000 + TIS_MEM_LEN - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + };
> +
> + if (!force)
> + return 0;
> +
> + /* The driver core will match the name tpm_tis of the device to
> + * the tpm_tis platform driver and complete the setup via
> + * tpm_tis_plat_probe
> + */
> + pdev = platform_device_register_simple("tpm_tis", -1, x86_resources,
> + ARRAY_SIZE(x86_resources));
> + if (IS_ERR(pdev))
> + return PTR_ERR(pdev);
> + force_pdev = pdev;
> +
> + return 0;
> +}
> +
> static int __init init_tis(void)
> {
> int rc;
> -#ifdef CONFIG_PNP
> - if (!force) {
> - rc = pnp_register_driver(&tis_pnp_driver);
> - if (rc)
> - return rc;
> - }
> -#endif
> +
> + rc = tpm_tis_force_device();
> + if (rc)
> + goto err_force;
> +
> + rc = platform_driver_register(&tis_drv);
> + if (rc)
> + goto err_platform;
> +
> #ifdef CONFIG_ACPI
> - if (!force) {
> - rc = acpi_bus_register_driver(&tis_acpi_driver);
> - if (rc) {
> -#ifdef CONFIG_PNP
> - pnp_unregister_driver(&tis_pnp_driver);
> -#endif
> - return rc;
> - }
> - }
> + rc = acpi_bus_register_driver(&tis_acpi_driver);
> + if (rc)
> + goto err_acpi;
> #endif
> - if (!force)
> - return 0;
>
> - rc = platform_driver_register(&tis_drv);
> - if (rc < 0)
> - return rc;
> - pdev = platform_device_register_simple("tpm_tis", -1, NULL, 0);
> - if (IS_ERR(pdev)) {
> - rc = PTR_ERR(pdev);
> - goto err_dev;
> + if (IS_ENABLED(CONFIG_PNP)) {
> + rc = pnp_register_driver(&tis_pnp_driver);
> + if (rc)
> + goto err_pnp;
> }
> - rc = tpm_tis_init(&pdev->dev, &tis_default_info, NULL);
> - if (rc)
> - goto err_init;
> +
> return 0;
> -err_init:
> - platform_device_unregister(pdev);
> -err_dev:
> - platform_driver_unregister(&tis_drv);
> +
> +err_pnp:
> +#ifdef CONFIG_ACPI
> + acpi_bus_unregister_driver(&tis_acpi_driver);
> +err_acpi:
> +#endif
> + platform_device_unregister(force_pdev);
> +err_platform:
> + if (force_pdev)
> + platform_device_unregister(force_pdev);
> +err_force:
> return rc;
> }
>
> static void __exit cleanup_tis(void)
> {
> - struct tpm_chip *chip;
> -#if defined(CONFIG_PNP) || defined(CONFIG_ACPI)
> - if (!force) {
> + pnp_unregister_driver(&tis_pnp_driver);
> #ifdef CONFIG_ACPI
> - acpi_bus_unregister_driver(&tis_acpi_driver);
> -#endif
> -#ifdef CONFIG_PNP
> - pnp_unregister_driver(&tis_pnp_driver);
> -#endif
> - return;
> - }
> + acpi_bus_unregister_driver(&tis_acpi_driver);
> #endif
> - chip = dev_get_drvdata(&pdev->dev);
> - tpm_chip_unregister(chip);
> - tpm_tis_remove(chip);
> - platform_device_unregister(pdev);
> platform_driver_unregister(&tis_drv);
> +
> + if (force_pdev)
> + platform_device_unregister(force_pdev);
> }
>
> module_init(init_tis);
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 6/7] tpm_crb: Drop le32_to_cpu(ioread32(..))
2016-01-08 0:36 ` Jason Gunthorpe
@ 2016-01-08 0:36 ` Jason Gunthorpe
-1 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
ioread32 and readl are defined to read from PCI style memory, ie little
endian and return the result in host order. On platforms where a
swap is required ioread32/readl do the swap internally (eg see ppc).
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/char/tpm/tpm_crb.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 8dd70696ebe8..0237006dc4d8 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -89,7 +89,7 @@ static u8 crb_status(struct tpm_chip *chip)
struct crb_priv *priv = chip->vendor.priv;
u8 sts = 0;
- if ((le32_to_cpu(ioread32(&priv->cca->start)) & CRB_START_INVOKE) !=
+ if ((ioread32(&priv->cca->start) & CRB_START_INVOKE) !=
CRB_START_INVOKE)
sts |= CRB_STS_COMPLETE;
@@ -105,7 +105,7 @@ static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
if (count < 6)
return -EIO;
- if (le32_to_cpu(ioread32(&priv->cca->sts)) & CRB_CA_STS_ERROR)
+ if (ioread32(&priv->cca->sts) & CRB_CA_STS_ERROR)
return -EIO;
memcpy_fromio(buf, priv->rsp, 6);
@@ -141,11 +141,11 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
struct crb_priv *priv = chip->vendor.priv;
int rc = 0;
- if (len > le32_to_cpu(ioread32(&priv->cca->cmd_size))) {
+ if (len > ioread32(&priv->cca->cmd_size)) {
dev_err(&chip->dev,
"invalid command count value %x %zx\n",
(unsigned int) len,
- (size_t) le32_to_cpu(ioread32(&priv->cca->cmd_size)));
+ (size_t) ioread32(&priv->cca->cmd_size));
return -E2BIG;
}
@@ -181,7 +181,7 @@ static void crb_cancel(struct tpm_chip *chip)
static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
{
struct crb_priv *priv = chip->vendor.priv;
- u32 cancel = le32_to_cpu(ioread32(&priv->cca->cancel));
+ u32 cancel = ioread32(&priv->cca->cancel);
return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE;
}
@@ -251,10 +251,10 @@ static int crb_acpi_add(struct acpi_device *device)
return -ENOMEM;
}
- pa = ((u64) le32_to_cpu(ioread32(&priv->cca->cmd_pa_high)) << 32) |
- (u64) le32_to_cpu(ioread32(&priv->cca->cmd_pa_low));
- priv->cmd = devm_ioremap_nocache(dev, pa,
- ioread32(&priv->cca->cmd_size));
+ pa = ((u64)ioread32(&priv->cca->cmd_pa_high) << 32) |
+ (u64)ioread32(&priv->cca->cmd_pa_low);
+ priv->cmd =
+ devm_ioremap_nocache(dev, pa, ioread32(&priv->cca->cmd_size));
if (!priv->cmd) {
dev_err(dev, "ioremap of the command buffer failed\n");
return -ENOMEM;
@@ -262,8 +262,8 @@ static int crb_acpi_add(struct acpi_device *device)
memcpy_fromio(&pa, &priv->cca->rsp_pa, 8);
pa = le64_to_cpu(pa);
- priv->rsp = devm_ioremap_nocache(dev, pa,
- ioread32(&priv->cca->rsp_size));
+ priv->rsp =
+ devm_ioremap_nocache(dev, pa, ioread32(&priv->cca->rsp_size));
if (!priv->rsp) {
dev_err(dev, "ioremap of the response buffer failed\n");
return -ENOMEM;
--
2.1.4
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 6/7] tpm_crb: Drop le32_to_cpu(ioread32(..))
@ 2016-01-08 0:36 ` Jason Gunthorpe
0 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel, linux-kernel; +Cc: Peter Huewe
ioread32 and readl are defined to read from PCI style memory, ie little
endian and return the result in host order. On platforms where a
swap is required ioread32/readl do the swap internally (eg see ppc).
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_crb.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 8dd70696ebe8..0237006dc4d8 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -89,7 +89,7 @@ static u8 crb_status(struct tpm_chip *chip)
struct crb_priv *priv = chip->vendor.priv;
u8 sts = 0;
- if ((le32_to_cpu(ioread32(&priv->cca->start)) & CRB_START_INVOKE) !=
+ if ((ioread32(&priv->cca->start) & CRB_START_INVOKE) !=
CRB_START_INVOKE)
sts |= CRB_STS_COMPLETE;
@@ -105,7 +105,7 @@ static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
if (count < 6)
return -EIO;
- if (le32_to_cpu(ioread32(&priv->cca->sts)) & CRB_CA_STS_ERROR)
+ if (ioread32(&priv->cca->sts) & CRB_CA_STS_ERROR)
return -EIO;
memcpy_fromio(buf, priv->rsp, 6);
@@ -141,11 +141,11 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
struct crb_priv *priv = chip->vendor.priv;
int rc = 0;
- if (len > le32_to_cpu(ioread32(&priv->cca->cmd_size))) {
+ if (len > ioread32(&priv->cca->cmd_size)) {
dev_err(&chip->dev,
"invalid command count value %x %zx\n",
(unsigned int) len,
- (size_t) le32_to_cpu(ioread32(&priv->cca->cmd_size)));
+ (size_t) ioread32(&priv->cca->cmd_size));
return -E2BIG;
}
@@ -181,7 +181,7 @@ static void crb_cancel(struct tpm_chip *chip)
static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
{
struct crb_priv *priv = chip->vendor.priv;
- u32 cancel = le32_to_cpu(ioread32(&priv->cca->cancel));
+ u32 cancel = ioread32(&priv->cca->cancel);
return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE;
}
@@ -251,10 +251,10 @@ static int crb_acpi_add(struct acpi_device *device)
return -ENOMEM;
}
- pa = ((u64) le32_to_cpu(ioread32(&priv->cca->cmd_pa_high)) << 32) |
- (u64) le32_to_cpu(ioread32(&priv->cca->cmd_pa_low));
- priv->cmd = devm_ioremap_nocache(dev, pa,
- ioread32(&priv->cca->cmd_size));
+ pa = ((u64)ioread32(&priv->cca->cmd_pa_high) << 32) |
+ (u64)ioread32(&priv->cca->cmd_pa_low);
+ priv->cmd =
+ devm_ioremap_nocache(dev, pa, ioread32(&priv->cca->cmd_size));
if (!priv->cmd) {
dev_err(dev, "ioremap of the command buffer failed\n");
return -ENOMEM;
@@ -262,8 +262,8 @@ static int crb_acpi_add(struct acpi_device *device)
memcpy_fromio(&pa, &priv->cca->rsp_pa, 8);
pa = le64_to_cpu(pa);
- priv->rsp = devm_ioremap_nocache(dev, pa,
- ioread32(&priv->cca->rsp_size));
+ priv->rsp =
+ devm_ioremap_nocache(dev, pa, ioread32(&priv->cca->rsp_size));
if (!priv->rsp) {
dev_err(dev, "ioremap of the response buffer failed\n");
return -ENOMEM;
--
2.1.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 7/7] tpm_crb: Use devm_ioremap_resource
2016-01-08 0:36 ` Jason Gunthorpe
@ 2016-01-08 0:36 ` Jason Gunthorpe
-1 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
To support the force mode in tpm_tis we need to use resource locking
in tpm_crb as well, via devm_ioremap_resource.
The light restructuring better aligns crb and tis and makes it easier
to see the that new changes make sense.
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/char/tpm/tpm_crb.c | 157 +++++++++++++++++++++++++++++++--------------
1 file changed, 108 insertions(+), 49 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 0237006dc4d8..1f844cc63016 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -77,6 +77,8 @@ enum crb_flags {
struct crb_priv {
unsigned int flags;
+ struct resource res;
+ void __iomem *iobase;
struct crb_control_area __iomem *cca;
u8 __iomem *cmd;
u8 __iomem *rsp;
@@ -196,22 +198,121 @@ static const struct tpm_class_ops tpm_crb = {
.req_complete_val = CRB_STS_COMPLETE,
};
-static int crb_acpi_add(struct acpi_device *device)
+static int crb_init(struct acpi_device *device, struct crb_priv *priv)
{
struct tpm_chip *chip;
+ int rc;
+
+ chip = tpmm_chip_alloc(&device->dev, &tpm_crb);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
+
+ chip->vendor.priv = priv;
+ chip->acpi_dev_handle = device->handle;
+ chip->flags = TPM_CHIP_FLAG_TPM2;
+
+ rc = tpm_get_timeouts(chip);
+ if (rc)
+ return rc;
+
+ rc = tpm2_do_selftest(chip);
+ if (rc)
+ return rc;
+
+ return tpm_chip_register(chip);
+}
+
+static int crb_check_resource(struct acpi_resource *ares, void *data)
+{
+ struct crb_priv *priv = data;
+ struct resource res;
+
+ if (acpi_dev_resource_memory(ares, &res))
+ priv->res = res;
+
+ return 1;
+}
+
+static void __iomem *crb_access(struct device *dev, struct crb_priv *priv,
+ u64 start, u32 size)
+{
+ struct resource tmp = {};
+
+ tmp.start = start;
+ tmp.end = start + size - 1;
+ tmp.flags = IORESOURCE_MEM;
+
+ /* Detect a 64 bit address on a 32 bit system */
+ if (start != tmp.start)
+ return ERR_PTR(-EINVAL);
+
+ if (!resource_contains(&priv->res, &tmp)) {
+ dev_err(dev,
+ FW_BUG "TPM2 ACPI sub resource %pR is not in the device's region of %pR\n",
+ &tmp, &priv->res);
+ return ERR_PTR(-EINVAL);
+ }
+
+ return priv->iobase + (tmp.start - priv->res.start);
+}
+
+static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
+ struct acpi_table_tpm2 *buf)
+{
+ struct list_head resources;
+ struct device *dev = &device->dev;
+ u64 pa;
+ int ret;
+
+ INIT_LIST_HEAD(&resources);
+ ret = acpi_dev_get_resources(device, &resources, crb_check_resource,
+ priv);
+ if (ret < 0)
+ return ret;
+ acpi_dev_free_resource_list(&resources);
+
+ if (resource_type(&priv->res) != IORESOURCE_MEM) {
+ dev_err(dev,
+ FW_BUG "TPM2 ACPI table does not define a memory resource\n");
+ return -EINVAL;
+ }
+
+ priv->iobase = devm_ioremap_resource(dev, &priv->res);
+ if (IS_ERR(priv->iobase))
+ return PTR_ERR(priv->iobase);
+
+ priv->cca = crb_access(dev, priv, buf->control_address, 0x1000);
+ if (IS_ERR(priv->cca))
+ return PTR_ERR(priv->cca);
+
+ pa = ((u64)ioread32(&priv->cca->cmd_pa_high) << 32) |
+ (u64)ioread32(&priv->cca->cmd_pa_low);
+ priv->cmd = crb_access(dev, priv, pa, ioread32(&priv->cca->cmd_size));
+ if (IS_ERR(priv->cmd))
+ return PTR_ERR(priv->cmd);
+
+ memcpy_fromio(&pa, &priv->cca->rsp_pa, 8);
+ pa = le64_to_cpu(pa);
+ priv->rsp = crb_access(dev, priv, pa, ioread32(&priv->cca->rsp_size));
+ if (IS_ERR(priv->rsp))
+ return PTR_ERR(priv->rsp);
+ return 0;
+}
+
+static int crb_acpi_add(struct acpi_device *device)
+{
struct acpi_table_tpm2 *buf;
struct crb_priv *priv;
struct device *dev = &device->dev;
acpi_status status;
u32 sm;
- u64 pa;
int rc;
status = acpi_get_table(ACPI_SIG_TPM2, 1,
(struct acpi_table_header **) &buf);
if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
- return -ENODEV;
+ return -EINVAL;
}
/* Should the FIFO driver handle this? */
@@ -219,18 +320,9 @@ static int crb_acpi_add(struct acpi_device *device)
if (sm == ACPI_TPM2_MEMORY_MAPPED)
return -ENODEV;
- chip = tpmm_chip_alloc(dev, &tpm_crb);
- if (IS_ERR(chip))
- return PTR_ERR(chip);
-
- chip->flags = TPM_CHIP_FLAG_TPM2;
-
- priv = (struct crb_priv *) devm_kzalloc(dev, sizeof(struct crb_priv),
- GFP_KERNEL);
- if (!priv) {
- dev_err(dev, "failed to devm_kzalloc for private data\n");
+ priv = devm_kzalloc(dev, sizeof(struct crb_priv), GFP_KERNEL);
+ if (!priv)
return -ENOMEM;
- }
/* 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
@@ -244,44 +336,11 @@ static int crb_acpi_add(struct acpi_device *device)
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_address, 0x1000);
- if (!priv->cca) {
- dev_err(dev, "ioremap of the control area failed\n");
- return -ENOMEM;
- }
-
- pa = ((u64)ioread32(&priv->cca->cmd_pa_high) << 32) |
- (u64)ioread32(&priv->cca->cmd_pa_low);
- priv->cmd =
- devm_ioremap_nocache(dev, pa, ioread32(&priv->cca->cmd_size));
- if (!priv->cmd) {
- dev_err(dev, "ioremap of the command buffer failed\n");
- return -ENOMEM;
- }
-
- memcpy_fromio(&pa, &priv->cca->rsp_pa, 8);
- pa = le64_to_cpu(pa);
- priv->rsp =
- devm_ioremap_nocache(dev, pa, ioread32(&priv->cca->rsp_size));
- if (!priv->rsp) {
- dev_err(dev, "ioremap of the response buffer failed\n");
- return -ENOMEM;
- }
-
- chip->vendor.priv = priv;
-
- rc = tpm_get_timeouts(chip);
- if (rc)
- return rc;
-
- chip->acpi_dev_handle = device->handle;
-
- rc = tpm2_do_selftest(chip);
+ rc = crb_map_io(device, priv, buf);
if (rc)
return rc;
- return tpm_chip_register(chip);
+ return crb_init(device, priv);
}
static int crb_acpi_remove(struct acpi_device *device)
--
2.1.4
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 7/7] tpm_crb: Use devm_ioremap_resource
@ 2016-01-08 0:36 ` Jason Gunthorpe
0 siblings, 0 replies; 29+ messages in thread
From: Jason Gunthorpe @ 2016-01-08 0:36 UTC (permalink / raw)
To: Jarkko Sakkinen, tpmdd-devel, linux-kernel; +Cc: Peter Huewe
To support the force mode in tpm_tis we need to use resource locking
in tpm_crb as well, via devm_ioremap_resource.
The light restructuring better aligns crb and tis and makes it easier
to see the that new changes make sense.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm_crb.c | 157 +++++++++++++++++++++++++++++++--------------
1 file changed, 108 insertions(+), 49 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 0237006dc4d8..1f844cc63016 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -77,6 +77,8 @@ enum crb_flags {
struct crb_priv {
unsigned int flags;
+ struct resource res;
+ void __iomem *iobase;
struct crb_control_area __iomem *cca;
u8 __iomem *cmd;
u8 __iomem *rsp;
@@ -196,22 +198,121 @@ static const struct tpm_class_ops tpm_crb = {
.req_complete_val = CRB_STS_COMPLETE,
};
-static int crb_acpi_add(struct acpi_device *device)
+static int crb_init(struct acpi_device *device, struct crb_priv *priv)
{
struct tpm_chip *chip;
+ int rc;
+
+ chip = tpmm_chip_alloc(&device->dev, &tpm_crb);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
+
+ chip->vendor.priv = priv;
+ chip->acpi_dev_handle = device->handle;
+ chip->flags = TPM_CHIP_FLAG_TPM2;
+
+ rc = tpm_get_timeouts(chip);
+ if (rc)
+ return rc;
+
+ rc = tpm2_do_selftest(chip);
+ if (rc)
+ return rc;
+
+ return tpm_chip_register(chip);
+}
+
+static int crb_check_resource(struct acpi_resource *ares, void *data)
+{
+ struct crb_priv *priv = data;
+ struct resource res;
+
+ if (acpi_dev_resource_memory(ares, &res))
+ priv->res = res;
+
+ return 1;
+}
+
+static void __iomem *crb_access(struct device *dev, struct crb_priv *priv,
+ u64 start, u32 size)
+{
+ struct resource tmp = {};
+
+ tmp.start = start;
+ tmp.end = start + size - 1;
+ tmp.flags = IORESOURCE_MEM;
+
+ /* Detect a 64 bit address on a 32 bit system */
+ if (start != tmp.start)
+ return ERR_PTR(-EINVAL);
+
+ if (!resource_contains(&priv->res, &tmp)) {
+ dev_err(dev,
+ FW_BUG "TPM2 ACPI sub resource %pR is not in the device's region of %pR\n",
+ &tmp, &priv->res);
+ return ERR_PTR(-EINVAL);
+ }
+
+ return priv->iobase + (tmp.start - priv->res.start);
+}
+
+static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
+ struct acpi_table_tpm2 *buf)
+{
+ struct list_head resources;
+ struct device *dev = &device->dev;
+ u64 pa;
+ int ret;
+
+ INIT_LIST_HEAD(&resources);
+ ret = acpi_dev_get_resources(device, &resources, crb_check_resource,
+ priv);
+ if (ret < 0)
+ return ret;
+ acpi_dev_free_resource_list(&resources);
+
+ if (resource_type(&priv->res) != IORESOURCE_MEM) {
+ dev_err(dev,
+ FW_BUG "TPM2 ACPI table does not define a memory resource\n");
+ return -EINVAL;
+ }
+
+ priv->iobase = devm_ioremap_resource(dev, &priv->res);
+ if (IS_ERR(priv->iobase))
+ return PTR_ERR(priv->iobase);
+
+ priv->cca = crb_access(dev, priv, buf->control_address, 0x1000);
+ if (IS_ERR(priv->cca))
+ return PTR_ERR(priv->cca);
+
+ pa = ((u64)ioread32(&priv->cca->cmd_pa_high) << 32) |
+ (u64)ioread32(&priv->cca->cmd_pa_low);
+ priv->cmd = crb_access(dev, priv, pa, ioread32(&priv->cca->cmd_size));
+ if (IS_ERR(priv->cmd))
+ return PTR_ERR(priv->cmd);
+
+ memcpy_fromio(&pa, &priv->cca->rsp_pa, 8);
+ pa = le64_to_cpu(pa);
+ priv->rsp = crb_access(dev, priv, pa, ioread32(&priv->cca->rsp_size));
+ if (IS_ERR(priv->rsp))
+ return PTR_ERR(priv->rsp);
+ return 0;
+}
+
+static int crb_acpi_add(struct acpi_device *device)
+{
struct acpi_table_tpm2 *buf;
struct crb_priv *priv;
struct device *dev = &device->dev;
acpi_status status;
u32 sm;
- u64 pa;
int rc;
status = acpi_get_table(ACPI_SIG_TPM2, 1,
(struct acpi_table_header **) &buf);
if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
- return -ENODEV;
+ return -EINVAL;
}
/* Should the FIFO driver handle this? */
@@ -219,18 +320,9 @@ static int crb_acpi_add(struct acpi_device *device)
if (sm == ACPI_TPM2_MEMORY_MAPPED)
return -ENODEV;
- chip = tpmm_chip_alloc(dev, &tpm_crb);
- if (IS_ERR(chip))
- return PTR_ERR(chip);
-
- chip->flags = TPM_CHIP_FLAG_TPM2;
-
- priv = (struct crb_priv *) devm_kzalloc(dev, sizeof(struct crb_priv),
- GFP_KERNEL);
- if (!priv) {
- dev_err(dev, "failed to devm_kzalloc for private data\n");
+ priv = devm_kzalloc(dev, sizeof(struct crb_priv), GFP_KERNEL);
+ if (!priv)
return -ENOMEM;
- }
/* 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
@@ -244,44 +336,11 @@ static int crb_acpi_add(struct acpi_device *device)
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_address, 0x1000);
- if (!priv->cca) {
- dev_err(dev, "ioremap of the control area failed\n");
- return -ENOMEM;
- }
-
- pa = ((u64)ioread32(&priv->cca->cmd_pa_high) << 32) |
- (u64)ioread32(&priv->cca->cmd_pa_low);
- priv->cmd =
- devm_ioremap_nocache(dev, pa, ioread32(&priv->cca->cmd_size));
- if (!priv->cmd) {
- dev_err(dev, "ioremap of the command buffer failed\n");
- return -ENOMEM;
- }
-
- memcpy_fromio(&pa, &priv->cca->rsp_pa, 8);
- pa = le64_to_cpu(pa);
- priv->rsp =
- devm_ioremap_nocache(dev, pa, ioread32(&priv->cca->rsp_size));
- if (!priv->rsp) {
- dev_err(dev, "ioremap of the response buffer failed\n");
- return -ENOMEM;
- }
-
- chip->vendor.priv = priv;
-
- rc = tpm_get_timeouts(chip);
- if (rc)
- return rc;
-
- chip->acpi_dev_handle = device->handle;
-
- rc = tpm2_do_selftest(chip);
+ rc = crb_map_io(device, priv, buf);
if (rc)
return rc;
- return tpm_chip_register(chip);
+ return crb_init(device, priv);
}
static int crb_acpi_remove(struct acpi_device *device)
--
2.1.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 7/7] tpm_crb: Use devm_ioremap_resource
2016-01-08 0:36 ` Jason Gunthorpe
(?)
@ 2016-01-08 2:04 ` kbuild test robot
[not found] ` <1452213386-21460-8-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
-1 siblings, 1 reply; 29+ messages in thread
From: kbuild test robot @ 2016-01-08 2:04 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: kbuild-all, Jarkko Sakkinen, tpmdd-devel, linux-kernel,
Peter Huewe
Hi Jason,
[auto build test WARNING on next-20160107]
[cannot apply to char-misc/char-misc-testing v4.4-rc8 v4.4-rc7 v4.4-rc6 v4.4-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Jason-Gunthorpe/tpm_tis-Clean-up-force-module-parameter/20160108-084227
coccinelle warnings: (new ones prefixed by >>)
>> drivers/char/tpm/tpm_crb.c:297:1-3: WARNING: PTR_ERR_OR_ZERO can be used
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 29+ messages in thread