diff mbox series

[1/1] tty: serial: handle HAS_IOPORT dependencies

Message ID 20240405152924.252598-2-schnelle@linux.ibm.com
State New
Headers show
Series tty: Handle HAS_IOPORT dependencies | expand

Commit Message

Niklas Schnelle April 5, 2024, 3:29 p.m. UTC 
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 <arnd@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
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(-)

Comments

Ilpo Järvinen April 8, 2024, 9:54 a.m. UTC | #1 
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 <arnd@kernel.org>
> Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
> 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
>
Arnd Bergmann April 8, 2024, 10:17 a.m. UTC | #2 
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
Ilpo Järvinen April 8, 2024, 10:25 a.m. UTC | #3 
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.
Niklas Schnelle April 8, 2024, 3:35 p.m. UTC | #4 
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 <arnd@kernel.org>
> > Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > ---
> > 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?
Ilpo Järvinen April 8, 2024, 3:41 p.m. UTC | #5 
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 <arnd@kernel.org>
> > > Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > ---
> > > 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.
Arnd Bergmann April 8, 2024, 3:50 p.m. UTC | #6 
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.
diff mbox series

Patch

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