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