Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753596AbcCWGUu (ORCPT ); Wed, 23 Mar 2016 02:20:50 -0400 Received: from mga11.intel.com ([192.55.52.93]:23763 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750861AbcCWGUn (ORCPT ); Wed, 23 Mar 2016 02:20:43 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,381,1455004800"; d="scan'208";a="939721235" Date: Wed, 23 Mar 2016 08:20:28 +0200 From: Jarkko Sakkinen To: Peter Huewe Cc: Marcel Selhorst , Jason Gunthorpe , "moderated list:TPM DEVICE DRIVER" , open list Subject: Re: [PATCH] tpm: drop 'base' from struct tpm_vendor_specific Message-ID: <20160323062028.GA15193@intel.com> References: <1458713769-11800-1-git-send-email-jarkko.sakkinen@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1458713769-11800-1-git-send-email-jarkko.sakkinen@linux.intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9076 Lines: 274 On Wed, Mar 23, 2016 at 08:16:09AM +0200, Jarkko Sakkinen wrote: > Dropped the field 'base' from struct tpm_vendor_specific and migrated > it to the private structures of tpm_atmel and tpm_nsc. > > Signed-off-by: Jarkko Sakkinen Ugh, forgot the final step: removing the field from the struct. Otherwise, this should be good (at least from my point of view). /Jarkko > --- > drivers/char/tpm/tpm_atmel.c | 6 ++--- > drivers/char/tpm/tpm_atmel.h | 7 ++++-- > drivers/char/tpm/tpm_nsc.c | 57 +++++++++++++++++++++++++++++--------------- > 3 files changed, 46 insertions(+), 24 deletions(-) > > diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c > index affc04b..c6daf6c 100644 > --- a/drivers/char/tpm/tpm_atmel.c > +++ b/drivers/char/tpm/tpm_atmel.c > @@ -36,6 +36,7 @@ enum tpm_atmel_read_status { > }; > > struct tpm_atmel_priv { > + unsigned long base; > int region_size; > int have_region; > }; > @@ -146,8 +147,7 @@ static void atml_plat_remove(void) > if (chip) { > tpm_chip_unregister(chip); > if (priv->have_region) > - atmel_release_region(chip->vendor.base, > - priv->region_size); > + atmel_release_region(priv->base, priv->region_size); > atmel_put_base_addr(chip->vendor.iobase); > platform_device_unregister(pdev); > } > @@ -196,6 +196,7 @@ static int __init init_atmel(void) > goto err_unreg_dev; > } > > + priv->base = base; > priv->have_region = have_region; > priv->region_size = region_size; > > @@ -206,7 +207,6 @@ static int __init init_atmel(void) > } > > chip->vendor.iobase = iobase; > - chip->vendor.base = base; > chip->vendor.priv = priv; > > rc = tpm_chip_register(chip); > diff --git a/drivers/char/tpm/tpm_atmel.h b/drivers/char/tpm/tpm_atmel.h > index 6c831f9..08607a6 100644 > --- a/drivers/char/tpm/tpm_atmel.h > +++ b/drivers/char/tpm/tpm_atmel.h > @@ -22,6 +22,8 @@ > * > */ > > +#define atmel_get_priv(chip) ((struct tpm_atmel_priv *) chip->vendor.priv) > + > #ifdef CONFIG_PPC64 > > #include > @@ -78,8 +80,9 @@ static void __iomem * atmel_get_base_addr(unsigned long *base, int *region_size) > return ioremap(*base, *region_size); > } > #else > -#define atmel_getb(chip, offset) inb(chip->vendor->base + offset) > -#define atmel_putb(val, chip, offset) outb(val, chip->vendor->base + offset) > +#define atmel_getb(chip, offset) inb(atmel_get_priv(chip)->base + offset) > +#define atmel_putb(val, chip, offset) \ > + outb(val, atmel_get_priv(chip)->base + offset) > #define atmel_request_region request_region > #define atmel_release_region release_region > /* Atmel definitions */ > diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c > index 766370b..8ccb1d5 100644 > --- a/drivers/char/tpm/tpm_nsc.c > +++ b/drivers/char/tpm/tpm_nsc.c > @@ -64,6 +64,13 @@ enum tpm_nsc_cmd_mode { > NSC_COMMAND_EOC = 0x03, > NSC_COMMAND_CANCEL = 0x22 > }; > + > +struct tpm_nsc_priv { > + unsigned long base; > +}; > + > +#define tpm_nsc_get_priv(chip) ((struct tpm_nsc_priv *) chip->vendor.priv) > + > /* > * Wait for a certain status to appear > */ > @@ -72,7 +79,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 mask, u8 val, u8 * data) > unsigned long stop; > > /* status immediately available check */ > - *data = inb(chip->vendor.base + NSC_STATUS); > + *data = inb(tpm_nsc_get_priv(chip)->base + NSC_STATUS); > if ((*data & mask) == val) > return 0; > > @@ -80,7 +87,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 mask, u8 val, u8 * data) > stop = jiffies + 10 * HZ; > do { > msleep(TPM_TIMEOUT); > - *data = inb(chip->vendor.base + 1); > + *data = inb(tpm_nsc_get_priv(chip)->base + 1); > if ((*data & mask) == val) > return 0; > } > @@ -95,9 +102,9 @@ static int nsc_wait_for_ready(struct tpm_chip *chip) > unsigned long stop; > > /* status immediately available check */ > - status = inb(chip->vendor.base + NSC_STATUS); > + status = inb(tpm_nsc_get_priv(chip)->base + NSC_STATUS); > if (status & NSC_STATUS_OBF) > - status = inb(chip->vendor.base + NSC_DATA); > + status = inb(tpm_nsc_get_priv(chip)->base + NSC_DATA); > if (status & NSC_STATUS_RDY) > return 0; > > @@ -105,9 +112,9 @@ static int nsc_wait_for_ready(struct tpm_chip *chip) > stop = jiffies + 100; > do { > msleep(TPM_TIMEOUT); > - status = inb(chip->vendor.base + NSC_STATUS); > + status = inb(tpm_nsc_get_priv(chip)->base + NSC_STATUS); > if (status & NSC_STATUS_OBF) > - status = inb(chip->vendor.base + NSC_DATA); > + status = inb(tpm_nsc_get_priv(chip)->base + NSC_DATA); > if (status & NSC_STATUS_RDY) > return 0; > } > @@ -132,8 +139,9 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count) > dev_err(&chip->dev, "F0 timeout\n"); > return -EIO; > } > - if ((data = > - inb(chip->vendor.base + NSC_DATA)) != NSC_COMMAND_NORMAL) { > + > + data = inb(tpm_nsc_get_priv(chip)->base + NSC_DATA); > + if (data != NSC_COMMAND_NORMAL) { > dev_err(&chip->dev, "not in normal mode (0x%x)\n", > data); > return -EIO; > @@ -149,7 +157,7 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count) > } > if (data & NSC_STATUS_F0) > break; > - *p = inb(chip->vendor.base + NSC_DATA); > + *p = inb(tpm_nsc_get_priv(chip)->base + NSC_DATA); > } > > if ((data & NSC_STATUS_F0) == 0 && > @@ -157,7 +165,9 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count) > dev_err(&chip->dev, "F0 not set\n"); > return -EIO; > } > - if ((data = inb(chip->vendor.base + NSC_DATA)) != NSC_COMMAND_EOC) { > + > + data = inb(tpm_nsc_get_priv(chip)->base + NSC_DATA); > + if (data != NSC_COMMAND_EOC) { > dev_err(&chip->dev, > "expected end of command(0x%x)\n", data); > return -EIO; > @@ -183,7 +193,7 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count) > * fix it. Not sure why this is needed, we followed the flow > * chart in the manual to the letter. > */ > - outb(NSC_COMMAND_CANCEL, chip->vendor.base + NSC_COMMAND); > + outb(NSC_COMMAND_CANCEL, tpm_nsc_get_priv(chip)->base + NSC_COMMAND); > > if (nsc_wait_for_ready(chip) != 0) > return -EIO; > @@ -193,7 +203,7 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count) > return -EIO; > } > > - outb(NSC_COMMAND_NORMAL, chip->vendor.base + NSC_COMMAND); > + outb(NSC_COMMAND_NORMAL, tpm_nsc_get_priv(chip)->base + NSC_COMMAND); > if (wait_for_stat(chip, NSC_STATUS_IBR, NSC_STATUS_IBR, &data) < 0) { > dev_err(&chip->dev, "IBR timeout\n"); > return -EIO; > @@ -205,26 +215,26 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count) > "IBF timeout (while writing data)\n"); > return -EIO; > } > - outb(buf[i], chip->vendor.base + NSC_DATA); > + outb(buf[i], tpm_nsc_get_priv(chip)->base + NSC_DATA); > } > > if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) { > dev_err(&chip->dev, "IBF timeout\n"); > return -EIO; > } > - outb(NSC_COMMAND_EOC, chip->vendor.base + NSC_COMMAND); > + outb(NSC_COMMAND_EOC, tpm_nsc_get_priv(chip)->base + NSC_COMMAND); > > return count; > } > > static void tpm_nsc_cancel(struct tpm_chip *chip) > { > - outb(NSC_COMMAND_CANCEL, chip->vendor.base + NSC_COMMAND); > + outb(NSC_COMMAND_CANCEL, tpm_nsc_get_priv(chip)->base + NSC_COMMAND); > } > > static u8 tpm_nsc_status(struct tpm_chip *chip) > { > - return inb(chip->vendor.base + NSC_STATUS); > + return inb(tpm_nsc_get_priv(chip)->base + NSC_STATUS); > } > > static bool tpm_nsc_req_canceled(struct tpm_chip *chip, u8 status) > @@ -249,7 +259,7 @@ static void tpm_nsc_remove(struct device *dev) > struct tpm_chip *chip = dev_get_drvdata(dev); > > tpm_chip_unregister(chip); > - release_region(chip->vendor.base, 2); > + release_region(tpm_nsc_get_priv(chip)->base, 2); > } > > static SIMPLE_DEV_PM_OPS(tpm_nsc_pm, tpm_pm_suspend, tpm_pm_resume); > @@ -268,6 +278,7 @@ static int __init init_nsc(void) > int nscAddrBase = TPM_ADDR; > struct tpm_chip *chip; > unsigned long base; > + struct tpm_nsc_priv *priv; > > /* verify that it is a National part (SID) */ > if (tpm_read_index(TPM_ADDR, NSC_SID_INDEX) != 0xEF) { > @@ -301,6 +312,14 @@ static int __init init_nsc(void) > if ((rc = platform_device_add(pdev)) < 0) > goto err_put_dev; > > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) { > + rc = -ENOMEM; > + goto err_del_dev; > + } > + > + priv->base = base; > + > if (request_region(base, 2, "tpm_nsc0") == NULL ) { > rc = -EBUSY; > goto err_del_dev; > @@ -312,6 +331,8 @@ static int __init init_nsc(void) > goto err_rel_reg; > } > > + chip->vendor.priv = priv; > + > rc = tpm_chip_register(chip); > if (rc) > goto err_rel_reg; > @@ -349,8 +370,6 @@ static int __init init_nsc(void) > "NSC TPM revision %d\n", > tpm_read_index(nscAddrBase, 0x27) & 0x1F); > > - chip->vendor.base = base; > - > return 0; > > err_rel_reg: > -- > 2.7.3 >