[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <56E8AC68.7040602@linux.vnet.ibm.com>
Date: Tue, 15 Mar 2016 20:44:24 -0400
From: Stefan Berger <stefanb@...ux.vnet.ibm.com>
To: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
Peter Huewe <peterhuewe@....de>
Cc: tpmdd-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [tpmdd-devel] [PATCH] tpm_crb: drop struct resource res from
struct crb_priv
On 03/15/2016 05:36 PM, Jarkko Sakkinen wrote:
> The iomem resource is needed only temporarily so it is better to pass
> it on instead of storing it permanently. Named the variable as io_res
> so that the code better documents itself.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Reviewed-by: Stefan Berger <stefanb@...ux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm_crb.c | 29 ++++++++++++++++-------------
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index a12b319..8e02c44 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -77,7 +77,6 @@ enum crb_flags {
>
> struct crb_priv {
> unsigned int flags;
> - struct resource res;
> void __iomem *iobase;
> struct crb_control_area __iomem *cca;
> u8 __iomem *cmd;
> @@ -224,19 +223,19 @@ static int crb_init(struct acpi_device *device, struct crb_priv *priv)
>
> static int crb_check_resource(struct acpi_resource *ares, void *data)
> {
> - struct crb_priv *priv = data;
> + struct resource *io_res = data;
> struct resource res;
>
> if (acpi_dev_resource_memory(ares, &res)) {
> - priv->res = res;
> - priv->res.name = NULL;
> + *io_res = res;
> + io_res->name = NULL;
> }
>
> return 1;
> }
>
> static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
> - u64 start, u32 size)
> + struct resource *io_res, u64 start, u32 size)
> {
> struct resource new_res = {
> .start = start,
> @@ -248,50 +247,54 @@ static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
> if (start != new_res.start)
> return ERR_PTR(-EINVAL);
>
> - if (!resource_contains(&priv->res, &new_res))
> + if (!resource_contains(io_res, &new_res))
> return devm_ioremap_resource(dev, &new_res);
>
> - return priv->iobase + (new_res.start - priv->res.start);
> + return priv->iobase + (new_res.start - io_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 resource io_res;
> struct device *dev = &device->dev;
> u64 pa;
> int ret;
>
> INIT_LIST_HEAD(&resources);
> ret = acpi_dev_get_resources(device, &resources, crb_check_resource,
> - priv);
> + &io_res);
> if (ret < 0)
> return ret;
> acpi_dev_free_resource_list(&resources);
>
> - if (resource_type(&priv->res) != IORESOURCE_MEM) {
> + if (resource_type(&io_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);
> + priv->iobase = devm_ioremap_resource(dev, &io_res);
> if (IS_ERR(priv->iobase))
> return PTR_ERR(priv->iobase);
>
> - priv->cca = crb_map_res(dev, priv, buf->control_address, 0x1000);
> + priv->cca = crb_map_res(dev, priv, &io_res, 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_map_res(dev, priv, pa, ioread32(&priv->cca->cmd_size));
> + priv->cmd = crb_map_res(dev, priv, &io_res, 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_map_res(dev, priv, pa, ioread32(&priv->cca->rsp_size));
> + priv->rsp = crb_map_res(dev, priv, &io_res, pa,
> + ioread32(&priv->cca->rsp_size));
> return PTR_ERR_OR_ZERO(priv->rsp);
> }
>
Powered by blists - more mailing lists