[RFC v2 04/39] char: impi, tpm: depend on HAS

Linux Integrity Measurement development
 help / color / mirror / Atom feed

* [RFC v2 04/39] char: impi, tpm: depend on HAS_IOPORT
       [not found] <20220429135108.2781579-1-schnelle@linux.ibm.com>
@ 2022-04-29 13:50 ` Niklas Schnelle
  2022-04-29 14:23   ` Niklas Schnelle
  2022-04-29 13:50 ` [PATCH 05/37] " Niklas Schnelle
  1 sibling, 1 reply; 6+ messages in thread
From: Niklas Schnelle @ 2022-04-29 13:50 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Greg Kroah-Hartman, linux-kernel, linux-arch, linux-pci,
	Arnd Bergmann, Corey Minyard, Peter Huewe, Jarkko Sakkinen,
	moderated list:IPMI SUBSYSTEM, open list:TPM DEVICE DRIVER

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add this dependency and ifdef
sections of code using inb()/outb() as alternative access methods.

Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/char/Kconfig             |  3 ++-
 drivers/char/ipmi/Makefile       | 11 ++++-------
 drivers/char/ipmi/ipmi_si_intf.c |  3 ++-
 drivers/char/ipmi/ipmi_si_pci.c  |  3 +++
 drivers/char/tpm/Kconfig         |  1 +
 drivers/char/tpm/tpm_infineon.c  | 14 ++++++++++----
 drivers/char/tpm/tpm_tis_core.c  | 19 ++++++++-----------
 7 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 55f48375e3fe..463b82935e78 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -33,6 +33,7 @@ config TTY_PRINTK_LEVEL
 config PRINTER
 	tristate "Parallel printer support"
 	depends on PARPORT
+	depends on HAS_IOPORT
 	help
 	  If you intend to attach a printer to the parallel port of your Linux
 	  box (as opposed to using a serial printer; if the connector at the
@@ -346,7 +347,7 @@ config NVRAM
 
 config DEVPORT
 	bool "/dev/port character device"
-	depends on ISA || PCI
+	depends on HAS_IOPORT
 	default y
 	help
 	  Say Y here if you want to support the /dev/port device. The /dev/port
diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index 7ce790efad92..439bed4feb3a 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -5,13 +5,10 @@
 
 ipmi_si-y := ipmi_si_intf.o ipmi_kcs_sm.o ipmi_smic_sm.o ipmi_bt_sm.o \
 	ipmi_si_hotmod.o ipmi_si_hardcode.o ipmi_si_platform.o \
-	ipmi_si_port_io.o ipmi_si_mem_io.o
-ifdef CONFIG_PCI
-ipmi_si-y += ipmi_si_pci.o
-endif
-ifdef CONFIG_PARISC
-ipmi_si-y += ipmi_si_parisc.o
-endif
+	ipmi_si_mem_io.o
+ipmi_si-$(CONFIG_HAS_IOPORT) += ipmi_si_port_io.o
+ipmi_si-$(CONFIG_PCI) += ipmi_si_pci.o
+ipmi_si-$(CONFIG_PARISC) += ipmi_si_parisc.o
 
 obj-$(CONFIG_IPMI_HANDLER) += ipmi_msghandler.o
 obj-$(CONFIG_IPMI_DEVICE_INTERFACE) += ipmi_devintf.o
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 64dedb3ef8ec..e8094b4007de 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1881,7 +1881,8 @@ int ipmi_si_add_smi(struct si_sm_io *io)
 	}
 
 	if (!io->io_setup) {
-		if (io->addr_space == IPMI_IO_ADDR_SPACE) {
+		if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
+		    io->addr_space == IPMI_IO_ADDR_SPACE) {
 			io->io_setup = ipmi_si_port_setup;
 		} else if (io->addr_space == IPMI_MEM_ADDR_SPACE) {
 			io->io_setup = ipmi_si_mem_setup;
diff --git a/drivers/char/ipmi/ipmi_si_pci.c b/drivers/char/ipmi/ipmi_si_pci.c
index 74fa2055868b..b83d55685b22 100644
--- a/drivers/char/ipmi/ipmi_si_pci.c
+++ b/drivers/char/ipmi/ipmi_si_pci.c
@@ -97,6 +97,9 @@ static int ipmi_pci_probe(struct pci_dev *pdev,
 	}
 
 	if (pci_resource_flags(pdev, 0) & IORESOURCE_IO) {
+		if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+			return -ENXIO;
+
 		io.addr_space = IPMI_IO_ADDR_SPACE;
 		io.io_setup = ipmi_si_port_setup;
 	} else {
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 4a5516406c22..4bc52ed08015 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -137,6 +137,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..2d2ae37153ba 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -51,34 +51,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 dc56b976d816..1efb58dc1b41 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -879,11 +879,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)
@@ -894,13 +889,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.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC v2 04/39] char: impi, tpm: depend on HAS_IOPORT
  2022-04-29 13:50 ` [RFC v2 04/39] char: impi, tpm: depend on HAS_IOPORT Niklas Schnelle
@ 2022-04-29 14:23   ` Niklas Schnelle
  2022-04-29 14:33     ` Ahmad Fatoum
  0 siblings, 1 reply; 6+ messages in thread
From: Niklas Schnelle @ 2022-04-29 14:23 UTC (permalink / raw)
  To: Arnd Bergmann, Ahmad Fatoum
  Cc: Greg Kroah-Hartman, linux-kernel, linux-arch, linux-pci,
	Arnd Bergmann, Corey Minyard, Peter Huewe, Jarkko Sakkinen,
	moderated list:IPMI SUBSYSTEM, open list:TPM DEVICE DRIVER

> Hello Niklas,
> 
> On 29.04.22 15:50, Niklas Schnelle wrote:
> > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > not being declared. We thus need to add this dependency and ifdef
> > sections of code using inb()/outb() as alternative access methods.
> > 
> > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 
> [snip]
> 
> > diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> > index 9c924a1440a9..2d2ae37153ba 100644
> > --- a/drivers/char/tpm/tpm_infineon.c
> > +++ b/drivers/char/tpm/tpm_infineon.c
> > @@ -51,34 +51,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
> 
> This looks ugly. Can't you declare inb/outb anyway and skip the definition,
> so you can use IS_ENABLED() here instead?
> 
> You can mark the declarations with __compiletime_error("some message"), so
> if an IS_ENABLED() reference is not removed at compile time, you get some
> readable error message instead of a link error.
> 
> Cheers,
> Ahmad

I didn't know about __compiletime_error() that certainly sounds
interesting even when using a normal #ifdef.

That said either with the function not being declared or this
__compiletime_error() mechanism I would think that using IS_ENABLED()
relies on compiler optimizations not to compile in the missing/error
function call, right? I'm not sure if that is something we should do.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC v2 04/39] char: impi, tpm: depend on HAS_IOPORT
  2022-04-29 14:23   ` Niklas Schnelle
@ 2022-04-29 14:33     ` Ahmad Fatoum
  2022-05-02 14:34       ` Niklas Schnelle
  0 siblings, 1 reply; 6+ messages in thread
From: Ahmad Fatoum @ 2022-04-29 14:33 UTC (permalink / raw)
  To: Niklas Schnelle, Arnd Bergmann
  Cc: Greg Kroah-Hartman, linux-kernel, linux-arch, linux-pci,
	Arnd Bergmann, Corey Minyard, Peter Huewe, Jarkko Sakkinen,
	moderated list:IPMI SUBSYSTEM, open list:TPM DEVICE DRIVER,
	Pengutronix Kernel Team

Hello Niklas,

On 29.04.22 16:23, Niklas Schnelle wrote:
>> Hello Niklas,
>>
>> On 29.04.22 15:50, Niklas Schnelle wrote:
>>> In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
>>> not being declared. We thus need to add this dependency and ifdef
>>> sections of code using inb()/outb() as alternative access methods.
>>>
>>> Co-developed-by: Arnd Bergmann <arnd@kernel.org>
>>> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
>>
>> [snip]
>>
>>> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
>>> index 9c924a1440a9..2d2ae37153ba 100644
>>> --- a/drivers/char/tpm/tpm_infineon.c
>>> +++ b/drivers/char/tpm/tpm_infineon.c
>>> @@ -51,34 +51,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
>>
>> This looks ugly. Can't you declare inb/outb anyway and skip the definition,
>> so you can use IS_ENABLED() here instead?
>>
>> You can mark the declarations with __compiletime_error("some message"), so
>> if an IS_ENABLED() reference is not removed at compile time, you get some
>> readable error message instead of a link error.
>>
>> Cheers,
>> Ahmad
> 
> I didn't know about __compiletime_error() that certainly sounds
> interesting even when using a normal #ifdef.
> 
> That said either with the function not being declared or this
> __compiletime_error() mechanism I would think that using IS_ENABLED()
> relies on compiler optimizations not to compile in the missing/error
> function call, right? I'm not sure if that is something we should do.

Yes, it assumes your compiler is able to discard the body of an if (0),
which we already assume, otherwise it wouldn't make sense for any existing
code to use __compiletime_error().

To me this sounds much cleaner than #ifdefs in the midst of functions,
which are a detriment to maintainability.

Cheers,
Ahmad




> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC v2 04/39] char: impi, tpm: depend on HAS_IOPORT
  2022-04-29 14:33     ` Ahmad Fatoum
@ 2022-05-02 14:34       ` Niklas Schnelle
  0 siblings, 0 replies; 6+ messages in thread
From: Niklas Schnelle @ 2022-05-02 14:34 UTC (permalink / raw)
  To: Ahmad Fatoum, Arnd Bergmann
  Cc: Greg Kroah-Hartman, linux-kernel, linux-arch, linux-pci,
	Arnd Bergmann, Corey Minyard, Peter Huewe, Jarkko Sakkinen,
	moderated list:IPMI SUBSYSTEM, open list:TPM DEVICE DRIVER,
	Pengutronix Kernel Team

On Fri, 2022-04-29 at 16:33 +0200, Ahmad Fatoum wrote:
> Hello Niklas,
> 
> On 29.04.22 16:23, Niklas Schnelle wrote:
> > > Hello Niklas,
> > > 
> > > On 29.04.22 15:50, Niklas Schnelle wrote:
> > > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > > > not being declared. We thus need to add this dependency and ifdef
> > > > sections of code using inb()/outb() as alternative access methods.
> > > > 
> > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > 
> > > [snip]
> > > 
> > > > diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> > > > index 9c924a1440a9..2d2ae37153ba 100644
> > > > --- a/drivers/char/tpm/tpm_infineon.c
> > > > +++ b/drivers/char/tpm/tpm_infineon.c
> > > > @@ -51,34 +51,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
> > > 
> > > This looks ugly. Can't you declare inb/outb anyway and skip the definition,
> > > so you can use IS_ENABLED() here instead?
> > > 
> > > You can mark the declarations with __compiletime_error("some message"), so
> > > if an IS_ENABLED() reference is not removed at compile time, you get some
> > > readable error message instead of a link error.
> > > 
> > > Cheers,
> > > Ahmad
> > 
> > I didn't know about __compiletime_error() that certainly sounds
> > interesting even when using a normal #ifdef.
> > 
> > That said either with the function not being declared or this
> > __compiletime_error() mechanism I would think that using IS_ENABLED()
> > relies on compiler optimizations not to compile in the missing/error
> > function call, right? I'm not sure if that is something we should do.
> 
> Yes, it assumes your compiler is able to discard the body of an if (0),
> which we already assume, otherwise it wouldn't make sense for any existing
> code to use __compiletime_error().
> 
> To me this sounds much cleaner than #ifdefs in the midst of functions,
> which are a detriment to maintainability.
> 
> Cheers,
> Ahmad
> 

Ok, makes sense. I'll look into using __compiletime_error() and
IS_ENABLED().



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 05/37] char: impi, tpm: depend on HAS_IOPORT
       [not found] <20220429135108.2781579-1-schnelle@linux.ibm.com>
  2022-04-29 13:50 ` [RFC v2 04/39] char: impi, tpm: depend on HAS_IOPORT Niklas Schnelle
@ 2022-04-29 13:50 ` Niklas Schnelle
  2022-04-29 14:02   ` Ahmad Fatoum
  1 sibling, 1 reply; 6+ messages in thread
From: Niklas Schnelle @ 2022-04-29 13:50 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Greg Kroah-Hartman, linux-kernel, linux-arch, linux-pci,
	Arnd Bergmann, Corey Minyard, Peter Huewe, Jarkko Sakkinen,
	moderated list:IPMI SUBSYSTEM, open list:TPM DEVICE DRIVER

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add this dependency and ifdef
sections of code using inb()/outb() as alternative access methods.

Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/char/Kconfig             |  3 ++-
 drivers/char/ipmi/Makefile       | 11 ++++-------
 drivers/char/ipmi/ipmi_si_intf.c |  3 ++-
 drivers/char/ipmi/ipmi_si_pci.c  |  3 +++
 drivers/char/tpm/Kconfig         |  1 +
 drivers/char/tpm/tpm_infineon.c  | 14 ++++++++++----
 drivers/char/tpm/tpm_tis_core.c  | 19 ++++++++-----------
 7 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 55f48375e3fe..463b82935e78 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -33,6 +33,7 @@ config TTY_PRINTK_LEVEL
 config PRINTER
 	tristate "Parallel printer support"
 	depends on PARPORT
+	depends on HAS_IOPORT
 	help
 	  If you intend to attach a printer to the parallel port of your Linux
 	  box (as opposed to using a serial printer; if the connector at the
@@ -346,7 +347,7 @@ config NVRAM
 
 config DEVPORT
 	bool "/dev/port character device"
-	depends on ISA || PCI
+	depends on HAS_IOPORT
 	default y
 	help
 	  Say Y here if you want to support the /dev/port device. The /dev/port
diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index 7ce790efad92..439bed4feb3a 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -5,13 +5,10 @@
 
 ipmi_si-y := ipmi_si_intf.o ipmi_kcs_sm.o ipmi_smic_sm.o ipmi_bt_sm.o \
 	ipmi_si_hotmod.o ipmi_si_hardcode.o ipmi_si_platform.o \
-	ipmi_si_port_io.o ipmi_si_mem_io.o
-ifdef CONFIG_PCI
-ipmi_si-y += ipmi_si_pci.o
-endif
-ifdef CONFIG_PARISC
-ipmi_si-y += ipmi_si_parisc.o
-endif
+	ipmi_si_mem_io.o
+ipmi_si-$(CONFIG_HAS_IOPORT) += ipmi_si_port_io.o
+ipmi_si-$(CONFIG_PCI) += ipmi_si_pci.o
+ipmi_si-$(CONFIG_PARISC) += ipmi_si_parisc.o
 
 obj-$(CONFIG_IPMI_HANDLER) += ipmi_msghandler.o
 obj-$(CONFIG_IPMI_DEVICE_INTERFACE) += ipmi_devintf.o
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 64dedb3ef8ec..e8094b4007de 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1881,7 +1881,8 @@ int ipmi_si_add_smi(struct si_sm_io *io)
 	}
 
 	if (!io->io_setup) {
-		if (io->addr_space == IPMI_IO_ADDR_SPACE) {
+		if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
+		    io->addr_space == IPMI_IO_ADDR_SPACE) {
 			io->io_setup = ipmi_si_port_setup;
 		} else if (io->addr_space == IPMI_MEM_ADDR_SPACE) {
 			io->io_setup = ipmi_si_mem_setup;
diff --git a/drivers/char/ipmi/ipmi_si_pci.c b/drivers/char/ipmi/ipmi_si_pci.c
index 74fa2055868b..b83d55685b22 100644
--- a/drivers/char/ipmi/ipmi_si_pci.c
+++ b/drivers/char/ipmi/ipmi_si_pci.c
@@ -97,6 +97,9 @@ static int ipmi_pci_probe(struct pci_dev *pdev,
 	}
 
 	if (pci_resource_flags(pdev, 0) & IORESOURCE_IO) {
+		if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+			return -ENXIO;
+
 		io.addr_space = IPMI_IO_ADDR_SPACE;
 		io.io_setup = ipmi_si_port_setup;
 	} else {
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 4a5516406c22..4bc52ed08015 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -137,6 +137,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..2d2ae37153ba 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -51,34 +51,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 dc56b976d816..1efb58dc1b41 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -879,11 +879,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)
@@ -894,13 +889,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.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 05/37] char: impi, tpm: depend on HAS_IOPORT
  2022-04-29 13:50 ` [PATCH 05/37] " Niklas Schnelle
@ 2022-04-29 14:02   ` Ahmad Fatoum
  0 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2022-04-29 14:02 UTC (permalink / raw)
  To: Niklas Schnelle, Arnd Bergmann
  Cc: Greg Kroah-Hartman, linux-kernel, linux-arch, linux-pci,
	Arnd Bergmann, Corey Minyard, Peter Huewe, Jarkko Sakkinen,
	moderated list:IPMI SUBSYSTEM, open list:TPM DEVICE DRIVER,
	Pengutronix Kernel Team

Hello Niklas,

On 29.04.22 15:50, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> not being declared. We thus need to add this dependency and ifdef
> sections of code using inb()/outb() as alternative access methods.
> 
> Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>

[snip]

> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 9c924a1440a9..2d2ae37153ba 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -51,34 +51,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

This looks ugly. Can't you declare inb/outb anyway and skip the definition,
so you can use IS_ENABLED() here instead?

You can mark the declarations with __compiletime_error("some message"), so
if an IS_ENABLED() reference is not removed at compile time, you get some
readable error message instead of a link error.

Cheers,
Ahmad

>  		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 dc56b976d816..1efb58dc1b41 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -879,11 +879,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)
> @@ -894,13 +889,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 = {


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-05-02 14:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20220429135108.2781579-1-schnelle@linux.ibm.com>
2022-04-29 13:50 ` [RFC v2 04/39] char: impi, tpm: depend on HAS_IOPORT Niklas Schnelle
2022-04-29 14:23   ` Niklas Schnelle
2022-04-29 14:33     ` Ahmad Fatoum
2022-05-02 14:34       ` Niklas Schnelle
2022-04-29 13:50 ` [PATCH 05/37] " Niklas Schnelle
2022-04-29 14:02   ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox