| Message ID | 20241007-b4-has_ioport-v6-4-03f7240da6e5@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | treewide: Remove I/O port accessors for HAS_IOPORT=n | expand |
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. > +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. > @@ -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. > @@ -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
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-lkp@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
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
On Tue, 8 Oct 2024, Niklas Schnelle wrote: > > > +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? With this change UAPI stays the same and setting ASYNC_FOURPORT (with `setserial', etc.) will just do nothing with non-port-I/O platforms, as expected. Arguably being able to set it for any serial port and cause the driver to poke at random I/O locations is already asking for trouble, but that the price of legacy. > > > @@ -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-lkp@intel.com/ Odd, but I'm not into UML so I need to accept your justification. My reservation about relying on compiler's __i386__ predefine rather than our CONFIG_X86_32 setting still stands, but that's beyond the scope of your change (as is switching from `#if ...' to `if (...)'). Thanks for your attention to such details. NB these `outb' calls look to me remarkably like remains of `outb_p' and I wonder if they could be abstracted somehow. For those who don't know: the port I/O location 0x80 in the IBM PC address space was reserved for use as a diagnostic port. Despite being in the mainboard's address space its chip select line was left floating and one could obtain an ISA option card that decoded this location and showed data values written to it on a hex display. As it was a location known to cause no side effect (beyond that optional hex display) it was commonly used to incur a small delay in execution. It was also used by BIOS POST to indicate progress. I've skimmed over v8 and it seems good to go as far as I'm concerned. Any fallout can be dealt with on a case-by-case basis. Thank you for working on these improvements. Maciej
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 +1672,9 @@ static int pci_fintek_setup(struct serial_private *priv, u8 config_base; u16 iobase; + if (!IS_ENABLED(CONFIG_HAS_IOPORT)) + return serial_8250_need_ioport(pdev); + config_base = 0x40 + 0x08 * idx; /* Get the io address from configuration space */ @@ -1686,6 +1706,9 @@ static int pci_fintek_init(struct pci_dev *dev) u8 config_base; struct serial_private *priv = pci_get_drvdata(dev); + if (!IS_ENABLED(CONFIG_HAS_IOPORT)) + return serial_8250_need_ioport(dev); + if (!(pci_resource_flags(dev, 5) & IORESOURCE_IO) || !(pci_resource_flags(dev, 4) & IORESOURCE_IO) || !(pci_resource_flags(dev, 3) & IORESOURCE_IO)) @@ -1864,6 +1887,9 @@ static int kt_serial_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); + port->port.flags |= UPF_BUG_THRE; port->port.serial_in = kt_serial_in; port->port.handle_break = kt_handle_break; @@ -1884,6 +1910,9 @@ pci_wch_ch353_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); + port->port.flags |= UPF_FIXED_TYPE; port->port.type = PORT_16550A; return pci_default_setup(priv, board, port, idx); @@ -1894,6 +1923,9 @@ pci_wch_ch355_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); + port->port.flags |= UPF_FIXED_TYPE; port->port.type = PORT_16550A; return pci_default_setup(priv, board, port, idx); @@ -1904,6 +1936,9 @@ pci_wch_ch38x_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); + port->port.flags |= UPF_FIXED_TYPE; port->port.type = PORT_16850; return pci_default_setup(priv, board, port, idx); @@ -1918,6 +1953,8 @@ static int pci_wch_ch38x_init(struct pci_dev *dev) int max_port; unsigned long iobase; + if (!IS_ENABLED(CONFIG_HAS_IOPORT)) + return serial_8250_need_ioport(dev); switch (dev->device) { case 0x3853: /* 8 ports */ @@ -1937,11 +1974,15 @@ static void pci_wch_ch38x_exit(struct pci_dev *dev) { unsigned long iobase; + if (!IS_ENABLED(CONFIG_HAS_IOPORT)) { + serial_8250_need_ioport(dev); + return; + } + 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, @@ -2052,6 +2093,9 @@ static int pci_moxa_init(struct pci_dev *dev) unsigned int i, num_ports = moxa_get_nports(device); u8 val, init_mode = MOXA_RS232; + if (!IS_ENABLED(CONFIG_HAS_IOPORT)) + return serial_8250_need_ioport(dev); + if (!(pci_moxa_supported_rs(dev) & MOXA_SUPP_RS232)) { init_mode = MOXA_RS422; } @@ -2084,6 +2128,9 @@ pci_moxa_setup(struct serial_private *priv, unsigned int bar = FL_GET_BASE(board->flags); int offset; + if (!IS_ENABLED(CONFIG_HAS_IOPORT)) + return serial_8250_need_ioport(priv->dev); + if (board->num_ports == 4 && idx == 3) offset = 7 * board->uart_offset; else 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; diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 3509af7dc52b8816f2f5ab58f0d999696f7483e7..74717675dc41db6a5b5b0dd1da84b4ea23bd9928 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,23 @@ 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) +{ +} + +static __always_inline bool is_upf_fourport(struct uart_port *port) +{ + if (!IS_ENABLED(CONFIG_HAS_IOPORT)) + return false; + + return port->flags & UPF_FOURPORT; +} static int serial8250_default_handle_irq(struct uart_port *port); @@ -422,10 +442,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 +468,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; @@ -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; @@ -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; - if (port->flags & UPF_FOURPORT) { + if (is_upf_fourport(port)) { ICP = (port->iobase & 0xfe0) | 0x1f; save_ICP = inb_p(ICP); outb_p(0x80, ICP); @@ -1330,7 +1357,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, @@ -1354,7 +1381,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; @@ -2435,7 +2462,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 @@ -2480,7 +2507,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 47ff50763c048c89b19b7c9f13f462bc5368ab43..1eb20350fcf432d41cb416bed3be72ed8ab129bb 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -72,7 +72,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 @@ -163,7 +163,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 28e4beeabf8f373fc76e09ea7d1c9d55a66f4964..45f0f779fbf960e9fa66375dbc44c379edc63bef 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -877,7 +877,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