All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Christian Marangi" <ansuelsmth@gmail.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 3/4] serial: 8250: map UAPI port type to internal enum
Date: Thu, 09 Jul 2026 21:18:22 +0000	[thread overview]
Message-ID: <20260709211823.653DB1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260709205656.319531-4-ansuelsmth@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new documentation guidelines encourage a split-brain ID registry that will cause integer collisions between internal 8250 port types and global UAPI port types.
--

--- Patch [3]: [PATCH 3/4] serial: 8250: map UAPI port type to internal enum ---
commit d2ffed5bdc0925967ba25a551f7c1657761a9cab
Author: Christian Marangi <ansuelsmth@gmail.com>

serial: 8250: map UAPI port type to internal enum

This commit maps the UAPI port types to an internal enum in the 8250 driver.
It creates a dedicated enum in 8250.h with all current UAPI values and documents
how new indexes should be defined. It also renames definitions in 8250_port.c
with a new UART_ prefix.

> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -73,6 +73,128 @@ struct serial8250_config {
>  	unsigned int	flags;
>  };
>  
> +/*
> + * The uart_config[] array index is referenced and defined from index
> + * in uapi/linux/serial.h and uapi/linux/serial_core.h.
> + *
> + * This was old practice and for anything that doesn't need to support
> + * userspace, new type should be limited and added only HERE.
> + *
> + * Any UART port that requires userspace support, should define the
> + * dedicated index in the UAPI header and reference it when added
> + * to this enum table.
> + */
> +enum uart_port_type {

[Severity: High]
Does this new documentation encourage a split-brain ID registry that could
cause integer collisions between internal 8250 port types and global UAPI
port types?

If a developer follows this documentation and defines a new internal port type
at the end of enum uart_port_type (e.g., getting value 124), what happens if
another subsystem concurrently adds a port to <uapi/linux/serial_core.h> that
also receives value 124?

Since the port->type field is a globally shared namespace across all serial
drivers, driver-local enum values that are not offset into a disjoint range
might collide with new UAPI additions. This could manifest as compile-time
errors when the uart_config[] array is populated, or runtime ABI
misidentification by userspace tools if setserial treats an internal 8250
port as a completely different architecture's port.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709205656.319531-1-ansuelsmth@gmail.com?part=3

  reply	other threads:[~2026-07-09 21:18 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 20:56 [PATCH 0/4] serial: 8250: Add AN7581 UART support Christian Marangi
2026-07-09 20:56 ` [PATCH 1/4] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi
2026-07-10 16:43   ` Conor Dooley
2026-07-09 20:56 ` [PATCH 2/4] serial: 8250: export serial8250_get_baud_rate() Christian Marangi
2026-07-09 21:35   ` Andy Shevchenko
2026-07-09 21:39     ` Christian Marangi
2026-07-10  8:20       ` Andy Shevchenko
2026-07-10  8:49         ` Christian Marangi
2026-07-10  9:14           ` Andy Shevchenko
2026-07-10  8:19   ` Ilpo Järvinen
2026-07-09 20:56 ` [PATCH 3/4] serial: 8250: map UAPI port type to internal enum Christian Marangi
2026-07-09 21:18   ` sashiko-bot [this message]
2026-07-09 21:37   ` Andy Shevchenko
2026-07-09 21:46     ` Christian Marangi
2026-07-10 10:09       ` Andy Shevchenko
2026-07-09 20:56 ` [PATCH 4/4] serial: 8250: Add Airoha SoC UART and HSUART support Christian Marangi
2026-07-09 21:19   ` sashiko-bot
2026-07-10  7:13   ` Jiri Slaby
2026-07-14 14:55     ` Benjamin Larsson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260709211823.653DB1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ansuelsmth@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.