Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1304071 > unrolled thread

[PATCH v4 0/7] tpm_tis: Clean up force module parameter

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/7] tpm_tis: Clean up force module parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-01-08 01:40 +0100
    [PATCH v4 4/7] tpm_tis: Use devm_ioremap_resource Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-01-08 01:40 +0100
    Re: [PATCH v4 0/7] tpm_tis: Clean up force module parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-01-08 16:20 +0100

#1304071 — [PATCH v4 0/7] tpm_tis: Clean up force module parameter

Drive the force=1 flow through the driver core. There are two main reasons to do this:
 1) To enable tpm_tis for OF environments requires a platform_device anyhow, so
    the force_device needs to be re-used for them.
 2) Recent changes in the core code break the assumption that a driver will be
    'attached' to things created through platform_device_register_simple,
    which causes the tpm core to blow up.

To make force probing reliable this also fixes both tpm_tis and tpm_crb to
properly use request_region to lock the TPM iomemory against multiple access.

v4:
- Alter the commit message for using the common ACPI definitions (Jarkko)
- Move the misplaced error check hunk from patch #4 to #3 (Jarkko)

v3:
- Fix some bugs in getting the struct resource for tpm_tis (Martin Wilck)
- Include tpm_crb in the request_resource cleanup as well, tpm_tis and tpm_crb
  tend to use the same address ranges so both should have locking for safety
- ACPI and endianness cleanups in both drivers

v2:
 - Make sure we request the mem resource in tpm_tis to avoid double-loading
   the driver
 - Re-order the init sequence so that a forced platform device gets first crack at
   loading, and excludes the other mechanisms via the above
 - Checkpatch clean
 - Gotos renamed

Jason Gunthorpe (7):
  tpm_crb: Use the common ACPI definition of struct acpi_tpm2
  tpm_tis: Disable interrupt auto probing on a per-device basis
  tpm_tis: Do not fall back to a hardcoded address for TPM2
  tpm_tis: Use devm_ioremap_resource
  tpm_tis: Clean up the force=1 module parameter
  tpm_crb: Drop le32_to_cpu(ioread32(..))
  tpm_crb: Use devm_ioremap_resource

 drivers/char/tpm/tpm.h     |   7 --
 drivers/char/tpm/tpm_crb.c | 196 +++++++++++++++++++++-------------
 drivers/char/tpm/tpm_tis.c | 254 +++++++++++++++++++++++++--------------------
 3 files changed, 264 insertions(+), 193 deletions(-)

-- 
2.1.4

[toc] | [next] | [standalone]

#1304072 — [PATCH v4 4/7] tpm_tis: Use devm_ioremap_resource

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

[toc] | [prev] | [next] | [standalone]

#1304641

On Thu, Jan 07, 2016 at 05:36:19PM -0700, Jason Gunthorpe wrote:
> Drive the force=1 flow through the driver core. There are two main reasons to do this:
>  1) To enable tpm_tis for OF environments requires a platform_device anyhow, so
>     the force_device needs to be re-used for them.
>  2) Recent changes in the core code break the assumption that a driver will be
>     'attached' to things created through platform_device_register_simple,
>     which causes the tpm core to blow up.
> 
> To make force probing reliable this also fixes both tpm_tis and tpm_crb to
> properly use request_region to lock the TPM iomemory against multiple access.

Applied. Thank you.

/Jarkko

> 
> v4:
> - Alter the commit message for using the common ACPI definitions (Jarkko)
> - Move the misplaced error check hunk from patch #4 to #3 (Jarkko)
> 
> v3:
> - Fix some bugs in getting the struct resource for tpm_tis (Martin Wilck)
> - Include tpm_crb in the request_resource cleanup as well, tpm_tis and tpm_crb
>   tend to use the same address ranges so both should have locking for safety
> - ACPI and endianness cleanups in both drivers
> 
> v2:
>  - Make sure we request the mem resource in tpm_tis to avoid double-loading
>    the driver
>  - Re-order the init sequence so that a forced platform device gets first crack at
>    loading, and excludes the other mechanisms via the above
>  - Checkpatch clean
>  - Gotos renamed
> 
> Jason Gunthorpe (7):
>   tpm_crb: Use the common ACPI definition of struct acpi_tpm2
>   tpm_tis: Disable interrupt auto probing on a per-device basis
>   tpm_tis: Do not fall back to a hardcoded address for TPM2
>   tpm_tis: Use devm_ioremap_resource
>   tpm_tis: Clean up the force=1 module parameter
>   tpm_crb: Drop le32_to_cpu(ioread32(..))
>   tpm_crb: Use devm_ioremap_resource
> 
>  drivers/char/tpm/tpm.h     |   7 --
>  drivers/char/tpm/tpm_crb.c | 196 +++++++++++++++++++++-------------
>  drivers/char/tpm/tpm_tis.c | 254 +++++++++++++++++++++++++--------------------
>  3 files changed, 264 insertions(+), 193 deletions(-)
> 
> -- 
> 2.1.4
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web