Hi Peter, Jarkko,
This is a follow up in my ongoing effort of making inb()/outb() and
similar I/O port accessors compile-time optional. Previously I sent this
as a treewide series titled "treewide: Remove I/O port accessors for
HAS_IOPORT=n" with the latest being its 5th version[0]. With a significant
subset of patches merged I've changed over to per-subsystem series. These
series are stand alone and should be merged via the relevant tree such
that with all subsystems complete we can follow this up with the final
patch that will make the I/O port accessors compile-time optional.
The current state of the full series with changes to the remaining
subsystems and the aforementioned final patch can be found for your
convenience on my git.kernel.org tree in the has_ioport_v6 branch[1] with
signed tags. As for compile-time vs runtime see Linus' reply to my first
attempt[2].
Thanks,
Niklas
[0] https://lore.kernel.org/all/[email protected]/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport_v6
[2] https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
Niklas Schnelle (1):
char: tpm: handle HAS_IOPORT dependencies
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(-)
--
2.40.1
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 <[email protected]>
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
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(-)
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 = {
--
2.40.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
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 <[email protected]>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
> ---
> 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
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
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
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);
}
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<---
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);
}
}