From: "Arnd Bergmann" <arnd@kernel.org>
To: "Niklas Schnelle" <schnelle@linux.ibm.com>,
"Peter Huewe" <peterhuewe@gmx.de>,
"Jarkko Sakkinen" <jarkko@kernel.org>
Cc: linux-integrity@vger.kernel.org,
"Heiko Carstens" <hca@linux.ibm.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] char: tpm: handle HAS_IOPORT dependencies
Date: Fri, 05 Apr 2024 23:02:47 +0200 [thread overview]
Message-ID: <fe5e2e29-6d60-4ec1-ab3e-492c463e6a12@app.fastmail.com> (raw)
In-Reply-To: <4d08af651b95744dc6cfa0c2624c596d21d83d09.camel@linux.ibm.com>
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);
}
}
next prev parent reply other threads:[~2024-04-05 21:03 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-04 10:58 [PATCH 0/1] char: tpm: Handle HAS_IOPORT dependencies Niklas Schnelle
2024-04-04 10:58 ` [PATCH 1/1] char: tpm: handle " Niklas Schnelle
2024-04-04 11:17 ` Arnd Bergmann
2024-04-04 15:22 ` Jarkko Sakkinen
2024-04-04 15:41 ` Niklas Schnelle
2024-04-04 15:56 ` Arnd Bergmann
2024-04-05 9:23 ` Niklas Schnelle
2024-04-05 21:02 ` Arnd Bergmann [this message]
2024-04-04 15:19 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fe5e2e29-6d60-4ec1-ab3e-492c463e6a12@app.fastmail.com \
--to=arnd@kernel.org \
--cc=hca@linux.ibm.com \
--cc=jarkko@kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterhuewe@gmx.de \
--cc=schnelle@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).