Hi Greg, Jiri, Ilpo,
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].
The patch was previously acked[3] by Greg but given this was almost
a year ago and didn't apply then I didn't carry the Ack over. That said
I don't think there were non trivial changes.
Thanks,
Niklas
[0] https://lore.kernel.org/all/[email protected]/
[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_-8g@mail.gmail.com/
[3] https://lore.kernel.org/all/2023053050-prodigal-shine-4d1c@gregkh/
Niklas Schnelle (1):
tty: serial: handle HAS_IOPORT dependencies
drivers/tty/Kconfig | 4 +--
drivers/tty/serial/8250/8250_early.c | 4 +++
drivers/tty/serial/8250/8250_pci.c | 14 ++++++++++
drivers/tty/serial/8250/8250_port.c | 42 +++++++++++++++++++++++-----
drivers/tty/serial/8250/Kconfig | 7 ++---
drivers/tty/serial/Kconfig | 2 +-
6 files changed, 59 insertions(+), 14 deletions(-)
--
2.40.1
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. For 8250 based drivers some support
MMIO only use so fence only the parts requiring I/O ports.
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
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.
Note 2: This was previously acked here:
https://lore.kernel.org/all/2023053050-prodigal-shine-4d1c@gregkh/
Given this was almost a year ago and didn't apply then I didn't
carry the Ack over though.
drivers/tty/Kconfig | 4 +--
drivers/tty/serial/8250/8250_early.c | 4 +++
drivers/tty/serial/8250/8250_pci.c | 14 ++++++++++
drivers/tty/serial/8250/8250_port.c | 42 +++++++++++++++++++++++-----
drivers/tty/serial/8250/Kconfig | 7 ++---
drivers/tty/serial/Kconfig | 2 +-
6 files changed, 59 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index a45d423ad10f..63a494d36a1f 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 e3f482fd3de4..8f9aec2e7c7d 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 0d35c77fad9e..38ac5236d2ea 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -928,6 +928,7 @@ static int pci_netmos_init(struct pci_dev *dev)
return num_serial;
}
+#ifdef CONFIG_HAS_IOPORT
/*
* These chips are available with optionally one parallel port and up to
* two serial ports. Unfortunately they all have the same product id.
@@ -1054,6 +1055,7 @@ static void pci_ite887x_exit(struct pci_dev *dev)
ioport &= 0xffff;
release_region(ioport, ITE_887x_IOSIZE);
}
+#endif /* CONFIG_HAS_IOPORT */
/*
* Oxford Semiconductor Inc.
@@ -1328,6 +1330,7 @@ static int pci_oxsemi_tornado_setup(struct serial_private *priv,
#define QOPR_CLOCK_X8 0x0003
#define QOPR_CLOCK_RATE_MASK 0x0003
+#ifdef CONFIG_HAS_IOPORT
/* Quatech devices have their own extra interface features */
static struct pci_device_id quatech_cards[] = {
{ PCI_DEVICE_DATA(QUATECH, QSC100, 1) },
@@ -1547,6 +1550,7 @@ static int pci_quatech_setup(struct serial_private *priv,
pci_warn(priv->dev, "software control of RS422 features not currently supported.\n");
return pci_default_setup(priv, board, port, idx);
}
+#endif /* CONFIG_HAS_IOPORT */
static int pci_default_setup(struct serial_private *priv,
const struct pciserial_board *board,
@@ -1826,6 +1830,7 @@ static int skip_tx_en_setup(struct serial_private *priv,
return pci_default_setup(priv, board, port, idx);
}
+#ifdef CONFIG_HAS_IOPORT
static void kt_handle_break(struct uart_port *p)
{
struct uart_8250_port *up = up_to_u8250p(p);
@@ -1869,6 +1874,7 @@ static int kt_serial_setup(struct serial_private *priv,
port->port.handle_break = kt_handle_break;
return skip_tx_en_setup(priv, board, port, idx);
}
+#endif /* CONFIG_HAS_IOPORT */
static int pci_eg20t_init(struct pci_dev *dev)
{
@@ -1913,6 +1919,7 @@ pci_wch_ch38x_setup(struct serial_private *priv,
#define CH384_XINT_ENABLE_REG 0xEB
#define CH384_XINT_ENABLE_BIT 0x02
+#ifdef CONFIG_HAS_IOPORT
static int pci_wch_ch38x_init(struct pci_dev *dev)
{
int max_port;
@@ -1940,6 +1947,7 @@ static void pci_wch_ch38x_exit(struct pci_dev *dev)
iobase = pci_resource_start(dev, 0);
outb(0x0, iobase + CH384_XINT_ENABLE_REG);
}
+#endif /* CONFIG_HAS_IOPORT */
static int
@@ -2171,6 +2179,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
.subdevice = PCI_ANY_ID,
.setup = ce4100_serial_setup,
},
+#ifdef CONFIG_HAS_IOPORT
{
.vendor = PCI_VENDOR_ID_INTEL,
.device = PCI_DEVICE_ID_INTEL_PATSBURG_KT,
@@ -2190,6 +2199,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
.setup = pci_default_setup,
.exit = pci_ite887x_exit,
},
+#endif
/*
* National Instruments
*/
@@ -2311,6 +2321,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
.exit = pci_ni8430_exit,
},
/* Quatech */
+#ifdef CONFIG_HAS_IOPORT
{
.vendor = PCI_VENDOR_ID_QUATECH,
.device = PCI_ANY_ID,
@@ -2319,6 +2330,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
.init = pci_quatech_init,
.setup = pci_quatech_setup,
},
+#endif
/*
* Panacom
*/
@@ -2836,6 +2848,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
.subdevice = PCI_ANY_ID,
.setup = pci_wch_ch38x_setup,
},
+#ifdef CONFIG_HAS_IOPORT
/* WCH CH384 8S card (16850 clone) */
{
.vendor = PCIE_VENDOR_ID_WCH,
@@ -2846,6 +2859,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
.exit = pci_wch_ch38x_exit,
.setup = pci_wch_ch38x_setup,
},
+#endif
/*
* Broadcom TruManage (NetXtreme)
*/
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index fc9dd5d45295..1c5e39c1a469 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -338,6 +338,7 @@ static void default_serial_dl_write(struct uart_8250_port *up, u32 value)
serial_out(up, UART_DLM, value >> 8 & 0xff);
}
+#ifdef CONFIG_HAS_IOPORT
static unsigned int hub6_serial_in(struct uart_port *p, int offset)
{
offset = offset << p->regshift;
@@ -351,6 +352,7 @@ static void hub6_serial_out(struct uart_port *p, int offset, int value)
outb(p->hub6 - 1 + offset, p->iobase);
outb(value, p->iobase + 1);
}
+#endif /* CONFIG_HAS_IOPORT */
static unsigned int mem_serial_in(struct uart_port *p, int offset)
{
@@ -400,6 +402,7 @@ static unsigned int mem32be_serial_in(struct uart_port *p, int offset)
return ioread32be(p->membase + offset);
}
+#ifdef CONFIG_HAS_IOPORT
static unsigned int io_serial_in(struct uart_port *p, int offset)
{
offset = offset << p->regshift;
@@ -411,6 +414,24 @@ static void io_serial_out(struct uart_port *p, int offset, int value)
offset = offset << p->regshift;
outb(value, p->iobase + offset);
}
+#endif
+static unsigned int no_serial_in(struct uart_port *p, int offset)
+{
+ return (unsigned int)-1;
+}
+
+static void no_serial_out(struct uart_port *p, int offset, int value)
+{
+}
+
+#ifdef CONFIG_HAS_IOPORT
+static inline bool is_upf_fourport(struct uart_port *port)
+{
+ return port->flags & UPF_FOURPORT;
+}
+#else
+#define is_upf_fourport(x) false
+#endif
static int serial8250_default_handle_irq(struct uart_port *port);
@@ -422,10 +443,12 @@ static void set_io_from_upio(struct uart_port *p)
up->dl_write = default_serial_dl_write;
switch (p->iotype) {
+#ifdef CONFIG_HAS_IOPORT
case UPIO_HUB6:
p->serial_in = hub6_serial_in;
p->serial_out = hub6_serial_out;
break;
+#endif
case UPIO_MEM:
p->serial_in = mem_serial_in;
@@ -446,11 +469,16 @@ static void set_io_from_upio(struct uart_port *p)
p->serial_in = mem32be_serial_in;
p->serial_out = mem32be_serial_out;
break;
-
- default:
+#ifdef CONFIG_HAS_IOPORT
+ case UPIO_PORT:
p->serial_in = io_serial_in;
p->serial_out = io_serial_out;
break;
+#endif
+ default:
+ WARN(1, "Unsupported UART type %x\n", p->iotype);
+ p->serial_in = no_serial_in;
+ p->serial_out = no_serial_out;
}
/* Remember loaded iotype */
up->cur_iotype = p->iotype;
@@ -1318,7 +1346,7 @@ static void autoconfig_irq(struct uart_8250_port *up)
unsigned long irqs;
int irq;
- if (port->flags & UPF_FOURPORT) {
+ if (is_upf_fourport(port)) {
ICP = (port->iobase & 0xfe0) | 0x1f;
save_ICP = inb_p(ICP);
outb_p(0x80, ICP);
@@ -1337,7 +1365,7 @@ static void autoconfig_irq(struct uart_8250_port *up)
irqs = probe_irq_on();
serial8250_out_MCR(up, 0);
udelay(10);
- if (port->flags & UPF_FOURPORT) {
+ if (is_upf_fourport(port)) {
serial8250_out_MCR(up, UART_MCR_DTR | UART_MCR_RTS);
} else {
serial8250_out_MCR(up,
@@ -1361,7 +1389,7 @@ static void autoconfig_irq(struct uart_8250_port *up)
serial_out(up, UART_IER, save_ier);
uart_port_unlock_irq(port);
- if (port->flags & UPF_FOURPORT)
+ if (is_upf_fourport(port))
outb_p(save_ICP, ICP);
port->irq = (irq > 0) ? irq : 0;
@@ -2438,7 +2466,7 @@ int serial8250_do_startup(struct uart_port *port)
*/
up->ier = UART_IER_RLSI | UART_IER_RDI;
- if (port->flags & UPF_FOURPORT) {
+ if (is_upf_fourport(port)) {
unsigned int icp;
/*
* Enable interrupts on the AST Fourport board
@@ -2483,7 +2511,7 @@ void serial8250_do_shutdown(struct uart_port *port)
serial8250_release_dma(up);
uart_port_lock_irqsave(port, &flags);
- if (port->flags & UPF_FOURPORT) {
+ if (is_upf_fourport(port)) {
/* reset interrupts on the AST Fourport board */
inb((port->iobase & 0xfe0) | 0x1f);
port->mctrl |= TIOCM_OUT1;
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 47ff50763c04..54bf98869abf 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
@@ -72,7 +71,7 @@ config SERIAL_8250_16550A_VARIANTS
config SERIAL_8250_FINTEK
bool "Support for Fintek variants"
- depends on SERIAL_8250
+ depends on SERIAL_8250 && HAS_IOPORT
help
Selecting this option will add support for the RS232 and RS485
capabilities of the Fintek F81216A LPC to 4 UART as well similar
@@ -136,7 +135,7 @@ config SERIAL_8250_PCILIB
config SERIAL_8250_PCI
tristate "8250/16550 PCI device support"
- depends on SERIAL_8250 && PCI
+ depends on SERIAL_8250 && PCI && HAS_IOPORT
select SERIAL_8250_PCILIB
default SERIAL_8250
help
@@ -163,7 +162,7 @@ config SERIAL_8250_HP300
config SERIAL_8250_CS
tristate "8250/16550 PCMCIA device support"
- depends on PCMCIA && SERIAL_8250
+ depends on PCMCIA && SERIAL_8250 && HAS_IOPORT
help
Say Y here to enable support for 16-bit PCMCIA serial devices,
including serial port cards, modems, and the modem functions of
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index ffcf4882b25f..92c85e805c2a 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -874,7 +874,7 @@ config SERIAL_TXX9_STDSERIAL
config SERIAL_JSM
tristate "Digi International NEO and Classic PCI Support"
- depends on PCI
+ depends on PCI && HAS_IOPORT
select SERIAL_CORE
help
This is a driver for Digi International's Neo and Classic series
--
2.40.1
On Fri, Apr 05, 2024 at 05:29:23PM +0200, Niklas Schnelle wrote:
> Hi Greg, Jiri, Ilpo,
>
> 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].
>
> The patch was previously acked[3] by Greg but given this was almost
> a year ago and didn't apply then I didn't carry the Ack over. That said
> I don't think there were non trivial changes.
Hmm... Can those drivers simply be converted to use ioreadXX/iowriteXX
instead?
--
With Best Regards,
Andy Shevchenko
On Sat, Apr 6, 2024, at 00:33, Andy Shevchenko wrote:
> On Fri, Apr 05, 2024 at 05:29:23PM +0200, Niklas Schnelle wrote:
>> Hi Greg, Jiri, Ilpo,
>>
>> 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].
>>
>> The patch was previously acked[3] by Greg but given this was almost
>> a year ago and didn't apply then I didn't carry the Ack over. That said
>> I don't think there were non trivial changes.
>
> Hmm... Can those drivers simply be converted to use ioreadXX/iowriteXX
> instead?
Not 8250, for a couple of reasons:
- the irq autoconfig code uses outb_p(), which has no iowrite()
equivalent
- the driver is used on machines that cannot implement
ioport_map() because of the nonlinear address translation,
e.g. certain early alpha and mips machines.
- it still needs its own I/O abstraction layer to deal with
different-sized registers, so the result would not be any
more readable even without the other issues.
Arnd
On Fri, 5 Apr 2024, 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. For 8250 based drivers some support
> MMIO only use so fence only the parts requiring I/O ports.
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
> ---
> 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.
>
> Note 2: This was previously acked here:
> https://lore.kernel.org/all/2023053050-prodigal-shine-4d1c@gregkh/
> Given this was almost a year ago and didn't apply then I didn't
> carry the Ack over though.
>
> drivers/tty/Kconfig | 4 +--
> drivers/tty/serial/8250/8250_early.c | 4 +++
> drivers/tty/serial/8250/8250_pci.c | 14 ++++++++++
> drivers/tty/serial/8250/8250_port.c | 42 +++++++++++++++++++++++-----
> drivers/tty/serial/8250/Kconfig | 7 ++---
> drivers/tty/serial/Kconfig | 2 +-
> 6 files changed, 59 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
> index a45d423ad10f..63a494d36a1f 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 e3f482fd3de4..8f9aec2e7c7d 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 0d35c77fad9e..38ac5236d2ea 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -928,6 +928,7 @@ static int pci_netmos_init(struct pci_dev *dev)
> return num_serial;
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> /*
> * These chips are available with optionally one parallel port and up to
> * two serial ports. Unfortunately they all have the same product id.
> @@ -1054,6 +1055,7 @@ static void pci_ite887x_exit(struct pci_dev *dev)
> ioport &= 0xffff;
> release_region(ioport, ITE_887x_IOSIZE);
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> /*
> * Oxford Semiconductor Inc.
> @@ -1328,6 +1330,7 @@ static int pci_oxsemi_tornado_setup(struct serial_private *priv,
> #define QOPR_CLOCK_X8 0x0003
> #define QOPR_CLOCK_RATE_MASK 0x0003
>
> +#ifdef CONFIG_HAS_IOPORT
> /* Quatech devices have their own extra interface features */
> static struct pci_device_id quatech_cards[] = {
> { PCI_DEVICE_DATA(QUATECH, QSC100, 1) },
> @@ -1547,6 +1550,7 @@ static int pci_quatech_setup(struct serial_private *priv,
> pci_warn(priv->dev, "software control of RS422 features not currently supported.\n");
> return pci_default_setup(priv, board, port, idx);
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static int pci_default_setup(struct serial_private *priv,
> const struct pciserial_board *board,
> @@ -1826,6 +1830,7 @@ static int skip_tx_en_setup(struct serial_private *priv,
> return pci_default_setup(priv, board, port, idx);
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> static void kt_handle_break(struct uart_port *p)
> {
> struct uart_8250_port *up = up_to_u8250p(p);
> @@ -1869,6 +1874,7 @@ static int kt_serial_setup(struct serial_private *priv,
> port->port.handle_break = kt_handle_break;
> return skip_tx_en_setup(priv, board, port, idx);
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static int pci_eg20t_init(struct pci_dev *dev)
> {
> @@ -1913,6 +1919,7 @@ pci_wch_ch38x_setup(struct serial_private *priv,
> #define CH384_XINT_ENABLE_REG 0xEB
> #define CH384_XINT_ENABLE_BIT 0x02
>
> +#ifdef CONFIG_HAS_IOPORT
> static int pci_wch_ch38x_init(struct pci_dev *dev)
> {
> int max_port;
> @@ -1940,6 +1947,7 @@ static void pci_wch_ch38x_exit(struct pci_dev *dev)
> iobase = pci_resource_start(dev, 0);
> outb(0x0, iobase + CH384_XINT_ENABLE_REG);
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
>
> static int
> @@ -2171,6 +2179,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
> .subdevice = PCI_ANY_ID,
> .setup = ce4100_serial_setup,
> },
> +#ifdef CONFIG_HAS_IOPORT
> {
> .vendor = PCI_VENDOR_ID_INTEL,
> .device = PCI_DEVICE_ID_INTEL_PATSBURG_KT,
> @@ -2190,6 +2199,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
> .setup = pci_default_setup,
> .exit = pci_ite887x_exit,
> },
> +#endif
> /*
> * National Instruments
> */
> @@ -2311,6 +2321,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
> .exit = pci_ni8430_exit,
> },
> /* Quatech */
> +#ifdef CONFIG_HAS_IOPORT
> {
> .vendor = PCI_VENDOR_ID_QUATECH,
> .device = PCI_ANY_ID,
> @@ -2319,6 +2330,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
> .init = pci_quatech_init,
> .setup = pci_quatech_setup,
> },
> +#endif
> /*
> * Panacom
> */
> @@ -2836,6 +2848,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
> .subdevice = PCI_ANY_ID,
> .setup = pci_wch_ch38x_setup,
> },
> +#ifdef CONFIG_HAS_IOPORT
> /* WCH CH384 8S card (16850 clone) */
> {
> .vendor = PCIE_VENDOR_ID_WCH,
> @@ -2846,6 +2859,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
> .exit = pci_wch_ch38x_exit,
> .setup = pci_wch_ch38x_setup,
> },
> +#endif
> /*
> * Broadcom TruManage (NetXtreme)
> */
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index fc9dd5d45295..1c5e39c1a469 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -338,6 +338,7 @@ static void default_serial_dl_write(struct uart_8250_port *up, u32 value)
> serial_out(up, UART_DLM, value >> 8 & 0xff);
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> static unsigned int hub6_serial_in(struct uart_port *p, int offset)
> {
> offset = offset << p->regshift;
> @@ -351,6 +352,7 @@ static void hub6_serial_out(struct uart_port *p, int offset, int value)
> outb(p->hub6 - 1 + offset, p->iobase);
> outb(value, p->iobase + 1);
> }
> +#endif /* CONFIG_HAS_IOPORT */
>
> static unsigned int mem_serial_in(struct uart_port *p, int offset)
> {
> @@ -400,6 +402,7 @@ static unsigned int mem32be_serial_in(struct uart_port *p, int offset)
> return ioread32be(p->membase + offset);
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> static unsigned int io_serial_in(struct uart_port *p, int offset)
> {
> offset = offset << p->regshift;
> @@ -411,6 +414,24 @@ static void io_serial_out(struct uart_port *p, int offset, int value)
> offset = offset << p->regshift;
> outb(value, p->iobase + offset);
> }
> +#endif
> +static unsigned int no_serial_in(struct uart_port *p, int offset)
> +{
> + return (unsigned int)-1;
> +}
> +
> +static void no_serial_out(struct uart_port *p, int offset, int value)
> +{
> +}
> +
> +#ifdef CONFIG_HAS_IOPORT
> +static inline bool is_upf_fourport(struct uart_port *port)
> +{
> + return port->flags & UPF_FOURPORT;
> +}
> +#else
> +#define is_upf_fourport(x) false
> +#endif
>
> static int serial8250_default_handle_irq(struct uart_port *port);
>
> @@ -422,10 +443,12 @@ static void set_io_from_upio(struct uart_port *p)
> up->dl_write = default_serial_dl_write;
>
> switch (p->iotype) {
> +#ifdef CONFIG_HAS_IOPORT
> case UPIO_HUB6:
> p->serial_in = hub6_serial_in;
> p->serial_out = hub6_serial_out;
> break;
> +#endif
>
> case UPIO_MEM:
> p->serial_in = mem_serial_in;
> @@ -446,11 +469,16 @@ static void set_io_from_upio(struct uart_port *p)
> p->serial_in = mem32be_serial_in;
> p->serial_out = mem32be_serial_out;
> break;
> -
> - default:
> +#ifdef CONFIG_HAS_IOPORT
> + case UPIO_PORT:
> p->serial_in = io_serial_in;
> p->serial_out = io_serial_out;
> break;
> +#endif
> + default:
> + WARN(1, "Unsupported UART type %x\n", p->iotype);
> + p->serial_in = no_serial_in;
> + p->serial_out = no_serial_out;
> }
> /* Remember loaded iotype */
> up->cur_iotype = p->iotype;
> @@ -1318,7 +1346,7 @@ static void autoconfig_irq(struct uart_8250_port *up)
> unsigned long irqs;
> int irq;
>
> - if (port->flags & UPF_FOURPORT) {
> + if (is_upf_fourport(port)) {
> ICP = (port->iobase & 0xfe0) | 0x1f;
> save_ICP = inb_p(ICP);
> outb_p(0x80, ICP);
> @@ -1337,7 +1365,7 @@ static void autoconfig_irq(struct uart_8250_port *up)
> irqs = probe_irq_on();
> serial8250_out_MCR(up, 0);
> udelay(10);
> - if (port->flags & UPF_FOURPORT) {
> + if (is_upf_fourport(port)) {
> serial8250_out_MCR(up, UART_MCR_DTR | UART_MCR_RTS);
> } else {
> serial8250_out_MCR(up,
> @@ -1361,7 +1389,7 @@ static void autoconfig_irq(struct uart_8250_port *up)
> serial_out(up, UART_IER, save_ier);
> uart_port_unlock_irq(port);
>
> - if (port->flags & UPF_FOURPORT)
> + if (is_upf_fourport(port))
> outb_p(save_ICP, ICP);
>
> port->irq = (irq > 0) ? irq : 0;
> @@ -2438,7 +2466,7 @@ int serial8250_do_startup(struct uart_port *port)
> */
> up->ier = UART_IER_RLSI | UART_IER_RDI;
>
> - if (port->flags & UPF_FOURPORT) {
> + if (is_upf_fourport(port)) {
> unsigned int icp;
> /*
> * Enable interrupts on the AST Fourport board
> @@ -2483,7 +2511,7 @@ void serial8250_do_shutdown(struct uart_port *port)
> serial8250_release_dma(up);
>
> uart_port_lock_irqsave(port, &flags);
> - if (port->flags & UPF_FOURPORT) {
> + if (is_upf_fourport(port)) {
> /* reset interrupts on the AST Fourport board */
> inb((port->iobase & 0xfe0) | 0x1f);
> port->mctrl |= TIOCM_OUT1;
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 47ff50763c04..54bf98869abf 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
Why? Your changelogs gives zero insight on this change.
> select SERIAL_CORE
> select SERIAL_MCTRL_GPIO if GPIOLIB
> help
> @@ -72,7 +71,7 @@ config SERIAL_8250_16550A_VARIANTS
>
> config SERIAL_8250_FINTEK
> bool "Support for Fintek variants"
> - depends on SERIAL_8250
> + depends on SERIAL_8250 && HAS_IOPORT
> help
> Selecting this option will add support for the RS232 and RS485
> capabilities of the Fintek F81216A LPC to 4 UART as well similar
> @@ -136,7 +135,7 @@ config SERIAL_8250_PCILIB
>
> config SERIAL_8250_PCI
> tristate "8250/16550 PCI device support"
> - depends on SERIAL_8250 && PCI
> + depends on SERIAL_8250 && PCI && HAS_IOPORT
> select SERIAL_8250_PCILIB
> default SERIAL_8250
> help
> @@ -163,7 +162,7 @@ config SERIAL_8250_HP300
>
> config SERIAL_8250_CS
> tristate "8250/16550 PCMCIA device support"
> - depends on PCMCIA && SERIAL_8250
> + depends on PCMCIA && SERIAL_8250 && HAS_IOPORT
> help
> Say Y here to enable support for 16-bit PCMCIA serial devices,
> including serial port cards, modems, and the modem functions of
What about drivers that use SERIAL8250_PORT()?
Also port provided in 8250_PNP might expect it I think.
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index ffcf4882b25f..92c85e805c2a 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -874,7 +874,7 @@ config SERIAL_TXX9_STDSERIAL
>
> config SERIAL_JSM
> tristate "Digi International NEO and Classic PCI Support"
> - depends on PCI
> + depends on PCI && HAS_IOPORT
> select SERIAL_CORE
> help
> This is a driver for Digi International's Neo and Classic series
>
--
i.
On Mon, Apr 8, 2024, at 11:54, Ilpo Järvinen wrote:
> On Fri, 5 Apr 2024, Niklas Schnelle wrote:
>> config SERIAL_8250_CS
>> tristate "8250/16550 PCMCIA device support"
>> - depends on PCMCIA && SERIAL_8250
>> + depends on PCMCIA && SERIAL_8250 && HAS_IOPORT
>> help
>> Say Y here to enable support for 16-bit PCMCIA serial devices,
>> including serial port cards, modems, and the modem functions of
>
> What about drivers that use SERIAL8250_PORT()?
It probably makes sense to hide these, since they won't ever
work. I probably missed them in my initial series because they
don't cause a compile-time error, but I agree that there is no
use in showing the options here.
> Also port provided in 8250_PNP might expect it I think.
I don't think these need any change: 8250_pnp.c supports
both IORESOURCE_IO and IORESOURCE_MEM based ports. It will
still create a 8250 port for the I/O based ones but they
will now correctly fail to probe in the main driver rather
than crashing the kernel. PNP devices that only use
memory BARs will keep working as before, on both machines
with and without CONFIG_HAS_IOPORT.
I think that most 8250_pnp variants are probably used only
with ISAPNP or PNPBIOS, neither of which exists without
HAS_IOPORT, but you could certainly have PNPACPI on arm
or riscv machines that don't have port I/O but come with
a memory-mapped 8250 port described by firmware.
Arnd
On Mon, 8 Apr 2024, Arnd Bergmann wrote:
> On Mon, Apr 8, 2024, at 11:54, Ilpo Järvinen wrote:
> > On Fri, 5 Apr 2024, Niklas Schnelle wrote:
>
> >> config SERIAL_8250_CS
> >> tristate "8250/16550 PCMCIA device support"
> >> - depends on PCMCIA && SERIAL_8250
> >> + depends on PCMCIA && SERIAL_8250 && HAS_IOPORT
> >> help
> >> Say Y here to enable support for 16-bit PCMCIA serial devices,
> >> including serial port cards, modems, and the modem functions of
> >
> > What about drivers that use SERIAL8250_PORT()?
>
> It probably makes sense to hide these, since they won't ever
> work. I probably missed them in my initial series because they
> don't cause a compile-time error, but I agree that there is no
> use in showing the options here.
>
> > Also port provided in 8250_PNP might expect it I think.
>
> I don't think these need any change: 8250_pnp.c supports
> both IORESOURCE_IO and IORESOURCE_MEM based ports. It will
> still create a 8250 port for the I/O based ones but they
> will now correctly fail to probe in the main driver rather
> than crashing the kernel. PNP devices that only use
> memory BARs will keep working as before, on both machines
> with and without CONFIG_HAS_IOPORT.
>
> I think that most 8250_pnp variants are probably used only
> with ISAPNP or PNPBIOS, neither of which exists without
> HAS_IOPORT,
Okay, seems fine then if that dependency is handled somewhere.
--
i.
> but you could certainly have PNPACPI on arm
> or riscv machines that don't have port I/O but come with
> a memory-mapped 8250 port described by firmware.
On Mon, 2024-04-08 at 12:54 +0300, Ilpo Järvinen wrote:
> On Fri, 5 Apr 2024, 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. For 8250 based drivers some support
> > MMIO only use so fence only the parts requiring I/O ports.
> >
> > Co-developed-by: Arnd Bergmann <[email protected]>
> > Signed-off-by: Arnd Bergmann <[email protected]>
> > Signed-off-by: Niklas Schnelle <[email protected]>
> > ---
> > 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.
> >
> > Note 2: This was previously acked here:
> > https://lore.kernel.org/all/2023053050-prodigal-shine-4d1c@gregkh/
> > Given this was almost a year ago and didn't apply then I didn't
> > carry the Ack over though.
> >
> >
---8<---
> > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > index 47ff50763c04..54bf98869abf 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
>
> Why? Your changelogs gives zero insight on this change.
I used this for compile testing since I build on s390 natively and this
would have hidden the missing HAS_IOPORT dependencies I'm pretty sure
it was added because of the I/O port problem too. I'll either add to
the commit description that it is no longer needed or drop this. Any
preference?
On Mon, Apr 8, 2024, at 17:41, Ilpo Järvinen wrote:
> On Mon, 8 Apr 2024, Niklas Schnelle wrote:
>> On Mon, 2024-04-08 at 12:54 +0300, Ilpo Järvinen wrote:
>> > On Fri, 5 Apr 2024, Niklas Schnelle wrote:
>> ---8<---
>> > > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
>> > > index 47ff50763c04..54bf98869abf 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
>> >
>> > Why? Your changelogs gives zero insight on this change.
>>
>> I used this for compile testing since I build on s390 natively and this
>> would have hidden the missing HAS_IOPORT dependencies I'm pretty sure
>> it was added because of the I/O port problem too. I'll either add to
>> the commit description that it is no longer needed or drop this. Any
>> preference?
>
> Okay, we might never know the reason for sure if that's old enough.
> I think the best approach would be to put it into own patch so this
> guessimation is limited to a single liner patch instead of it being
> hidden among the other clearer cases.
From the description in commit 1598e38c0770 ("serial: forbid
8250 on s390") I would just leave the dependency there.
Arnd
On Mon, 8 Apr 2024, Niklas Schnelle wrote:
> On Mon, 2024-04-08 at 12:54 +0300, Ilpo J?rvinen wrote:
> > On Fri, 5 Apr 2024, 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. For 8250 based drivers some support
> > > MMIO only use so fence only the parts requiring I/O ports.
> > >
> > > Co-developed-by: Arnd Bergmann <[email protected]>
> > > Signed-off-by: Arnd Bergmann <[email protected]>
> > > Signed-off-by: Niklas Schnelle <[email protected]>
> > > ---
> > > 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.
> > >
> > > Note 2: This was previously acked here:
> > > https://lore.kernel.org/all/2023053050-prodigal-shine-4d1c@gregkh/
> > > Given this was almost a year ago and didn't apply then I didn't
> > > carry the Ack over though.
> > >
> > >
> ---8<---
> > > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > > index 47ff50763c04..54bf98869abf 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
> >
> > Why? Your changelogs gives zero insight on this change.
>
> I used this for compile testing since I build on s390 natively and this
> would have hidden the missing HAS_IOPORT dependencies I'm pretty sure
> it was added because of the I/O port problem too. I'll either add to
> the commit description that it is no longer needed or drop this. Any
> preference?
Okay, we might never know the reason for sure if that's old enough.
I think the best approach would be to put it into own patch so this
guessimation is limited to a single liner patch instead of it being
hidden among the other clearer cases.
--
i.
On Fri, 5 Apr 2024, Niklas Schnelle wrote:
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 0d35c77fad9e..38ac5236d2ea 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -928,6 +928,7 @@ static int pci_netmos_init(struct pci_dev *dev)
> return num_serial;
> }
>
> +#ifdef CONFIG_HAS_IOPORT
> /*
> * These chips are available with optionally one parallel port and up to
> * two serial ports. Unfortunately they all have the same product id.
[...]
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 47ff50763c04..54bf98869abf 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -136,7 +135,7 @@ config SERIAL_8250_PCILIB
>
> config SERIAL_8250_PCI
> tristate "8250/16550 PCI device support"
> - depends on SERIAL_8250 && PCI
> + depends on SERIAL_8250 && PCI && HAS_IOPORT
> select SERIAL_8250_PCILIB
> default SERIAL_8250
> help
This is clearly wrong, there is PCIe 8250 serial hardware that does MMIO
only, so the option has to stay possible to enable. I have such hardware
as shown in this log:
Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
serial 0001:01:00.0: enabling device (0140 -> 0142)
serial 0001:01:00.0: detected caps 00000700 should be 00000500
0001:01:00.0: ttyS0 at MMIO 0x600c080401000 (irq = 40, base_baud = 15625000) is a 16C950/954
serial 0001:01:00.0: detected caps 00000700 should be 00000500
0001:01:00.0: ttyS1 at MMIO 0x600c080401200 (irq = 40, base_baud = 15625000) is a 16C950/954
which is from a POWER9 system. Which as you may know has no PCI port I/O
support in hardware, so it is quite relevant here. I'd like to keep this
PCIe serial option functional with my system.
Also your change itself modifies 8250_pci.c (cited above for reference),
which would make no sense if SERIAL_8250_PCI was permanently disabled for
!HAS_IOPORT. Shall I take it than that the Kconfig change I question has
been made merely by mistake?
Maciej