[PATCH v2 4/5] serial: 8250: document doubled "type == PORT_8250_CIR" check

Jiri Slaby (SUSE) posted 5 patches 3 months, 2 weeks ago
[PATCH v2 4/5] serial: 8250: document doubled "type == PORT_8250_CIR" check
Posted by Jiri Slaby (SUSE) 3 months, 2 weeks ago
The check for "port.type == PORT_8250_CIR" is present twice in
serial8250_register_8250_port(). The latter was already tried to be
dropped by 1104321a7b3b ("serial: Delete dead code for CIR serial
ports") and then reverted by 9527b82ae3af ("Revert "serial: Delete dead
code for CIR serial ports"").

Document this weirdness with a reason.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Link: https://lore.kernel.org/all/aFcDOx1bdB34I5hS@surfacebook.localdomain/
---
 drivers/tty/serial/8250/8250_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a6ecb8575da4..feb920c5b2e8 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -717,6 +717,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 		nr_uarts++;
 	}
 
+	/* Check if it is CIR already. We check this below again, see there why. */
 	if (uart->port.type == PORT_8250_CIR) {
 		ret = -ENODEV;
 		goto unlock;
@@ -815,6 +816,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 	if (up->dl_write)
 		uart->dl_write = up->dl_write;
 
+	/* Check the type (again)! It might have changed by the port.type assignment above. */
 	if (uart->port.type != PORT_8250_CIR) {
 		if (uart_console_registered(&uart->port))
 			pm_runtime_get_sync(uart->port.dev);
-- 
2.50.0
Re: [PATCH v2 4/5] serial: 8250: document doubled "type == PORT_8250_CIR" check
Posted by Ilpo Järvinen 3 months, 2 weeks ago
On Tue, 24 Jun 2025, Jiri Slaby (SUSE) wrote:

> The check for "port.type == PORT_8250_CIR" is present twice in
> serial8250_register_8250_port(). The latter was already tried to be
> dropped by 1104321a7b3b ("serial: Delete dead code for CIR serial
> ports") and then reverted by 9527b82ae3af ("Revert "serial: Delete dead
> code for CIR serial ports"").
> 
> Document this weirdness with a reason.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> Link: https://lore.kernel.org/all/aFcDOx1bdB34I5hS@surfacebook.localdomain/
> ---
>  drivers/tty/serial/8250/8250_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index a6ecb8575da4..feb920c5b2e8 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -717,6 +717,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>  		nr_uarts++;
>  	}
>  
> +	/* Check if it is CIR already. We check this below again, see there why. */
>  	if (uart->port.type == PORT_8250_CIR) {
>  		ret = -ENODEV;
>  		goto unlock;
> @@ -815,6 +816,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>  	if (up->dl_write)
>  		uart->dl_write = up->dl_write;
>  
> +	/* Check the type (again)! It might have changed by the port.type assignment above. */
>  	if (uart->port.type != PORT_8250_CIR) {
>  		if (uart_console_registered(&uart->port))
>  			pm_runtime_get_sync(uart->port.dev);
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.