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);
}
}
