There is no point in a long 'if' in serial8250_register_8250_port() to
just return ENOSPC for PORT_8250_CIR ports. Invert the condition and
return immediately.
'gpios' variable was moved to its set location.
And return ENODEV instead of ENOSPC. The latter is a leftover from the
previous find-uart 'if'. The former makes a lot more sense in this case.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/serial/8250/8250_core.c | 253 ++++++++++++++--------------
1 file changed, 127 insertions(+), 126 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 2bac9c7827de..10f25bae9f46 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -725,139 +725,140 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
nr_uarts++;
}
- if (uart->port.type != PORT_8250_CIR) {
- struct mctrl_gpios *gpios;
-
- if (uart->port.dev)
- uart_remove_one_port(&serial8250_reg, &uart->port);
-
- uart->port.ctrl_id = up->port.ctrl_id;
- uart->port.port_id = up->port.port_id;
- uart->port.iobase = up->port.iobase;
- uart->port.membase = up->port.membase;
- uart->port.irq = up->port.irq;
- uart->port.irqflags = up->port.irqflags;
- uart->port.uartclk = up->port.uartclk;
- uart->port.fifosize = up->port.fifosize;
- uart->port.regshift = up->port.regshift;
- uart->port.iotype = up->port.iotype;
- uart->port.flags = up->port.flags | UPF_BOOT_AUTOCONF;
- uart->bugs = up->bugs;
- uart->port.mapbase = up->port.mapbase;
- uart->port.mapsize = up->port.mapsize;
- uart->port.private_data = up->port.private_data;
- uart->tx_loadsz = up->tx_loadsz;
- uart->capabilities = up->capabilities;
- uart->port.throttle = up->port.throttle;
- uart->port.unthrottle = up->port.unthrottle;
- uart->port.rs485_config = up->port.rs485_config;
- uart->port.rs485_supported = up->port.rs485_supported;
- uart->port.rs485 = up->port.rs485;
- uart->rs485_start_tx = up->rs485_start_tx;
- uart->rs485_stop_tx = up->rs485_stop_tx;
- uart->lsr_save_mask = up->lsr_save_mask;
- uart->dma = up->dma;
-
- /* Take tx_loadsz from fifosize if it wasn't set separately */
- if (uart->port.fifosize && !uart->tx_loadsz)
- uart->tx_loadsz = uart->port.fifosize;
-
- if (up->port.dev) {
- uart->port.dev = up->port.dev;
- ret = uart_get_rs485_mode(&uart->port);
- if (ret)
- goto err;
- }
+ if (uart->port.type == PORT_8250_CIR) {
+ ret = -ENODEV;
+ goto unlock;
+ }
- if (up->port.flags & UPF_FIXED_TYPE)
- uart->port.type = up->port.type;
+ if (uart->port.dev)
+ uart_remove_one_port(&serial8250_reg, &uart->port);
+
+ uart->port.ctrl_id = up->port.ctrl_id;
+ uart->port.port_id = up->port.port_id;
+ uart->port.iobase = up->port.iobase;
+ uart->port.membase = up->port.membase;
+ uart->port.irq = up->port.irq;
+ uart->port.irqflags = up->port.irqflags;
+ uart->port.uartclk = up->port.uartclk;
+ uart->port.fifosize = up->port.fifosize;
+ uart->port.regshift = up->port.regshift;
+ uart->port.iotype = up->port.iotype;
+ uart->port.flags = up->port.flags | UPF_BOOT_AUTOCONF;
+ uart->bugs = up->bugs;
+ uart->port.mapbase = up->port.mapbase;
+ uart->port.mapsize = up->port.mapsize;
+ uart->port.private_data = up->port.private_data;
+ uart->tx_loadsz = up->tx_loadsz;
+ uart->capabilities = up->capabilities;
+ uart->port.throttle = up->port.throttle;
+ uart->port.unthrottle = up->port.unthrottle;
+ uart->port.rs485_config = up->port.rs485_config;
+ uart->port.rs485_supported = up->port.rs485_supported;
+ uart->port.rs485 = up->port.rs485;
+ uart->rs485_start_tx = up->rs485_start_tx;
+ uart->rs485_stop_tx = up->rs485_stop_tx;
+ uart->lsr_save_mask = up->lsr_save_mask;
+ uart->dma = up->dma;
+
+ /* Take tx_loadsz from fifosize if it wasn't set separately */
+ if (uart->port.fifosize && !uart->tx_loadsz)
+ uart->tx_loadsz = uart->port.fifosize;
+
+ if (up->port.dev) {
+ uart->port.dev = up->port.dev;
+ ret = uart_get_rs485_mode(&uart->port);
+ if (ret)
+ goto err;
+ }
- /*
- * Only call mctrl_gpio_init(), if the device has no ACPI
- * companion device
- */
- if (!has_acpi_companion(uart->port.dev)) {
- gpios = mctrl_gpio_init(&uart->port, 0);
- if (IS_ERR(gpios)) {
- ret = PTR_ERR(gpios);
- goto err;
- } else {
- uart->gpios = gpios;
- }
- }
+ if (up->port.flags & UPF_FIXED_TYPE)
+ uart->port.type = up->port.type;
- serial8250_set_defaults(uart);
-
- /* Possibly override default I/O functions. */
- if (up->port.serial_in)
- uart->port.serial_in = up->port.serial_in;
- if (up->port.serial_out)
- uart->port.serial_out = up->port.serial_out;
- if (up->port.handle_irq)
- uart->port.handle_irq = up->port.handle_irq;
- /* Possibly override set_termios call */
- if (up->port.set_termios)
- uart->port.set_termios = up->port.set_termios;
- if (up->port.set_ldisc)
- uart->port.set_ldisc = up->port.set_ldisc;
- if (up->port.get_mctrl)
- uart->port.get_mctrl = up->port.get_mctrl;
- if (up->port.set_mctrl)
- uart->port.set_mctrl = up->port.set_mctrl;
- if (up->port.get_divisor)
- uart->port.get_divisor = up->port.get_divisor;
- if (up->port.set_divisor)
- uart->port.set_divisor = up->port.set_divisor;
- if (up->port.startup)
- uart->port.startup = up->port.startup;
- if (up->port.shutdown)
- uart->port.shutdown = up->port.shutdown;
- if (up->port.pm)
- uart->port.pm = up->port.pm;
- if (up->port.handle_break)
- uart->port.handle_break = up->port.handle_break;
- if (up->dl_read)
- uart->dl_read = up->dl_read;
- if (up->dl_write)
- uart->dl_write = up->dl_write;
-
- if (uart->port.type != PORT_8250_CIR) {
- if (uart_console_registered(&uart->port))
- pm_runtime_get_sync(uart->port.dev);
-
- if (serial8250_isa_config != NULL)
- serial8250_isa_config(0, &uart->port,
- &uart->capabilities);
-
- serial8250_apply_quirks(uart);
- ret = uart_add_one_port(&serial8250_reg,
- &uart->port);
- if (ret)
- goto err;
-
- ret = uart->port.line;
+ /*
+ * Only call mctrl_gpio_init(), if the device has no ACPI
+ * companion device
+ */
+ if (!has_acpi_companion(uart->port.dev)) {
+ struct mctrl_gpios *gpios = mctrl_gpio_init(&uart->port, 0);
+ if (IS_ERR(gpios)) {
+ ret = PTR_ERR(gpios);
+ goto err;
} else {
- dev_info(uart->port.dev,
- "skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n",
- uart->port.iobase,
- (unsigned long long)uart->port.mapbase,
- uart->port.irq);
-
- ret = 0;
+ uart->gpios = gpios;
}
+ }
- if (!uart->lsr_save_mask)
- uart->lsr_save_mask = LSR_SAVE_FLAGS; /* Use default LSR mask */
+ serial8250_set_defaults(uart);
+
+ /* Possibly override default I/O functions. */
+ if (up->port.serial_in)
+ uart->port.serial_in = up->port.serial_in;
+ if (up->port.serial_out)
+ uart->port.serial_out = up->port.serial_out;
+ if (up->port.handle_irq)
+ uart->port.handle_irq = up->port.handle_irq;
+ /* Possibly override set_termios call */
+ if (up->port.set_termios)
+ uart->port.set_termios = up->port.set_termios;
+ if (up->port.set_ldisc)
+ uart->port.set_ldisc = up->port.set_ldisc;
+ if (up->port.get_mctrl)
+ uart->port.get_mctrl = up->port.get_mctrl;
+ if (up->port.set_mctrl)
+ uart->port.set_mctrl = up->port.set_mctrl;
+ if (up->port.get_divisor)
+ uart->port.get_divisor = up->port.get_divisor;
+ if (up->port.set_divisor)
+ uart->port.set_divisor = up->port.set_divisor;
+ if (up->port.startup)
+ uart->port.startup = up->port.startup;
+ if (up->port.shutdown)
+ uart->port.shutdown = up->port.shutdown;
+ if (up->port.pm)
+ uart->port.pm = up->port.pm;
+ if (up->port.handle_break)
+ uart->port.handle_break = up->port.handle_break;
+ if (up->dl_read)
+ uart->dl_read = up->dl_read;
+ if (up->dl_write)
+ uart->dl_write = up->dl_write;
- /* Initialise interrupt backoff work if required */
- if (up->overrun_backoff_time_ms > 0) {
- uart->overrun_backoff_time_ms =
- up->overrun_backoff_time_ms;
- INIT_DELAYED_WORK(&uart->overrun_backoff,
- serial_8250_overrun_backoff_work);
- } else {
- uart->overrun_backoff_time_ms = 0;
- }
+ if (uart->port.type != PORT_8250_CIR) {
+ if (uart_console_registered(&uart->port))
+ pm_runtime_get_sync(uart->port.dev);
+
+ if (serial8250_isa_config != NULL)
+ serial8250_isa_config(0, &uart->port,
+ &uart->capabilities);
+
+ serial8250_apply_quirks(uart);
+ ret = uart_add_one_port(&serial8250_reg,
+ &uart->port);
+ if (ret)
+ goto err;
+
+ ret = uart->port.line;
+ } else {
+ dev_info(uart->port.dev,
+ "skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n",
+ uart->port.iobase,
+ (unsigned long long)uart->port.mapbase,
+ uart->port.irq);
+
+ ret = 0;
+ }
+
+ if (!uart->lsr_save_mask)
+ uart->lsr_save_mask = LSR_SAVE_FLAGS; /* Use default LSR mask */
+
+ /* Initialise interrupt backoff work if required */
+ if (up->overrun_backoff_time_ms > 0) {
+ uart->overrun_backoff_time_ms =
+ up->overrun_backoff_time_ms;
+ INIT_DELAYED_WORK(&uart->overrun_backoff,
+ serial_8250_overrun_backoff_work);
+ } else {
+ uart->overrun_backoff_time_ms = 0;
}
unlock:
--
2.49.0On Wed, Jun 11, 2025 at 12:03:16PM +0200, Jiri Slaby (SUSE) wrote:
> There is no point in a long 'if' in serial8250_register_8250_port() to
> just return ENOSPC for PORT_8250_CIR ports. Invert the condition and
> return immediately.
>
> 'gpios' variable was moved to its set location.
>
> And return ENODEV instead of ENOSPC. The latter is a leftover from the
> previous find-uart 'if'. The former makes a lot more sense in this case.
...
> + if (uart->port.type == PORT_8250_CIR) {
> + ret = -ENODEV;
> + goto unlock;
> + }
> + if (up->port.flags & UPF_FIXED_TYPE)
> + uart->port.type = up->port.type;
> + if (uart->port.type != PORT_8250_CIR) {
I admit that there tons of mysterious ways of UART initialisation, but can you
elaborate how this is not a always-true conditional?
> + if (uart_console_registered(&uart->port))
> + pm_runtime_get_sync(uart->port.dev);
> +
> + if (serial8250_isa_config != NULL)
> + serial8250_isa_config(0, &uart->port,
> + &uart->capabilities);
> +
> + serial8250_apply_quirks(uart);
> + ret = uart_add_one_port(&serial8250_reg,
> + &uart->port);
> + if (ret)
> + goto err;
> +
> + ret = uart->port.line;
> + } else {
> + dev_info(uart->port.dev,
> + "skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n",
> + uart->port.iobase,
> + (unsigned long long)uart->port.mapbase,
> + uart->port.irq);
> +
> + ret = 0;
> + }
--
With Best Regards,
Andy Shevchenko
On 18.06.2025 07:48, Andy Shevchenko wrote:
> On Wed, Jun 11, 2025 at 12:03:16PM +0200, Jiri Slaby (SUSE) wrote:
>> There is no point in a long 'if' in serial8250_register_8250_port() to
>> just return ENOSPC for PORT_8250_CIR ports. Invert the condition and
>> return immediately.
>>
>> 'gpios' variable was moved to its set location.
>>
>> And return ENODEV instead of ENOSPC. The latter is a leftover from the
>> previous find-uart 'if'. The former makes a lot more sense in this case.
>
> ...
>
>> + if (uart->port.type == PORT_8250_CIR) {
>> + ret = -ENODEV;
>> + goto unlock;
>> + }
>
>> + if (up->port.flags & UPF_FIXED_TYPE)
>> + uart->port.type = up->port.type;
>
>> + if (uart->port.type != PORT_8250_CIR) {
>
> I admit that there tons of mysterious ways of UART initialisation, but can you
> elaborate how this is not a always-true conditional?
>
Careful here, someone had an idea in the past that this is indeed
a dead code/branch and ended causing a regression [1].
It would definitely make sense to add a comment describing the code
flow there though as it proven to bewilder people.
Thanks,
Maciej
[1]: https://lore.kernel.org/lkml/c599065a-60f5-070e-c1e3-12b3ab2cbf0a@suse.com/
Fri, Jun 20, 2025 at 11:48:09PM +0200, Maciej S. Szmigiero kirjoitti:
> On 18.06.2025 07:48, Andy Shevchenko wrote:
> > On Wed, Jun 11, 2025 at 12:03:16PM +0200, Jiri Slaby (SUSE) wrote:
...
> > > + if (uart->port.type == PORT_8250_CIR) {
> > > + ret = -ENODEV;
> > > + goto unlock;
> > > + }
> >
> > > + if (up->port.flags & UPF_FIXED_TYPE)
> > > + uart->port.type = up->port.type;
> >
> > > + if (uart->port.type != PORT_8250_CIR) {
> >
> > I admit that there tons of mysterious ways of UART initialisation, but can you
> > elaborate how this is not a always-true conditional?
>
> Careful here, someone had an idea in the past that this is indeed
> a dead code/branch and ended causing a regression [1].
> It would definitely make sense to add a comment describing the code
> flow there though as it proven to bewilder people.
Yes, this is my point between the lines. I left the code that may affect the
type change and the second check needs a comment explaining these cases, if any.
"If any" defines "always-true" or not conditional. W//o a comment this code
tends to be updated again and lead to a regression.
--
With Best Regards,
Andy Shevchenko
On 21. 06. 25, 21:08, Andy Shevchenko wrote:
> Fri, Jun 20, 2025 at 11:48:09PM +0200, Maciej S. Szmigiero kirjoitti:
>> On 18.06.2025 07:48, Andy Shevchenko wrote:
>>> On Wed, Jun 11, 2025 at 12:03:16PM +0200, Jiri Slaby (SUSE) wrote:
>
> ...
>
>>>> + if (uart->port.type == PORT_8250_CIR) {
>>>> + ret = -ENODEV;
>>>> + goto unlock;
>>>> + }
>>>
>>>> + if (up->port.flags & UPF_FIXED_TYPE)
>>>> + uart->port.type = up->port.type;
>>>
>>>> + if (uart->port.type != PORT_8250_CIR) {
>>>
>>> I admit that there tons of mysterious ways of UART initialisation, but can you
>>> elaborate how this is not a always-true conditional?
>>
>> Careful here, someone had an idea in the past that this is indeed
>> a dead code/branch and ended causing a regression [1].
Right, I was confused too, but then I noticed there is:
uart->port.type = up->port.type;
in between the tests.
>> It would definitely make sense to add a comment describing the code
>> flow there though as it proven to bewilder people.
>
> Yes, this is my point between the lines. I left the code that may affect the
> type change and the second check needs a comment explaining these cases, if any.
> "If any" defines "always-true" or not conditional. W//o a comment this code
> tends to be updated again and lead to a regression.
ACK, I will.
--
js
suse labs
On Mon, Jun 23, 2025 at 09:05:46AM +0200, Jiri Slaby wrote:
> On 21. 06. 25, 21:08, Andy Shevchenko wrote:
> > Fri, Jun 20, 2025 at 11:48:09PM +0200, Maciej S. Szmigiero kirjoitti:
> > > On 18.06.2025 07:48, Andy Shevchenko wrote:
> > > > On Wed, Jun 11, 2025 at 12:03:16PM +0200, Jiri Slaby (SUSE) wrote:
...
> > > > > + if (uart->port.type == PORT_8250_CIR) {
> > > > > + ret = -ENODEV;
> > > > > + goto unlock;
> > > > > + }
> > > >
> > > > > + if (up->port.flags & UPF_FIXED_TYPE)
> > > > > + uart->port.type = up->port.type;
> > > >
> > > > > + if (uart->port.type != PORT_8250_CIR) {
> > > >
> > > > I admit that there tons of mysterious ways of UART initialisation, but can you
> > > > elaborate how this is not a always-true conditional?
> > >
> > > Careful here, someone had an idea in the past that this is indeed
> > > a dead code/branch and ended causing a regression [1].
>
> Right, I was confused too, but then I noticed there is:
> uart->port.type = up->port.type;
> in between the tests.
>
> > > It would definitely make sense to add a comment describing the code
> > > flow there though as it proven to bewilder people.
> >
> > Yes, this is my point between the lines. I left the code that may affect the
> > type change and the second check needs a comment explaining these cases, if any.
> > "If any" defines "always-true" or not conditional. W//o a comment this code
> > tends to be updated again and lead to a regression.
>
> ACK, I will.
Thanks!
Looking at the code again, I think it deserves actually two comments, on top of
each of the checks against PORT_8250_CIR.
--
With Best Regards,
Andy Shevchenko
On Wed, 11 Jun 2025, Jiri Slaby (SUSE) wrote:
> There is no point in a long 'if' in serial8250_register_8250_port() to
> just return ENOSPC for PORT_8250_CIR ports. Invert the condition and
> return immediately.
>
> 'gpios' variable was moved to its set location.
>
> And return ENODEV instead of ENOSPC. The latter is a leftover from the
> previous find-uart 'if'. The former makes a lot more sense in this case.
>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
> drivers/tty/serial/8250/8250_core.c | 253 ++++++++++++++--------------
> 1 file changed, 127 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 2bac9c7827de..10f25bae9f46 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -725,139 +725,140 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
> nr_uarts++;
> }
>
> - if (uart->port.type != PORT_8250_CIR) {
> - struct mctrl_gpios *gpios;
> -
> - if (uart->port.dev)
> - uart_remove_one_port(&serial8250_reg, &uart->port);
> -
> - uart->port.ctrl_id = up->port.ctrl_id;
> - uart->port.port_id = up->port.port_id;
> - uart->port.iobase = up->port.iobase;
> - uart->port.membase = up->port.membase;
> - uart->port.irq = up->port.irq;
> - uart->port.irqflags = up->port.irqflags;
> - uart->port.uartclk = up->port.uartclk;
> - uart->port.fifosize = up->port.fifosize;
> - uart->port.regshift = up->port.regshift;
> - uart->port.iotype = up->port.iotype;
> - uart->port.flags = up->port.flags | UPF_BOOT_AUTOCONF;
> - uart->bugs = up->bugs;
> - uart->port.mapbase = up->port.mapbase;
> - uart->port.mapsize = up->port.mapsize;
> - uart->port.private_data = up->port.private_data;
> - uart->tx_loadsz = up->tx_loadsz;
> - uart->capabilities = up->capabilities;
> - uart->port.throttle = up->port.throttle;
> - uart->port.unthrottle = up->port.unthrottle;
> - uart->port.rs485_config = up->port.rs485_config;
> - uart->port.rs485_supported = up->port.rs485_supported;
> - uart->port.rs485 = up->port.rs485;
> - uart->rs485_start_tx = up->rs485_start_tx;
> - uart->rs485_stop_tx = up->rs485_stop_tx;
> - uart->lsr_save_mask = up->lsr_save_mask;
> - uart->dma = up->dma;
> -
> - /* Take tx_loadsz from fifosize if it wasn't set separately */
> - if (uart->port.fifosize && !uart->tx_loadsz)
> - uart->tx_loadsz = uart->port.fifosize;
> -
> - if (up->port.dev) {
> - uart->port.dev = up->port.dev;
> - ret = uart_get_rs485_mode(&uart->port);
> - if (ret)
> - goto err;
> - }
> + if (uart->port.type == PORT_8250_CIR) {
> + ret = -ENODEV;
> + goto unlock;
> + }
>
> - if (up->port.flags & UPF_FIXED_TYPE)
> - uart->port.type = up->port.type;
> + if (uart->port.dev)
> + uart_remove_one_port(&serial8250_reg, &uart->port);
> +
> + uart->port.ctrl_id = up->port.ctrl_id;
> + uart->port.port_id = up->port.port_id;
> + uart->port.iobase = up->port.iobase;
> + uart->port.membase = up->port.membase;
> + uart->port.irq = up->port.irq;
> + uart->port.irqflags = up->port.irqflags;
> + uart->port.uartclk = up->port.uartclk;
> + uart->port.fifosize = up->port.fifosize;
> + uart->port.regshift = up->port.regshift;
> + uart->port.iotype = up->port.iotype;
> + uart->port.flags = up->port.flags | UPF_BOOT_AUTOCONF;
> + uart->bugs = up->bugs;
> + uart->port.mapbase = up->port.mapbase;
> + uart->port.mapsize = up->port.mapsize;
> + uart->port.private_data = up->port.private_data;
> + uart->tx_loadsz = up->tx_loadsz;
> + uart->capabilities = up->capabilities;
> + uart->port.throttle = up->port.throttle;
> + uart->port.unthrottle = up->port.unthrottle;
> + uart->port.rs485_config = up->port.rs485_config;
> + uart->port.rs485_supported = up->port.rs485_supported;
> + uart->port.rs485 = up->port.rs485;
> + uart->rs485_start_tx = up->rs485_start_tx;
> + uart->rs485_stop_tx = up->rs485_stop_tx;
> + uart->lsr_save_mask = up->lsr_save_mask;
> + uart->dma = up->dma;
> +
> + /* Take tx_loadsz from fifosize if it wasn't set separately */
> + if (uart->port.fifosize && !uart->tx_loadsz)
> + uart->tx_loadsz = uart->port.fifosize;
> +
> + if (up->port.dev) {
> + uart->port.dev = up->port.dev;
> + ret = uart_get_rs485_mode(&uart->port);
> + if (ret)
> + goto err;
> + }
>
> - /*
> - * Only call mctrl_gpio_init(), if the device has no ACPI
> - * companion device
> - */
> - if (!has_acpi_companion(uart->port.dev)) {
> - gpios = mctrl_gpio_init(&uart->port, 0);
> - if (IS_ERR(gpios)) {
> - ret = PTR_ERR(gpios);
> - goto err;
> - } else {
> - uart->gpios = gpios;
> - }
> - }
> + if (up->port.flags & UPF_FIXED_TYPE)
> + uart->port.type = up->port.type;
>
> - serial8250_set_defaults(uart);
> -
> - /* Possibly override default I/O functions. */
> - if (up->port.serial_in)
> - uart->port.serial_in = up->port.serial_in;
> - if (up->port.serial_out)
> - uart->port.serial_out = up->port.serial_out;
> - if (up->port.handle_irq)
> - uart->port.handle_irq = up->port.handle_irq;
> - /* Possibly override set_termios call */
> - if (up->port.set_termios)
> - uart->port.set_termios = up->port.set_termios;
> - if (up->port.set_ldisc)
> - uart->port.set_ldisc = up->port.set_ldisc;
> - if (up->port.get_mctrl)
> - uart->port.get_mctrl = up->port.get_mctrl;
> - if (up->port.set_mctrl)
> - uart->port.set_mctrl = up->port.set_mctrl;
> - if (up->port.get_divisor)
> - uart->port.get_divisor = up->port.get_divisor;
> - if (up->port.set_divisor)
> - uart->port.set_divisor = up->port.set_divisor;
> - if (up->port.startup)
> - uart->port.startup = up->port.startup;
> - if (up->port.shutdown)
> - uart->port.shutdown = up->port.shutdown;
> - if (up->port.pm)
> - uart->port.pm = up->port.pm;
> - if (up->port.handle_break)
> - uart->port.handle_break = up->port.handle_break;
> - if (up->dl_read)
> - uart->dl_read = up->dl_read;
> - if (up->dl_write)
> - uart->dl_write = up->dl_write;
> -
> - if (uart->port.type != PORT_8250_CIR) {
> - if (uart_console_registered(&uart->port))
> - pm_runtime_get_sync(uart->port.dev);
> -
> - if (serial8250_isa_config != NULL)
> - serial8250_isa_config(0, &uart->port,
> - &uart->capabilities);
> -
> - serial8250_apply_quirks(uart);
> - ret = uart_add_one_port(&serial8250_reg,
> - &uart->port);
> - if (ret)
> - goto err;
> -
> - ret = uart->port.line;
> + /*
> + * Only call mctrl_gpio_init(), if the device has no ACPI
> + * companion device
> + */
> + if (!has_acpi_companion(uart->port.dev)) {
> + struct mctrl_gpios *gpios = mctrl_gpio_init(&uart->port, 0);
> + if (IS_ERR(gpios)) {
> + ret = PTR_ERR(gpios);
> + goto err;
> } else {
> - dev_info(uart->port.dev,
> - "skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n",
> - uart->port.iobase,
> - (unsigned long long)uart->port.mapbase,
> - uart->port.irq);
> -
> - ret = 0;
> + uart->gpios = gpios;
> }
> + }
>
> - if (!uart->lsr_save_mask)
> - uart->lsr_save_mask = LSR_SAVE_FLAGS; /* Use default LSR mask */
> + serial8250_set_defaults(uart);
> +
> + /* Possibly override default I/O functions. */
> + if (up->port.serial_in)
> + uart->port.serial_in = up->port.serial_in;
> + if (up->port.serial_out)
> + uart->port.serial_out = up->port.serial_out;
> + if (up->port.handle_irq)
> + uart->port.handle_irq = up->port.handle_irq;
> + /* Possibly override set_termios call */
> + if (up->port.set_termios)
> + uart->port.set_termios = up->port.set_termios;
> + if (up->port.set_ldisc)
> + uart->port.set_ldisc = up->port.set_ldisc;
> + if (up->port.get_mctrl)
> + uart->port.get_mctrl = up->port.get_mctrl;
> + if (up->port.set_mctrl)
> + uart->port.set_mctrl = up->port.set_mctrl;
> + if (up->port.get_divisor)
> + uart->port.get_divisor = up->port.get_divisor;
> + if (up->port.set_divisor)
> + uart->port.set_divisor = up->port.set_divisor;
> + if (up->port.startup)
> + uart->port.startup = up->port.startup;
> + if (up->port.shutdown)
> + uart->port.shutdown = up->port.shutdown;
> + if (up->port.pm)
> + uart->port.pm = up->port.pm;
> + if (up->port.handle_break)
> + uart->port.handle_break = up->port.handle_break;
> + if (up->dl_read)
> + uart->dl_read = up->dl_read;
> + if (up->dl_write)
> + uart->dl_write = up->dl_write;
>
> - /* Initialise interrupt backoff work if required */
> - if (up->overrun_backoff_time_ms > 0) {
> - uart->overrun_backoff_time_ms =
> - up->overrun_backoff_time_ms;
> - INIT_DELAYED_WORK(&uart->overrun_backoff,
> - serial_8250_overrun_backoff_work);
> - } else {
> - uart->overrun_backoff_time_ms = 0;
> - }
> + if (uart->port.type != PORT_8250_CIR) {
> + if (uart_console_registered(&uart->port))
> + pm_runtime_get_sync(uart->port.dev);
> +
> + if (serial8250_isa_config != NULL)
> + serial8250_isa_config(0, &uart->port,
> + &uart->capabilities);
> +
> + serial8250_apply_quirks(uart);
> + ret = uart_add_one_port(&serial8250_reg,
> + &uart->port);
> + if (ret)
> + goto err;
> +
> + ret = uart->port.line;
> + } else {
> + dev_info(uart->port.dev,
> + "skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n",
> + uart->port.iobase,
> + (unsigned long long)uart->port.mapbase,
> + uart->port.irq);
> +
> + ret = 0;
> + }
> +
> + if (!uart->lsr_save_mask)
> + uart->lsr_save_mask = LSR_SAVE_FLAGS; /* Use default LSR mask */
> +
> + /* Initialise interrupt backoff work if required */
> + if (up->overrun_backoff_time_ms > 0) {
> + uart->overrun_backoff_time_ms =
> + up->overrun_backoff_time_ms;
> + INIT_DELAYED_WORK(&uart->overrun_backoff,
> + serial_8250_overrun_backoff_work);
> + } else {
> + uart->overrun_backoff_time_ms = 0;
> }
>
> unlock:
>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
© 2016 - 2026 Red Hat, Inc.