diff mbox

[tpmdd-devel,v4,7/7] tpm_crb: Use devm_ioremap_resource

Message ID 1452213386-21460-8-git-send-email-jgunthorpe@obsidianresearch.com
State New
Headers show

Commit Message

Jason Gunthorpe Jan. 8, 2016, 12:36 a.m. UTC 
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(-)

Comments

kernel test robot Jan. 8, 2016, 2:04 a.m. UTC | #1 
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

------------------------------------------------------------------------------
Jarkko Sakkinen Jan. 8, 2016, noon UTC | #2 
On Thu, Jan 07, 2016 at 05:36:26PM -0700, Jason Gunthorpe wrote:
> 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>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

PS. There's one cocci warning. I'll amend the fixup for that. Thanks for
the good work!

/Jarkko

> ---
>  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
> 

------------------------------------------------------------------------------
Jarkko Sakkinen Jan. 8, 2016, 12:02 p.m. UTC | #3 
On Thu, Jan 07, 2016 at 05:36:26PM -0700, Jason Gunthorpe wrote:
> 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>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko
> ---
>  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
> 

------------------------------------------------------------------------------
diff mbox

Patch

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)