diff mbox series

[1/1] char: tpm: handle HAS_IOPORT dependencies

Message ID 20240404105840.3396821-2-schnelle@linux.ibm.com (mailing list archive)
State New
Headers show
Series char: tpm: Handle HAS_IOPORT dependencies | expand

Commit Message

Niklas Schnelle April 4, 2024, 10:58 a.m. UTC 
In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add this dependency and ifdef sections of
code using inb()/outb() as alternative access methods.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
Note: This patch does not depend any not-yet-mainline HAS_IOPORT changes
and may be merged via subsystem specific trees at your earliest
convenience.

 drivers/char/tpm/Kconfig        |  1 +
 drivers/char/tpm/tpm_infineon.c | 16 ++++++++++++----
 drivers/char/tpm/tpm_tis_core.c | 19 ++++++++-----------
 3 files changed, 21 insertions(+), 15 deletions(-)

Comments

Arnd Bergmann April 4, 2024, 11:17 a.m. UTC | #1 
On Thu, Apr 4, 2024, at 12:58, Niklas Schnelle wrote:
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 9c924a1440a9..99c6e565ec8d 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -26,7 +26,9 @@
>  #define	TPM_MAX_TRIES		5000
>  #define	TPM_INFINEON_DEV_VEN_VALUE	0x15D1
> 
> +#ifdef CONFIG_HAS_IOPORT
>  #define TPM_INF_IO_PORT		0x0
> +#endif
>  #define TPM_INF_IO_MEM		0x1

I think hiding this definition in this version of a patch
results in a build failure because of the assignment that
you are not stubbing out:

        /* read IO-ports through PnP */
        if (pnp_port_valid(dev, 0) && pnp_port_valid(dev, 1) &&
            !(pnp_port_flags(dev, 0) & IORESOURCE_DISABLED)) {
                tpm_dev.iotype = TPM_INF_IO_PORT;

I don't know what changed since the earlier versions I tested,
or if I just missed it, but I think you either have to remove
the #ifdef above or add another one in tpm_inf_pnp_probe().

    Arnd
Jarkko Sakkinen April 4, 2024, 3:19 p.m. UTC | #2 
On Thu Apr 4, 2024 at 1:58 PM EEST, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
> compile time. We thus need to add this dependency and ifdef sections of
> code using inb()/outb() as alternative access methods.
>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
> Note: This patch does not depend any not-yet-mainline HAS_IOPORT changes
> and may be merged via subsystem specific trees at your earliest
> convenience.

Thanks I applied it.

BR, Jarkko
Jarkko Sakkinen April 4, 2024, 3:22 p.m. UTC | #3 
On Thu Apr 4, 2024 at 2:17 PM EEST, Arnd Bergmann wrote:
> On Thu, Apr 4, 2024, at 12:58, Niklas Schnelle wrote:
> > diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> > index 9c924a1440a9..99c6e565ec8d 100644
> > --- a/drivers/char/tpm/tpm_infineon.c
> > +++ b/drivers/char/tpm/tpm_infineon.c
> > @@ -26,7 +26,9 @@
> >  #define	TPM_MAX_TRIES		5000
> >  #define	TPM_INFINEON_DEV_VEN_VALUE	0x15D1
> > 
> > +#ifdef CONFIG_HAS_IOPORT
> >  #define TPM_INF_IO_PORT		0x0
> > +#endif
> >  #define TPM_INF_IO_MEM		0x1
>
> I think hiding this definition in this version of a patch
> results in a build failure because of the assignment that
> you are not stubbing out:
>
>         /* read IO-ports through PnP */
>         if (pnp_port_valid(dev, 0) && pnp_port_valid(dev, 1) &&
>             !(pnp_port_flags(dev, 0) & IORESOURCE_DISABLED)) {
>                 tpm_dev.iotype = TPM_INF_IO_PORT;
>
> I don't know what changed since the earlier versions I tested,
> or if I just missed it, but I think you either have to remove
> the #ifdef above or add another one in tpm_inf_pnp_probe().
>
>     Arnd

Thanks for the remark. I placed the current patch to my master branch
which is not too critical ('next' is mirrored to linux-next) if anyone
wants to try it out:

git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git

I can repeal and replace it with a newer one later on.

BR, Jarkko
Niklas Schnelle April 4, 2024, 3:41 p.m. UTC | #4 
On Thu, 2024-04-04 at 13:17 +0200, Arnd Bergmann wrote:
> On Thu, Apr 4, 2024, at 12:58, Niklas Schnelle wrote:
> > diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> > index 9c924a1440a9..99c6e565ec8d 100644
> > --- a/drivers/char/tpm/tpm_infineon.c
> > +++ b/drivers/char/tpm/tpm_infineon.c
> > @@ -26,7 +26,9 @@
> >  #define	TPM_MAX_TRIES		5000
> >  #define	TPM_INFINEON_DEV_VEN_VALUE	0x15D1
> > 
> > +#ifdef CONFIG_HAS_IOPORT
> >  #define TPM_INF_IO_PORT		0x0
> > +#endif
> >  #define TPM_INF_IO_MEM		0x1
> 
> I think hiding this definition in this version of a patch
> results in a build failure because of the assignment that
> you are not stubbing out:
> 
>         /* read IO-ports through PnP */
>         if (pnp_port_valid(dev, 0) && pnp_port_valid(dev, 1) &&
>             !(pnp_port_flags(dev, 0) & IORESOURCE_DISABLED)) {
>                 tpm_dev.iotype = TPM_INF_IO_PORT;
> 
> I don't know what changed since the earlier versions I tested,
> or if I just missed it, but I think you either have to remove
> the #ifdef above or add another one in tpm_inf_pnp_probe().
> 
>     Arnd

Good find! I do see the same #ifdef in v5 but maybe something else
changed. I think this was also hidden during both my local test builds
and kernel test robot because of the PNP -> ACPI || ISA dependency
which I think implies HAS_IOPORT. So unless I'm missing something we
can't currently get here with HAS_IOPORT=n. Maybe that changed?

I'm now thinking maybe keeping TPM_INF_IO_PORT is the cleaner choice.
It saves us 4 lines of #ifdeffery at the cost of one sometimes "unused"
define.

Thanks,
Niklas
Arnd Bergmann April 4, 2024, 3:56 p.m. UTC | #5 
On Thu, Apr 4, 2024, at 17:41, Niklas Schnelle wrote:
>
> Good find! I do see the same #ifdef in v5 but maybe something else
> changed. I think this was also hidden during both my local test builds
> and kernel test robot because of the PNP -> ACPI || ISA dependency
> which I think implies HAS_IOPORT. So unless I'm missing something we
> can't currently get here with HAS_IOPORT=n. Maybe that changed?

Rihgt, I just found that as well, so the TCG_INFINEON driver
won't ever be enabled without CONFIG_HAS_IOPORT after all.

> I'm now thinking maybe keeping TPM_INF_IO_PORT is the cleaner choice.
> It saves us 4 lines of #ifdeffery at the cost of one sometimes "unused"
> define.

Agreed. I tried it out for reference, but it does get quite ugly,
see below. Alternatively, we could just add a 'depends on HAS_IOPORT'
to this driver after all. Even if it can be used on MMIO, it might
never actually be built without PIO.

     Arnd

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 418c9ed59ffd..852bb9344788 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -157,7 +157,7 @@ config TCG_ATMEL
 
 config TCG_INFINEON
 	tristate "Infineon Technologies TPM Interface"
-	depends on PNP
+	depends on PNP || COMPILE_TEST
 	help
 	  If you have a TPM security chip from Infineon Technologies
 	  (either SLD 9630 TT 1.1 or SLB 9635 TT 1.2) say Yes and it
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 99c6e565ec8d..768ca65960d8 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -51,10 +51,19 @@ struct tpm_inf_dev {
 
 static struct tpm_inf_dev tpm_dev;
 
+static inline bool tpm_is_ioport(struct tpm_inf_dev *dev)
+{
+#ifdef CONFIG_HAS_IOPORT
+	return tpm_dev.iotype == TPM_INF_IO_PORT;
+#else
+	return false;
+#endif
+}
+
 static inline void tpm_data_out(unsigned char data, unsigned char offset)
 {
 #ifdef CONFIG_HAS_IOPORT
-	if (tpm_dev.iotype == TPM_INF_IO_PORT)
+	if (tpm_is_ioport(&tpm_dev))
 		outb(data, tpm_dev.data_regs + offset);
 	else
 #endif
@@ -64,7 +73,7 @@ static inline void tpm_data_out(unsigned char data, unsigned char offset)
 static inline unsigned char tpm_data_in(unsigned char offset)
 {
 #ifdef CONFIG_HAS_IOPORT
-	if (tpm_dev.iotype == TPM_INF_IO_PORT)
+	if (tpm_is_ioport(&tpm_dev))
 		return inb(tpm_dev.data_regs + offset);
 #endif
 	return readb(tpm_dev.mem_base + tpm_dev.data_regs + offset);
@@ -73,7 +82,7 @@ static inline unsigned char tpm_data_in(unsigned char offset)
 static inline void tpm_config_out(unsigned char data, unsigned char offset)
 {
 #ifdef CONFIG_HAS_IOPORT
-	if (tpm_dev.iotype == TPM_INF_IO_PORT)
+	if (tpm_is_ioport(&tpm_dev))
 		outb(data, tpm_dev.config_port + offset);
 	else
 #endif
@@ -83,7 +92,7 @@ static inline void tpm_config_out(unsigned char data, unsigned char offset)
 static inline unsigned char tpm_config_in(unsigned char offset)
 {
 #ifdef CONFIG_HAS_IOPORT
-	if (tpm_dev.iotype == TPM_INF_IO_PORT)
+	if (tpm_is_ioport(&tpm_dev))
 		return inb(tpm_dev.config_port + offset);
 #endif
 	return readb(tpm_dev.mem_base + tpm_dev.index_off + offset);
@@ -404,6 +413,7 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
 	const char *chipname;
 	struct tpm_chip *chip;
 
+#ifdef CONFIG_HAS_IOPORT
 	/* read IO-ports through PnP */
 	if (pnp_port_valid(dev, 0) && pnp_port_valid(dev, 1) &&
 	    !(pnp_port_flags(dev, 0) & IORESOURCE_DISABLED)) {
@@ -436,8 +446,10 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
 			rc = -EINVAL;
 			goto err_last;
 		}
-	} else if (pnp_mem_valid(dev, 0) &&
-		   !(pnp_mem_flags(dev, 0) & IORESOURCE_DISABLED)) {
+	} else
+#endif
+	if (pnp_mem_valid(dev, 0) &&
+	   !(pnp_mem_flags(dev, 0) & IORESOURCE_DISABLED)) {
 
 		tpm_dev.iotype = TPM_INF_IO_MEM;
 
@@ -540,10 +552,10 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
 			 "vendor id 0x%x%x (Infineon), "
 			 "product id 0x%02x%02x"
 			 "%s\n",
-			 tpm_dev.iotype == TPM_INF_IO_PORT ?
+			 tpm_is_ioport(&tpm_dev) ?
 			 tpm_dev.config_port :
 			 tpm_dev.map_base + tpm_dev.index_off,
-			 tpm_dev.iotype == TPM_INF_IO_PORT ?
+			 tpm_is_ioport(&tpm_dev) ?
 			 tpm_dev.data_regs :
 			 tpm_dev.map_base + tpm_dev.data_regs,
 			 version[0], version[1],
@@ -567,7 +579,7 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
 	}
 
 err_release_region:
-	if (tpm_dev.iotype == TPM_INF_IO_PORT) {
+	if (tpm_is_ioport(&tpm_dev)) {
 		release_region(tpm_dev.data_regs, tpm_dev.data_size);
 		release_region(tpm_dev.config_port, tpm_dev.config_size);
 	} else {
@@ -585,11 +597,14 @@ static void tpm_inf_pnp_remove(struct pnp_dev *dev)
 
 	tpm_chip_unregister(chip);
 
-	if (tpm_dev.iotype == TPM_INF_IO_PORT) {
+#ifdef CONFIG_HAS_IOPORT
+	if (tpm_is_ioport(&tpm_dev)) {
 		release_region(tpm_dev.data_regs, tpm_dev.data_size);
 		release_region(tpm_dev.config_port,
 			       tpm_dev.config_size);
-	} else {
+	} else
+#endif
+	{
 		iounmap(tpm_dev.mem_base);
 		release_mem_region(tpm_dev.map_base, tpm_dev.map_size);
 	}
Niklas Schnelle April 5, 2024, 9:23 a.m. UTC | #6 
On Thu, 2024-04-04 at 17:56 +0200, Arnd Bergmann wrote:
> On Thu, Apr 4, 2024, at 17:41, Niklas Schnelle wrote:
> > 
> > Good find! I do see the same #ifdef in v5 but maybe something else
> > changed. I think this was also hidden during both my local test builds
> > and kernel test robot because of the PNP -> ACPI || ISA dependency
> > which I think implies HAS_IOPORT. So unless I'm missing something we
> > can't currently get here with HAS_IOPORT=n. Maybe that changed?
> 
> Rihgt, I just found that as well, so the TCG_INFINEON driver
> won't ever be enabled without CONFIG_HAS_IOPORT after all.
> 
> > I'm now thinking maybe keeping TPM_INF_IO_PORT is the cleaner choice.
> > It saves us 4 lines of #ifdeffery at the cost of one sometimes "unused"
> > define.
> 
> Agreed. I tried it out for reference, but it does get quite ugly,
> see below. Alternatively, we could just add a 'depends on HAS_IOPORT'
> to this driver after all. Even if it can be used on MMIO, it might
> never actually be built without PIO.

Oh yeah thats an even bigger change than I thought if we want to remove
the TPM_INF_IO_PORT define for HAS_IOPORT=n. It's also still bit of an
arbitrary point to stop since we could just as well argue that struct
tpm_inf_dev::iotype then also shouldn't be there. I think one could
handle this still with a bit of regrouping but not sure if it is worth
it.

I just confirmed that keeping the define it also compiles but I do
wonder if it's not even cleaner to just add an explicit HAS_IOPORT
dependency and no new #ifdefs in the code. I'm willing to send a patch
for any of these solutions though.

> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 418c9ed59ffd..852bb9344788 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -157,7 +157,7 @@ config TCG_ATMEL
>  
>  config TCG_INFINEON
>  	tristate "Infineon Technologies TPM Interface"
> -	depends on PNP
> +	depends on PNP || COMPILE_TEST
>  	help
>  	  If you have a TPM security chip from Infineon Technologies
>  	  (either SLD 9630 TT 1.1 or SLB 9635 TT 1.2) say Yes and it
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 99c6e565ec8d..768ca65960d8 100644
> --- a/drivers/char/tpm/tpm_infineon.c
---8<---
Arnd Bergmann April 5, 2024, 9:02 p.m. UTC | #7 
On Fri, Apr 5, 2024, at 11:23, Niklas Schnelle wrote:
>
> I just confirmed that keeping the define it also compiles but I do
> wonder if it's not even cleaner to just add an explicit HAS_IOPORT
> dependency and no new #ifdefs in the code. I'm willing to send a patch
> for any of these solutions though.

It depends a bit on where the driver is used in the end. We
currently set HAS_IOPORT on arm64 and riscv, but we could make
that dependent on which PCI host drivers are actually being
built, as a lot of modern hardware doesn't actually support
port I/O.

Is this driver still expected to be used on modern PCIe hosts
with no port I/O, or would new machines all use the i2c version?

If we do need the driver in configurations without CONFIG_HAS_IOPORT,
then the patch below wouldn't be awful (on top of the patch that
got merged already).

     Arnd

diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 99c6e565ec8d..5b45ad619900 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -37,7 +37,8 @@
 struct tpm_inf_dev {
 	int iotype;
 
-	void __iomem *mem_base;	/* MMIO ioremap'd addr */
+	void __iomem *data_base;	/* MMIO ioremap'd addr */
+	void __iomem *config_base;	/* MMIO ioremap'd config */
 	unsigned long map_base;	/* phys MMIO base */
 	unsigned long map_size;	/* MMIO region size */
 	unsigned int index_off;	/* index register offset */
@@ -53,40 +54,22 @@ static struct tpm_inf_dev tpm_dev;
 
 static inline void tpm_data_out(unsigned char data, unsigned char offset)
 {
-#ifdef CONFIG_HAS_IOPORT
-	if (tpm_dev.iotype == TPM_INF_IO_PORT)
-		outb(data, tpm_dev.data_regs + offset);
-	else
-#endif
-		writeb(data, tpm_dev.mem_base + tpm_dev.data_regs + offset);
+	iowrite8(data, tpm_dev.data_base + offset);
 }
 
 static inline unsigned char tpm_data_in(unsigned char offset)
 {
-#ifdef CONFIG_HAS_IOPORT
-	if (tpm_dev.iotype == TPM_INF_IO_PORT)
-		return inb(tpm_dev.data_regs + offset);
-#endif
-	return readb(tpm_dev.mem_base + tpm_dev.data_regs + offset);
+	return ioread8(tpm_dev.data_base + offset);
 }
 
 static inline void tpm_config_out(unsigned char data, unsigned char offset)
 {
-#ifdef CONFIG_HAS_IOPORT
-	if (tpm_dev.iotype == TPM_INF_IO_PORT)
-		outb(data, tpm_dev.config_port + offset);
-	else
-#endif
-		writeb(data, tpm_dev.mem_base + tpm_dev.index_off + offset);
+	iowrite8(data, tpm_dev.config_base + offset);
 }
 
 static inline unsigned char tpm_config_in(unsigned char offset)
 {
-#ifdef CONFIG_HAS_IOPORT
-	if (tpm_dev.iotype == TPM_INF_IO_PORT)
-		return inb(tpm_dev.config_port + offset);
-#endif
-	return readb(tpm_dev.mem_base + tpm_dev.index_off + offset);
+	return ioread8(tpm_dev.config_base + offset);
 }
 
 /* TPM header definitions */
@@ -425,16 +408,27 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
 			goto err_last;
 		}
 		/* publish my base address and request region */
+		tpm_dev.data_base = ioport_map(tpm_dev.data_regs, tpm_dev.data_size);
+		if (!tpm_dev.data_base) {
+			rc = -EINVAL;
+			goto err_last;
+		}
 		if (request_region(tpm_dev.data_regs, tpm_dev.data_size,
 				   "tpm_infineon0") == NULL) {
 			rc = -EINVAL;
+			ioport_unmap(tpm_dev.config_base);
 			goto err_last;
 		}
+		tpm_dev.config_base = ioport_map(tpm_dev.config_port, tpm_dev.config_size);
+		if (!tpm_dev.config_base) {
+			rc = -EINVAL;
+			goto err_release_data_region;
+		}
 		if (request_region(tpm_dev.config_port, tpm_dev.config_size,
 				   "tpm_infineon0") == NULL) {
 			release_region(tpm_dev.data_regs, tpm_dev.data_size);
 			rc = -EINVAL;
-			goto err_last;
+			goto err_release_data_region;
 		}
 	} else if (pnp_mem_valid(dev, 0) &&
 		   !(pnp_mem_flags(dev, 0) & IORESOURCE_DISABLED)) {
@@ -454,8 +448,8 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
 			goto err_last;
 		}
 
-		tpm_dev.mem_base = ioremap(tpm_dev.map_base, tpm_dev.map_size);
-		if (tpm_dev.mem_base == NULL) {
+		tpm_dev.data_base = ioremap(tpm_dev.map_base, tpm_dev.map_size);
+		if (tpm_dev.data_base == NULL) {
 			release_mem_region(tpm_dev.map_base, tpm_dev.map_size);
 			rc = -EINVAL;
 			goto err_last;
@@ -468,8 +462,7 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
 		 * seem like they could be placed anywhere within the MMIO
 		 * region, but lets just put them at zero offset.
 		 */
-		tpm_dev.index_off = TPM_ADDR;
-		tpm_dev.data_regs = 0x0;
+		tpm_dev.config_base = tpm_dev.data_base + TPM_ADDR;
 	} else {
 		rc = -EINVAL;
 		goto err_last;
@@ -568,10 +561,16 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
 
 err_release_region:
 	if (tpm_dev.iotype == TPM_INF_IO_PORT) {
-		release_region(tpm_dev.data_regs, tpm_dev.data_size);
+		ioport_unmap(tpm_dev.config_base);
 		release_region(tpm_dev.config_port, tpm_dev.config_size);
+	}
+
+err_release_data_region:
+	if (tpm_dev.iotype == TPM_INF_IO_PORT) {
+		ioport_unmap(tpm_dev.data_base);
+		release_region(tpm_dev.data_regs, tpm_dev.data_size);
 	} else {
-		iounmap(tpm_dev.mem_base);
+		iounmap(tpm_dev.data_base);
 		release_mem_region(tpm_dev.map_base, tpm_dev.map_size);
 	}
 
@@ -586,11 +585,13 @@ static void tpm_inf_pnp_remove(struct pnp_dev *dev)
 	tpm_chip_unregister(chip);
 
 	if (tpm_dev.iotype == TPM_INF_IO_PORT) {
+		ioport_unmap(tpm_dev.data_base);
 		release_region(tpm_dev.data_regs, tpm_dev.data_size);
+		ioport_unmap(tpm_dev.config_base);
 		release_region(tpm_dev.config_port,
 			       tpm_dev.config_size);
 	} else {
-		iounmap(tpm_dev.mem_base);
+		iounmap(tpm_dev.data_base);
 		release_mem_region(tpm_dev.map_base, tpm_dev.map_size);
 	}
 }
diff mbox series

Patch

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 927088b2c3d3..418c9ed59ffd 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -149,6 +149,7 @@  config TCG_NSC
 config TCG_ATMEL
 	tristate "Atmel TPM Interface"
 	depends on PPC64 || HAS_IOPORT_MAP
+	depends on HAS_IOPORT
 	help
 	  If you have a TPM security chip from Atmel say Yes and it 
 	  will be accessible from within Linux.  To compile this driver 
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 9c924a1440a9..99c6e565ec8d 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -26,7 +26,9 @@ 
 #define	TPM_MAX_TRIES		5000
 #define	TPM_INFINEON_DEV_VEN_VALUE	0x15D1
 
+#ifdef CONFIG_HAS_IOPORT
 #define TPM_INF_IO_PORT		0x0
+#endif
 #define TPM_INF_IO_MEM		0x1
 
 #define TPM_INF_ADDR		0x0
@@ -51,34 +53,40 @@  static struct tpm_inf_dev tpm_dev;
 
 static inline void tpm_data_out(unsigned char data, unsigned char offset)
 {
+#ifdef CONFIG_HAS_IOPORT
 	if (tpm_dev.iotype == TPM_INF_IO_PORT)
 		outb(data, tpm_dev.data_regs + offset);
 	else
+#endif
 		writeb(data, tpm_dev.mem_base + tpm_dev.data_regs + offset);
 }
 
 static inline unsigned char tpm_data_in(unsigned char offset)
 {
+#ifdef CONFIG_HAS_IOPORT
 	if (tpm_dev.iotype == TPM_INF_IO_PORT)
 		return inb(tpm_dev.data_regs + offset);
-	else
-		return readb(tpm_dev.mem_base + tpm_dev.data_regs + offset);
+#endif
+	return readb(tpm_dev.mem_base + tpm_dev.data_regs + offset);
 }
 
 static inline void tpm_config_out(unsigned char data, unsigned char offset)
 {
+#ifdef CONFIG_HAS_IOPORT
 	if (tpm_dev.iotype == TPM_INF_IO_PORT)
 		outb(data, tpm_dev.config_port + offset);
 	else
+#endif
 		writeb(data, tpm_dev.mem_base + tpm_dev.index_off + offset);
 }
 
 static inline unsigned char tpm_config_in(unsigned char offset)
 {
+#ifdef CONFIG_HAS_IOPORT
 	if (tpm_dev.iotype == TPM_INF_IO_PORT)
 		return inb(tpm_dev.config_port + offset);
-	else
-		return readb(tpm_dev.mem_base + tpm_dev.index_off + offset);
+#endif
+	return readb(tpm_dev.mem_base + tpm_dev.index_off + offset);
 }
 
 /* TPM header definitions */
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 714070ebb6e7..176cd8dbf1db 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1057,11 +1057,6 @@  static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value)
 		clkrun_val &= ~LPC_CLKRUN_EN;
 		iowrite32(clkrun_val, data->ilb_base_addr + LPC_CNTRL_OFFSET);
 
-		/*
-		 * Write any random value on port 0x80 which is on LPC, to make
-		 * sure LPC clock is running before sending any TPM command.
-		 */
-		outb(0xCC, 0x80);
 	} else {
 		data->clkrun_enabled--;
 		if (data->clkrun_enabled)
@@ -1072,13 +1067,15 @@  static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value)
 		/* Enable LPC CLKRUN# */
 		clkrun_val |= LPC_CLKRUN_EN;
 		iowrite32(clkrun_val, data->ilb_base_addr + LPC_CNTRL_OFFSET);
-
-		/*
-		 * Write any random value on port 0x80 which is on LPC, to make
-		 * sure LPC clock is running before sending any TPM command.
-		 */
-		outb(0xCC, 0x80);
 	}
+
+#ifdef CONFIG_HAS_IOPORT
+	/*
+	 * Write any random value on port 0x80 which is on LPC, to make
+	 * sure LPC clock is running before sending any TPM command.
+	 */
+	outb(0xCC, 0x80);
+#endif
 }
 
 static const struct tpm_class_ops tpm_tis = {