Re: [PATCH v9 0/5] treewide: Remove I/O port accessors for HAS_IOPORT=n

[Niklas Schnelle]

On Fri, 2024-10-25 at 13:41 +, Arnd Bergmann wrote:
> On Thu, Oct 24, 2024, at 17:54, Niklas Schnelle wrote:
> > Hi All,
> > 
> > This is a follow up in my long running effort of making inb()/outb() and
> > similar I/O port accessors compile-time optional. After initially
> > sending this as a treewide series with the latest revision at[0]
> > we switched to per subsystem series. Now though as we're left with only
> > 5 patches left I'm going back to a single series with Arnd planning
> > to take this via the the asm-generic tree.
> > 
> > This series may also be viewed for your convenience on my git.kernel.org
> > tree[1] under the b4/has_ioport branch. As for compile-time vs runtime
> > see Linus' reply to my first attempt[2].
> 
> Hi Niklas,
> 
> Thanks for your endless work on this. I have now pulled it into
> the asm-generic tree as I want to ensure we get enough time to
> test this as part of linux-next before the merge window.
> 
> If minor issues still come up, I would try to fix those as
> add-on patches to avoid rebasing my tree.
> 
> I also expect that we will continue with add-on patches in
> the future, in particular I hope to make HAS_IOPORT optional
> on arm, arm64 and powerpc, and only enabled for
> configurations that actually want it.
> 
>  Arnd
> 

Thanks for taking it and sticking by my side through this! Now let's
just hope there won't be too much fallout but I will be here to help if
needed. As for arm, arm64, and powerpc I like it, having more
!HAS_IOPORT targets will help to share the load of new inb()/outb()
which "worked for me on x86". I definitely learned a lot in the
process. Of course I wished and originally expected it to go a lot
faster but hey looks like we might persevere in the end. And yes, I
will pour myself a drink when this finally made it into Linus' tree :-)
And then when we meet at some conference in the future we can laugh
about how this turned from a 5 line patch into at least 53 commits over
3 years.

Thanks,
Niklas

[PATCH v9 3/5] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them. In the bochs driver there is optional MMIO support
detected at runtime, warn if this isn't taken when HAS_IOPORT is not
defined.

There is also a direct and hard coded use in cirrus.c which according to
the comment is only necessary during resume.  Let's just skip this as
for example s390 which doesn't have I/O port support also doesen't
support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Acked-by: Lucas De Marchi  # xe
Signed-off-by: Niklas Schnelle 
---
 drivers/gpu/drm/gma500/Kconfig |  2 +-
 drivers/gpu/drm/qxl/Kconfig|  2 +-
 drivers/gpu/drm/tiny/bochs.c   | 19 ++-
 drivers/gpu/drm/tiny/cirrus.c  |  2 ++
 drivers/gpu/drm/xe/Kconfig |  2 +-
 5 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
index 
efb4a2dd2f80885cb59c925d09401002278d7d61..23b7c14de5e29238ece939d5822d8a9ffc4675cc
 100644
--- a/drivers/gpu/drm/gma500/Kconfig
+++ b/drivers/gpu/drm/gma500/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_GMA500
tristate "Intel GMA500/600/3600/3650 KMS Framebuffer"
-   depends on DRM && PCI && X86 && MMU
+   depends on DRM && PCI && X86 && MMU && HAS_IOPORT
select DRM_KMS_HELPER
select FB_IOMEM_HELPERS if DRM_FBDEV_EMULATION
select I2C
diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index 
ca3f51c2a8fe1a383f8a2479f04b5c0b3fb14e44..17d6927e5e23402786117fd0f99186978956c1c2
 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_QXL
tristate "QXL virtual GPU"
-   depends on DRM && PCI && MMU
+   depends on DRM && PCI && MMU && HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 
31fc5d839e106ea4d5c8fe42d1bfc3c70291e3fb..e738bb85831667f55c436e21e761435def113b9a
 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
+#include 
 #include 
 #include 
 
@@ -95,12 +96,17 @@ struct bochs_device {
 
 /* -- */
 
+static __always_inline bool bochs_uses_mmio(struct bochs_device *bochs)
+{
+   return !IS_ENABLED(CONFIG_HAS_IOPORT) || bochs->mmio;
+}
+
 static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val)
 {
if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df))
return;
 
-   if (bochs->mmio) {
+   if (bochs_uses_mmio(bochs)) {
int offset = ioport - 0x3c0 + 0x400;
 
writeb(val, bochs->mmio + offset);
@@ -114,7 +120,7 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 
ioport)
if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df))
return 0xff;
 
-   if (bochs->mmio) {
+   if (bochs_uses_mmio(bochs)) {
int offset = ioport - 0x3c0 + 0x400;
 
return readb(bochs->mmio + offset);
@@ -127,7 +133,7 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, u16 
reg)
 {
u16 ret = 0;
 
-   if (bochs->mmio) {
+   if (bochs_uses_mmio(bochs)) {
int offset = 0x500 + (reg << 1);
 
ret = readw(bochs->mmio + offset);
@@ -140,7 +146,7 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, u16 
reg)
 
 static void bochs_dispi_write(struct bochs_device *bochs, u16 reg, u16 val)
 {
-   if (bochs->mmio) {
+   if (bochs_uses_mmio(bochs)) {
int offset = 0x500 + (reg << 1);
 
writew(val, bochs->mmio + offset);
@@ -228,7 +234,7 @@ static int bochs_hw_init(struct drm_device *dev)
DRM_ERROR("Cannot map mmio region\n");
return -ENOMEM;
}
-   } else {
+   } else if (IS_ENABLED(CONFIG_HAS_IOPORT)) {
ioaddr = VBE_DISPI_IOPORT_INDEX;
iosize = 2;
if (!request_region(ioaddr, iosize, "bochs-drm")) {
@@ -236,6 +242,9 @@ static int bochs_hw_init(struct drm_device *dev)
return -EBUSY;
}
bochs->ioports = 1;
+   } else {
+   dev_err(dev->dev, "I/O ports are not supported\n");
+   return -EIO;
}
 
id = bochs_dispi_read(bochs, VBE_DISPI_INDEX_ID);
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 
75132

[PATCH v9 5/5] asm-generic/io.h: Remove I/O port accessors for HAS_IOPORT=n

[Niklas Schnelle]

With all subsystems and drivers either declaring their dependence on
HAS_IOPORT or fencing I/O port specific code sections we can finally
make inb()/outb() and friends compile-time dependent on HAS_IOPORT as
suggested by Linus in the linked mail. The main benefit of this is that
on platforms such as s390 which have no meaningful way of implementing
inb()/outb() their use without the proper HAS_IOPORT dependency will
result in easy to catch and fix compile-time errors instead of compiling
code that can never work.

Link: 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/
Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 include/asm-generic/io.h | 60 
 1 file changed, 60 insertions(+)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 
80de699bf6af4b7b77f582c0469c43e978f67a43..1027be6a62bcbcd5f6797e0fa42035208d0ca79f
 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -540,6 +540,7 @@ static inline void writesq(volatile void __iomem *addr, 
const void *buffer,
 
 #if !defined(inb) && !defined(_inb)
 #define _inb _inb
+#ifdef CONFIG_HAS_IOPORT
 static inline u8 _inb(unsigned long addr)
 {
u8 val;
@@ -549,10 +550,15 @@ static inline u8 _inb(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u8 _inb(unsigned long addr)
+   __compiletime_error("inb()) requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(inw) && !defined(_inw)
 #define _inw _inw
+#ifdef CONFIG_HAS_IOPORT
 static inline u16 _inw(unsigned long addr)
 {
u16 val;
@@ -562,10 +568,15 @@ static inline u16 _inw(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u16 _inw(unsigned long addr)
+   __compiletime_error("inw() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(inl) && !defined(_inl)
 #define _inl _inl
+#ifdef CONFIG_HAS_IOPORT
 static inline u32 _inl(unsigned long addr)
 {
u32 val;
@@ -575,36 +586,55 @@ static inline u32 _inl(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u32 _inl(unsigned long addr)
+   __compiletime_error("inl() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outb) && !defined(_outb)
 #define _outb _outb
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outb(u8 value, unsigned long addr)
 {
__io_pbw();
__raw_writeb(value, PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outb(u8 value, unsigned long addr)
+   __compiletime_error("outb() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outw) && !defined(_outw)
 #define _outw _outw
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outw(u16 value, unsigned long addr)
 {
__io_pbw();
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outw(u16 value, unsigned long addr)
+   __compiletime_error("outw() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outl) && !defined(_outl)
 #define _outl _outl
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outl(u32 value, unsigned long addr)
 {
__io_pbw();
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outl(u32 value, unsigned long addr)
+   __compiletime_error("outl() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #include 
@@ -688,53 +718,83 @@ static inline void outl_p(u32 value, unsigned long addr)
 
 #ifndef insb
 #define insb insb
+#ifdef CONFIG_HAS_IOPORT
 static inline void insb(unsigned long addr, void *buffer, unsigned int count)
 {
readsb(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insb(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insb() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef insw
 #define insw insw
+#ifdef CONFIG_HAS_IOPORT
 static inline void insw(unsigned long addr, void *buffer, unsigned int count)
 {
readsw(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insw(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insw() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef insl
 #define insl insl
+#ifdef CONFIG_HAS_IOPORT
 static inline void insl(unsigned long addr, void *buffer, unsigned int count)
 {
readsl(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insl(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insl() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef outsb
 #define outsb outsb
+#ifdef CONFIG_HAS_IOPORT
 static inline void outsb(unsigned long addr, const void *buffer,
 unsigned int count)
 {
writesb(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void outsb(unsigned long addr, const void *buf

[PATCH v9 4/5] tty: serial: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them unconditionally. Some 8250 serial drivers support
MMIO only use, so fence only the parts requiring I/O ports and print an
error message if a device can't be supported with the current
configuration.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Acked-by: Greg Kroah-Hartman 
Reviewed-by: Maciej W. Rozycki 
Signed-off-by: Niklas Schnelle 
---
 drivers/tty/Kconfig   |  4 ++--
 drivers/tty/serial/8250/8250_early.c  |  4 
 drivers/tty/serial/8250/8250_pci.c| 40 +++
 drivers/tty/serial/8250/8250_pcilib.c | 12 ++-
 drivers/tty/serial/8250/8250_pcilib.h |  2 ++
 drivers/tty/serial/8250/8250_port.c   | 27 +++
 drivers/tty/serial/8250/Kconfig   |  4 ++--
 drivers/tty/serial/Kconfig|  2 +-
 include/linux/serial_core.h   |  4 
 9 files changed, 89 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 
a45d423ad10f02c3a818021bbb18655a8b690500..63a494d36a1fdceba5a7b39f4516060e48af0cc6
 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -220,7 +220,7 @@ config MOXA_INTELLIO
 
 config MOXA_SMARTIO
tristate "Moxa SmartIO support v. 2.0"
-   depends on SERIAL_NONSTANDARD && PCI
+   depends on SERIAL_NONSTANDARD && PCI && HAS_IOPORT
help
  Say Y here if you have a Moxa SmartIO multiport serial card and/or
  want to help develop a new version of this driver.
@@ -302,7 +302,7 @@ config GOLDFISH_TTY_EARLY_CONSOLE
 
 config IPWIRELESS
tristate "IPWireless 3G UMTS PCMCIA card support"
-   depends on PCMCIA && NETDEVICES
+   depends on PCMCIA && NETDEVICES && HAS_IOPORT
select PPP
help
  This is a driver for 3G UMTS PCMCIA card from IPWireless company. In
diff --git a/drivers/tty/serial/8250/8250_early.c 
b/drivers/tty/serial/8250/8250_early.c
index 
6176083d0341ca10edebe5c4eebfffc922a61472..84242292176570cd2c92afbd4755d303827a4abc
 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -46,8 +46,10 @@ static unsigned int serial8250_early_in(struct uart_port 
*port, int offset)
return readl(port->membase + offset);
case UPIO_MEM32BE:
return ioread32be(port->membase + offset);
+#ifdef CONFIG_HAS_IOPORT
case UPIO_PORT:
return inb(port->iobase + offset);
+#endif
default:
return 0;
}
@@ -70,9 +72,11 @@ static void serial8250_early_out(struct uart_port *port, int 
offset, int value)
case UPIO_MEM32BE:
iowrite32be(value, port->membase + offset);
break;
+#ifdef CONFIG_HAS_IOPORT
case UPIO_PORT:
outb(value, port->iobase + offset);
break;
+#endif
}
 }
 
diff --git a/drivers/tty/serial/8250/8250_pci.c 
b/drivers/tty/serial/8250/8250_pci.c
index 
6709b6a5f3011db38acc58dc7223158fe4fcf72e..7d7a6d62c09ceabcf4094d539c565ed09c153561
 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -964,6 +964,9 @@ static int pci_ite887x_init(struct pci_dev *dev)
struct resource *iobase = NULL;
u32 miscr, uartbar, ioport;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(dev);
+
/* search for the base-ioport */
for (i = 0; i < ARRAY_SIZE(inta_addr); i++) {
iobase = request_region(inta_addr[i], ITE_887x_IOSIZE,
@@ -1514,6 +1517,9 @@ static int pci_quatech_init(struct pci_dev *dev)
const struct pci_device_id *match;
bool amcc = false;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(dev);
+
match = pci_match_id(quatech_cards, dev);
if (match)
amcc = match->driver_data;
@@ -1538,6 +1544,9 @@ static int pci_quatech_setup(struct serial_private *priv,
  const struct pciserial_board *board,
  struct uart_8250_port *port, int idx)
 {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(priv->dev);
+
/* Needed by pci_quatech calls below */
port->port.iobase = pci_resource_start(priv->dev, 
FL_GET_BASE(board->flags));
/* Set up the clocking */
@@ -1655,6 +1664,9 @@ static int pci_fintek_setup(struct serial_private *priv,
u8 config_base;
u16 iobase;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(pdev);
+
config_base = 0x40 + 0x08 * idx;
 
/* Get the io address from configuration space */
@@ -1686,6 +1698,9 @@ s

[PATCH v9 1/5] hexagon: Don't select GENERIC_IOMAP without HAS_IOPORT support

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. As hexagon does not support I/O port access it also
the GENERIC_IOMAP mechanism of dynamically choosing between I/O port and
MMIO access doesn't work so don't select it.

Reviewed-by: Brian Cain 
Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 arch/hexagon/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
index 
e233b5efa2761e35c416cbc147f6b6422a7c5b8f..5ea1bf4b7d4f5471a9c39ee9fb5c701455d21342
 100644
--- a/arch/hexagon/Kconfig
+++ b/arch/hexagon/Kconfig
@@ -31,7 +31,6 @@ config HEXAGON
select HAVE_ARCH_TRACEHOOK
select NEED_SG_DMA_LENGTH
select NO_IOPORT_MAP
-   select GENERIC_IOMAP
select GENERIC_IOREMAP
select GENERIC_SMP_IDLE_THREAD
select STACKTRACE_SUPPORT

-- 
2.45.2

[PATCH v9 0/5] treewide: Remove I/O port accessors for HAS_IOPORT=n

[Niklas Schnelle]

Hi All,

This is a follow up in my long running effort of making inb()/outb() and
similar I/O port accessors compile-time optional. After initially
sending this as a treewide series with the latest revision at[0]
we switched to per subsystem series. Now though as we're left with only
5 patches left I'm going back to a single series with Arnd planning
to take this via the the asm-generic tree.

This series may also be viewed for your convenience on my git.kernel.org
tree[1] under the b4/has_ioport branch. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Thanks,
Niklas

[0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git
[2] 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/

Signed-off-by: Niklas Schnelle 
---
Changes in v9:
- In drm patch sort includes and reformat if (Thomas). Use IS_ENABELD()
  and add a helper instead of #ifdef (Arnd, Thomas)
- Rebased on v6.12-rc4
- Compile tested applied to next. There are a few conflicts with drm
  next but they're all just context changes
- Link to v8: 
https://lore.kernel.org/r/20241008-b4-has_ioport-v8-0-793e68aea...@linux.ibm.com

Changes in v8:
- Don't remove "depends on !S390" for SERIAL_8250
- Link to v7: 
https://lore.kernel.org/r/20241008-b4-has_ioport-v7-0-8624c09a4...@linux.ibm.com

Changes in v7:
- Renamed serial_8250_need_ioport() helper to
  serial_8250_warn_need_ioport() and move it to 8250_pcilib.c so it can
  be used in serial8250_pci_setup_port()
- Flattened if in serial8250_pci_setup_port() (Maciej)
- Removed gratuituous changes (Maciej)
- Removed is_upf_fourport() helper in favor of zeroing UPF_FOURPORT
  if CONFIG_HAS_IOPORT is not set (Maciej)
- Link to v6: 
https://lore.kernel.org/r/20241007-b4-has_ioport-v6-0-03f7240da...@linux.ibm.com

Changes since v5 / per subsystem patches:

drm:
- Add HAS_IOPORT dependency for GMA500
tty: serial:
- Make 8250 PCI driver emit an error message when trying to use devices
  which require I/O ports without CONFIG_HAS_IOPORT (Maciej)
- Use early returns + dead code elimination to skip inb()/outb() uses
  in quirks (Arnd)
- In 8250 PCI driver also handle fintek and moxi quirks
- In 8250 ports code handle um's defined(__i385__) &&
  defined(CONFIG_HAS_IOPORT) case
- Use IS_ENABLED() early return also in is_upf_fourport()
  __always_inline to force constant folding

---
Niklas Schnelle (5):
  hexagon: Don't select GENERIC_IOMAP without HAS_IOPORT support
  Bluetooth: add HAS_IOPORT dependencies
  drm: handle HAS_IOPORT dependencies
  tty: serial: handle HAS_IOPORT dependencies
  asm-generic/io.h: Remove I/O port accessors for HAS_IOPORT=n

 arch/hexagon/Kconfig  |  1 -
 drivers/bluetooth/Kconfig |  6 ++--
 drivers/gpu/drm/gma500/Kconfig|  2 +-
 drivers/gpu/drm/qxl/Kconfig   |  2 +-
 drivers/gpu/drm/tiny/bochs.c  | 19 ---
 drivers/gpu/drm/tiny/cirrus.c |  2 ++
 drivers/gpu/drm/xe/Kconfig|  2 +-
 drivers/tty/Kconfig   |  4 +--
 drivers/tty/serial/8250/8250_early.c  |  4 +++
 drivers/tty/serial/8250/8250_pci.c| 40 +++
 drivers/tty/serial/8250/8250_pcilib.c | 12 ++-
 drivers/tty/serial/8250/8250_pcilib.h |  2 ++
 drivers/tty/serial/8250/8250_port.c   | 27 +---
 drivers/tty/serial/8250/Kconfig   |  4 +--
 drivers/tty/serial/Kconfig|  2 +-
 include/asm-generic/io.h  | 60 +++
 include/linux/serial_core.h   |  4 +++
 17 files changed, 171 insertions(+), 22 deletions(-)
---
base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
change-id: 20241004-b4-has_ioport-60ac6ce1deb6

Best regards,
-- 
Niklas Schnelle

[PATCH v9 2/5] Bluetooth: add HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/bluetooth/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 
18767b54df352e929692850dc789d9377839e094..4ab32abf0f48644715d4c27ec0d2fdaccef62b76
 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -336,7 +336,7 @@ config BT_HCIBFUSB
 
 config BT_HCIDTL1
tristate "HCI DTL1 (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
help
  Bluetooth HCI DTL1 (PC Card) driver.
  This driver provides support for Bluetooth PCMCIA devices with
@@ -349,7 +349,7 @@ config BT_HCIDTL1
 
 config BT_HCIBT3C
tristate "HCI BT3C (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
select FW_LOADER
help
  Bluetooth HCI BT3C (PC Card) driver.
@@ -363,7 +363,7 @@ config BT_HCIBT3C
 
 config BT_HCIBLUECARD
tristate "HCI BlueCard (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
help
  Bluetooth HCI BlueCard (PC Card) driver.
  This driver provides support for Bluetooth PCMCIA devices with

-- 
2.45.2

Re: [PATCH v8 3/5] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Mon, Oct 21, 2024 at 01:18:20PM +0200, Niklas Schnelle wrote:
> On Mon, 2024-10-21 at 12:58 +0200, Thomas Zimmermann wrote:
> > Hi
> > 
> > Am 21.10.24 um 12:08 schrieb Arnd Bergmann:
> > > On Mon, Oct 21, 2024, at 07:52, Thomas Zimmermann wrote:
> > > > Am 08.10.24 um 14:39 schrieb Niklas Schnelle:
> > > d 100644
> > > > > --- a/drivers/gpu/drm/qxl/Kconfig
> > > > > +++ b/drivers/gpu/drm/qxl/Kconfig
> > > > > @@ -2,6 +2,7 @@
> > > > >config DRM_QXL
> > > > >   tristate "QXL virtual GPU"
> > > > >   depends on DRM && PCI && MMU
> > > > > + depends on HAS_IOPORT
> > > > Is there a difference between this style (multiple 'depends on') and the
> > > > one used for gma500 (&& && &&)?
> > > No, it's the same. Doing it in one line is mainly useful
> > > if you have some '||' as well.
> > > 
> > > > > @@ -105,7 +106,9 @@ static void bochs_vga_writeb(struct bochs_device 
> > > > > *bochs, u16 ioport, u8 val)
> > > > >
> > > > >   writeb(val, bochs->mmio + offset);
> > > > >   } else {
> > > > > +#ifdef CONFIG_HAS_IOPORT
> > > > >   outb(val, ioport);
> > > > > +#endif
> > > > Could you provide empty defines for the out() interfaces at the top of
> > > > the file?
> > > That no longer works since there are now __compiletime_error()
> > > versions of these funcitons. However we can do it more nicely like:
> > > 
> > > diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> > > index 9b337f948434..034af6e32200 100644
> > > --- a/drivers/gpu/drm/tiny/bochs.c
> > > +++ b/drivers/gpu/drm/tiny/bochs.c
> > > @@ -112,14 +112,12 @@ static void bochs_vga_writeb(struct bochs_device 
> > > *bochs, u16 ioport, u8 val)
> > >   if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df))
> > >   return;
> > >   
> > > - if (bochs->mmio) {
> > > + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) {
> > >   int offset = ioport - 0x3c0 + 0x400;
> > >   
> > >   writeb(val, bochs->mmio + offset);
> > >   } else {
> > > -#ifdef CONFIG_HAS_IOPORT
> > >   outb(val, ioport);
> > > -#endif
> > >   }
> > 
> > For all functions with such a pattern, could we use:
> > 
> > bool bochs_uses_mmio(bochs)
> > {
> >      return !IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio
> > }
> > 
> > void writeb_func()
> > {
> >      if (bochs_uses_mmio()) {
> >    writeb()
> > #if CONFIG_HAS_IOPORT
> >      } else {
> >    outb()
> > #endif
> >      }
> > }
> > 
> 
> I think if the helper were __always_inline we could still take
> advantage of the dead code elimination and combine this with Arnd's
> approach. Though I feel like it is a bit odd to try to do the MMIO
> approach despite bochs->mmio being false on !HAS_IOPORT systems.
> Is that what you wanted to correct by keeping the #ifdef
> CONFIG_HAS_IOPORT around the else? And yes the warning makes sense to
> me too.

Working on this now, I think we don't need a warning in the bochs_uses_mmio()
helper because we should never get here with !IS_ENABLED(CONFIG_HAS_IOPORT)
at runtime thanks to the check added in bochs_hw_init(). This also takes
care of my original worry that we might try writeb()/readb() with an invalid
bochs->mmio value. I'll sent a v9 with the helper added and #ifdefs's removed.

Thanks,
Niklas

Re: [PATCH v8 3/5] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Mon, 2024-10-21 at 12:58 +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 21.10.24 um 12:08 schrieb Arnd Bergmann:
> > On Mon, Oct 21, 2024, at 07:52, Thomas Zimmermann wrote:
> > > Am 08.10.24 um 14:39 schrieb Niklas Schnelle:
> > d 100644
> > > > --- a/drivers/gpu/drm/qxl/Kconfig
> > > > +++ b/drivers/gpu/drm/qxl/Kconfig
> > > > @@ -2,6 +2,7 @@
> > > >config DRM_QXL
> > > > tristate "QXL virtual GPU"
> > > > depends on DRM && PCI && MMU
> > > > +   depends on HAS_IOPORT
> > > Is there a difference between this style (multiple 'depends on') and the
> > > one used for gma500 (&& && &&)?
> > No, it's the same. Doing it in one line is mainly useful
> > if you have some '||' as well.
> > 
> > > > @@ -105,7 +106,9 @@ static void bochs_vga_writeb(struct bochs_device 
> > > > *bochs, u16 ioport, u8 val)
> > > >
> > > > writeb(val, bochs->mmio + offset);
> > > > } else {
> > > > +#ifdef CONFIG_HAS_IOPORT
> > > > outb(val, ioport);
> > > > +#endif
> > > Could you provide empty defines for the out() interfaces at the top of
> > > the file?
> > That no longer works since there are now __compiletime_error()
> > versions of these funcitons. However we can do it more nicely like:
> > 
> > diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> > index 9b337f948434..034af6e32200 100644
> > --- a/drivers/gpu/drm/tiny/bochs.c
> > +++ b/drivers/gpu/drm/tiny/bochs.c
> > @@ -112,14 +112,12 @@ static void bochs_vga_writeb(struct bochs_device 
> > *bochs, u16 ioport, u8 val)
> > if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df))
> > return;
> >   
> > -   if (bochs->mmio) {
> > +   if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) {
> > int offset = ioport - 0x3c0 + 0x400;
> >   
> > writeb(val, bochs->mmio + offset);
> > } else {
> > -#ifdef CONFIG_HAS_IOPORT
> > outb(val, ioport);
> > -#endif
> > }
> 
> For all functions with such a pattern, could we use:
> 
> bool bochs_uses_mmio(bochs)
> {
>      return !IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio
> }
> 
> void writeb_func()
> {
>      if (bochs_uses_mmio()) {
>    writeb()
> #if CONFIG_HAS_IOPORT
>      } else {
>    outb()
> #endif
>      }
> }
> 

I think if the helper were __always_inline we could still take
advantage of the dead code elimination and combine this with Arnd's
approach. Though I feel like it is a bit odd to try to do the MMIO
approach despite bochs->mmio being false on !HAS_IOPORT systems.
Is that what you wanted to correct by keeping the #ifdef
CONFIG_HAS_IOPORT around the else? And yes the warning makes sense to
me too.

Thanks,
Niklas

[PATCH v8 5/5] asm-generic/io.h: Remove I/O port accessors for HAS_IOPORT=n

[Niklas Schnelle]

With all subsystems and drivers either declaring their dependence on
HAS_IOPORT or fencing I/O port specific code sections we can finally
make inb()/outb() and friends compile-time dependent on HAS_IOPORT as
suggested by Linus in the linked mail. The main benefit of this is that
on platforms such as s390 which have no meaningful way of implementing
inb()/outb() their use without the proper HAS_IOPORT dependency will
result in easy to catch and fix compile-time errors instead of compiling
code that can never work.

Link: 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/
Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 include/asm-generic/io.h | 60 
 1 file changed, 60 insertions(+)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 
80de699bf6af4b7b77f582c0469c43e978f67a43..1027be6a62bcbcd5f6797e0fa42035208d0ca79f
 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -540,6 +540,7 @@ static inline void writesq(volatile void __iomem *addr, 
const void *buffer,
 
 #if !defined(inb) && !defined(_inb)
 #define _inb _inb
+#ifdef CONFIG_HAS_IOPORT
 static inline u8 _inb(unsigned long addr)
 {
u8 val;
@@ -549,10 +550,15 @@ static inline u8 _inb(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u8 _inb(unsigned long addr)
+   __compiletime_error("inb()) requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(inw) && !defined(_inw)
 #define _inw _inw
+#ifdef CONFIG_HAS_IOPORT
 static inline u16 _inw(unsigned long addr)
 {
u16 val;
@@ -562,10 +568,15 @@ static inline u16 _inw(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u16 _inw(unsigned long addr)
+   __compiletime_error("inw() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(inl) && !defined(_inl)
 #define _inl _inl
+#ifdef CONFIG_HAS_IOPORT
 static inline u32 _inl(unsigned long addr)
 {
u32 val;
@@ -575,36 +586,55 @@ static inline u32 _inl(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u32 _inl(unsigned long addr)
+   __compiletime_error("inl() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outb) && !defined(_outb)
 #define _outb _outb
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outb(u8 value, unsigned long addr)
 {
__io_pbw();
__raw_writeb(value, PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outb(u8 value, unsigned long addr)
+   __compiletime_error("outb() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outw) && !defined(_outw)
 #define _outw _outw
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outw(u16 value, unsigned long addr)
 {
__io_pbw();
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outw(u16 value, unsigned long addr)
+   __compiletime_error("outw() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outl) && !defined(_outl)
 #define _outl _outl
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outl(u32 value, unsigned long addr)
 {
__io_pbw();
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outl(u32 value, unsigned long addr)
+   __compiletime_error("outl() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #include 
@@ -688,53 +718,83 @@ static inline void outl_p(u32 value, unsigned long addr)
 
 #ifndef insb
 #define insb insb
+#ifdef CONFIG_HAS_IOPORT
 static inline void insb(unsigned long addr, void *buffer, unsigned int count)
 {
readsb(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insb(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insb() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef insw
 #define insw insw
+#ifdef CONFIG_HAS_IOPORT
 static inline void insw(unsigned long addr, void *buffer, unsigned int count)
 {
readsw(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insw(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insw() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef insl
 #define insl insl
+#ifdef CONFIG_HAS_IOPORT
 static inline void insl(unsigned long addr, void *buffer, unsigned int count)
 {
readsl(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insl(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insl() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef outsb
 #define outsb outsb
+#ifdef CONFIG_HAS_IOPORT
 static inline void outsb(unsigned long addr, const void *buffer,
 unsigned int count)
 {
writesb(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void outsb(unsigned long addr, const void *buf

[PATCH v8 4/5] tty: serial: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them unconditionally. Some 8250 serial drivers support
MMIO only use, so fence only the parts requiring I/O ports and print an
error message if a device can't be supported with the current
configuration.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/tty/Kconfig   |  4 ++--
 drivers/tty/serial/8250/8250_early.c  |  4 
 drivers/tty/serial/8250/8250_pci.c| 40 +++
 drivers/tty/serial/8250/8250_pcilib.c | 12 ++-
 drivers/tty/serial/8250/8250_pcilib.h |  2 ++
 drivers/tty/serial/8250/8250_port.c   | 27 +++
 drivers/tty/serial/8250/Kconfig   |  4 ++--
 drivers/tty/serial/Kconfig|  2 +-
 include/linux/serial_core.h   |  4 
 9 files changed, 89 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 
a45d423ad10f02c3a818021bbb18655a8b690500..63a494d36a1fdceba5a7b39f4516060e48af0cc6
 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -220,7 +220,7 @@ config MOXA_INTELLIO
 
 config MOXA_SMARTIO
tristate "Moxa SmartIO support v. 2.0"
-   depends on SERIAL_NONSTANDARD && PCI
+   depends on SERIAL_NONSTANDARD && PCI && HAS_IOPORT
help
  Say Y here if you have a Moxa SmartIO multiport serial card and/or
  want to help develop a new version of this driver.
@@ -302,7 +302,7 @@ config GOLDFISH_TTY_EARLY_CONSOLE
 
 config IPWIRELESS
tristate "IPWireless 3G UMTS PCMCIA card support"
-   depends on PCMCIA && NETDEVICES
+   depends on PCMCIA && NETDEVICES && HAS_IOPORT
select PPP
help
  This is a driver for 3G UMTS PCMCIA card from IPWireless company. In
diff --git a/drivers/tty/serial/8250/8250_early.c 
b/drivers/tty/serial/8250/8250_early.c
index 
6176083d0341ca10edebe5c4eebfffc922a61472..84242292176570cd2c92afbd4755d303827a4abc
 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -46,8 +46,10 @@ static unsigned int serial8250_early_in(struct uart_port 
*port, int offset)
return readl(port->membase + offset);
case UPIO_MEM32BE:
return ioread32be(port->membase + offset);
+#ifdef CONFIG_HAS_IOPORT
case UPIO_PORT:
return inb(port->iobase + offset);
+#endif
default:
return 0;
}
@@ -70,9 +72,11 @@ static void serial8250_early_out(struct uart_port *port, int 
offset, int value)
case UPIO_MEM32BE:
iowrite32be(value, port->membase + offset);
break;
+#ifdef CONFIG_HAS_IOPORT
case UPIO_PORT:
outb(value, port->iobase + offset);
break;
+#endif
}
 }
 
diff --git a/drivers/tty/serial/8250/8250_pci.c 
b/drivers/tty/serial/8250/8250_pci.c
index 
6709b6a5f3011db38acc58dc7223158fe4fcf72e..7d7a6d62c09ceabcf4094d539c565ed09c153561
 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -964,6 +964,9 @@ static int pci_ite887x_init(struct pci_dev *dev)
struct resource *iobase = NULL;
u32 miscr, uartbar, ioport;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(dev);
+
/* search for the base-ioport */
for (i = 0; i < ARRAY_SIZE(inta_addr); i++) {
iobase = request_region(inta_addr[i], ITE_887x_IOSIZE,
@@ -1514,6 +1517,9 @@ static int pci_quatech_init(struct pci_dev *dev)
const struct pci_device_id *match;
bool amcc = false;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(dev);
+
match = pci_match_id(quatech_cards, dev);
if (match)
amcc = match->driver_data;
@@ -1538,6 +1544,9 @@ static int pci_quatech_setup(struct serial_private *priv,
  const struct pciserial_board *board,
  struct uart_8250_port *port, int idx)
 {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(priv->dev);
+
/* Needed by pci_quatech calls below */
port->port.iobase = pci_resource_start(priv->dev, 
FL_GET_BASE(board->flags));
/* Set up the clocking */
@@ -1655,6 +1664,9 @@ static int pci_fintek_setup(struct serial_private *priv,
u8 config_base;
u16 iobase;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(pdev);
+
config_base = 0x40 + 0x08 * idx;
 
/* Get the io address from configuration space */
@@ -1686,6 +1698,9 @@ static int pci_fintek_init(struct pci_dev *dev)
u8 config_bas

[PATCH v8 2/5] Bluetooth: add HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/bluetooth/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 
18767b54df352e929692850dc789d9377839e094..4ab32abf0f48644715d4c27ec0d2fdaccef62b76
 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -336,7 +336,7 @@ config BT_HCIBFUSB
 
 config BT_HCIDTL1
tristate "HCI DTL1 (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
help
  Bluetooth HCI DTL1 (PC Card) driver.
  This driver provides support for Bluetooth PCMCIA devices with
@@ -349,7 +349,7 @@ config BT_HCIDTL1
 
 config BT_HCIBT3C
tristate "HCI BT3C (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
select FW_LOADER
help
  Bluetooth HCI BT3C (PC Card) driver.
@@ -363,7 +363,7 @@ config BT_HCIBT3C
 
 config BT_HCIBLUECARD
tristate "HCI BlueCard (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
help
  Bluetooth HCI BlueCard (PC Card) driver.
  This driver provides support for Bluetooth PCMCIA devices with

-- 
2.43.0

[PATCH v8 3/5] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them. In the bochs driver there is optional MMIO support
detected at runtime, warn if this isn't taken when HAS_IOPORT is not
defined.

There is also a direct and hard coded use in cirrus.c which according to
the comment is only necessary during resume.  Let's just skip this as
for example s390 which doesn't have I/O port support also doesen't
support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/gpu/drm/gma500/Kconfig |  2 +-
 drivers/gpu/drm/qxl/Kconfig|  1 +
 drivers/gpu/drm/tiny/bochs.c   | 17 +
 drivers/gpu/drm/tiny/cirrus.c  |  2 ++
 drivers/gpu/drm/xe/Kconfig |  2 +-
 5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
index 
efb4a2dd2f80885cb59c925d09401002278d7d61..23b7c14de5e29238ece939d5822d8a9ffc4675cc
 100644
--- a/drivers/gpu/drm/gma500/Kconfig
+++ b/drivers/gpu/drm/gma500/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_GMA500
tristate "Intel GMA500/600/3600/3650 KMS Framebuffer"
-   depends on DRM && PCI && X86 && MMU
+   depends on DRM && PCI && X86 && MMU && HAS_IOPORT
select DRM_KMS_HELPER
select FB_IOMEM_HELPERS if DRM_FBDEV_EMULATION
select I2C
diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index 
ca3f51c2a8fe1a383f8a2479f04b5c0b3fb14e44..d0e0d440c8d96564cb7b8ffd2385c44fc43f873d
 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
 config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 
31fc5d839e106ea4d5c8fe42d1bfc3c70291e3fb..0ed78d3d5774778f91de972ac27056938036e722
 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -2,6 +2,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -105,7 +106,9 @@ static void bochs_vga_writeb(struct bochs_device *bochs, 
u16 ioport, u8 val)
 
writeb(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outb(val, ioport);
+#endif
}
 }
 
@@ -119,7 +122,11 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 
ioport)
 
return readb(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
return inb(ioport);
+#else
+   return 0xff;
+#endif
}
 }
 
@@ -132,8 +139,12 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, 
u16 reg)
 
ret = readw(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
ret = inw(VBE_DISPI_IOPORT_DATA);
+#else
+   ret = 0x;
+#endif
}
return ret;
 }
@@ -145,8 +156,10 @@ static void bochs_dispi_write(struct bochs_device *bochs, 
u16 reg, u16 val)
 
writew(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
outw(val, VBE_DISPI_IOPORT_DATA);
+#endif
}
 }
 
@@ -229,6 +242,10 @@ static int bochs_hw_init(struct drm_device *dev)
return -ENOMEM;
}
} else {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
+   DRM_ERROR("I/O ports are not supported\n");
+   return -EIO;
+   }
ioaddr = VBE_DISPI_IOPORT_INDEX;
iosize = 2;
if (!request_region(ioaddr, iosize, "bochs-drm")) {
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 
751326e3d9c374baf72115492aeefff2b73869f0..e31e1df029ab0272c4a1ff0ab3eb026ca679b560
 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -509,8 +509,10 @@ static void cirrus_crtc_helper_atomic_enable(struct 
drm_crtc *crtc,
 
cirrus_mode_set(cirrus, &crtc_state->mode);
 
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(VGA_AR_ENABLE_DISPLAY, VGA_ATT_W);
+#endif
 
drm_dev_exit(idx);
 }
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 
7bbe46a98ff1f449bc2af30686585a00e9e8af93..116f58774135fc3a9f37d6d72d41340f5c812297
 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -49,7 +49,7 @@ config DRM_XE
 
 config DRM_XE_DISPLAY
bool "Enable display sup

[PATCH v8 1/5] hexagon: Don't select GENERIC_IOMAP without HAS_IOPORT support

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. As hexagon does not support I/O port access it also
the GENERIC_IOMAP mechanism of dynamically choosing between I/O port and
MMIO access doesn't work so don't select it.

Reviewed-by: Brian Cain 
Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 arch/hexagon/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
index 
e233b5efa2761e35c416cbc147f6b6422a7c5b8f..5ea1bf4b7d4f5471a9c39ee9fb5c701455d21342
 100644
--- a/arch/hexagon/Kconfig
+++ b/arch/hexagon/Kconfig
@@ -31,7 +31,6 @@ config HEXAGON
select HAVE_ARCH_TRACEHOOK
select NEED_SG_DMA_LENGTH
select NO_IOPORT_MAP
-   select GENERIC_IOMAP
select GENERIC_IOREMAP
select GENERIC_SMP_IDLE_THREAD
select STACKTRACE_SUPPORT

-- 
2.43.0

[PATCH v8 0/5] treewide: Remove I/O port accessors for HAS_IOPORT=n

[Niklas Schnelle]

Hi All,

This is a follow up in my long running effort of making inb()/outb() and
similar I/O port accessors compile-time optional. After initially
sending this as a treewide series with the latest revision at[0]
we switched to per subsystem series. Now though as we're left with only
5 patches left I'm going back to a single series with Arnd planning
to take this via the the asm-generic tree.

This series may also be viewed for your convenience on my git.kernel.org
tree[1] under the b4/has_ioport branch. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Thanks,
Niklas

[0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git
[2] 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/

Signed-off-by: Niklas Schnelle 
---
Changes in v8:
- Don't remove "depends on !S390" for SERIAL_8250
- Link to v7: 
https://lore.kernel.org/r/20241008-b4-has_ioport-v7-0-8624c09a4...@linux.ibm.com

Changes in v7:
- Renamed serial_8250_need_ioport() helper to
  serial_8250_warn_need_ioport() and move it to 8250_pcilib.c so it can
  be used in serial8250_pci_setup_port()
- Flattened if in serial8250_pci_setup_port() (Maciej)
- Removed gratuituous changes (Maciej)
- Removed is_upf_fourport() helper in favor of zeroing UPF_FOURPORT
  if CONFIG_HAS_IOPORT is not set (Maciej)
- Link to v6: 
https://lore.kernel.org/r/20241007-b4-has_ioport-v6-0-03f7240da...@linux.ibm.com

Changes since v5 / per subsystem patches:

drm:
- Add HAS_IOPORT dependency for GMA500
tty: serial:
- Make 8250 PCI driver emit an error message when trying to use devices
  which require I/O ports without CONFIG_HAS_IOPORT (Maciej)
- Use early returns + dead code elimination to skip inb()/outb() uses
  in quirks (Arnd)
- In 8250 PCI driver also handle fintek and moxi quirks
- In 8250 ports code handle um's defined(__i385__) &&
  defined(CONFIG_HAS_IOPORT) case
- Use IS_ENABLED() early return also in is_upf_fourport()
  __always_inline to force constant folding

---
Niklas Schnelle (5):
  hexagon: Don't select GENERIC_IOMAP without HAS_IOPORT support
  Bluetooth: add HAS_IOPORT dependencies
  drm: handle HAS_IOPORT dependencies
  tty: serial: handle HAS_IOPORT dependencies
  asm-generic/io.h: Remove I/O port accessors for HAS_IOPORT=n

 arch/hexagon/Kconfig  |  1 -
 drivers/bluetooth/Kconfig |  6 ++--
 drivers/gpu/drm/gma500/Kconfig|  2 +-
 drivers/gpu/drm/qxl/Kconfig   |  1 +
 drivers/gpu/drm/tiny/bochs.c  | 17 ++
 drivers/gpu/drm/tiny/cirrus.c |  2 ++
 drivers/gpu/drm/xe/Kconfig|  2 +-
 drivers/tty/Kconfig   |  4 +--
 drivers/tty/serial/8250/8250_early.c  |  4 +++
 drivers/tty/serial/8250/8250_pci.c| 40 +++
 drivers/tty/serial/8250/8250_pcilib.c | 12 ++-
 drivers/tty/serial/8250/8250_pcilib.h |  2 ++
 drivers/tty/serial/8250/8250_port.c   | 27 +---
 drivers/tty/serial/8250/Kconfig   |  4 +--
 drivers/tty/serial/Kconfig|  2 +-
 include/asm-generic/io.h  | 60 +++
 include/linux/serial_core.h   |  4 +++
 17 files changed, 174 insertions(+), 16 deletions(-)
---
base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
change-id: 20241004-b4-has_ioport-60ac6ce1deb6

Best regards,
-- 
Niklas Schnelle

Re: [PATCH v7 4/5] tty: serial: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Tue, 2024-10-08 at 13:32 +0200, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
> compile time. We thus need to add HAS_IOPORT as dependency for those
> drivers using them unconditionally. Some 8250 serial drivers support
> MMIO only use, so fence only the parts requiring I/O ports and print an
> error message if a device can't be supported with the current
> configuration.
> 
> Co-developed-by: Arnd Bergmann 
> Signed-off-by: Arnd Bergmann 
> Signed-off-by: Niklas Schnelle 
> ---
> 
---8<---
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 
> 47ff50763c048c89b19b7c9f13f462bc5368ab43..afa75020d132de2210545fbe54e0d03df4b72a41
>  100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -6,7 +6,6 @@
>  
>  config SERIAL_8250
>   tristate "8250/16550 and compatible serial support"
> - depends on !S390
>   select SERIAL_CORE
>   select SERIAL_MCTRL_GPIO if GPIOLIB
>   help

Sorry ;-( I didn't mean to commit the above hunk. I always need to
change this for my test compiles and it slipped in. With this series it
all builds but we previously decided we don't want this as part of this
patch. I'll send a v8.

[PATCH v7 4/5] tty: serial: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them unconditionally. Some 8250 serial drivers support
MMIO only use, so fence only the parts requiring I/O ports and print an
error message if a device can't be supported with the current
configuration.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/tty/Kconfig   |  4 ++--
 drivers/tty/serial/8250/8250_early.c  |  4 
 drivers/tty/serial/8250/8250_pci.c| 40 +++
 drivers/tty/serial/8250/8250_pcilib.c | 12 ++-
 drivers/tty/serial/8250/8250_pcilib.h |  2 ++
 drivers/tty/serial/8250/8250_port.c   | 27 +++
 drivers/tty/serial/8250/Kconfig   |  5 ++---
 drivers/tty/serial/Kconfig|  2 +-
 include/linux/serial_core.h   |  4 
 9 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 
a45d423ad10f02c3a818021bbb18655a8b690500..63a494d36a1fdceba5a7b39f4516060e48af0cc6
 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -220,7 +220,7 @@ config MOXA_INTELLIO
 
 config MOXA_SMARTIO
tristate "Moxa SmartIO support v. 2.0"
-   depends on SERIAL_NONSTANDARD && PCI
+   depends on SERIAL_NONSTANDARD && PCI && HAS_IOPORT
help
  Say Y here if you have a Moxa SmartIO multiport serial card and/or
  want to help develop a new version of this driver.
@@ -302,7 +302,7 @@ config GOLDFISH_TTY_EARLY_CONSOLE
 
 config IPWIRELESS
tristate "IPWireless 3G UMTS PCMCIA card support"
-   depends on PCMCIA && NETDEVICES
+   depends on PCMCIA && NETDEVICES && HAS_IOPORT
select PPP
help
  This is a driver for 3G UMTS PCMCIA card from IPWireless company. In
diff --git a/drivers/tty/serial/8250/8250_early.c 
b/drivers/tty/serial/8250/8250_early.c
index 
6176083d0341ca10edebe5c4eebfffc922a61472..84242292176570cd2c92afbd4755d303827a4abc
 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -46,8 +46,10 @@ static unsigned int serial8250_early_in(struct uart_port 
*port, int offset)
return readl(port->membase + offset);
case UPIO_MEM32BE:
return ioread32be(port->membase + offset);
+#ifdef CONFIG_HAS_IOPORT
case UPIO_PORT:
return inb(port->iobase + offset);
+#endif
default:
return 0;
}
@@ -70,9 +72,11 @@ static void serial8250_early_out(struct uart_port *port, int 
offset, int value)
case UPIO_MEM32BE:
iowrite32be(value, port->membase + offset);
break;
+#ifdef CONFIG_HAS_IOPORT
case UPIO_PORT:
outb(value, port->iobase + offset);
break;
+#endif
}
 }
 
diff --git a/drivers/tty/serial/8250/8250_pci.c 
b/drivers/tty/serial/8250/8250_pci.c
index 
6709b6a5f3011db38acc58dc7223158fe4fcf72e..7d7a6d62c09ceabcf4094d539c565ed09c153561
 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -964,6 +964,9 @@ static int pci_ite887x_init(struct pci_dev *dev)
struct resource *iobase = NULL;
u32 miscr, uartbar, ioport;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(dev);
+
/* search for the base-ioport */
for (i = 0; i < ARRAY_SIZE(inta_addr); i++) {
iobase = request_region(inta_addr[i], ITE_887x_IOSIZE,
@@ -1514,6 +1517,9 @@ static int pci_quatech_init(struct pci_dev *dev)
const struct pci_device_id *match;
bool amcc = false;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(dev);
+
match = pci_match_id(quatech_cards, dev);
if (match)
amcc = match->driver_data;
@@ -1538,6 +1544,9 @@ static int pci_quatech_setup(struct serial_private *priv,
  const struct pciserial_board *board,
  struct uart_8250_port *port, int idx)
 {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(priv->dev);
+
/* Needed by pci_quatech calls below */
port->port.iobase = pci_resource_start(priv->dev, 
FL_GET_BASE(board->flags));
/* Set up the clocking */
@@ -1655,6 +1664,9 @@ static int pci_fintek_setup(struct serial_private *priv,
u8 config_base;
u16 iobase;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_warn_need_ioport(pdev);
+
config_base = 0x40 + 0x08 * idx;
 
/* Get the io address from configuration space */
@@ -1686,6 +1698,9 @@ static int pci_fintek_init(struct pci_dev *dev)
u8 config_bas

[PATCH v7 5/5] asm-generic/io.h: Remove I/O port accessors for HAS_IOPORT=n

[Niklas Schnelle]

With all subsystems and drivers either declaring their dependence on
HAS_IOPORT or fencing I/O port specific code sections we can finally
make inb()/outb() and friends compile-time dependent on HAS_IOPORT as
suggested by Linus in the linked mail. The main benefit of this is that
on platforms such as s390 which have no meaningful way of implementing
inb()/outb() their use without the proper HAS_IOPORT dependency will
result in easy to catch and fix compile-time errors instead of compiling
code that can never work.

Link: 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/
Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 include/asm-generic/io.h | 60 
 1 file changed, 60 insertions(+)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 
80de699bf6af4b7b77f582c0469c43e978f67a43..1027be6a62bcbcd5f6797e0fa42035208d0ca79f
 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -540,6 +540,7 @@ static inline void writesq(volatile void __iomem *addr, 
const void *buffer,
 
 #if !defined(inb) && !defined(_inb)
 #define _inb _inb
+#ifdef CONFIG_HAS_IOPORT
 static inline u8 _inb(unsigned long addr)
 {
u8 val;
@@ -549,10 +550,15 @@ static inline u8 _inb(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u8 _inb(unsigned long addr)
+   __compiletime_error("inb()) requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(inw) && !defined(_inw)
 #define _inw _inw
+#ifdef CONFIG_HAS_IOPORT
 static inline u16 _inw(unsigned long addr)
 {
u16 val;
@@ -562,10 +568,15 @@ static inline u16 _inw(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u16 _inw(unsigned long addr)
+   __compiletime_error("inw() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(inl) && !defined(_inl)
 #define _inl _inl
+#ifdef CONFIG_HAS_IOPORT
 static inline u32 _inl(unsigned long addr)
 {
u32 val;
@@ -575,36 +586,55 @@ static inline u32 _inl(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u32 _inl(unsigned long addr)
+   __compiletime_error("inl() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outb) && !defined(_outb)
 #define _outb _outb
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outb(u8 value, unsigned long addr)
 {
__io_pbw();
__raw_writeb(value, PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outb(u8 value, unsigned long addr)
+   __compiletime_error("outb() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outw) && !defined(_outw)
 #define _outw _outw
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outw(u16 value, unsigned long addr)
 {
__io_pbw();
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outw(u16 value, unsigned long addr)
+   __compiletime_error("outw() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outl) && !defined(_outl)
 #define _outl _outl
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outl(u32 value, unsigned long addr)
 {
__io_pbw();
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outl(u32 value, unsigned long addr)
+   __compiletime_error("outl() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #include 
@@ -688,53 +718,83 @@ static inline void outl_p(u32 value, unsigned long addr)
 
 #ifndef insb
 #define insb insb
+#ifdef CONFIG_HAS_IOPORT
 static inline void insb(unsigned long addr, void *buffer, unsigned int count)
 {
readsb(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insb(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insb() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef insw
 #define insw insw
+#ifdef CONFIG_HAS_IOPORT
 static inline void insw(unsigned long addr, void *buffer, unsigned int count)
 {
readsw(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insw(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insw() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef insl
 #define insl insl
+#ifdef CONFIG_HAS_IOPORT
 static inline void insl(unsigned long addr, void *buffer, unsigned int count)
 {
readsl(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insl(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insl() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef outsb
 #define outsb outsb
+#ifdef CONFIG_HAS_IOPORT
 static inline void outsb(unsigned long addr, const void *buffer,
 unsigned int count)
 {
writesb(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void outsb(unsigned long addr, const void *buf

[PATCH v7 3/5] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them. In the bochs driver there is optional MMIO support
detected at runtime, warn if this isn't taken when HAS_IOPORT is not
defined.

There is also a direct and hard coded use in cirrus.c which according to
the comment is only necessary during resume.  Let's just skip this as
for example s390 which doesn't have I/O port support also doesen't
support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/gpu/drm/gma500/Kconfig |  2 +-
 drivers/gpu/drm/qxl/Kconfig|  1 +
 drivers/gpu/drm/tiny/bochs.c   | 17 +
 drivers/gpu/drm/tiny/cirrus.c  |  2 ++
 drivers/gpu/drm/xe/Kconfig |  2 +-
 5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
index 
efb4a2dd2f80885cb59c925d09401002278d7d61..23b7c14de5e29238ece939d5822d8a9ffc4675cc
 100644
--- a/drivers/gpu/drm/gma500/Kconfig
+++ b/drivers/gpu/drm/gma500/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_GMA500
tristate "Intel GMA500/600/3600/3650 KMS Framebuffer"
-   depends on DRM && PCI && X86 && MMU
+   depends on DRM && PCI && X86 && MMU && HAS_IOPORT
select DRM_KMS_HELPER
select FB_IOMEM_HELPERS if DRM_FBDEV_EMULATION
select I2C
diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index 
ca3f51c2a8fe1a383f8a2479f04b5c0b3fb14e44..d0e0d440c8d96564cb7b8ffd2385c44fc43f873d
 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
 config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 
31fc5d839e106ea4d5c8fe42d1bfc3c70291e3fb..0ed78d3d5774778f91de972ac27056938036e722
 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -2,6 +2,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -105,7 +106,9 @@ static void bochs_vga_writeb(struct bochs_device *bochs, 
u16 ioport, u8 val)
 
writeb(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outb(val, ioport);
+#endif
}
 }
 
@@ -119,7 +122,11 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 
ioport)
 
return readb(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
return inb(ioport);
+#else
+   return 0xff;
+#endif
}
 }
 
@@ -132,8 +139,12 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, 
u16 reg)
 
ret = readw(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
ret = inw(VBE_DISPI_IOPORT_DATA);
+#else
+   ret = 0x;
+#endif
}
return ret;
 }
@@ -145,8 +156,10 @@ static void bochs_dispi_write(struct bochs_device *bochs, 
u16 reg, u16 val)
 
writew(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
outw(val, VBE_DISPI_IOPORT_DATA);
+#endif
}
 }
 
@@ -229,6 +242,10 @@ static int bochs_hw_init(struct drm_device *dev)
return -ENOMEM;
}
} else {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
+   DRM_ERROR("I/O ports are not supported\n");
+   return -EIO;
+   }
ioaddr = VBE_DISPI_IOPORT_INDEX;
iosize = 2;
if (!request_region(ioaddr, iosize, "bochs-drm")) {
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 
751326e3d9c374baf72115492aeefff2b73869f0..e31e1df029ab0272c4a1ff0ab3eb026ca679b560
 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -509,8 +509,10 @@ static void cirrus_crtc_helper_atomic_enable(struct 
drm_crtc *crtc,
 
cirrus_mode_set(cirrus, &crtc_state->mode);
 
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(VGA_AR_ENABLE_DISPLAY, VGA_ATT_W);
+#endif
 
drm_dev_exit(idx);
 }
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 
7bbe46a98ff1f449bc2af30686585a00e9e8af93..116f58774135fc3a9f37d6d72d41340f5c812297
 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -49,7 +49,7 @@ config DRM_XE
 
 config DRM_XE_DISPLAY
bool "Enable display sup

[PATCH v7 2/5] Bluetooth: add HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/bluetooth/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 
18767b54df352e929692850dc789d9377839e094..4ab32abf0f48644715d4c27ec0d2fdaccef62b76
 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -336,7 +336,7 @@ config BT_HCIBFUSB
 
 config BT_HCIDTL1
tristate "HCI DTL1 (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
help
  Bluetooth HCI DTL1 (PC Card) driver.
  This driver provides support for Bluetooth PCMCIA devices with
@@ -349,7 +349,7 @@ config BT_HCIDTL1
 
 config BT_HCIBT3C
tristate "HCI BT3C (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
select FW_LOADER
help
  Bluetooth HCI BT3C (PC Card) driver.
@@ -363,7 +363,7 @@ config BT_HCIBT3C
 
 config BT_HCIBLUECARD
tristate "HCI BlueCard (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
help
  Bluetooth HCI BlueCard (PC Card) driver.
  This driver provides support for Bluetooth PCMCIA devices with

-- 
2.43.0

[PATCH v7 1/5] hexagon: Don't select GENERIC_IOMAP without HAS_IOPORT support

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. As hexagon does not support I/O port access it also
the GENERIC_IOMAP mechanism of dynamically choosing between I/O port and
MMIO access doesn't work so don't select it.

Reviewed-by: Brian Cain 
Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 arch/hexagon/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
index 
e233b5efa2761e35c416cbc147f6b6422a7c5b8f..5ea1bf4b7d4f5471a9c39ee9fb5c701455d21342
 100644
--- a/arch/hexagon/Kconfig
+++ b/arch/hexagon/Kconfig
@@ -31,7 +31,6 @@ config HEXAGON
select HAVE_ARCH_TRACEHOOK
select NEED_SG_DMA_LENGTH
select NO_IOPORT_MAP
-   select GENERIC_IOMAP
select GENERIC_IOREMAP
select GENERIC_SMP_IDLE_THREAD
select STACKTRACE_SUPPORT

-- 
2.43.0

[PATCH v7 0/5] treewide: Remove I/O port accessors for HAS_IOPORT=n

[Niklas Schnelle]

Hi All,

This is a follow up in my long running effort of making inb()/outb() and
similar I/O port accessors compile-time optional. After initially
sending this as a treewide series with the latest revision at[0]
we switched to per subsystem series. Now though as we're left with only
5 patches left I'm going back to a single series with Arnd planning
to take this via the the asm-generic tree.

This series may also be viewed for your convenience on my git.kernel.org
tree[1] under the b4/has_ioport branch. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Thanks,
Niklas

[0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git
[2] 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/

Signed-off-by: Niklas Schnelle 
---
Changes in v7:
- Renamed serial_8250_need_ioport() helper to
  serial_8250_warn_need_ioport() and move it to 8250_pcilib.c so it can
  be used in serial8250_pci_setup_port()
- Flattened if in serial8250_pci_setup_port() (Maciej)
- Removed gratuituous changes (Maciej)
- Removed is_upf_fourport() helper in favor of zeroing UPF_FOURPORT
  if CONFIG_HAS_IOPORT is not set (Maciej)
- Link to v6: 
https://lore.kernel.org/r/20241007-b4-has_ioport-v6-0-03f7240da...@linux.ibm.com

Changes since v5 / per subsystem patches:

drm:
- Add HAS_IOPORT dependency for GMA500
tty: serial:
- Make 8250 PCI driver emit an error message when trying to use devices
  which require I/O ports without CONFIG_HAS_IOPORT (Maciej)
- Use early returns + dead code elimination to skip inb()/outb() uses
  in quirks (Arnd)
- In 8250 PCI driver also handle fintek and moxi quirks
- In 8250 ports code handle um's defined(__i385__) &&
  defined(CONFIG_HAS_IOPORT) case
- Use IS_ENABLED() early return also in is_upf_fourport()
  __always_inline to force constant folding

---
Niklas Schnelle (5):
  hexagon: Don't select GENERIC_IOMAP without HAS_IOPORT support
  Bluetooth: add HAS_IOPORT dependencies
  drm: handle HAS_IOPORT dependencies
  tty: serial: handle HAS_IOPORT dependencies
  asm-generic/io.h: Remove I/O port accessors for HAS_IOPORT=n

 arch/hexagon/Kconfig  |  1 -
 drivers/bluetooth/Kconfig |  6 ++--
 drivers/gpu/drm/gma500/Kconfig|  2 +-
 drivers/gpu/drm/qxl/Kconfig   |  1 +
 drivers/gpu/drm/tiny/bochs.c  | 17 ++
 drivers/gpu/drm/tiny/cirrus.c |  2 ++
 drivers/gpu/drm/xe/Kconfig|  2 +-
 drivers/tty/Kconfig   |  4 +--
 drivers/tty/serial/8250/8250_early.c  |  4 +++
 drivers/tty/serial/8250/8250_pci.c| 40 +++
 drivers/tty/serial/8250/8250_pcilib.c | 12 ++-
 drivers/tty/serial/8250/8250_pcilib.h |  2 ++
 drivers/tty/serial/8250/8250_port.c   | 27 +---
 drivers/tty/serial/8250/Kconfig   |  5 ++-
 drivers/tty/serial/Kconfig|  2 +-
 include/asm-generic/io.h  | 60 +++
 include/linux/serial_core.h   |  4 +++
 17 files changed, 174 insertions(+), 17 deletions(-)
---
base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
change-id: 20241004-b4-has_ioport-60ac6ce1deb6

Best regards,
-- 
Niklas Schnelle

Re: [PATCH v6 4/5] tty: serial: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Tue, 2024-10-08 at 10:16 +0200, Niklas Schnelle wrote:
> On Mon, 2024-10-07 at 22:09 +0100, Maciej W. Rozycki wrote:
> > On Mon, 7 Oct 2024, Niklas Schnelle wrote:
> > 
> > > diff --git a/drivers/tty/serial/8250/8250_pci.c 
> > > b/drivers/tty/serial/8250/8250_pci.c
> > > index 
> > > 6709b6a5f3011db38acc58dc7223158fe4fcf72e..6a638feb44e443a1998980dd037748f227ec1bc8
> > >  100644
> > > --- a/drivers/tty/serial/8250/8250_pci.c
> > > +++ b/drivers/tty/serial/8250/8250_pci.c
> > [...]
> > >   iobase = pci_resource_start(dev, 0);
> > >   outb(0x0, iobase + CH384_XINT_ENABLE_REG);
> > >  }
> > >  
> > > -
> > >  static int
> > >  pci_sunix_setup(struct serial_private *priv,
> > >   const struct pciserial_board *board,
> > 
> >  Gratuitous change here.
> > 
> > > diff --git a/drivers/tty/serial/8250/8250_pcilib.c 
> > > b/drivers/tty/serial/8250/8250_pcilib.c
> > > index 
> > > ea906d721b2c3eac15c9e8d62cc6fa56c3ef6150..fc1882d7515b5814ff1240ffdbe1009ab908ad6b
> > >  100644
> > > --- a/drivers/tty/serial/8250/8250_pcilib.c
> > > +++ b/drivers/tty/serial/8250/8250_pcilib.c
> > > @@ -28,6 +28,10 @@ int serial8250_pci_setup_port(struct pci_dev *dev, 
> > > struct uart_8250_port *port,
> > >   port->port.membase = pcim_iomap_table(dev)[bar] + offset;
> > >   port->port.regshift = regshift;
> > >   } else {
> > > + if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
> > > + pr_err("Serial port %lx requires I/O port support\n", 
> > > port->port.iobase);
> > > + return -EINVAL;
> > > + }
> > >   port->port.iotype = UPIO_PORT;
> > >   port->port.iobase = pci_resource_start(dev, bar) + offset;
> > >   port->port.mapbase = 0;
> > 
> >  Can we please flatten this conditional and get rid of the negation, and 
> > also use `pci_err' for clear identification (`port->port.iobase' may not 
> > even have been set to anything meaningful if this triggers)?  I.e.:
> > 
> > /* ... */
> > } else if (IS_ENABLED(CONFIG_HAS_IOPORT)) {
> > /* ... */
> > } else {
> > pci_err(dev, "serial port requires I/O port support\n");
> > return -EINVAL;
> > }
> > 
> > I'd also say "port I/O" (by analogy to "memory-mapped I/O") rather than 
> > "I/O port", but I can imagine it might be debatable.
> 
> Agree this looks better, will change it.

While changing this I noticed that this isn't aligned with the other
print. There we use dev_warn() and -ENXIO vs -EINVAL. How about we move
serial_8250_need_ioport() to 8250_pcilib.c and use it here too? Then we
also only have a single place for the message.

Thanks,
Niklas

Re: [PATCH v6 4/5] tty: serial: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Mon, 2024-10-07 at 22:09 +0100, Maciej W. Rozycki wrote:
> On Mon, 7 Oct 2024, Niklas Schnelle wrote:
> 
> > diff --git a/drivers/tty/serial/8250/8250_pci.c 
> > b/drivers/tty/serial/8250/8250_pci.c
> > index 
> > 6709b6a5f3011db38acc58dc7223158fe4fcf72e..6a638feb44e443a1998980dd037748f227ec1bc8
> >  100644
> > --- a/drivers/tty/serial/8250/8250_pci.c
> > +++ b/drivers/tty/serial/8250/8250_pci.c
> [...]
> > iobase = pci_resource_start(dev, 0);
> > outb(0x0, iobase + CH384_XINT_ENABLE_REG);
> >  }
> >  
> > -
> >  static int
> >  pci_sunix_setup(struct serial_private *priv,
> > const struct pciserial_board *board,
> 
>  Gratuitous change here.
> 
> > diff --git a/drivers/tty/serial/8250/8250_pcilib.c 
> > b/drivers/tty/serial/8250/8250_pcilib.c
> > index 
> > ea906d721b2c3eac15c9e8d62cc6fa56c3ef6150..fc1882d7515b5814ff1240ffdbe1009ab908ad6b
> >  100644
> > --- a/drivers/tty/serial/8250/8250_pcilib.c
> > +++ b/drivers/tty/serial/8250/8250_pcilib.c
> > @@ -28,6 +28,10 @@ int serial8250_pci_setup_port(struct pci_dev *dev, 
> > struct uart_8250_port *port,
> > port->port.membase = pcim_iomap_table(dev)[bar] + offset;
> > port->port.regshift = regshift;
> > } else {
> > +   if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
> > +   pr_err("Serial port %lx requires I/O port support\n", 
> > port->port.iobase);
> > +   return -EINVAL;
> > +   }
> > port->port.iotype = UPIO_PORT;
> > port->port.iobase = pci_resource_start(dev, bar) + offset;
> > port->port.mapbase = 0;
> 
>  Can we please flatten this conditional and get rid of the negation, and 
> also use `pci_err' for clear identification (`port->port.iobase' may not 
> even have been set to anything meaningful if this triggers)?  I.e.:
> 
>   /* ... */
>   } else if (IS_ENABLED(CONFIG_HAS_IOPORT)) {
>   /* ... */
>   } else {
>   pci_err(dev, "serial port requires I/O port support\n");
>   return -EINVAL;
>   }
> 
> I'd also say "port I/O" (by analogy to "memory-mapped I/O") rather than 
> "I/O port", but I can imagine it might be debatable.

Agree this looks better, will change it.

> 
> > +static __always_inline bool is_upf_fourport(struct uart_port *port)
> > +{
> > +   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
> > +   return false;
> > +
> > +   return port->flags & UPF_FOURPORT;
> > +}
> 
>  Can we perhaps avoid adding this helper and then tweaking code throughout 
> by having:
> 
> #ifdef CONFIG_SERIAL_8250_FOURPORT
> #define UPF_FOURPORT  ((__force upf_t) ASYNC_FOURPORT   /* 1  */ )
> #else
> #define UPF_FOURPORT  0
> #endif
> 
> in include/linux/serial_core.h instead?  I can see the flag is reused by 
> drivers/tty/serial/sunsu.c, but from a glance over it seems rubbish to me 
> and such a change won't hurt the driver anyway.

I'll look at this, do you think this is okay regarding matching the
user-space definitions in include/uapi/linux/tty_flags.h?

> 
> > @@ -1174,7 +1201,7 @@ static void autoconfig(struct uart_8250_port *up)
> >  */
> > scratch = serial_in(up, UART_IER);
> > serial_out(up, UART_IER, 0);
> > -#ifdef __i386__
> > +#if defined(__i386__) && defined(CONFIG_HAS_IOPORT)
> > outb(0xff, 0x080);
> >  #endif
> > /*
> > @@ -1183,7 +1210,7 @@ static void autoconfig(struct uart_8250_port *up)
> >  */
> > scratch2 = serial_in(up, UART_IER) & UART_IER_ALL_INTR;
> > serial_out(up, UART_IER, UART_IER_ALL_INTR);
> > -#ifdef __i386__
> > +#if defined(__i386__) && defined(CONFIG_HAS_IOPORT)
> > outb(0, 0x080);
> >  #endif
> > scratch3 = serial_in(up, UART_IER) & UART_IER_ALL_INTR;
> 
>  Nah, i386 does have machine OUTB instructions, it has the port I/O 
> address space in the ISA, so these two changes make no sense to me.  
> 
>  Though this #ifdef should likely be converted to CONFIG_X86_32 via a 
> separate change.

This is needed for Usermode Linux (UM) which sets __i386__ but also
doesn't have CONFIG_HAS_IOPORT. This was spotted by the kernel test bot
here: https://lore.kernel.org/all/202410031712.bwfgjrqy-...@intel.com/

> 
> > @@ -1306,12 +1333,12 @@ static void autoconfig_irq(struct uart_8250_port 
> > *up)
> >  {
> > struct uart_port *port = &up->port;
> > unsigned char save_mcr, save_ier;
> > +   unsigned long irqs;
> > unsigned char save_ICP = 0;
> > unsigned int ICP = 0;
> > -   unsigned long irqs;
> > int irq;
> 
>  Gratuitous change here (also breaking the reverse Christmas tree order).
> 
>  Thanks for making the clean-ups we discussed.
> 
>   Maciej

WIll drop this hunk

[PATCH v6 5/5] asm-generic/io.h: Remove I/O port accessors for HAS_IOPORT=n

[Niklas Schnelle]

With all subsystems and drivers either declaring their dependence on
HAS_IOPORT or fencing I/O port specific code sections we can finally
make inb()/outb() and friends compile-time dependent on HAS_IOPORT as
suggested by Linus in the linked mail. The main benefit of this is that
on platforms such as s390 which have no meaningful way of implementing
inb()/outb() their use without the proper HAS_IOPORT dependency will
result in easy to catch and fix compile-time errors instead of compiling
code that can never work.

Link: 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/
Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 include/asm-generic/io.h | 60 
 1 file changed, 60 insertions(+)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 
80de699bf6af4b7b77f582c0469c43e978f67a43..1027be6a62bcbcd5f6797e0fa42035208d0ca79f
 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -540,6 +540,7 @@ static inline void writesq(volatile void __iomem *addr, 
const void *buffer,
 
 #if !defined(inb) && !defined(_inb)
 #define _inb _inb
+#ifdef CONFIG_HAS_IOPORT
 static inline u8 _inb(unsigned long addr)
 {
u8 val;
@@ -549,10 +550,15 @@ static inline u8 _inb(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u8 _inb(unsigned long addr)
+   __compiletime_error("inb()) requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(inw) && !defined(_inw)
 #define _inw _inw
+#ifdef CONFIG_HAS_IOPORT
 static inline u16 _inw(unsigned long addr)
 {
u16 val;
@@ -562,10 +568,15 @@ static inline u16 _inw(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u16 _inw(unsigned long addr)
+   __compiletime_error("inw() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(inl) && !defined(_inl)
 #define _inl _inl
+#ifdef CONFIG_HAS_IOPORT
 static inline u32 _inl(unsigned long addr)
 {
u32 val;
@@ -575,36 +586,55 @@ static inline u32 _inl(unsigned long addr)
__io_par(val);
return val;
 }
+#else
+u32 _inl(unsigned long addr)
+   __compiletime_error("inl() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outb) && !defined(_outb)
 #define _outb _outb
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outb(u8 value, unsigned long addr)
 {
__io_pbw();
__raw_writeb(value, PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outb(u8 value, unsigned long addr)
+   __compiletime_error("outb() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outw) && !defined(_outw)
 #define _outw _outw
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outw(u16 value, unsigned long addr)
 {
__io_pbw();
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outw(u16 value, unsigned long addr)
+   __compiletime_error("outw() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #if !defined(outl) && !defined(_outl)
 #define _outl _outl
+#ifdef CONFIG_HAS_IOPORT
 static inline void _outl(u32 value, unsigned long addr)
 {
__io_pbw();
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
__io_paw();
 }
+#else
+void _outl(u32 value, unsigned long addr)
+   __compiletime_error("outl() requires CONFIG_HAS_IOPORT");
+#endif
 #endif
 
 #include 
@@ -688,53 +718,83 @@ static inline void outl_p(u32 value, unsigned long addr)
 
 #ifndef insb
 #define insb insb
+#ifdef CONFIG_HAS_IOPORT
 static inline void insb(unsigned long addr, void *buffer, unsigned int count)
 {
readsb(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insb(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insb() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef insw
 #define insw insw
+#ifdef CONFIG_HAS_IOPORT
 static inline void insw(unsigned long addr, void *buffer, unsigned int count)
 {
readsw(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insw(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insw() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef insl
 #define insl insl
+#ifdef CONFIG_HAS_IOPORT
 static inline void insl(unsigned long addr, void *buffer, unsigned int count)
 {
readsl(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void insl(unsigned long addr, void *buffer, unsigned int count)
+   __compiletime_error("insl() requires HAS_IOPORT");
+#endif
 #endif
 
 #ifndef outsb
 #define outsb outsb
+#ifdef CONFIG_HAS_IOPORT
 static inline void outsb(unsigned long addr, const void *buffer,
 unsigned int count)
 {
writesb(PCI_IOBASE + addr, buffer, count);
 }
+#else
+void outsb(unsigned long addr, const void *buf

[PATCH v6 3/5] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them. In the bochs driver there is optional MMIO support
detected at runtime, warn if this isn't taken when HAS_IOPORT is not
defined.

There is also a direct and hard coded use in cirrus.c which according to
the comment is only necessary during resume.  Let's just skip this as
for example s390 which doesn't have I/O port support also doesen't
support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/gpu/drm/gma500/Kconfig |  2 +-
 drivers/gpu/drm/qxl/Kconfig|  1 +
 drivers/gpu/drm/tiny/bochs.c   | 17 +
 drivers/gpu/drm/tiny/cirrus.c  |  2 ++
 drivers/gpu/drm/xe/Kconfig |  2 +-
 5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
index 
efb4a2dd2f80885cb59c925d09401002278d7d61..23b7c14de5e29238ece939d5822d8a9ffc4675cc
 100644
--- a/drivers/gpu/drm/gma500/Kconfig
+++ b/drivers/gpu/drm/gma500/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config DRM_GMA500
tristate "Intel GMA500/600/3600/3650 KMS Framebuffer"
-   depends on DRM && PCI && X86 && MMU
+   depends on DRM && PCI && X86 && MMU && HAS_IOPORT
select DRM_KMS_HELPER
select FB_IOMEM_HELPERS if DRM_FBDEV_EMULATION
select I2C
diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index 
ca3f51c2a8fe1a383f8a2479f04b5c0b3fb14e44..d0e0d440c8d96564cb7b8ffd2385c44fc43f873d
 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
 config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 
31fc5d839e106ea4d5c8fe42d1bfc3c70291e3fb..0ed78d3d5774778f91de972ac27056938036e722
 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -2,6 +2,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -105,7 +106,9 @@ static void bochs_vga_writeb(struct bochs_device *bochs, 
u16 ioport, u8 val)
 
writeb(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outb(val, ioport);
+#endif
}
 }
 
@@ -119,7 +122,11 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 
ioport)
 
return readb(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
return inb(ioport);
+#else
+   return 0xff;
+#endif
}
 }
 
@@ -132,8 +139,12 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, 
u16 reg)
 
ret = readw(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
ret = inw(VBE_DISPI_IOPORT_DATA);
+#else
+   ret = 0x;
+#endif
}
return ret;
 }
@@ -145,8 +156,10 @@ static void bochs_dispi_write(struct bochs_device *bochs, 
u16 reg, u16 val)
 
writew(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
outw(val, VBE_DISPI_IOPORT_DATA);
+#endif
}
 }
 
@@ -229,6 +242,10 @@ static int bochs_hw_init(struct drm_device *dev)
return -ENOMEM;
}
} else {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
+   DRM_ERROR("I/O ports are not supported\n");
+   return -EIO;
+   }
ioaddr = VBE_DISPI_IOPORT_INDEX;
iosize = 2;
if (!request_region(ioaddr, iosize, "bochs-drm")) {
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 
751326e3d9c374baf72115492aeefff2b73869f0..e31e1df029ab0272c4a1ff0ab3eb026ca679b560
 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -509,8 +509,10 @@ static void cirrus_crtc_helper_atomic_enable(struct 
drm_crtc *crtc,
 
cirrus_mode_set(cirrus, &crtc_state->mode);
 
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(VGA_AR_ENABLE_DISPLAY, VGA_ATT_W);
+#endif
 
drm_dev_exit(idx);
 }
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 
7bbe46a98ff1f449bc2af30686585a00e9e8af93..116f58774135fc3a9f37d6d72d41340f5c812297
 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -49,7 +49,7 @@ config DRM_XE
 
 config DRM_XE_DISPLAY
bool "Enable display sup

[PATCH v6 4/5] tty: serial: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them unconditionally. Some 8250 serial drivers support
MMIO only use, so fence only the parts requiring I/O ports and print an
error message if a device can't be supported with the current
configuration.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/tty/Kconfig   |  4 +--
 drivers/tty/serial/8250/8250_early.c  |  4 +++
 drivers/tty/serial/8250/8250_pci.c| 49 ++-
 drivers/tty/serial/8250/8250_pcilib.c |  4 +++
 drivers/tty/serial/8250/8250_port.c   | 47 ++---
 drivers/tty/serial/8250/Kconfig   |  4 +--
 drivers/tty/serial/Kconfig|  2 +-
 7 files changed, 98 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 
a45d423ad10f02c3a818021bbb18655a8b690500..63a494d36a1fdceba5a7b39f4516060e48af0cc6
 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -220,7 +220,7 @@ config MOXA_INTELLIO
 
 config MOXA_SMARTIO
tristate "Moxa SmartIO support v. 2.0"
-   depends on SERIAL_NONSTANDARD && PCI
+   depends on SERIAL_NONSTANDARD && PCI && HAS_IOPORT
help
  Say Y here if you have a Moxa SmartIO multiport serial card and/or
  want to help develop a new version of this driver.
@@ -302,7 +302,7 @@ config GOLDFISH_TTY_EARLY_CONSOLE
 
 config IPWIRELESS
tristate "IPWireless 3G UMTS PCMCIA card support"
-   depends on PCMCIA && NETDEVICES
+   depends on PCMCIA && NETDEVICES && HAS_IOPORT
select PPP
help
  This is a driver for 3G UMTS PCMCIA card from IPWireless company. In
diff --git a/drivers/tty/serial/8250/8250_early.c 
b/drivers/tty/serial/8250/8250_early.c
index 
6176083d0341ca10edebe5c4eebfffc922a61472..84242292176570cd2c92afbd4755d303827a4abc
 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -46,8 +46,10 @@ static unsigned int serial8250_early_in(struct uart_port 
*port, int offset)
return readl(port->membase + offset);
case UPIO_MEM32BE:
return ioread32be(port->membase + offset);
+#ifdef CONFIG_HAS_IOPORT
case UPIO_PORT:
return inb(port->iobase + offset);
+#endif
default:
return 0;
}
@@ -70,9 +72,11 @@ static void serial8250_early_out(struct uart_port *port, int 
offset, int value)
case UPIO_MEM32BE:
iowrite32be(value, port->membase + offset);
break;
+#ifdef CONFIG_HAS_IOPORT
case UPIO_PORT:
outb(value, port->iobase + offset);
break;
+#endif
}
 }
 
diff --git a/drivers/tty/serial/8250/8250_pci.c 
b/drivers/tty/serial/8250/8250_pci.c
index 
6709b6a5f3011db38acc58dc7223158fe4fcf72e..6a638feb44e443a1998980dd037748f227ec1bc8
 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -156,6 +156,14 @@ static const struct pci_device_id pci_use_msi[] = {
 static int pci_default_setup(struct serial_private*,
  const struct pciserial_board*, struct uart_8250_port *, int);
 
+static int serial_8250_need_ioport(struct pci_dev *dev)
+{
+   dev_warn(&dev->dev,
+"Serial port not supported because of missing I/O resource\n");
+
+   return -ENXIO;
+}
+
 static void moan_device(const char *str, struct pci_dev *dev)
 {
pci_err(dev, "%s\n"
@@ -964,6 +972,9 @@ static int pci_ite887x_init(struct pci_dev *dev)
struct resource *iobase = NULL;
u32 miscr, uartbar, ioport;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_need_ioport(dev);
+
/* search for the base-ioport */
for (i = 0; i < ARRAY_SIZE(inta_addr); i++) {
iobase = request_region(inta_addr[i], ITE_887x_IOSIZE,
@@ -1514,6 +1525,9 @@ static int pci_quatech_init(struct pci_dev *dev)
const struct pci_device_id *match;
bool amcc = false;
 
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_need_ioport(dev);
+
match = pci_match_id(quatech_cards, dev);
if (match)
amcc = match->driver_data;
@@ -1538,6 +1552,9 @@ static int pci_quatech_setup(struct serial_private *priv,
  const struct pciserial_board *board,
  struct uart_8250_port *port, int idx)
 {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+   return serial_8250_need_ioport(priv->dev);
+
/* Needed by pci_quatech calls below */
port->port.iobase = pci_resource_start(priv->dev, 
FL_GET_BASE(board->flags));
/* Set up the clocking */
@@ -1655,6 +167

[PATCH v6 2/5] Bluetooth: add HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/bluetooth/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 
18767b54df352e929692850dc789d9377839e094..4ab32abf0f48644715d4c27ec0d2fdaccef62b76
 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -336,7 +336,7 @@ config BT_HCIBFUSB
 
 config BT_HCIDTL1
tristate "HCI DTL1 (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
help
  Bluetooth HCI DTL1 (PC Card) driver.
  This driver provides support for Bluetooth PCMCIA devices with
@@ -349,7 +349,7 @@ config BT_HCIDTL1
 
 config BT_HCIBT3C
tristate "HCI BT3C (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
select FW_LOADER
help
  Bluetooth HCI BT3C (PC Card) driver.
@@ -363,7 +363,7 @@ config BT_HCIBT3C
 
 config BT_HCIBLUECARD
tristate "HCI BlueCard (PC Card) driver"
-   depends on PCMCIA
+   depends on PCMCIA && HAS_IOPORT
help
  Bluetooth HCI BlueCard (PC Card) driver.
  This driver provides support for Bluetooth PCMCIA devices with

-- 
2.43.0

[PATCH v6 1/5] hexagon: Don't select GENERIC_IOMAP without HAS_IOPORT support

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. As hexagon does not support I/O port access it also
the GENERIC_IOMAP mechanism of dynamically choosing between I/O port and
MMIO access doesn't work so don't select it.

Reviewed-by: Brian Cain 
Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 arch/hexagon/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
index 
e233b5efa2761e35c416cbc147f6b6422a7c5b8f..5ea1bf4b7d4f5471a9c39ee9fb5c701455d21342
 100644
--- a/arch/hexagon/Kconfig
+++ b/arch/hexagon/Kconfig
@@ -31,7 +31,6 @@ config HEXAGON
select HAVE_ARCH_TRACEHOOK
select NEED_SG_DMA_LENGTH
select NO_IOPORT_MAP
-   select GENERIC_IOMAP
select GENERIC_IOREMAP
select GENERIC_SMP_IDLE_THREAD
select STACKTRACE_SUPPORT

-- 
2.43.0

[PATCH v6 0/5] treewide: Remove I/O port accessors for HAS_IOPORT=n

[Niklas Schnelle]

Hi All,

This is a follow up in my long running effort of making inb()/outb() and
similar I/O port accessors compile-time optional. After initially
sending this as a treewide series with the latest revision at[0]
we switched to per subsystem series. Now though as we're left with only
5 patches left I'm going back to a single series with Arnd planning
to take this via the the asm-generic tree.

This series may also be viewed for your convenience on my git.kernel.org
tree[1] under the b4/has_ioport branch. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Thanks,
Niklas

[0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git
[2] 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/

Changes since v5 / per subsystem patches:

drm:
- Add HAS_IOPORT dependency for GMA500
tty: serial:
- Make 8250 PCI driver emit an error message when trying to use devices
  which require I/O ports without CONFIG_HAS_IOPORT (Maciej)
- Use early returns + dead code elimination to skip inb()/outb() uses
  in quirks (Arnd)
- In 8250 PCI driver also handle fintek and moxi quirks
- In 8250 ports code handle um's defined(__i385__) &&
  defined(CONFIG_HAS_IOPORT) case
- Use IS_ENABLED() early return also in is_upf_fourport()
  __always_inline to force constant folding

Signed-off-by: Niklas Schnelle 
---
Niklas Schnelle (5):
  hexagon: Don't select GENERIC_IOMAP without HAS_IOPORT support
  Bluetooth: add HAS_IOPORT dependencies
  drm: handle HAS_IOPORT dependencies
  tty: serial: handle HAS_IOPORT dependencies
  asm-generic/io.h: Remove I/O port accessors for HAS_IOPORT=n

 arch/hexagon/Kconfig  |  1 -
 drivers/bluetooth/Kconfig |  6 ++--
 drivers/gpu/drm/gma500/Kconfig|  2 +-
 drivers/gpu/drm/qxl/Kconfig   |  1 +
 drivers/gpu/drm/tiny/bochs.c  | 17 ++
 drivers/gpu/drm/tiny/cirrus.c |  2 ++
 drivers/gpu/drm/xe/Kconfig|  2 +-
 drivers/tty/Kconfig   |  4 +--
 drivers/tty/serial/8250/8250_early.c  |  4 +++
 drivers/tty/serial/8250/8250_pci.c| 49 +++-
 drivers/tty/serial/8250/8250_pcilib.c |  4 +++
 drivers/tty/serial/8250/8250_port.c   | 47 +--
 drivers/tty/serial/8250/Kconfig   |  4 +--
 drivers/tty/serial/Kconfig|  2 +-
 include/asm-generic/io.h  | 60 +++
 15 files changed, 183 insertions(+), 22 deletions(-)
---
base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
change-id: 20241004-b4-has_ioport-60ac6ce1deb6

Best regards,
-- 
Niklas Schnelle

Re: [PATCH net-next v10 05/14] netdev: netdevice devmem allocator

[Niklas Schnelle]

On Tue, 2024-06-04 at 20:27 -0400, Steven Rostedt wrote:
> On Wed, 5 Jun 2024 01:44:37 +0200
> Andrew Lunn  wrote:
> 
> > > Interesting, as I sped up the ftrace ring buffer by a substantial amount 
> > > by
> > > adding strategic __always_inline, noinline, likely() and unlikely()
> > > throughout the code. It had to do with what was considered the fast path
> > > and slow path, and not actually the size of the function. gcc got it
> > > horribly wrong.  
> > 
> > And what did the compiler people say when you reported gcc was getting
> > it wrong?
> > 
> > Our assumption is, the compiler is better than a human at deciding
> > this. Or at least, a human who does not spend a long time profiling
> > and tuning. If this assumption is not true, we probably should be
> > trying to figure out why, and improving the compiler when
> > possible. That will benefit everybody.
> > 
> 
> How is the compiler going to know which path is going to be taken the most?
> There's two main paths in the ring buffer logic. One when an event stays on
> the sub-buffer, the other when the event crosses over to a new sub buffer.
> As there's 100s of events that happen on the same sub-buffer for every one
> time there's a cross over, I optimized the paths that stayed on the
> sub-buffer, which caused the time for those events to go from 250ns down to
> 150 ns!. That's a 40% speed up.
> 
> I added the unlikely/likely and 'always_inline' and 'noinline' paths to
> make sure the "staying on the buffer" path was always the hot path, and
> keeping it tight in cache.
> 
> How is a compiler going to know that?
> 
> -- Steve
> 

Isn't this basically a perfect example of something where profile
guided optimization should work?

Thanks,
Niklas

[PATCH v2 1/1] video: Handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to #ifdef functions and their callsites which
unconditionally use these I/O accessors. In the include/video/vga.h
these are conveniently all those functions with the vga_io_* prefix.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
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.

v1 -> v2:
- Moved vga_mm_r(), vga_mm_w(), vga_mm_w_fast() above #ifdef CONFIG_HAS_IOPORT
  to use them in with or without I/O port variants.
- Duplicated vga_r(), vga_w(), vga_w_fast() functions as non-I/O port variants
  to get rid of in-code #ifdef (Arnd)
- Got rid of if (regbase) logic inversion needed for in-code #ifdef

 include/video/vga.h | 58 -
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/include/video/vga.h b/include/video/vga.h
index 947c0abd04ef..468764d6727a 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -197,9 +197,26 @@ struct vgastate {
 extern int save_vga(struct vgastate *state);
 extern int restore_vga(struct vgastate *state);
 
+static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short 
port)
+{
+   return readb (regbase + port);
+}
+
+static inline void vga_mm_w (void __iomem *regbase, unsigned short port, 
unsigned char val)
+{
+   writeb (val, regbase + port);
+}
+
+static inline void vga_mm_w_fast (void __iomem *regbase, unsigned short port,
+ unsigned char reg, unsigned char val)
+{
+   writew (VGA_OUT16VAL (val, reg), regbase + port);
+}
+
 /*
  * generic VGA port read/write
  */
+#ifdef CONFIG_HAS_IOPORT
 
 static inline unsigned char vga_io_r (unsigned short port)
 {
@@ -217,22 +234,6 @@ static inline void vga_io_w_fast (unsigned short port, 
unsigned char reg,
outw(VGA_OUT16VAL (val, reg), port);
 }
 
-static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short 
port)
-{
-   return readb (regbase + port);
-}
-
-static inline void vga_mm_w (void __iomem *regbase, unsigned short port, 
unsigned char val)
-{
-   writeb (val, regbase + port);
-}
-
-static inline void vga_mm_w_fast (void __iomem *regbase, unsigned short port,
- unsigned char reg, unsigned char val)
-{
-   writew (VGA_OUT16VAL (val, reg), regbase + port);
-}
-
 static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
 {
if (regbase)
@@ -258,7 +259,24 @@ static inline void vga_w_fast (void __iomem *regbase, 
unsigned short port,
else
vga_io_w_fast (port, reg, val);
 }
+#else /* CONFIG_HAS_IOPORT */
+static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
+{
+   return vga_mm_r (regbase, port);
+}
 
+static inline void vga_w (void __iomem *regbase, unsigned short port, unsigned 
char val)
+{
+   vga_mm_w (regbase, port, val);
+}
+
+
+static inline void vga_w_fast (void __iomem *regbase, unsigned short port,
+  unsigned char reg, unsigned char val)
+{
+   vga_mm_w_fast (regbase, port, reg, val);
+}
+#endif /* CONFIG_HAS_IOPORT */
 
 /*
  * VGA CRTC register read/write
@@ -280,6 +298,7 @@ static inline void vga_wcrt (void __iomem *regbase, 
unsigned char reg, unsigned
 #endif /* VGA_OUTW_WRITE */
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rcrt (unsigned char reg)
 {
 vga_io_w (VGA_CRT_IC, reg);
@@ -295,6 +314,7 @@ static inline void vga_io_wcrt (unsigned char reg, unsigned 
char val)
 vga_io_w (VGA_CRT_DC, val);
 #endif /* VGA_OUTW_WRITE */
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rcrt (void __iomem *regbase, unsigned char 
reg)
 {
@@ -333,6 +353,7 @@ static inline void vga_wseq (void __iomem *regbase, 
unsigned char reg, unsigned
 #endif /* VGA_OUTW_WRITE */
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rseq (unsigned char reg)
 {
 vga_io_w (VGA_SEQ_I, reg);
@@ -348,6 +369,7 @@ static inline void vga_io_wseq (unsigned char reg, unsigned 
char val)
 vga_io_w (VGA_SEQ_D, val);
 #endif /* VGA_OUTW_WRITE */
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rseq (void __iomem *regbase, unsigned char 
reg)
 {
@@ -385,6 +407,7 @@ static inline void vga_wgfx (void __iomem *regbase, 
unsigned char reg, unsigned
 #endif /* VGA_OUTW_WRITE */
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rgfx (unsigned char reg)
 {
 vga_io_w (VGA_GFX_I, reg);
@@ -400,6 +423,7 @@ static inline void vga_io_wgfx (unsigned char reg, unsigned 
char val)
 vga_io_w (VGA_GFX_D, val);
 #endif /* VGA_OUTW_WRITE */
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rgfx (void __iomem *regbase, unsigned char 
reg)
 {
@@ -43

[PATCH v2 0/1] video: Handle HAS_IOPORT dependencies

[Niklas Schnelle]

Hi,

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 branch[1]. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Changes since v1:
- Moved vga_mm_r(), vga_mm_w(), vga_mm_w_fast() above #ifdef CONFIG_HAS_IOPORT
  to use them in with or without I/O port variants.
- Duplicated vga_r(), vga_w(), vga_w_fast() functions as non-I/O port variants
  to get rid of in-code #ifdef (Arnd)
- Got rid of if (regbase) logic inversion needed for in-code #ifdef

Thanks,
Niklas

[0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/
[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport
[2] 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/

Niklas Schnelle (1):
  video: Handle HAS_IOPORT dependencies

 include/video/vga.h | 58 -
 1 file changed, 42 insertions(+), 16 deletions(-)

-- 
2.40.1

Re: [PATCH 1/1] video: Handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Thu, 2024-04-11 at 16:00 +0200, Helge Deller wrote:
> * Niklas Schnelle :
> > In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
> > compile time. We thus need to #ifdef functions and their callsites which
> > unconditionally use these I/O accessors. In the include/video/vga.h
> > these are conveniently all those functions with the vga_io_* prefix.
> 
> Why don't you code it like in the patch below?
> inb_p(), outb_p() and outw() would then need to be defined externally
> without an implementation so that they would generate link time errors
> (instead of compile time errors).
> 
> diff --git a/include/video/vga.h b/include/video/vga.h
> index 947c0abd04ef..32c915e109fa 100644
> --- a/include/video/vga.h
> +++ b/include/video/vga.h
> @@ -203,18 +203,20 @@ extern int restore_vga(struct vgastate *state);
>  
>  static inline unsigned char vga_io_r (unsigned short port)
>  {
> - return inb_p(port);
> + return IS_ENABLED(CONFIG_HAS_IOPORT) ? inb_p(port) : 0;
>  }
>  
>  static inline void vga_io_w (unsigned short port, unsigned char val)
>  {
> - outb_p(val, port);
> + if (IS_ENABLED(CONFIG_HAS_IOPORT))
> + outb_p(val, port);
>  }
>  
>  static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
> unsigned char val)
>  {
> - outw(VGA_OUT16VAL (val, reg), port);
> + if (IS_ENABLED(CONFIG_HAS_IOPORT))
> + outw(VGA_OUT16VAL (val, reg), port);
>  }
>  
>  static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short 
> port)
> 

This may be personal preference but I feel like link time errors would
be very late to catch a configuration that can't work. Also this would
bypass the __compiletime_error("inb()) requires CONFIG_HAS_IOPORT");
added instead of the in*()/out*() helpers to make it easy to spot the
problem.


I'm not a fan of #ifdeffery either but I think in this case it is
simple, well enough contained and overall there aren't that many spots
where we need to exclude just some sections of code vs entire drivers
with vga.h probably being the worst of them all.

Thanks,
Niklas

[PATCH 0/1] video: Handle HAS_IOPORT dependencies

[Niklas Schnelle]

Hi Helge (again ;-)),

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 branch[1]. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Thanks,
Niklas

[0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/
[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport
[2] 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/

Niklas Schnelle (1):
  video: Handle HAS_IOPORT dependencies

 include/video/vga.h | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

-- 
2.40.1

[PATCH 1/1] video: Handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to #ifdef functions and their callsites which
unconditionally use these I/O accessors. In the include/video/vga.h
these are conveniently all those functions with the vga_io_* prefix.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
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.

 include/video/vga.h | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/include/video/vga.h b/include/video/vga.h
index 947c0abd04ef..ed89295941c4 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -201,6 +201,7 @@ extern int restore_vga(struct vgastate *state);
  * generic VGA port read/write
  */
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_r (unsigned short port)
 {
return inb_p(port);
@@ -210,12 +211,12 @@ static inline void vga_io_w (unsigned short port, 
unsigned char val)
 {
outb_p(val, port);
 }
-
 static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
  unsigned char val)
 {
outw(VGA_OUT16VAL (val, reg), port);
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short 
port)
 {
@@ -235,28 +236,34 @@ static inline void vga_mm_w_fast (void __iomem *regbase, 
unsigned short port,
 
 static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
 {
-   if (regbase)
-   return vga_mm_r (regbase, port);
-   else
+#ifdef CONFIG_HAS_IOPORT
+   if (!regbase)
return vga_io_r (port);
+   else
+#endif /* CONFIG_HAS_IOPORT */
+   return vga_mm_r (regbase, port);
 }
 
 static inline void vga_w (void __iomem *regbase, unsigned short port, unsigned 
char val)
 {
-   if (regbase)
-   vga_mm_w (regbase, port, val);
-   else
+#ifdef CONFIG_HAS_IOPORT
+   if (!regbase)
vga_io_w (port, val);
+   else
+#endif /* CONFIG_HAS_IOPORT */
+   vga_mm_w (regbase, port, val);
 }
 
 
 static inline void vga_w_fast (void __iomem *regbase, unsigned short port,
   unsigned char reg, unsigned char val)
 {
-   if (regbase)
-   vga_mm_w_fast (regbase, port, reg, val);
-   else
+#ifdef CONFIG_HAS_IOPORT
+   if (!regbase)
vga_io_w_fast (port, reg, val);
+   else
+#endif /* CONFIG_HAS_IOPORT */
+   vga_mm_w_fast (regbase, port, reg, val);
 }
 
 
@@ -280,6 +287,7 @@ static inline void vga_wcrt (void __iomem *regbase, 
unsigned char reg, unsigned
 #endif /* VGA_OUTW_WRITE */
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rcrt (unsigned char reg)
 {
 vga_io_w (VGA_CRT_IC, reg);
@@ -295,6 +303,7 @@ static inline void vga_io_wcrt (unsigned char reg, unsigned 
char val)
 vga_io_w (VGA_CRT_DC, val);
 #endif /* VGA_OUTW_WRITE */
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rcrt (void __iomem *regbase, unsigned char 
reg)
 {
@@ -333,6 +342,7 @@ static inline void vga_wseq (void __iomem *regbase, 
unsigned char reg, unsigned
 #endif /* VGA_OUTW_WRITE */
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rseq (unsigned char reg)
 {
 vga_io_w (VGA_SEQ_I, reg);
@@ -348,6 +358,7 @@ static inline void vga_io_wseq (unsigned char reg, unsigned 
char val)
 vga_io_w (VGA_SEQ_D, val);
 #endif /* VGA_OUTW_WRITE */
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rseq (void __iomem *regbase, unsigned char 
reg)
 {
@@ -385,6 +396,7 @@ static inline void vga_wgfx (void __iomem *regbase, 
unsigned char reg, unsigned
 #endif /* VGA_OUTW_WRITE */
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rgfx (unsigned char reg)
 {
 vga_io_w (VGA_GFX_I, reg);
@@ -400,6 +412,7 @@ static inline void vga_io_wgfx (unsigned char reg, unsigned 
char val)
 vga_io_w (VGA_GFX_D, val);
 #endif /* VGA_OUTW_WRITE */
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rgfx (void __iomem *regbase, unsigned char 
reg)
 {
@@ -434,6 +447,7 @@ static inline void vga_wattr (void __iomem *regbase, 
unsigned char reg, unsigned
 vga_w (regbase, VGA_ATT_W, val);
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rattr (unsigned char reg)
 {
 vga_io_w (VGA_ATT_IW, reg);
@@ -445,6 +459,7 @@ static inline void vga_io_wattr (unsigned char reg, 
unsigned char val)
 vga_io_w (VGA_ATT_IW, reg);
 vga_io_w (VGA_ATT_W, val);
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rattr (void __iomem *regbase, unsigned char 
reg)
 {
-- 
2.40.1

[PATCH v2 0/1] fbdev: Handle HAS_IOPORT dependencies

[Niklas Schnelle]

Hi Helge,

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 branch[1]. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Thanks,
Niklas

[0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/
[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport
[2] 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/

v1 -> v2:
- Add dependency for FB_ARC

Niklas Schnelle (1):
  fbdev: add HAS_IOPORT dependencies

 drivers/video/fbdev/Kconfig | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

-- 
2.40.1

[PATCH v2 1/1] fbdev: add HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
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.

v1 -> v2:
- Add dependency for FB_ARC

 drivers/video/fbdev/Kconfig | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 197b6d5268e9..76bbfd3767da 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -157,7 +157,7 @@ config FB_IMX
 
 config FB_CYBER2000
tristate "CyberPro 2000/2010/5000 support"
-   depends on FB && PCI && (BROKEN || !SPARC64)
+   depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64)
select FB_IOMEM_HELPERS
help
  This enables support for the Integraphics CyberPro 20x0 and 5000
@@ -245,7 +245,7 @@ config FB_FM2
 
 config FB_ARC
tristate "Arc Monochrome LCD board support"
-   depends on FB && (X86 || COMPILE_TEST)
+   depends on FB && HAS_IOPORT && (X86 || COMPILE_TEST)
select FB_SYSMEM_HELPERS_DEFERRED
help
  This enables support for the Arc Monochrome LCD board. The board
@@ -1046,7 +1046,7 @@ config FB_ATY_BACKLIGHT
 
 config FB_S3
tristate "S3 Trio/Virge support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1107,7 +1107,7 @@ config FB_SAVAGE_ACCEL
 
 config FB_SIS
tristate "SiS/XGI display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select BOOT_VESA_SUPPORT if FB_SIS = y
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1138,7 +1138,7 @@ config FB_SIS_315
 
 config FB_VIA
tristate "VIA UniChrome (Pro) and Chrome9 display support"
-   depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST)
+   depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || 
COMPILE_TEST)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1177,7 +1177,7 @@ endif
 
 config FB_NEOMAGIC
tristate "NeoMagic display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1204,7 +1204,7 @@ config FB_KYRO
 
 config FB_3DFX
tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1252,7 +1252,7 @@ config FB_VOODOO1
 
 config FB_VT8623
tristate "VIA VT8623 support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1267,7 +1267,7 @@ config FB_VT8623
 
 config FB_TRIDENT
tristate "Trident/CyberXXX/CyberBlade support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1290,7 +1290,7 @@ config FB_TRIDENT
 
 config FB_ARK
tristate "ARK 2000PV support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1814,7 +1814,7 @@ config FB_SSD1307
 
 config FB_SM712
tristate "Silicon Motion SM712 framebuffer support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_IOMEM_HELPERS
help
  Frame buffer driver for the Silicon Motion SM710, SM712, SM721
-- 
2.40.1

Re: [PATCH 1/1] fbdev: add HAS_IOPORT dependencies

[Niklas Schnelle]

On Wed, 2024-04-10 at 10:27 +0200, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
> compile time. We thus need to add HAS_IOPORT as dependency for those
> drivers using them.
> 
> Co-developed-by: Arnd Bergmann 
> Signed-off-by: Arnd Bergmann 
> Signed-off-by: Niklas Schnelle 
> ---
> 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/video/fbdev/Kconfig | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)

Sorry, I just noticed that one of the fbdev Kconfig changes slipped
into my patch for video. This one is missing the below hunk and I'll
send a v2.

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index f90b191dc68b..76bbfd3767da 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -245,7 +245,7 @@ config FB_FM2
 
 config FB_ARC
tristate "Arc Monochrome LCD board support"
-   depends on FB && (X86 || COMPILE_TEST)
+   depends on FB && HAS_IOPORT && (X86 || COMPILE_TEST)
select FB_SYSMEM_HELPERS_DEFERRED
help
  This enables support for the Arc Monochrome LCD board. The board

[PATCH 1/1] fbdev: add HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
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/video/fbdev/Kconfig | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 197b6d5268e9..f90b191dc68b 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -157,7 +157,7 @@ config FB_IMX
 
 config FB_CYBER2000
tristate "CyberPro 2000/2010/5000 support"
-   depends on FB && PCI && (BROKEN || !SPARC64)
+   depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64)
select FB_IOMEM_HELPERS
help
  This enables support for the Integraphics CyberPro 20x0 and 5000
@@ -1046,7 +1046,7 @@ config FB_ATY_BACKLIGHT
 
 config FB_S3
tristate "S3 Trio/Virge support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1107,7 +1107,7 @@ config FB_SAVAGE_ACCEL
 
 config FB_SIS
tristate "SiS/XGI display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select BOOT_VESA_SUPPORT if FB_SIS = y
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1138,7 +1138,7 @@ config FB_SIS_315
 
 config FB_VIA
tristate "VIA UniChrome (Pro) and Chrome9 display support"
-   depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST)
+   depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || 
COMPILE_TEST)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1177,7 +1177,7 @@ endif
 
 config FB_NEOMAGIC
tristate "NeoMagic display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1204,7 +1204,7 @@ config FB_KYRO
 
 config FB_3DFX
tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1252,7 +1252,7 @@ config FB_VOODOO1
 
 config FB_VT8623
tristate "VIA VT8623 support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1267,7 +1267,7 @@ config FB_VT8623
 
 config FB_TRIDENT
tristate "Trident/CyberXXX/CyberBlade support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1290,7 +1290,7 @@ config FB_TRIDENT
 
 config FB_ARK
tristate "ARK 2000PV support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1814,7 +1814,7 @@ config FB_SSD1307
 
 config FB_SM712
tristate "Silicon Motion SM712 framebuffer support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_IOMEM_HELPERS
help
  Frame buffer driver for the Silicon Motion SM710, SM712, SM721
-- 
2.40.1

[PATCH 0/1] fbdev: Handle HAS_IOPORT dependencies

[Niklas Schnelle]

Hi Helge,

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 branch[1]. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Thanks,
Niklas

[0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/
[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport
[2] 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/

Niklas Schnelle (1):
  fbdev: add HAS_IOPORT dependencies

 drivers/video/fbdev/Kconfig | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

-- 
2.40.1

Re: [PATCH 1/1] vgacon: add HAS_IOPORT dependencies

[Niklas Schnelle]

On Fri, 2024-04-05 at 17:47 +0200, Arnd Bergmann wrote:
> On Fri, Apr 5, 2024, at 17:43, Niklas Schnelle wrote:
> > In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
> > compile time. We thus need to add HAS_IOPORT as dependency for
> > those drivers using them.
> > 
> > Co-developed-by: Arnd Bergmann 
> > Signed-off-by: Arnd Bergmann 
> > Signed-off-by: Niklas Schnelle 
> > ---
> > 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.
> 
> I think this patch can just get dropped now, no need to merge
> it because it's already handled by e9e3300b6e77 ("vgacon:
> rework Kconfig dependencies").
> 
>  Arnd

Makes sense then let's drop this patch.

Thanks,
Niklas

[PATCH 0/1] vgacon: Handle HAS_IOPORT dependencies

[Niklas Schnelle]

Hi Greg, Helge,

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 branch[1]. As for compile-time vs runtime
see Linus' reply to my first attempt[2].

Thanks,
Niklas

[0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/
[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport
[2] 
https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/

Niklas Schnelle (1):
  vgacon: add HAS_IOPORT dependencies

 drivers/video/console/Kconfig | 1 +
 1 file changed, 1 insertion(+)

-- 
2.40.1

[PATCH 1/1] vgacon: add HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for
those drivers using them.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
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/video/console/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index bc31db6ef7d2..a053a2de4432 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -10,6 +10,7 @@ config VGA_CONSOLE
depends on ALPHA || X86 || \
(ARM && ARCH_FOOTBRIDGE) || \
(MIPS && (MIPS_MALTA || SIBYTE_BCM112X || SIBYTE_SB1250 || 
SIBYTE_BCM1x80 || SNI_RM))
+   depends on HAS_IOPORT
select APERTURE_HELPERS if (DRM || FB || VFIO_PCI_CORE)
default y
help
-- 
2.40.1

[PATCH v5 41/44] video: Handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to #ifdef functions and their callsites
which unconditionally use these I/O accessors. In the
include/video/vga.h these are conveniently all those functions with the
vga_io_* prefix.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/video/fbdev/Kconfig |  3 ++-
 include/video/vga.h | 35 +--
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 4eea398ebbfe..598d06bddf6a 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -532,7 +532,8 @@ config FB_IMSTT
 
 config FB_VGA16
tristate "VGA 16-color graphics support"
-   depends on FB && HAS_IOPORT
+   depends on FB && (X86 || PPC)
+   depends on HAS_IOPORT
select APERTURE_HELPERS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
diff --git a/include/video/vga.h b/include/video/vga.h
index 947c0abd04ef..ed89295941c4 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -201,6 +201,7 @@ extern int restore_vga(struct vgastate *state);
  * generic VGA port read/write
  */
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_r (unsigned short port)
 {
return inb_p(port);
@@ -210,12 +211,12 @@ static inline void vga_io_w (unsigned short port, 
unsigned char val)
 {
outb_p(val, port);
 }
-
 static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
  unsigned char val)
 {
outw(VGA_OUT16VAL (val, reg), port);
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short 
port)
 {
@@ -235,28 +236,34 @@ static inline void vga_mm_w_fast (void __iomem *regbase, 
unsigned short port,
 
 static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
 {
-   if (regbase)
-   return vga_mm_r (regbase, port);
-   else
+#ifdef CONFIG_HAS_IOPORT
+   if (!regbase)
return vga_io_r (port);
+   else
+#endif /* CONFIG_HAS_IOPORT */
+   return vga_mm_r (regbase, port);
 }
 
 static inline void vga_w (void __iomem *regbase, unsigned short port, unsigned 
char val)
 {
-   if (regbase)
-   vga_mm_w (regbase, port, val);
-   else
+#ifdef CONFIG_HAS_IOPORT
+   if (!regbase)
vga_io_w (port, val);
+   else
+#endif /* CONFIG_HAS_IOPORT */
+   vga_mm_w (regbase, port, val);
 }
 
 
 static inline void vga_w_fast (void __iomem *regbase, unsigned short port,
   unsigned char reg, unsigned char val)
 {
-   if (regbase)
-   vga_mm_w_fast (regbase, port, reg, val);
-   else
+#ifdef CONFIG_HAS_IOPORT
+   if (!regbase)
vga_io_w_fast (port, reg, val);
+   else
+#endif /* CONFIG_HAS_IOPORT */
+   vga_mm_w_fast (regbase, port, reg, val);
 }
 
 
@@ -280,6 +287,7 @@ static inline void vga_wcrt (void __iomem *regbase, 
unsigned char reg, unsigned
 #endif /* VGA_OUTW_WRITE */
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rcrt (unsigned char reg)
 {
 vga_io_w (VGA_CRT_IC, reg);
@@ -295,6 +303,7 @@ static inline void vga_io_wcrt (unsigned char reg, unsigned 
char val)
 vga_io_w (VGA_CRT_DC, val);
 #endif /* VGA_OUTW_WRITE */
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rcrt (void __iomem *regbase, unsigned char 
reg)
 {
@@ -333,6 +342,7 @@ static inline void vga_wseq (void __iomem *regbase, 
unsigned char reg, unsigned
 #endif /* VGA_OUTW_WRITE */
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rseq (unsigned char reg)
 {
 vga_io_w (VGA_SEQ_I, reg);
@@ -348,6 +358,7 @@ static inline void vga_io_wseq (unsigned char reg, unsigned 
char val)
 vga_io_w (VGA_SEQ_D, val);
 #endif /* VGA_OUTW_WRITE */
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rseq (void __iomem *regbase, unsigned char 
reg)
 {
@@ -385,6 +396,7 @@ static inline void vga_wgfx (void __iomem *regbase, 
unsigned char reg, unsigned
 #endif /* VGA_OUTW_WRITE */
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rgfx (unsigned char reg)
 {
 vga_io_w (VGA_GFX_I, reg);
@@ -400,6 +412,7 @@ static inline void vga_io_wgfx (unsigned char reg, unsigned 
char val)
 vga_io_w (VGA_GFX_D, val);
 #endif /* VGA_OUTW_WRITE */
 }
+#endif /* CONFIG_HAS_IOPORT */
 
 static inline unsigned char vga_mm_rgfx (void __iomem *regbase, unsigned char 
reg)
 {
@@ -434,6 +447,7 @@ static inline void vga_wattr (void __iomem *regbase, 
unsigned char reg, unsigned
 vga_w (regbase, VGA_ATT_W, val);
 }
 
+#ifdef CONFIG_HAS_IOPORT
 static inline unsigned char vga_io_rattr (unsigned char reg)
 {

[PATCH v5 39/44] vgacon: add HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/video/console/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 22cea5082ac4..64974eaa3ac5 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -10,6 +10,7 @@ config VGA_CONSOLE
depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC &&  !SUPERH 
&& \
(!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER) 
&& \
!ARM64 && !ARC && !MICROBLAZE && !OPENRISC && !S390 && !UML
+   depends on HAS_IOPORT
select APERTURE_HELPERS if (DRM || FB || VFIO_PCI_CORE)
default y
help
-- 
2.39.2

[PATCH v5 40/44] fbdev: add HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/video/fbdev/Kconfig | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 0fdf5f46802c..4eea398ebbfe 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -335,7 +335,7 @@ config FB_IMX
 
 config FB_CYBER2000
tristate "CyberPro 2000/2010/5000 support"
-   depends on FB && PCI && (BROKEN || !SPARC64)
+   depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -429,6 +429,7 @@ config FB_FM2
 config FB_ARC
tristate "Arc Monochrome LCD board support"
depends on FB && (X86 || COMPILE_TEST)
+   depends on HAS_IOPORT
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
select FB_SYS_IMAGEBLIT
@@ -531,7 +532,7 @@ config FB_IMSTT
 
 config FB_VGA16
tristate "VGA 16-color graphics support"
-   depends on FB && (X86 || PPC)
+   depends on FB && HAS_IOPORT
select APERTURE_HELPERS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1332,7 +1333,7 @@ config FB_ATY_BACKLIGHT
 
 config FB_S3
tristate "S3 Trio/Virge support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1393,7 +1394,7 @@ config FB_SAVAGE_ACCEL
 
 config FB_SIS
tristate "SiS/XGI display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1424,7 +1425,7 @@ config FB_SIS_315
 
 config FB_VIA
tristate "VIA UniChrome (Pro) and Chrome9 display support"
-   depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST)
+   depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || 
COMPILE_TEST)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1463,7 +1464,7 @@ endif
 
 config FB_NEOMAGIC
tristate "NeoMagic display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_MODE_HELPERS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1493,7 +1494,7 @@ config FB_KYRO
 
 config FB_3DFX
tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_IMAGEBLIT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1543,7 +1544,7 @@ config FB_VOODOO1
 
 config FB_VT8623
tristate "VIA VT8623 support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1558,7 +1559,7 @@ config FB_VT8623
 
 config FB_TRIDENT
tristate "Trident/CyberXXX/CyberBlade support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1581,7 +1582,7 @@ config FB_TRIDENT
 
 config FB_ARK
tristate "ARK 2000PV support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -2195,7 +2196,7 @@ config FB_SSD1307
 
 config FB_SM712
tristate "Silicon Motion SM712 framebuffer support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
-- 
2.39.2

[PATCH v5 09/44] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them. In the bochs driver there is optional MMIO
support detected at runtime, warn if this isn't taken when
HAS_IOPORT is not defined.

There is also a direct and hard coded use in cirrus.c which according to
the comment is only necessary during resume.  Let's just skip this as
for example s390 which doesn't have I/O port support also doesen't
support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/gpu/drm/qxl/Kconfig   |  1 +
 drivers/gpu/drm/tiny/bochs.c  | 17 +
 drivers/gpu/drm/tiny/cirrus.c |  2 ++
 3 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index ca3f51c2a8fe..d0e0d440c8d9 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
 config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index d254679a136e..3710339407cc 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -2,6 +2,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -105,7 +106,9 @@ static void bochs_vga_writeb(struct bochs_device *bochs, 
u16 ioport, u8 val)
 
writeb(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outb(val, ioport);
+#endif
}
 }
 
@@ -119,7 +122,11 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 
ioport)
 
return readb(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
return inb(ioport);
+#else
+   return 0xff;
+#endif
}
 }
 
@@ -132,8 +139,12 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, 
u16 reg)
 
ret = readw(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
ret = inw(VBE_DISPI_IOPORT_DATA);
+#else
+   ret = 0x;
+#endif
}
return ret;
 }
@@ -145,8 +156,10 @@ static void bochs_dispi_write(struct bochs_device *bochs, 
u16 reg, u16 val)
 
writew(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
outw(val, VBE_DISPI_IOPORT_DATA);
+#endif
}
 }
 
@@ -229,6 +242,10 @@ static int bochs_hw_init(struct drm_device *dev)
return -ENOMEM;
}
} else {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
+   DRM_ERROR("I/O ports are not supported\n");
+   return -EIO;
+   }
ioaddr = VBE_DISPI_IOPORT_INDEX;
iosize = 2;
if (!request_region(ioaddr, iosize, "bochs-drm")) {
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 594bc472862f..c65fea049bc7 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -508,8 +508,10 @@ static void cirrus_crtc_helper_atomic_enable(struct 
drm_crtc *crtc,
 
cirrus_mode_set(cirrus, &crtc_state->mode);
 
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(VGA_AR_ENABLE_DISPLAY, VGA_ATT_W);
+#endif
 
drm_dev_exit(idx);
 }
-- 
2.39.2

Re: [PATCH v4 38/41] video: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Tue, 2023-05-16 at 19:21 +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 16.05.23 um 13:00 schrieb Niklas Schnelle:
> > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > not being declared. We thus need to add HAS_IOPORT as dependency for
> > those drivers using them and guard inline code in headers.
> > 
> > Co-developed-by: Arnd Bergmann 
> > Signed-off-by: Arnd Bergmann 
> > Signed-off-by: Niklas Schnelle 
> > ---
> > Note: The HAS_IOPORT Kconfig option was added in v6.4-rc1 so
> >per-subsystem patches may be applied independently
> > 
> >   drivers/video/console/Kconfig |  1 +
> >   drivers/video/fbdev/Kconfig   | 21 +++--
> >   include/video/vga.h   |  8 
> 
> Those are 3 different things. It might be preferable to not handle them 
> under the video/ umbrella.
> 
> >   3 files changed, 20 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> > index 22cea5082ac4..64974eaa3ac5 100644
> > --- a/drivers/video/console/Kconfig
> > +++ b/drivers/video/console/Kconfig
> > @@ -10,6 +10,7 @@ config VGA_CONSOLE
> > depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC &&  !SUPERH 
> > && \
> > (!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER) 
> > && \
> > !ARM64 && !ARC && !MICROBLAZE && !OPENRISC && !S390 && !UML
> > +   depends on HAS_IOPORT
> > select APERTURE_HELPERS if (DRM || FB || VFIO_PCI_CORE)
> > default y
> > help
> > diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> > index 96e91570cdd3..a56c57dd839b 100644
> > --- a/drivers/video/fbdev/Kconfig
> > +++ b/drivers/video/fbdev/Kconfig
> > @@ -335,7 +335,7 @@ config FB_IMX
> >   
> >   config FB_CYBER2000
> > tristate "CyberPro 2000/2010/5000 support"
> > -   depends on FB && PCI && (BROKEN || !SPARC64)
> > +   depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64)
> > select FB_CFB_FILLRECT
> > select FB_CFB_COPYAREA
> > select FB_CFB_IMAGEBLIT
> > @@ -429,6 +429,7 @@ config FB_FM2
> >   config FB_ARC
> > tristate "Arc Monochrome LCD board support"
> > depends on FB && (X86 || COMPILE_TEST)
> > +   depends on HAS_IOPORT
> > select FB_SYS_FILLRECT
> > select FB_SYS_COPYAREA
> > select FB_SYS_IMAGEBLIT
> > @@ -1332,7 +1333,7 @@ config FB_ATY_BACKLIGHT
> >   
> >   config FB_S3
> > tristate "S3 Trio/Virge support"
> > -   depends on FB && PCI
> > +   depends on FB && PCI && HAS_IOPORT
> > select FB_CFB_FILLRECT
> > select FB_CFB_COPYAREA
> > select FB_CFB_IMAGEBLIT
> > @@ -1393,7 +1394,7 @@ config FB_SAVAGE_ACCEL
> >   
> >   config FB_SIS
> > tristate "SiS/XGI display support"
> > -   depends on FB && PCI
> > +   depends on FB && PCI && HAS_IOPORT
> > select FB_CFB_FILLRECT
> > select FB_CFB_COPYAREA
> > select FB_CFB_IMAGEBLIT
> > @@ -1424,7 +1425,7 @@ config FB_SIS_315
> >   
> >   config FB_VIA
> > tristate "VIA UniChrome (Pro) and Chrome9 display support"
> > -   depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST)
> > +   depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || 
> > COMPILE_TEST)
> > select FB_CFB_FILLRECT
> > select FB_CFB_COPYAREA
> > select FB_CFB_IMAGEBLIT
> > @@ -1463,7 +1464,7 @@ endif
> >   
> >   config FB_NEOMAGIC
> > tristate "NeoMagic display support"
> > -   depends on FB && PCI
> > +   depends on FB && PCI && HAS_IOPORT
> > select FB_MODE_HELPERS
> > select FB_CFB_FILLRECT
> > select FB_CFB_COPYAREA
> > @@ -1493,7 +1494,7 @@ config FB_KYRO
> >   
> >   config FB_3DFX
> > tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support"
> > -   depends on FB && PCI
> > +   depends on FB && PCI && HAS_IOPORT
> > select FB_CFB_IMAGEBLIT
> > select FB_CFB_FILLRECT
> > select FB_CFB_COPYAREA
> > @@ -1543,7 +1544,7 @@ config FB_VOODOO1
> >   
> >   config FB_VT8623
> > tristate "VIA VT86

[PATCH v4 38/41] video: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them and guard inline code in headers.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
Note: The HAS_IOPORT Kconfig option was added in v6.4-rc1 so
  per-subsystem patches may be applied independently

 drivers/video/console/Kconfig |  1 +
 drivers/video/fbdev/Kconfig   | 21 +++--
 include/video/vga.h   |  8 
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 22cea5082ac4..64974eaa3ac5 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -10,6 +10,7 @@ config VGA_CONSOLE
depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC &&  !SUPERH 
&& \
(!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER) 
&& \
!ARM64 && !ARC && !MICROBLAZE && !OPENRISC && !S390 && !UML
+   depends on HAS_IOPORT
select APERTURE_HELPERS if (DRM || FB || VFIO_PCI_CORE)
default y
help
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 96e91570cdd3..a56c57dd839b 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -335,7 +335,7 @@ config FB_IMX
 
 config FB_CYBER2000
tristate "CyberPro 2000/2010/5000 support"
-   depends on FB && PCI && (BROKEN || !SPARC64)
+   depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -429,6 +429,7 @@ config FB_FM2
 config FB_ARC
tristate "Arc Monochrome LCD board support"
depends on FB && (X86 || COMPILE_TEST)
+   depends on HAS_IOPORT
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
select FB_SYS_IMAGEBLIT
@@ -1332,7 +1333,7 @@ config FB_ATY_BACKLIGHT
 
 config FB_S3
tristate "S3 Trio/Virge support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1393,7 +1394,7 @@ config FB_SAVAGE_ACCEL
 
 config FB_SIS
tristate "SiS/XGI display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1424,7 +1425,7 @@ config FB_SIS_315
 
 config FB_VIA
tristate "VIA UniChrome (Pro) and Chrome9 display support"
-   depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST)
+   depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || 
COMPILE_TEST)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1463,7 +1464,7 @@ endif
 
 config FB_NEOMAGIC
tristate "NeoMagic display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_MODE_HELPERS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1493,7 +1494,7 @@ config FB_KYRO
 
 config FB_3DFX
tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_IMAGEBLIT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1543,7 +1544,7 @@ config FB_VOODOO1
 
 config FB_VT8623
tristate "VIA VT8623 support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1558,7 +1559,7 @@ config FB_VT8623
 
 config FB_TRIDENT
tristate "Trident/CyberXXX/CyberBlade support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1581,7 +1582,7 @@ config FB_TRIDENT
 
 config FB_ARK
tristate "ARK 2000PV support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -2195,7 +2196,7 @@ config FB_SSD1307
 
 config FB_SM712
tristate "Silicon Motion SM712 framebuffer support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
  

[PATCH v4 07/41] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them. In the bochs driver there is optional MMIO
support detected at runtime, warn if this isn't taken when
HAS_IOPORT is not defined.

There is also a direct and hard coded use in cirrus.c which according to
the comment is only necessary during resume.  Let's just skip this as
for example s390 which doesn't have I/O port support also doesen't
support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
Note: The HAS_IOPORT Kconfig option was added in v6.4-rc1 so
  per-subsystem patches may be applied independently

 drivers/gpu/drm/qxl/Kconfig   |  1 +
 drivers/gpu/drm/tiny/bochs.c  | 17 +
 drivers/gpu/drm/tiny/cirrus.c |  2 ++
 3 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index ca3f51c2a8fe..d0e0d440c8d9 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
 config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index d254679a136e..3710339407cc 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -2,6 +2,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -105,7 +106,9 @@ static void bochs_vga_writeb(struct bochs_device *bochs, 
u16 ioport, u8 val)
 
writeb(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outb(val, ioport);
+#endif
}
 }
 
@@ -119,7 +122,11 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 
ioport)
 
return readb(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
return inb(ioport);
+#else
+   return 0xff;
+#endif
}
 }
 
@@ -132,8 +139,12 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, 
u16 reg)
 
ret = readw(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
ret = inw(VBE_DISPI_IOPORT_DATA);
+#else
+   ret = 0x;
+#endif
}
return ret;
 }
@@ -145,8 +156,10 @@ static void bochs_dispi_write(struct bochs_device *bochs, 
u16 reg, u16 val)
 
writew(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
outw(val, VBE_DISPI_IOPORT_DATA);
+#endif
}
 }
 
@@ -229,6 +242,10 @@ static int bochs_hw_init(struct drm_device *dev)
return -ENOMEM;
}
} else {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
+   DRM_ERROR("I/O ports are not supported\n");
+   return -EIO;
+   }
ioaddr = VBE_DISPI_IOPORT_INDEX;
iosize = 2;
if (!request_region(ioaddr, iosize, "bochs-drm")) {
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 594bc472862f..c65fea049bc7 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -508,8 +508,10 @@ static void cirrus_crtc_helper_atomic_enable(struct 
drm_crtc *crtc,
 
cirrus_mode_set(cirrus, &crtc_state->mode);
 
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(VGA_AR_ENABLE_DISPLAY, VGA_ATT_W);
+#endif
 
drm_dev_exit(idx);
 }
-- 
2.39.2

[PATCH v4 37/41] fbdev: atyfb: Remove unused clock determination

[Niklas Schnelle]

Just below the removed lines par->clk_wr_offset is hard coded to 3 so
there is no use in determining a different clock just to then ignore it
anyway. This also removes the only I/O port use remaining in the driver
allowing it to be built without CONFIG_HAS_IOPORT.

Link: https://lore.kernel.org/all/zbx5alo5h546b...@intel.com/
Suggested-by: Ville Syrjälä 
Signed-off-by: Niklas Schnelle 
---
Note: The HAS_IOPORT Kconfig option was added in v6.4-rc1 so
  per-subsystem patches may be applied independently

 drivers/video/fbdev/aty/atyfb_base.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/video/fbdev/aty/atyfb_base.c 
b/drivers/video/fbdev/aty/atyfb_base.c
index b02e4e645035..cba2b113b28b 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -3498,11 +3498,6 @@ static int atyfb_setup_generic(struct pci_dev *pdev, 
struct fb_info *info,
if (ret)
goto atyfb_setup_generic_fail;
 #endif
-   if (!(aty_ld_le32(CRTC_GEN_CNTL, par) & CRTC_EXT_DISP_EN))
-   par->clk_wr_offset = (inb(R_GENMO) & 0x0CU) >> 2;
-   else
-   par->clk_wr_offset = aty_ld_8(CLOCK_CNTL, par) & 0x03U;
-
/* according to ATI, we should use clock 3 for acelerated mode */
par->clk_wr_offset = 3;
 
-- 
2.39.2

Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Thu, 2023-03-23 at 18:08 +0200, Ville Syrjälä wrote:
> On Thu, Mar 23, 2023 at 03:17:38PM +0100, Niklas Schnelle wrote:
> > On Wed, 2023-03-15 at 12:19 +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 15, 2023 at 09:16:50AM +0100, Geert Uytterhoeven wrote:
> > > > Hi Niklas,
> > > > 
> > > > On Tue, Mar 14, 2023 at 1:13 PM 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 HAS_IOPORT as dependency for
> > > > > those drivers using them and guard inline code in headers.
> > > > > 
> > > > > Co-developed-by: Arnd Bergmann 
> > > > > Signed-off-by: Niklas Schnelle 
> > > > 
> > > > Thanks for your patch!
> > > > 
> > > > > --- a/drivers/video/fbdev/Kconfig
> > > > > +++ b/drivers/video/fbdev/Kconfig
> > > > 
> > > > > @@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT
> > > > > 
> > > > >  config FB_ATY
> > > > > tristate "ATI Mach64 display support" if PCI || ATARI
> > > > > -   depends on FB && !SPARC32
> > > > > +   depends on FB && HAS_IOPORT && !SPARC32
> > > > 
> > > > On Atari, this works without ATARI_ROM_ISA, hence it must not depend
> > > > on HAS_IOPORT.
> > > > The only call to inb() is inside a section protected by #ifdef
> > > > CONFIG_PCI. So:
> > > 
> > > That piece of code is a nop anyway. We immediately overwrite
> > > clk_wr_offset with a hardcoded selection after the register reads.
> > > So if you nuke that nop code then no IOPORT dependency required
> > > at all.
> > > 
> > 
> > I agree this "looks" like a nop but are we sure the inb() doesn't have
> > side effects? 
> 
> Yes. It's just trying to check which PLL dividers/etc. are currently
> used. In VGA mode it gets it from a the GENMO and in non-VGA mode from
> CLOCK_CNTL. And then it says "screw that" and just uses index 3 instead.
> 

Ok, I've added a patch to remove this part of the code and with that
the driver actually builds on s390 (no HAS_IOPORT) so I also removed
the HAS_IOPORT dependency. Both will be in my v4.

Thanks,
Niklas

Re: [PATCH v3 07/38] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Wed, 2023-03-15 at 13:54 +0200, Jani Nikula wrote:
> On Tue, 14 Mar 2023, 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 HAS_IOPORT as dependency for
> > those drivers using them. In the bochs driver there is optional MMIO
> > support detected at runtime, warn if this isn't taken when
> > HAS_IOPORT is not defined.
> 
> Not that I care a whole lot, but there should really only be one warning
> or even failure to probe at bochs_hw_init() for !bochs->mmio &&
> !IS_ENABLED(CONFIG_HAS_IOPORT), not warnings all over the place.
> 
> Moreover, the config macro is CONFIG_HAS_IOPORT instead of HAS_IOPORT
> that you check for below.
> 
> BR,
> Jani.
> 

Good Find! Yes the #ifdefs need CONFIG_HAS_IOPORT I even had it
correctly for cirrus in the same patch. For v4 I remove the separate
warnings and instead went with the below:

@@ -229,6 +242,10 @@ static int bochs_hw_init(struct drm_device *dev)
return -ENOMEM;
}
} else {
+   if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
+   DRM_ERROR("I/O ports are not supported\n");
+   return -EIO;
+   }
ioaddr = VBE_DISPI_IOPORT_INDEX;
iosize = 2;
if (!request_region(ioaddr, iosize, "bochs-drm")) {


Thanks,
Niklas

Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Wed, 2023-03-15 at 12:19 +0200, Ville Syrjälä wrote:
> On Wed, Mar 15, 2023 at 09:16:50AM +0100, Geert Uytterhoeven wrote:
> > Hi Niklas,
> > 
> > On Tue, Mar 14, 2023 at 1:13 PM 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 HAS_IOPORT as dependency for
> > > those drivers using them and guard inline code in headers.
> > > 
> > > Co-developed-by: Arnd Bergmann 
> > > Signed-off-by: Niklas Schnelle 
> > 
> > Thanks for your patch!
> > 
> > > --- a/drivers/video/fbdev/Kconfig
> > > +++ b/drivers/video/fbdev/Kconfig
> > 
> > > @@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT
> > > 
> > >  config FB_ATY
> > > tristate "ATI Mach64 display support" if PCI || ATARI
> > > -   depends on FB && !SPARC32
> > > +   depends on FB && HAS_IOPORT && !SPARC32
> > 
> > On Atari, this works without ATARI_ROM_ISA, hence it must not depend
> > on HAS_IOPORT.
> > The only call to inb() is inside a section protected by #ifdef
> > CONFIG_PCI. So:
> 
> That piece of code is a nop anyway. We immediately overwrite
> clk_wr_offset with a hardcoded selection after the register reads.
> So if you nuke that nop code then no IOPORT dependency required
> at all.
> 

I agree this "looks" like a nop but are we sure the inb() doesn't have
side effects? 
(for reference drivers/video/fbdev/aty/aty/atyfb_base.c:
atyfb_setup_generc() towards the end)

It does feel a bit out of scope for this series but if it's really a
nop nuking it surely is the cleaner solution.

Thanks,
Niklas

[PATCH v3 35/38] video: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them and guard inline code in headers.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/video/console/Kconfig |  1 +
 drivers/video/fbdev/Kconfig   | 25 +
 include/video/vga.h   |  8 
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 22cea5082ac4..64974eaa3ac5 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -10,6 +10,7 @@ config VGA_CONSOLE
depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC &&  !SUPERH 
&& \
(!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER) 
&& \
!ARM64 && !ARC && !MICROBLAZE && !OPENRISC && !S390 && !UML
+   depends on HAS_IOPORT
select APERTURE_HELPERS if (DRM || FB || VFIO_PCI_CORE)
default y
help
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index ff3646c30d0d..b21a37497d22 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -338,7 +338,7 @@ config FB_IMX
 
 config FB_CYBER2000
tristate "CyberPro 2000/2010/5000 support"
-   depends on FB && PCI && (BROKEN || !SPARC64)
+   depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -432,6 +432,7 @@ config FB_FM2
 config FB_ARC
tristate "Arc Monochrome LCD board support"
depends on FB && (X86 || COMPILE_TEST)
+   depends on HAS_IOPORT
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
select FB_SYS_IMAGEBLIT
@@ -1260,7 +1261,7 @@ config FB_RADEON_DEBUG
 
 config FB_ATY128
tristate "ATI Rage128 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT
 
 config FB_ATY
tristate "ATI Mach64 display support" if PCI || ATARI
-   depends on FB && !SPARC32
+   depends on FB && HAS_IOPORT && !SPARC32
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1335,7 +1336,7 @@ config FB_ATY_BACKLIGHT
 
 config FB_S3
tristate "S3 Trio/Virge support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1396,7 +1397,7 @@ config FB_SAVAGE_ACCEL
 
 config FB_SIS
tristate "SiS/XGI display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1427,7 +1428,7 @@ config FB_SIS_315
 
 config FB_VIA
tristate "VIA UniChrome (Pro) and Chrome9 display support"
-   depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST)
+   depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || 
COMPILE_TEST)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1466,7 +1467,7 @@ endif
 
 config FB_NEOMAGIC
tristate "NeoMagic display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_MODE_HELPERS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1496,7 +1497,7 @@ config FB_KYRO
 
 config FB_3DFX
tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_IMAGEBLIT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1546,7 +1547,7 @@ config FB_VOODOO1
 
 config FB_VT8623
tristate "VIA VT8623 support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1561,7 +1562,7 @@ config FB_VT8623
 
 config FB_TRIDENT
tristate "Trident/CyberXXX/CyberBlade support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1584,7 +1585,7 @@ config FB_TRIDENT
 
 config FB_A

[PATCH v3 07/38] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them. In the bochs driver there is optional MMIO
support detected at runtime, warn if this isn't taken when
HAS_IOPORT is not defined.

There is also a direct and hard coded use in cirrus.c which according to
the comment is only necessary during resume.  Let's just skip this as
for example s390 which doesn't have I/O port support also doesen't
support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/gpu/drm/qxl/Kconfig   |  1 +
 drivers/gpu/drm/tiny/bochs.c  | 19 +++
 drivers/gpu/drm/tiny/cirrus.c |  2 ++
 3 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index ca3f51c2a8fe..d0e0d440c8d9 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
 config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 024346054c70..da4a5d53b003 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -2,6 +2,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -105,7 +106,11 @@ static void bochs_vga_writeb(struct bochs_device *bochs, 
u16 ioport, u8 val)
 
writeb(val, bochs->mmio + offset);
} else {
+#ifdef HAS_IOPORT
outb(val, ioport);
+#else
+   WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT");
+#endif
}
 }
 
@@ -119,7 +124,12 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 
ioport)
 
return readb(bochs->mmio + offset);
} else {
+#ifdef HAS_IOPORT
return inb(ioport);
+#else
+   WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT");
+   return 0xff;
+#endif
}
 }
 
@@ -132,8 +142,13 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, 
u16 reg)
 
ret = readw(bochs->mmio + offset);
} else {
+#ifdef HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
ret = inw(VBE_DISPI_IOPORT_DATA);
+#else
+   WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT");
+   ret = 0x;
+#endif
}
return ret;
 }
@@ -145,8 +160,12 @@ static void bochs_dispi_write(struct bochs_device *bochs, 
u16 reg, u16 val)
 
writew(val, bochs->mmio + offset);
} else {
+#ifdef HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
outw(val, VBE_DISPI_IOPORT_DATA);
+#else
+   WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT");
+#endif
}
 }
 
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index accfa52e78c5..9da89732c5ac 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -308,8 +308,10 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 
cirrus_set_start_address(cirrus, 0);
 
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(0x20, 0x3c0);
+#endif
 
drm_dev_exit(idx);
return 0;
-- 
2.37.2

Re: [PATCH 8/8] iommu/s390: Push the gfp parameter to the kmem_cache_alloc()'s

[Niklas Schnelle]

On Fri, 2023-01-06 at 12:42 -0400, Jason Gunthorpe wrote:
> dma_alloc_cpu_table() and dma_alloc_page_table() are eventually called by
> iommufd through s390_iommu_map_pages() and it should not be forced to
> atomic. Thread the gfp parameter through the call chain starting from
> s390_iommu_map_pages().
> 
> Signed-off-by: Jason Gunthorpe 
> ---
>  arch/s390/include/asm/pci_dma.h |  5 +++--
>  arch/s390/pci/pci_dma.c | 31 +--
>  drivers/iommu/s390-iommu.c  | 15 +--
>  3 files changed, 29 insertions(+), 22 deletions(-)
> 
---8<---
>  

Looks good to me and I have no objections.

Reviewed-by: Niklas Schnelle 

[PATCH 31/37] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them. There is also a direct and hard coded use in
cirrus.c which according to the comment is only necessary during resume.
Let's just skip this as for example s390 which doesn't have I/O port
support also doesen't support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/gpu/drm/qxl/Kconfig   | 1 +
 drivers/gpu/drm/tiny/Kconfig  | 1 +
 drivers/gpu/drm/tiny/cirrus.c | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index ca3f51c2a8fe..d0e0d440c8d9 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
 config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 627d637a1e7e..7102783809c3 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -13,6 +13,7 @@ config DRM_ARCPGU
 config DRM_BOCHS
tristate "DRM Support for bochs dispi vga interface (qemu stdvga)"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_VRAM_HELPER
select DRM_TTM
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index c8e791840862..0dc4788c5399 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -306,8 +306,10 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 
cirrus_set_start_address(cirrus, 0);
 
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(0x20, 0x3c0);
+#endif
 
drm_dev_exit(idx);
return 0;
-- 
2.32.0

[RFC v2 36/39] video: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them and guard inline code in headers.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/video/fbdev/Kconfig | 25 +
 include/video/vga.h |  8 
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 93b8d84c34cf..e7d27c0602d5 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -343,7 +343,7 @@ config FB_IMX
 
 config FB_CYBER2000
tristate "CyberPro 2000/2010/5000 support"
-   depends on FB && PCI && (BROKEN || !SPARC64)
+   depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -436,6 +436,7 @@ config FB_FM2
 config FB_ARC
tristate "Arc Monochrome LCD board support"
depends on FB && (X86 || COMPILE_TEST)
+   depends on HAS_IOPORT
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
select FB_SYS_IMAGEBLIT
@@ -1242,7 +1243,7 @@ config FB_RADEON_DEBUG
 
 config FB_ATY128
tristate "ATI Rage128 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1265,7 +1266,7 @@ config FB_ATY128_BACKLIGHT
 
 config FB_ATY
tristate "ATI Mach64 display support" if PCI || ATARI
-   depends on FB && !SPARC32
+   depends on FB && HAS_IOPORT && !SPARC32
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1315,7 +1316,7 @@ config FB_ATY_BACKLIGHT
 
 config FB_S3
tristate "S3 Trio/Virge support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1374,7 +1375,7 @@ config FB_SAVAGE_ACCEL
 
 config FB_SIS
tristate "SiS/XGI display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1404,7 +1405,7 @@ config FB_SIS_315
 
 config FB_VIA
tristate "VIA UniChrome (Pro) and Chrome9 display support"
-   depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST)
+   depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || 
COMPILE_TEST)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1442,7 +1443,7 @@ endif
 
 config FB_NEOMAGIC
tristate "NeoMagic display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_MODE_HELPERS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1470,7 +1471,7 @@ config FB_KYRO
 
 config FB_3DFX
tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_IMAGEBLIT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1518,7 +1519,7 @@ config FB_VOODOO1
 
 config FB_VT8623
tristate "VIA VT8623 support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1532,7 +1533,7 @@ config FB_VT8623
 
 config FB_TRIDENT
tristate "Trident/CyberXXX/CyberBlade support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1554,7 +1555,7 @@ config FB_TRIDENT
 
 config FB_ARK
tristate "ARK 2000PV support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -2226,7 +2227,7 @@ config FB_SSD1307
 
 config FB_SM712
tristate "Silicon Motion SM712 framebuffer support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
diff --git a/include/video/vga.h b/include/video/vga.h
index d334e64c1c19..53cb52c0fddb 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -201,18 +201,26

Re: [PATCH 25/37] video: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Fri, 2022-04-29 at 15:50 +0200, 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 HAS_IOPORT as dependency for
> those drivers using them and guard inline code in headers.
> 
> Co-developed-by: Arnd Bergmann 
> Signed-off-by: Niklas Schnelle 
> ---

Sorry everyone. I sent this as PATCH in error while preparing to sent
the same series as RFC. Since e-mail has no remote delete and I lack a
time machine let's just all pretend you only got the RFC.

Re: [PATCH 31/37] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

On Fri, 2022-04-29 at 15:50 +0200, 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 HAS_IOPORT as dependency for
> those drivers using them. There is also a direct and hard coded use in
> cirrus.c which according to the comment is only necessary during resume.
> Let's just skip this as for example s390 which doesn't have I/O port
> support also doesen't support suspend/resume.
> 
> Co-developed-by: Arnd Bergmann 
> Signed-off-by: Niklas Schnelle 
> ---

Sorry everyone. I sent this as PATCH in error while preparing to sent
the same series as RFC. Since e-mail has no remote delete and I lack a
time machine let's just all pretend you only got the RFC.

[RFC v2 08/39] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them. In the bochs driver there is optional MMIO
support detected at runtime, warn if this isn't taken when
HAS_IOPORT is not defined.

There is also a direct and hard coded use in cirrus.c which according to
the comment is only necessary during resume.  Let's just skip this as
for example s390 which doesn't have I/O port support also doesen't
support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/gpu/drm/qxl/Kconfig   |  1 +
 drivers/gpu/drm/tiny/bochs.c  | 19 +++
 drivers/gpu/drm/tiny/cirrus.c |  2 ++
 3 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index ca3f51c2a8fe..d0e0d440c8d9 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
 config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index ed971c8bb446..9acc726d99ec 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
+#include 
 #include 
 
 #include 
@@ -102,7 +103,11 @@ static void bochs_vga_writeb(struct bochs_device *bochs, 
u16 ioport, u8 val)
 
writeb(val, bochs->mmio + offset);
} else {
+#ifdef HAS_IOPORT
outb(val, ioport);
+#else
+   WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT");
+#endif
}
 }
 
@@ -116,7 +121,12 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 
ioport)
 
return readb(bochs->mmio + offset);
} else {
+#ifdef HAS_IOPORT
return inb(ioport);
+#else
+   WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT");
+   return 0xff;
+#endif
}
 }
 
@@ -129,8 +139,13 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, 
u16 reg)
 
ret = readw(bochs->mmio + offset);
} else {
+#ifdef HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
ret = inw(VBE_DISPI_IOPORT_DATA);
+#else
+   WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT");
+   ret = 0x;
+#endif
}
return ret;
 }
@@ -142,8 +157,12 @@ static void bochs_dispi_write(struct bochs_device *bochs, 
u16 reg, u16 val)
 
writew(val, bochs->mmio + offset);
} else {
+#ifdef HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
outw(val, VBE_DISPI_IOPORT_DATA);
+#else
+   WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT");
+#endif
}
 }
 
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index c8e791840862..0dc4788c5399 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -306,8 +306,10 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 
cirrus_set_start_address(cirrus, 0);
 
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(0x20, 0x3c0);
+#endif
 
drm_dev_exit(idx);
return 0;
-- 
2.32.0

[PATCH 25/37] video: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them and guard inline code in headers.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/video/fbdev/Kconfig | 25 +
 include/video/vga.h |  8 
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 93b8d84c34cf..e7d27c0602d5 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -343,7 +343,7 @@ config FB_IMX
 
 config FB_CYBER2000
tristate "CyberPro 2000/2010/5000 support"
-   depends on FB && PCI && (BROKEN || !SPARC64)
+   depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -436,6 +436,7 @@ config FB_FM2
 config FB_ARC
tristate "Arc Monochrome LCD board support"
depends on FB && (X86 || COMPILE_TEST)
+   depends on HAS_IOPORT
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
select FB_SYS_IMAGEBLIT
@@ -1242,7 +1243,7 @@ config FB_RADEON_DEBUG
 
 config FB_ATY128
tristate "ATI Rage128 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1265,7 +1266,7 @@ config FB_ATY128_BACKLIGHT
 
 config FB_ATY
tristate "ATI Mach64 display support" if PCI || ATARI
-   depends on FB && !SPARC32
+   depends on FB && HAS_IOPORT && !SPARC32
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1315,7 +1316,7 @@ config FB_ATY_BACKLIGHT
 
 config FB_S3
tristate "S3 Trio/Virge support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1374,7 +1375,7 @@ config FB_SAVAGE_ACCEL
 
 config FB_SIS
tristate "SiS/XGI display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1404,7 +1405,7 @@ config FB_SIS_315
 
 config FB_VIA
tristate "VIA UniChrome (Pro) and Chrome9 display support"
-   depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST)
+   depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || 
COMPILE_TEST)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1442,7 +1443,7 @@ endif
 
 config FB_NEOMAGIC
tristate "NeoMagic display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_MODE_HELPERS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1470,7 +1471,7 @@ config FB_KYRO
 
 config FB_3DFX
tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_IMAGEBLIT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1518,7 +1519,7 @@ config FB_VOODOO1
 
 config FB_VT8623
tristate "VIA VT8623 support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1532,7 +1533,7 @@ config FB_VT8623
 
 config FB_TRIDENT
tristate "Trident/CyberXXX/CyberBlade support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -1554,7 +1555,7 @@ config FB_TRIDENT
 
 config FB_ARK
tristate "ARK 2000PV support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -2226,7 +2227,7 @@ config FB_SSD1307
 
 config FB_SM712
tristate "Silicon Motion SM712 framebuffer support"
-   depends on FB && PCI
+   depends on FB && PCI && HAS_IOPORT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
diff --git a/include/video/vga.h b/include/video/vga.h
index d334e64c1c19..53cb52c0fddb 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -201,18 +201,26

Re: [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI

[Niklas Schnelle]

On Thu, 2022-01-06 at 17:41 +, John Garry wrote:
> On 05/01/2022 19:47, Bjorn Helgaas wrote:
> > > > > >   ok if the PCI maintainers decide otherwise.
> > > > > I don't really like the "LEGACY_PCI" Kconfig option.  "Legacy" just
> > > > > means something old and out of favor; it doesn't say*what*  that
> > > > > something is.
> > > > > 
> > > > > I think you're specifically interested in I/O port space usage, and it
> > > > > seems that you want all PCI drivers that*only*  use I/O port space to
> > > > > depend on LEGACY_PCI?  Drivers that can use either I/O or memory
> > > > > space or both would not depend on LEGACY_PCI?  This seems a little
> > > > > murky and error-prone.
> > > > I'd like to hear Arnd's opinion on this but you're the PCI maintainer
> > > > so of course your buy-in would be quite important for such an option.
> > I'd like to hear Arnd's opinion, too.  If we do add LEGACY_PCI, I
> > think we need a clear guide for when to use it, e.g., "a PCI driver
> > that uses inb() must depend on LEGACY_PCI" or whatever it is.
> > 
> > I must be missing something because I don't see what we gain from
> > this.  We have PCI drivers, e.g., megaraid [1], for devices that have
> > either MEM or I/O BARs.  I think we want to build drivers like that on
> > any arch that supports PCI.
> > 
> > If the arch doesn't support I/O port space, devices that only have I/O
> > BARs won't work, of course, and hopefully the PCI core and driver can
> > figure that out and gracefully fail the probe.
> > 
> > But that same driver should still work with devices that have MEM
> > BARs.  If inb() isn't always present, I guess we could litter these
> > drivers with #ifdefs, but that would be pretty ugly.

I think this is the big question here. If we do go with a compile-time
solution as requested by Linus we will either get a lot of #ifdeffery,
coarse driver dependencies or as proposed by Alan Stern for the USB
#ifdefs might end up turning inb() into a compile-time nop.

The originally proposed change that returned ~0 from inb() and printed
a warning clearly is the simpler change and sure we could also drop the
warning. I'm honestly torn, I do agree with Linus that we shouldn't
have run-time things that we know at compile-time will not work but I
also dislike all the #ifdeffery a compile-time solution requires. Sadly
C really doesn't give us any better tools here.

Also I 100% agree with you Bjorn how likely it is to see a device on a
platform really shouldn't matter. Without going into details, on s390
we have already beneffited from PCI drivers working with 0 changes to
support devices we previously didn't have on the platform or
anticipated we would get in the future. Consequently drivers that could
work in principle should be built.

> >  
> 
> There were some ifdefs added to the 8250 drivers in Arnd's original 
> patch [0], but it does not seem included here.
> 
> Niklas, what happened to the 8250 and the other driver changes?

I missed it during the rebase, likely because the changed files compile
depend on !S390 via config SERIAL_8250 and thus didn't cause any errors
for my allyesconfig. That !S390 dependency is of course not really what
we want if the driver can use MEM BARs.

Re: [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI

[Niklas Schnelle]

On Wed, 2021-12-29 at 10:03 -0600, Bjorn Helgaas wrote:
> On Wed, Dec 29, 2021 at 01:12:07PM +0100, Mauro Carvalho Chehab wrote:
> > Em Wed, 29 Dec 2021 12:45:38 +0100
> > Niklas Schnelle  escreveu:
> > > ...
> > > I do think we agree that once done correctly there is value in
> > > such an option independent of HAS_IOPORT only gating inb() etc uses.
> 
> I'm not sure I'm convinced about this.  For s390, you could do this
> patch series, where you don't define inb() at all, and you add new
> dependencies to prevent compile errors.  Or you could define inb() to
> return ~0, which is what happens on other platforms when the device is
> not present.
> 
> > Personally, I don't see much value on a Kconfig var for legacy PCI I/O 
> > space. From maintenance PoV, bots won't be triggered if someone use
> > HAS_IOPORT instead of the PCI specific one - or vice-versa. So, we
> > could end having a mix of both at the wrong places, in long term.
> > 
> > Also, assuming that PCIe hardware will some day abandon support for 
> > "legacy" PCI I/O space, I guess some runtime logic would be needed, 
> > in order to work with both kinds of PCIe controllers. So, having a
> > Kconfig option won't help much, IMO.
> > 
> > So, my personal preference would be to have just one Kconfig var, but
> > I'm ok if the PCI maintainers decide otherwise.
> 
> I don't really like the "LEGACY_PCI" Kconfig option.  "Legacy" just
> means something old and out of favor; it doesn't say *what* that
> something is.
> 
> I think you're specifically interested in I/O port space usage, and it
> seems that you want all PCI drivers that *only* use I/O port space to
> depend on LEGACY_PCI?  Drivers that can use either I/O or memory
> space or both would not depend on LEGACY_PCI?  This seems a little
> murky and error-prone.

I'd like to hear Arnd's opinion on this but you're the PCI maintainer
so of course your buy-in would be quite important for such an option.

> 
> What if you used the approach from [1] but just dropped the warning?
> The inb() would return ~0 if the platform doesn't support I/O port
> space.  Drivers should be prepared to handle that because that's what
> happens if the device doesn't exist.

Hmm, in that mail Linus very clearly and specifically asked for this to
be a compile-time thing. So, if we do want to make it compile-time but
keep the potential errors to a minimum I guess just having HAS_IOPORT
might be valid compromise. It gets caught by bots through allyesconfig
or randconfig on HAS_IOPORT=n architectures. Also it has a nice
symmetry with the existing HAS_IOMEM. 

>  
> 
> HAS_IOPORT and LEGACY_PCI is a lot of Kconfiggery that basically just
> avoids building drivers that aren't useful on s390.  I'm not sure the
> benefit outweighs the complication.
> 
> Bjorn
> 
> [1] 
> https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/
> 

Despite s390 I believe it would also affect nds32, um, h8300, nios2,
openrisc, hexagon, csky, and xtensa. But yes none of these is any less
niche than us. I do wonder if we will see a new drivers using I/O
ports?

[RFC 22/32] video: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them and guard inline code in headers.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/video/fbdev/Kconfig | 1 +
 include/video/vga.h | 8 
 2 files changed, 9 insertions(+)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index a3446d44c118..d95f638f4deb 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -436,6 +436,7 @@ config FB_FM2
 config FB_ARC
tristate "Arc Monochrome LCD board support"
depends on FB && (X86 || COMPILE_TEST)
+   depends on HAS_IOPORT
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
select FB_SYS_IMAGEBLIT
diff --git a/include/video/vga.h b/include/video/vga.h
index d334e64c1c19..53cb52c0fddb 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -201,18 +201,26 @@ extern int restore_vga(struct vgastate *state);
  
 static inline unsigned char vga_io_r (unsigned short port)
 {
+#ifdef CONFIG_HAS_IOPORT
return inb_p(port);
+#else
+   return 0xff;
+#endif
 }
 
 static inline void vga_io_w (unsigned short port, unsigned char val)
 {
+#ifdef CONFIG_HAS_IOPORT
outb_p(val, port);
+#endif
 }
 
 static inline void vga_io_w_fast (unsigned short port, unsigned char reg,
  unsigned char val)
 {
+#ifdef CONFIG_HAS_IOPORT
outw(VGA_OUT16VAL (val, reg), port);
+#endif
 }
 
 static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short 
port)
-- 
2.32.0

Re: [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI

[Niklas Schnelle]

On Tue, 2021-12-28 at 10:15 +0100, Mauro Carvalho Chehab wrote:
> Em Tue, 28 Dec 2021 09:21:23 +0100
> Greg Kroah-Hartman  escreveu:
> 
> > On Mon, Dec 27, 2021 at 05:42:46PM +0100, Niklas Schnelle wrote:
> > > --- a/drivers/pci/Kconfig
> > > +++ b/drivers/pci/Kconfig
> > > @@ -23,6 +23,17 @@ menuconfig PCI
> > >  
> > >  if PCI
> > >  
> > > +config LEGACY_PCI
> > > + bool "Enable support for legacy PCI devices"
> > > + depends on HAVE_PCI
> > > + help
> > > +This option enables support for legacy PCI devices. This includes
> > > +PCI devices attached directly or via a bridge on a PCI Express bus.
> > > +It also includes compatibility features on PCI Express devices which
> > > +make use of legacy I/O spaces.  
> 
> This Kconfig doesn't seem what it is needed there, as this should be an 
> arch-dependent feature, and not something that the poor user should be
> aware if a given architecture supports it or not. Also, the above will keep
> causing warnings or errors with randconfigs.
> 
> Also, the "depends on HAVE_CPI" is bogus, as PCI already depends on 
> HAVE_PCI:

Ah yes you're right.

> 
>   menuconfig PCI
>   bool "PCI support"
>   depends on HAVE_PCI
>   help
> This option enables support for the PCI local bus, including
> support for PCI-X and the foundations for PCI Express support.
> Say 'Y' here unless you know what you are doing.
> 
> So, instead, I would expect that a new HAVE_xxx option would be
> added at arch/*/Kconfig, like:
> 
>   config X86
>   ...
>   select HAVE_PCI_DIRECT_IO
> 
> It would also make sense to document it at Documentation/features/.

I'll look into that, thanks.

> 
> > All you really care about is the "legacy" I/O spaces here, this isn't
> > tied to PCI specifically at all, right?
> > 
> > So why not just have a OLD_STYLE_IO config option or something like
> > that, to show that it's the i/o functions we care about here, not PCI at
> > all?
> > 
> > And maybe not call it "old" or "legacy" as time constantly goes forward,
> > just describe it as it is, "DIRECT_IO"?
> 
> Agreed. HAVE_PCI_DIRECT_IO (or something similar) seems a more appropriate
> name for it.
> 
> Thanks,
> Mauro

Hmm, I might be missing something here but that sounds a lot like the
HAS_IOPORT option added in patch 02.

We add both LEGACY_PCI and HAS_IOPORT to differentiate between two
cases. HAS_IOPORT is for PC-style devices that are not on a PCI card
while LEGACY_PCI is for PCI drivers that require port I/O. This
includes pre-PCIe devices as well as PCIe devices which require
features like I/O spaces. The "legacy" naming is comes from the PCIe
spec which in section 2.1.1.2 says "PCI Express supports I/O Space for
compatibility with legacy devices which require their use. Future
revisions of this specification may deprecate the use of I/O Space."

These two separate config options allow us to compile without support
for these legacy PCI devices even on a system where inb()/outb() and
friends are required for some PC style devices and for example ACPI.

[RFC 26/32] drm: handle HAS_IOPORT dependencies

[Niklas Schnelle]

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them. There is also a direct and hard coded use in
cirrus.c which according to the comment is only necessary during resume.
Let's just skip this as for example s390 which doesn't have I/O port
support also doesen't support suspend/resume.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/gpu/drm/qxl/Kconfig   | 1 +
 drivers/gpu/drm/tiny/Kconfig  | 1 +
 drivers/gpu/drm/tiny/cirrus.c | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index ca3f51c2a8fe..d0e0d440c8d9 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
 config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 1ceb93fbdc50..81749943af13 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -13,6 +13,7 @@ config DRM_ARCPGU
 config DRM_BOCHS
tristate "DRM Support for bochs dispi vga interface (qemu stdvga)"
depends on DRM && PCI && MMU
+   depends on HAS_IOPORT
select DRM_KMS_HELPER
select DRM_VRAM_HELPER
select DRM_TTM
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 4611ec408506..c9b6e9779d18 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -306,8 +306,10 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 
cirrus_set_start_address(cirrus, 0);
 
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(0x20, 0x3c0);
+#endif
 
drm_dev_exit(idx);
return 0;
-- 
2.32.0

Re: [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI

[Niklas Schnelle]

On Tue, 2021-12-28 at 18:12 +0100, Mauro Carvalho Chehab wrote:
> Em Tue, 28 Dec 2021 16:06:44 +0100
> Niklas Schnelle  escreveu:
> 
> (on a side note: the c/c list of this patch is too long. I would try to
> avoid using a too long list, as otherwise this e-mail may end being rejected
> by mail servers)
> 
> > On Tue, 2021-12-28 at 13:54 +0100, Mauro Carvalho Chehab wrote:
> > >  
> > ---8<---
> > > 
> > > > > > All you really care about is the "legacy" I/O spaces here, this 
> > > > > > isn't
> > > > > > tied to PCI specifically at all, right?
> > > > > > 
> > > > > > So why not just have a OLD_STYLE_IO config option or something like
> > > > > > that, to show that it's the i/o functions we care about here, not 
> > > > > > PCI at
> > > > > > all?
> > > > > > 
> > > > > > And maybe not call it "old" or "legacy" as time constantly goes 
> > > > > > forward,
> > > > > > just describe it as it is, "DIRECT_IO"?
> > > > > 
> > > > > Agreed. HAVE_PCI_DIRECT_IO (or something similar) seems a more 
> > > > > appropriate
> > > > > name for it.
> > > > > 
> > > > > Thanks,
> > > > > Mauro
> > > > 
> > > > Hmm, I might be missing something here but that sounds a lot like the
> > > > HAS_IOPORT option added in patch 02.
> > > > 
> > > > We add both LEGACY_PCI and HAS_IOPORT to differentiate between two
> > > > cases. HAS_IOPORT is for PC-style devices that are not on a PCI card
> > > > while LEGACY_PCI is for PCI drivers that require port I/O.   
> > > 
> > > I didn't look at the other patches on this series, but why it is needed
> > > to deal with them on a separate way? Won't "PCI" and "HAS_IOPORT" be 
> > > enough? 
> > > 
> > > I mean, are there any architecture where HAVE_PCI=y and HAS_IOPORT=y
> > > where LEGACY_PCI shall be "n"?  
> > 
> > In the current patch set LEGACY_PCI is not currently selected by
> > architectures, though of course it could be if we know that an
> > architecture requires it. We should probably also set it in any
> > defconfig that has devices depending on it so as not to break these.
> > 
> > Other than that it would be set during kernel configuration if one
> > wants/needs support for legacy PCI devices. For testing I ran with
> > HAVE_PCI=y, HAS_IOPORT=y and LEGACY_PCI=n on both my local Ryzen 3990X
> > based workstation and Raspberry Pi 4 (DT). I guess at the moment it
> > would make most sense for special configs such as those tailored for
> > vitualization guets but in the end that would be something for
> > distributions to decide.
> 
> IMO, it makes sense to have a "default y" there, as on systems that
> support I/O space, disabling it will just randomly disable some drivers
> that could be required by some hardware. I won't doubt that some of 
> those could be ported from using inb/outb to use, instead, readb/writeb.

Makes sense, if these get more legacy over time we can always change
the default. This would also mean we don't need to change defconfigs
that include legacy PCI devices.

> 
> > Arnd described the options here:
> > https://lore.kernel.org/lkml/cak8p3a3hhep+gw_k2p7qtig0omerf0hn30g22+qhic_uzth...@mail.gmail.com/
> 
> Based on Arnd's description, LEGACY_PCI should depend on HAS_IOPORT.
> This is missing on patch 1. You should probably reorder your patch
> series to first create HAS_IOPORT and then add LEGACY_PCI with
> depends on, as otherwise it may cause randconfig build issues
> at robots and/or git bisect.
> 
> I would also suggest to first introduce such change and then send
> a per-subsystem LEGACY_PCI patch, as it would be a lot easier for
> maintainers to review.

Playing around with the reordering I think it might make sense to
introduce HAS_IOPORT in patch 01, then LEGACY_PCI in patch 02 and then
add dependencies for both on a per subsystem basis. I think it would be
overkill to have two series of per subsystem patches.

> 
> > >   
> > > > This
> > > > includes pre-PCIe devices as well as PCIe devices which require
> > > > features like I/O spaces. The "legacy" naming is comes from the PCIe
> > > > spec which in section 2.1.1.2 says "PCI Express supports I/O Space for
> > > > compatibility with le

[RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI

[Niklas Schnelle]

Introduce a new LEGACY_PCI Kconfig option which gates support for legacy
PCI devices including those attached to a PCI-to-PCI Express bridge and
PCI Express devices using legacy I/O spaces. Note that this is different
from non PCI uses of I/O ports such as by ACPI.

Add dependencies on LEGACY_PCI for all PCI drivers which only target
legacy PCI devices and ifdef legacy PCI specific functions in ata
handling.

Co-developed-by: Arnd Bergmann 
Signed-off-by: Arnd Bergmann 
Signed-off-by: Niklas Schnelle 
---
 drivers/ata/Kconfig  | 34 
 drivers/ata/ata_generic.c|  3 +-
 drivers/ata/libata-sff.c |  2 +
 drivers/comedi/Kconfig   | 42 +++
 drivers/gpio/Kconfig |  2 +-
 drivers/hwmon/Kconfig|  6 +--
 drivers/i2c/busses/Kconfig   | 24 +--
 drivers/input/gameport/Kconfig   |  4 +-
 drivers/isdn/hardware/mISDN/Kconfig  | 14 +++
 drivers/media/cec/platform/Kconfig   |  2 +-
 drivers/media/pci/dm1105/Kconfig |  2 +-
 drivers/media/radio/Kconfig  |  2 +-
 drivers/message/fusion/Kconfig   |  8 ++--
 drivers/net/arcnet/Kconfig   |  2 +-
 drivers/net/ethernet/8390/Kconfig|  2 +-
 drivers/net/ethernet/amd/Kconfig |  2 +-
 drivers/net/ethernet/intel/Kconfig   |  4 +-
 drivers/net/ethernet/sis/Kconfig |  6 +--
 drivers/net/ethernet/ti/Kconfig  |  4 +-
 drivers/net/ethernet/via/Kconfig |  4 +-
 drivers/net/fddi/Kconfig |  4 +-
 drivers/net/wan/Kconfig  |  2 +-
 drivers/net/wireless/atmel/Kconfig   |  4 +-
 drivers/net/wireless/intersil/hostap/Kconfig |  4 +-
 drivers/pci/Kconfig  | 11 +
 drivers/scsi/Kconfig | 20 -
 drivers/scsi/aic7xxx/Kconfig.aic79xx |  2 +-
 drivers/scsi/aic7xxx/Kconfig.aic7xxx |  2 +-
 drivers/scsi/aic94xx/Kconfig |  2 +-
 drivers/scsi/megaraid/Kconfig.megaraid   |  2 +-
 drivers/scsi/mvsas/Kconfig   |  2 +-
 drivers/scsi/qla2xxx/Kconfig |  2 +-
 drivers/spi/Kconfig  |  1 +
 drivers/staging/sm750fb/Kconfig  |  2 +-
 drivers/staging/vt6655/Kconfig   |  2 +-
 drivers/tty/Kconfig  |  2 +-
 drivers/tty/serial/Kconfig   |  2 +-
 drivers/video/fbdev/Kconfig  | 22 +-
 drivers/watchdog/Kconfig |  4 +-
 sound/pci/Kconfig| 43 
 40 files changed, 194 insertions(+), 110 deletions(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index a7da8ea7b3ed..32e0489bd01c 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -556,7 +556,7 @@ comment "PATA SFF controllers with BMDMA"
 
 config PATA_ALI
tristate "ALi PATA support"
-   depends on PCI
+   depends on LEGACY_PCI
select PATA_TIMINGS
help
  This option enables support for the ALi ATA interfaces
@@ -566,7 +566,7 @@ config PATA_ALI
 
 config PATA_AMD
tristate "AMD/NVidia PATA support"
-   depends on PCI
+   depends on LEGACY_PCI
select PATA_TIMINGS
help
  This option enables support for the AMD and NVidia PATA
@@ -584,7 +584,7 @@ config PATA_ARASAN_CF
 
 config PATA_ARTOP
tristate "ARTOP 6210/6260 PATA support"
-   depends on PCI
+   depends on LEGACY_PCI
help
  This option enables support for ARTOP PATA controllers.
 
@@ -621,7 +621,7 @@ config PATA_BK3710
 
 config PATA_CMD64X
tristate "CMD64x PATA support"
-   depends on PCI
+   depends on LEGACY_PCI
select PATA_TIMINGS
help
  This option enables support for the CMD64x series chips
@@ -667,7 +667,7 @@ config PATA_CS5536
 
 config PATA_CYPRESS
tristate "Cypress CY82C693 PATA support (Very Experimental)"
-   depends on PCI
+   depends on LEGACY_PCI
select PATA_TIMINGS
help
  This option enables support for the Cypress/Contaq CY82C693
@@ -707,7 +707,7 @@ config PATA_FTIDE010
 
 config PATA_HPT366
tristate "HPT 366/368 PATA support"
-   depends on PCI
+   depends on LEGACY_PCI
help
  This option enables support for the HPT 366 and 368
  PATA controllers via the new ATA layer.
@@ -716,7 +716,7 @@ config PATA_HPT366
 
 config PATA_HPT37X
tristate "HPT 370/370A/371/372/374/302 PATA support"
-   depends on PCI
+   depends on LEGACY_PCI
help
  This option enables support for the majority of the later HPT
  PATA controllers via the new ATA layer.
@@ -725,7 +725,7 @@ config PATA

Re: [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI

[Niklas Schnelle]

On Tue, 2021-12-28 at 13:54 +0100, Mauro Carvalho Chehab wrote:
>
---8<---
>   
> > > > All you really care about is the "legacy" I/O spaces here, this isn't
> > > > tied to PCI specifically at all, right?
> > > > 
> > > > So why not just have a OLD_STYLE_IO config option or something like
> > > > that, to show that it's the i/o functions we care about here, not PCI at
> > > > all?
> > > > 
> > > > And maybe not call it "old" or "legacy" as time constantly goes forward,
> > > > just describe it as it is, "DIRECT_IO"?  
> > > 
> > > Agreed. HAVE_PCI_DIRECT_IO (or something similar) seems a more appropriate
> > > name for it.
> > > 
> > > Thanks,
> > > Mauro  
> > 
> > Hmm, I might be missing something here but that sounds a lot like the
> > HAS_IOPORT option added in patch 02.
> > 
> > We add both LEGACY_PCI and HAS_IOPORT to differentiate between two
> > cases. HAS_IOPORT is for PC-style devices that are not on a PCI card
> > while LEGACY_PCI is for PCI drivers that require port I/O. 
> 
> I didn't look at the other patches on this series, but why it is needed
> to deal with them on a separate way? Won't "PCI" and "HAS_IOPORT" be enough? 
> 
> I mean, are there any architecture where HAVE_PCI=y and HAS_IOPORT=y
> where LEGACY_PCI shall be "n"?

In the current patch set LEGACY_PCI is not currently selected by
architectures, though of course it could be if we know that an
architecture requires it. We should probably also set it in any
defconfig that has devices depending on it so as not to break these.

Other than that it would be set during kernel configuration if one
wants/needs support for legacy PCI devices. For testing I ran with
HAVE_PCI=y, HAS_IOPORT=y and LEGACY_PCI=n on both my local Ryzen 3990X
based workstation and Raspberry Pi 4 (DT). I guess at the moment it
would make most sense for special configs such as those tailored for
vitualization guets but in the end that would be something for
distributions to decide.

Arnd described the options here:
https://lore.kernel.org/lkml/cak8p3a3hhep+gw_k2p7qtig0omerf0hn30g22+qhic_uzth...@mail.gmail.com/

> 
> > This
> > includes pre-PCIe devices as well as PCIe devices which require
> > features like I/O spaces. The "legacy" naming is comes from the PCIe
> > spec which in section 2.1.1.2 says "PCI Express supports I/O Space for
> > compatibility with legacy devices which require their use. Future
> > revisions of this specification may deprecate the use of I/O Space."
> 
> I would still avoid calling it LEGACY_PCI, as this sounds too generic.
> 
> I didn't read the PCI/PCIe specs, but I suspect that are a lot more
> features that were/will be deprecated on PCI specs as time goes by.
> 
> So, I would, instead, use something like PCI_LEGACY_IO_SPACE or 
> HAVE_PCI_LEGACY_IO_SPACE, in order to let it clear what "legacy"
> means.

Hmm, I'd like to hear Bjorn's opinion on this. Personally I feel like
LEGACY_PCI is pretty clear since most devices are either pre-PCIe
devices or a compatibility feature allowing drivers for a pre-PCIe
device to work with a PCIe device.

> 
> > These two separate config options allow us to compile without support
> > for these legacy PCI devices even on a system where inb()/outb() and
> > friends are required for some PC style devices and for example ACPI.
> 
> Hmm... why this patch make SND_BT87X dependent on LEGACY_PCI?
> 
> > @@ -172,6 +177,7 @@ config SND_AZT3328
> >  
> >  config SND_BT87X
> > tristate "Bt87x Audio Capture"
> > +   depends on LEGACY_PCI
> > select SND_PCM
> > help
> >   If you want to record audio from TV cards based on
> 
> I couldn't find any usage of inb/outb & friends on it:
> 
>   $ grep -E '(inb|outb|inw|outw|inl|outl)\b' ./sound/pci/bt87x.c
> 
> It uses, instead, readl/writel:
> 
>   static inline u32 snd_bt87x_readl(struct snd_bt87x *chip, u32 reg)
>   {
>   return readl(chip->mmio + reg);
>   }
> 
>   static inline void snd_bt87x_writel(struct snd_bt87x *chip, u32 reg, 
> u32 value)
>   {
>   writel(value, chip->mmio + reg);
>   }
> 
> I failed to see what makes it different from VIDEO_BT848 and
> DVB_BT8XX drivers. They all support exactly the same chipset:
> Brooktree/Conexant BT8xx. On those devices, depending on the exact
> model, up to three separate interfaces are provided, each one with
> its own Kconfig var:
> 
>   - audio I/O (SND_BT87X);
>   - video I/O (VIDEO_BT848);
>   - MPEG-TS I/O (DVB_BT8XX).
> 
> Thanks,
> Mauro

You're right, that makes no sense this doesn't seem to require
LEGACY_PCI. Maybe this was added by Arnd because it lacks a "depends on
PCI" which could have caused issues with HAVE_PCI=n rand configs.

Re: [PATCH v3 08/16] s390/pci: Remove races against pte updates

[Niklas Schnelle]

On 10/21/20 10:56 AM, Daniel Vetter wrote:
> Way back it was a reasonable assumptions that iomem mappings never
> change the pfn range they point at. But this has changed:
> 
> - gpu drivers dynamically manage their memory nowadays, invalidating
> ptes with unmap_mapping_range when buffers get moved
> 
> - contiguous dma allocations have moved from dedicated carvetouts to
> cma regions. This means if we miss the unmap the pfn might contain
> pagecache or anon memory (well anything allocated with GFP_MOVEABLE)
> 
> - even /dev/mem now invalidates mappings when the kernel requests that
> iomem region when CONFIG_IO_STRICT_DEVMEM is set, see commit
> 3234ac664a87 ("/dev/mem: Revoke mappings when a driver claims the
> region")
> 
> Accessing pfns obtained from ptes without holding all the locks is
> therefore no longer a good idea. Fix this.
> 
> Since zpci_memcpy_from|toio seems to not do anything nefarious with
> locks we just need to open code get_pfn and follow_pfn and make sure
> we drop the locks only after we're done. The write function also needs
> the copy_from_user move, since we can't take userspace faults while
> holding the mmap sem.
> 
> v2: Move VM_IO | VM_PFNMAP checks around so they keep returning EINVAL
> like before (Gerard)
> 
> v3: Polish commit message (Niklas)
> 
> Reviewed-by: Gerald Schaefer 
> Signed-off-by: Daniel Vetter 
> Cc: Jason Gunthorpe 
> Cc: Dan Williams 
> Cc: Kees Cook 
> Cc: Andrew Morton 
> Cc: John Hubbard 
> Cc: Jérôme Glisse 
> Cc: Jan Kara 
> Cc: linux...@kvack.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: Niklas Schnelle 
> Cc: Gerald Schaefer 
> Cc: linux-s...@vger.kernel.org
> Cc: Niklas Schnelle 
> Signed-off-by: Daniel Vetter 
   
This should be ".ch", but since this is clearly a typo and you also send from 
@ffwll.ch,
I took the liberty and fixed it for this commit and applied your patch to our 
internal
branch, Heiko or Vasily will then pick it up for the s390 tree.

Thanks!

> ---
>  arch/s390/pci/pci_mmio.c | 98 +++-
>  1 file changed, 57 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
> index 401cf670a243..1a6adbc68ee8 100644
> --- a/arch/s390/pci/pci_mmio.c
> +++ b/arch/s390/pci/pci_mmio.c
> @@ -119,33 +119,15 @@ static inline int __memcpy_toio_inuser(void __iomem 
> *dst,
>   return rc;
>  }
>  
> -static long get_pfn(unsigned long user_addr, unsigned long access,
> - unsigned long *pfn)
> -{
> - struct vm_area_struct *vma;
> - long ret;
> -
> - mmap_read_lock(current->mm);
> - ret = -EINVAL;
> - vma = find_vma(current->mm, user_addr);
> - if (!vma)
> - goto out;
> - ret = -EACCES;
> - if (!(vma->vm_flags & access))
> - goto out;
> - ret = follow_pfn(vma, user_addr, pfn);
> -out:
> - mmap_read_unlock(current->mm);
> - return ret;
> -}
> -
>  SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
>   const void __user *, user_buffer, size_t, length)
>  {
>   u8 local_buf[64];
>   void __iomem *io_addr;
>   void *buf;
> - unsigned long pfn;
> + struct vm_area_struct *vma;
> + pte_t *ptep;
> + spinlock_t *ptl;
>   long ret;
>  
>   if (!zpci_is_enabled())
> @@ -158,7 +140,7 @@ SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, 
> mmio_addr,
>* We only support write access to MIO capable devices if we are on
>* a MIO enabled system. Otherwise we would have to check for every
>* address if it is a special ZPCI_ADDR and would have to do
> -  * a get_pfn() which we don't need for MIO capable devices.  Currently
> +  * a pfn lookup which we don't need for MIO capable devices.  Currently
>* ISM devices are the only devices without MIO support and there is no
>* known need for accessing these from userspace.
>*/
> @@ -176,21 +158,37 @@ SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, 
> mmio_addr,
>   } else
>   buf = local_buf;
>  
> - ret = get_pfn(mmio_addr, VM_WRITE, &pfn);
> + ret = -EFAULT;
> + if (copy_from_user(buf, user_buffer, length))
> + goto out_free;
> +
> + mmap_read_lock(current->mm);
> + ret = -EINVAL;
> + vma = find_vma(current->mm, mmio_addr);
> + if (!vma)
> + goto out_unlock_mmap;
> + if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))

Re: [PATCH v2 08/17] s390/pci: Remove races against pte updates

[Niklas Schnelle]

Hi Daniel,

friendly ping. I haven't seen a new version of this patch series,
as I said I think your change for s390/pci is generally useful so
I'm curious, are you planning on sending a new version soon?
If you want you can also just sent this patch with the last few
nitpicks (primarily the mail address) fixed and I'll happily apply.

Best regards,
Niklas Schnelle

On 10/12/20 4:19 PM, Daniel Vetter wrote:
> On Mon, Oct 12, 2020 at 04:03:28PM +0200, Niklas Schnelle wrote:
... snip 
>>> Cc: Jason Gunthorpe 
>>> Cc: Dan Williams 
>>> Cc: Kees Cook 
>>> Cc: Andrew Morton 
>>> Cc: John Hubbard 
>>> Cc: Jérôme Glisse 
>>> Cc: Jan Kara 
>>> Cc: Dan Williams 
>>
>> The above Cc: line for Dan Williams is a duplicate
>>
>>> Cc: linux...@kvack.org
>>> Cc: linux-arm-ker...@lists.infradead.org
>>> Cc: linux-samsung-...@vger.kernel.org
>>> Cc: linux-me...@vger.kernel.org
>>> Cc: Niklas Schnelle 
>>> Cc: Gerald Schaefer 
>>> Cc: linux-s...@vger.kernel.org
>>> --
>>> v2: Move VM_IO | VM_PFNMAP checks around so they keep returning EINVAL
>>> like before (Gerard)
>>
>> I think the above should go before the CC/Signed-off/Reviewev block.
> 
> This is a per-subsystem bikeshed :-) drivers/gpu definitely wants it
> above, but most core subsystems want it below. I'll move it.
> 
>>> ---
>>>  arch/s390/pci/pci_mmio.c | 98 +++-
>>>  1 file changed, 57 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
>>> index 401cf670a243..1a6adbc68ee8 100644
>>> --- a/arch/s390/pci/pci_mmio.c
>>> +++ b/arch/s390/pci/pci_mmio.c
>>> @@ -119,33 +119,15 @@ static inline int __memcpy_toio_inuser(void __iomem 
>>> *dst,
>>> return rc;
>>>  }
>>>  
>>> -static long get_pfn(unsigned long user_addr, unsigned long access,
>>> -   unsigned long *pfn)
>>> -{
>>> -   struct vm_area_struct *vma;
>>> -   long ret;
>>> -
>>> -   mmap_read_lock(current->mm);
>>> -   ret = -EINVAL;
>>> -   vma = find_vma(current->mm, user_addr);
>>> -   if (!vma)
>>> -   goto out;
>>> -   ret = -EACCES;
>>> -   if (!(vma->vm_flags & access))
>>> -   goto out;
>>> -   ret = follow_pfn(vma, user_addr, pfn);
>>> -out:
>>> -   mmap_read_unlock(current->mm);
>>> -   return ret;
>>> -}
>>> -
>>>  SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
>>> const void __user *, user_buffer, size_t, length)
>>>  {
>>> u8 local_buf[64];
>>> void __iomem *io_addr;
>>> void *buf;
>>> -   unsigned long pfn;
>>> +   struct vm_area_struct *vma;
>>> +   pte_t *ptep;
>>> +   spinlock_t *ptl;
>>
>> With checkpatch.pl --strict the above yields a complained
>> "CHECK: spinlock_t definition without comment" but I think
>> that's really okay since your commit description is very clear.
>> Same oin line 277.
> 
> I think this is a falls positive, checkpatch doesn't realize that
> SYSCALL_DEFINE3 is a function, not a structure. And in a structure I'd
> have added the kerneldoc or comment.
> 
> I'll fix up all the nits you've found for the next round. Thanks for
> taking a look.
> -Daniel
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 08/17] s390/pci: Remove races against pte updates

[Niklas Schnelle]

Hi Daniel,

freshly back from my vacation I've just taken a look at your patch.
First thanks for this fix and the detailed commit description.
Definitely makes sense to fix this and you can add my

Acked-by: Niklas Schnelle 

Content wise it all looks sane and clear and since Gerald did the testing,
I would have applied it to our tree already, but I got some trivial
checkpatch violations that probably apply to the whole series.
I've commented them inline below.
If you confirm there I can do the fixups when applying or you can resend.

On 10/9/20 9:59 AM, Daniel Vetter wrote:
> Way back it was a reasonable assumptions that iomem mappings never
> change the pfn range they point at. But this has changed:
> 
> - gpu drivers dynamically manage their memory nowadays, invalidating
> ptes with unmap_mapping_range when buffers get moved
> 
> - contiguous dma allocations have moved from dedicated carvetouts to
> cma regions. This means if we miss the unmap the pfn might contain
> pagecache or anon memory (well anything allocated with GFP_MOVEABLE)
> 
> - even /dev/mem now invalidates mappings when the kernel requests that
> iomem region when CONFIG_IO_STRICT_DEVMEM is set, see 3234ac664a87

The above commit mention should use the format
'commit 3234ac664a87 ("/dev/mem: Revoke mappings when a driver claims the 
region")'
otherwise this results in a checkpatch ERROR.

> ("/dev/mem: Revoke mappings when a driver claims the region")
> 
> Accessing pfns obtained from ptes without holding all the locks is
> therefore no longer a good idea. Fix this.
> 
> Since zpci_memcpy_from|toio seems to not do anything nefarious with
> locks we just need to open code get_pfn and follow_pfn and make sure
> we drop the locks only after we've done. The write function also needs

just a typo but just saw it "we're" instead of "we've"

> the copy_from_user move, since we can't take userspace faults while
> holding the mmap sem.
> 
> Reviewed-by: Gerald Schaefer 
> 
No empty line after the Revied-by tag.

> Signed-off-by: Daniel Vetter 

Your Signed-off-by mail address does not match the one you're sending from,
this yields a checkpatch warning when using git am with your mail.
This is probably just a silly misconfiguration but since Signed-offs
are signatures should I change this to 
"Daniel Vetter " which is the one you're
sending from and also in the MAINTAINERS file?


> Cc: Jason Gunthorpe 
> Cc: Dan Williams 
> Cc: Kees Cook 
> Cc: Andrew Morton 
> Cc: John Hubbard 
> Cc: Jérôme Glisse 
> Cc: Jan Kara 
> Cc: Dan Williams 

The above Cc: line for Dan Williams is a duplicate

> Cc: linux...@kvack.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: Niklas Schnelle 
> Cc: Gerald Schaefer 
> Cc: linux-s...@vger.kernel.org
> --
> v2: Move VM_IO | VM_PFNMAP checks around so they keep returning EINVAL
> like before (Gerard)

I think the above should go before the CC/Signed-off/Reviewev block.

> ---
>  arch/s390/pci/pci_mmio.c | 98 +++-
>  1 file changed, 57 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
> index 401cf670a243..1a6adbc68ee8 100644
> --- a/arch/s390/pci/pci_mmio.c
> +++ b/arch/s390/pci/pci_mmio.c
> @@ -119,33 +119,15 @@ static inline int __memcpy_toio_inuser(void __iomem 
> *dst,
>   return rc;
>  }
>  
> -static long get_pfn(unsigned long user_addr, unsigned long access,
> - unsigned long *pfn)
> -{
> - struct vm_area_struct *vma;
> - long ret;
> -
> - mmap_read_lock(current->mm);
> - ret = -EINVAL;
> - vma = find_vma(current->mm, user_addr);
> - if (!vma)
> - goto out;
> - ret = -EACCES;
> - if (!(vma->vm_flags & access))
> - goto out;
> - ret = follow_pfn(vma, user_addr, pfn);
> -out:
> - mmap_read_unlock(current->mm);
> - return ret;
> -}
> -
>  SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
>   const void __user *, user_buffer, size_t, length)
>  {
>   u8 local_buf[64];
>   void __iomem *io_addr;
>   void *buf;
> - unsigned long pfn;
> + struct vm_area_struct *vma;
> + pte_t *ptep;
> + spinlock_t *ptl;

With checkpatch.pl --strict the above yields a complained
"CHECK: spinlock_t definition without comment" but I think
that's really okay since your commit description is very clear.
Same oin line 277.

... snip ...
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 08/17] s390/pci: Remove races against pte updates

[Niklas Schnelle]

... snip ...
>>> Cc: linux-me...@vger.kernel.org
>>> Cc: Niklas Schnelle 
>>> Cc: Gerald Schaefer 
>>> Cc: linux-s...@vger.kernel.org
>>> --
>>> v2: Move VM_IO | VM_PFNMAP checks around so they keep returning EINVAL
>>> like before (Gerard)
>>
>> I think the above should go before the CC/Signed-off/Reviewev block.
> 
> This is a per-subsystem bikeshed :-) drivers/gpu definitely wants it
> above, but most core subsystems want it below. I'll move it.

Today I learned, thanks! That said I think most of the time I've
actually not seen version change information in the commit message itself
only in the cover letters. I really don't care just looked odd to me.

> 
>>> ---
>>>  arch/s390/pci/pci_mmio.c | 98 +++-
>>>  1 file changed, 57 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
>>> index 401cf670a243..1a6adbc68ee8 100644
>>> --- a/arch/s390/pci/pci_mmio.c
>>> +++ b/arch/s390/pci/pci_mmio.c
>>> @@ -119,33 +119,15 @@ static inline int __memcpy_toio_inuser(void __iomem 
>>> *dst,
>>> return rc;
>>>  }
>>>  
>>> -static long get_pfn(unsigned long user_addr, unsigned long access,
>>> -   unsigned long *pfn)
>>> -{
>>> -   struct vm_area_struct *vma;
>>> -   long ret;
>>> -
>>> -   mmap_read_lock(current->mm);
>>> -   ret = -EINVAL;
>>> -   vma = find_vma(current->mm, user_addr);
>>> -   if (!vma)
>>> -   goto out;
>>> -   ret = -EACCES;
>>> -   if (!(vma->vm_flags & access))
>>> -   goto out;
>>> -   ret = follow_pfn(vma, user_addr, pfn);
>>> -out:
>>> -   mmap_read_unlock(current->mm);
>>> -   return ret;
>>> -}
>>> -
>>>  SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
>>> const void __user *, user_buffer, size_t, length)
>>>  {
>>> u8 local_buf[64];
>>> void __iomem *io_addr;
>>> void *buf;
>>> -   unsigned long pfn;
>>> +   struct vm_area_struct *vma;
>>> +   pte_t *ptep;
>>> +   spinlock_t *ptl;
>>
>> With checkpatch.pl --strict the above yields a complained
>> "CHECK: spinlock_t definition without comment" but I think
>> that's really okay since your commit description is very clear.
>> Same oin line 277.
> 
> I think this is a falls positive, checkpatch doesn't realize that
> SYSCALL_DEFINE3 is a function, not a structure. And in a structure I'd
> have added the kerneldoc or comment.

Interesting, your theory sounds convincing, I too thought this
was a bit too pedantic.

> 
> I'll fix up all the nits you've found for the next round. Thanks for
> taking a look.

You're welcome hope I didn't sound pedantic. I think you've a lot
more experience actually and this can indeed turn into bikeshedding
but since I was answering anyway and most of this was checkpatch…

> -Daniel
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel