* Re: [PATCH v2 2/6] serial: sb1250-duart: Fix bootconsole handover lockup
From: Philippe Mathieu-Daudé @ 2026-06-17 11:56 UTC (permalink / raw)
To: Maciej W. Rozycki, Thomas Bogendoerfer, Greg Kroah-Hartman,
Jiri Slaby
Cc: Elena Reshetova, David Windsor, Kees Cook, Hans Liljestrand,
linux-mips, linux-serial, linux-kernel
In-Reply-To: <alpine.DEB.2.21.2605241623110.1450@angie.orcam.me.uk>
On 25/5/26 01:12, Maciej W. Rozycki wrote:
> Calling sbd_init_port() in the course of setting up the serial device
> causes line parameters to be messed up and the transmitter disabled.
> We've been lucky in that no message is usually produced to the kernel
> log between this call and the later call to uart_set_options() in the
> course of console setup done by sbd_serial_console_init(), or the system
> would hang as the console output handler in CFE tried to access a port
> whose transmitter has been disabled and line parameters messed up.
>
> It'll change with the next change to the driver, so fix sbd_init_port()
> such that line parameters are set for 115200n8 console operation as with
> the CFE firmware and the transmitter re-enabled after reset.
>
> Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Cc: stable@vger.kernel.org # v6.5+
> ---
> Changes from v1 (2/4),
> <https://lore.kernel.org/r/alpine.DEB.2.21.2604130338210.29980@angie.orcam.me.uk/>:
>
> - Sanitise the change heading.
> ---
> drivers/tty/serial/sb1250-duart.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v2 1/6] serial: sb1250-duart: Fix console message clobbering at channel resets
From: Philippe Mathieu-Daudé @ 2026-06-17 11:55 UTC (permalink / raw)
To: Maciej W. Rozycki, Thomas Bogendoerfer, Greg Kroah-Hartman,
Jiri Slaby
Cc: Elena Reshetova, David Windsor, Kees Cook, Hans Liljestrand,
linux-mips, linux-serial, linux-kernel
In-Reply-To: <alpine.DEB.2.21.2605241620470.1450@angie.orcam.me.uk>
On 25/5/26 01:12, Maciej W. Rozycki wrote:
> Ensure any characters outstanding have been sent before issuing channel
> resets so as to prevent messages issued to the bootconsole from getting
> clobbered.
>
> Contrary to device documentation at the time the transmitter empty bit
> is set only the transmit FIFO has been drained and there is still data
> outstanding in the transmitter shift register, so wait an extra amount
> of time for that register to drain too. This also prevents subsequent
> messages produced to the console from getting clobbered, owing to what
> seems a transmitter synchronisation issue.
>
> When called from sbd_serial_console_init() it is too early for fsleep()
> to work and even before lpj has been calculated and therefore the delay
> is actually not sufficient for the transmitter to drain and is merely a
> placeholder now. This will be addressed in a follow-up change.
>
> Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Cc: stable@vger.kernel.org # v6.5+
> ---
> Changes from v1 (1/4),
> <https://lore.kernel.org/r/alpine.DEB.2.21.2604130321540.29980@angie.orcam.me.uk/>:
>
> - Sanitise the change heading.
> ---
> drivers/tty/serial/sb1250-duart.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
^ permalink raw reply
* [PATCH v1 1/1] serdev: acpi: Free resource list at appropriate time
From: Andy Shevchenko @ 2026-06-17 9:25 UTC (permalink / raw)
To: linux-serial, linux-kernel
Cc: Rob Herring, Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko
We do unneeded "double free" (emptying an empty list) in one case.
This is not a critical issue at all, the fix just makes code robust
against any possible future changes in the flow.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/tty/serdev/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index e9d044a331b0..7500efcdfc21 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -651,11 +651,11 @@ static int acpi_serdev_do_lookup(struct acpi_device *adev,
INIT_LIST_HEAD(&resource_list);
ret = acpi_dev_get_resources(adev, &resource_list,
acpi_serdev_parse_resource, lookup);
- acpi_dev_free_resource_list(&resource_list);
-
if (ret < 0)
return -EINVAL;
+ acpi_dev_free_resource_list(&resource_list);
+
return 0;
}
--
2.50.1
^ permalink raw reply related
* RE: [PATCH v2 0/2] tty: n_gsm: fix gsm_queue() UAF and add a base regression test
From: Starke, Daniel @ 2026-06-17 7:26 UTC (permalink / raw)
To: Weiming Shi, Greg Kroah-Hartman, Jiri Slaby, Shuah Khan
Cc: Xiang Mei, linux-serial@vger.kernel.org,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20260616173240.3665059-1-bestswngs@gmail.com>
> The receive worker walks gsm->dlci[] without gsm->mutex while a
> concurrent GSMIOC_SETCONF -> gsm_cleanup_mux() frees the DLCIs, so the
> control handlers can dereference a freed gsm_dlci. v1's NULL check only
> narrowed the window; v2 fixes the use-after-free itself.
>
> The fix pins each DLCI the dispatch dereferences with its existing
> tty_port reference (option 2), so the data path stays lock-free. See the
> patch 1 commit message for details, including why the late destructor
> uses cmpxchg() so it cannot wipe a re-created mux (Daniel's teardown
> concern).
>
> Changes since v1:
> - Fix the UAF by reference-pinning instead of a NULL check in the
> handlers; no gsm->mutex in the data path (Greg, Daniel).
> - Pin every DLCI the dispatch touches, not just the addressed one:
> MSC/RLS/PN operate on gsm->dlci[k] named in the payload.
> - Add a base selftest (patch 2), as Greg asked.
>
> Verification (KASAN, panic_on_warn=1): the originally reported splat is
> the gsm_control_reply() / CMD_TEST path (see the Link in patch 1). A
> reproducer targeting the MSC handler crashes the unpatched kernel and
> survives 270 race rounds on v2. The selftest passes on both the clean
> and patched kernel (pass:3 fail:0 skip:0).
>
> Weiming Shi (2):
> tty: n_gsm: fix use-after-free in gsm_queue() control frame dispatch
> selftests: tty: add base regression test for n_gsm line discipline
So now there is a race on device node level. We have the old virtual
gsmttys still waiting for the current pending operations to finish while
the reconfiguration of the ldisc triggers a recreation of these. How is
this handled?
PATCH 1/2:
> - if (addr == 0 || addr >= NUM_DLCI || gsm->dlci[addr] == NULL)
> + if (addr == 0)
> return;
> - dlci = gsm->dlci[addr];
Control octets can be transmitted on non-control DLCIs in convergence layer
type 2. You do not guard against invalid DLCIs anymore. Same for parameter
negotiation and RLS.
> Fixes: e1eaea46bb40 ("tty: n_gsm line discipline")
> Cc: stable@vger.kernel.org
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Link: https://lore.kernel.org/all/DJ7OKN8EMAK8.22CE0B8NZXD73@gmail.com/
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
This whole worker separation was added later, not in e1eaea46bb40. Maybe
worth updating to the corresponding commit.
Also, the code comments have a strong AI flavor but AI use was only
disclosed for PATCH 2/2.
Best regards,
Daniel Starke
^ permalink raw reply
* Re: [PATCH v2 0/2] tty: n_gsm: fix gsm_queue() UAF and add a base regression test
From: Weiming Shi @ 2026-06-17 5:54 UTC (permalink / raw)
To: Greg Kroah-Hartman, Weiming Shi
Cc: Jiri Slaby, Shuah Khan, Starke, Daniel, Xiang Mei, linux-serial,
linux-kselftest, linux-kernel
In-Reply-To: <2026061722-explode-predator-59f4@gregkh>
On Wed Jun 17, 2026 at 9:24 AM CST, Greg Kroah-Hartman wrote:
> On Tue, Jun 16, 2026 at 10:32:38AM -0700, Weiming Shi wrote:
>> The receive worker walks gsm->dlci[] without gsm->mutex while a
>> concurrent GSMIOC_SETCONF -> gsm_cleanup_mux() frees the DLCIs, so the
>> control handlers can dereference a freed gsm_dlci. v1's NULL check only
>> narrowed the window; v2 fixes the use-after-free itself.
>>
>> The fix pins each DLCI the dispatch dereferences with its existing
>> tty_port reference (option 2), so the data path stays lock-free. See the
>> patch 1 commit message for details, including why the late destructor
>> uses cmpxchg() so it cannot wipe a re-created mux (Daniel's teardown
>> concern).
>
> Cool, but wow, that's complex for something that will never actually
> happen in a real device :)
>
> So do we want to add that complexity? if so, why?
>
> Ideally Daniel can verfiy this change is ok as they are the only known
> user here.
>
> And thanks for the test patch, but that's just a functional test, while
> great to have, and not one that can actually mimic a real device with
> its timing constraints, right?
>
> thanks,
>
> greg k-h
Hi,
The complexity is just the cmpxchg() in gsm_dlci_free(). The actual UAF fix
is only the tty_port pin (dlci_get/put) around the dispatch, which doesn't
touch any fast path. The cmpxchg() only guards the teardown+recreate case
Daniel mentioned, and isn't needed for the UAF.
So should I drop it and respin a v3 with the pin only? The use-after-free
is fully fixed either way.
Daniel, does the pin approach look right to you?
And yes, the test is functional only (pty bring-up/teardown, no timing).
It's the base regression test, not a race reproducer.
Thanks,
Weiming Shi
^ permalink raw reply
* Re: [PATCH v2 0/2] tty: n_gsm: fix gsm_queue() UAF and add a base regression test
From: Greg Kroah-Hartman @ 2026-06-17 1:24 UTC (permalink / raw)
To: Weiming Shi
Cc: Jiri Slaby, Shuah Khan, Starke, Daniel, Xiang Mei, linux-serial,
linux-kselftest, linux-kernel
In-Reply-To: <20260616173240.3665059-1-bestswngs@gmail.com>
On Tue, Jun 16, 2026 at 10:32:38AM -0700, Weiming Shi wrote:
> The receive worker walks gsm->dlci[] without gsm->mutex while a
> concurrent GSMIOC_SETCONF -> gsm_cleanup_mux() frees the DLCIs, so the
> control handlers can dereference a freed gsm_dlci. v1's NULL check only
> narrowed the window; v2 fixes the use-after-free itself.
>
> The fix pins each DLCI the dispatch dereferences with its existing
> tty_port reference (option 2), so the data path stays lock-free. See the
> patch 1 commit message for details, including why the late destructor
> uses cmpxchg() so it cannot wipe a re-created mux (Daniel's teardown
> concern).
Cool, but wow, that's complex for something that will never actually
happen in a real device :)
So do we want to add that complexity? if so, why?
Ideally Daniel can verfiy this change is ok as they are the only known
user here.
And thanks for the test patch, but that's just a functional test, while
great to have, and not one that can actually mimic a real device with
its timing constraints, right?
thanks,
greg k-h
^ permalink raw reply
* [PATCH v2 2/2] selftests: tty: add base regression test for n_gsm line discipline
From: Weiming Shi @ 2026-06-16 17:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Shuah Khan
Cc: Starke, Daniel, Xiang Mei, linux-serial, linux-kselftest,
linux-kernel, Weiming Shi
In-Reply-To: <20260616173240.3665059-1-bestswngs@gmail.com>
n_gsm has no selftest coverage. Add a base functional regression test
that drives the line discipline over a local pty pair, so no real serial
hardware or modem is required, and exercises the GSM 07.10 / 3GPP TS 27.010
basic-option mux from userspace.
The test attaches N_GSM to the pty master, then:
- basic: brings up the mux (SETCONF, initiator side), drives the
DLCI 0 control channel SABM/UA handshake and tears it down.
- getconf: round-trips GSMIOC_GETCONF/GSMIOC_SETCONF and checks the
configuration is preserved.
- data_dlci: opens a data DLCI (DLCI 1) via the SABM/UA exchange and
verifies the responder side answers, covering the control
-> data DLCI path.
Frames are encoded by hand against 3GPP TS 27.010 (address EA/C-R/DLCI
bits, SABM/UA/UIH control fields, the reversed CRC-8 FCS) with the clause
numbers referenced in the comments, so the test doubles as a small,
readable description of the on-wire format.
It is a functional/regression test, not a race reproducer: it gives the
subsystem a green baseline to catch behavioural regressions, including in
the gsm_queue() control-frame dispatch path.
Wire it into the tty selftest Makefile, add CONFIG_N_GSM=y to the config
fragment, and ignore the built binary. The test SKIPs cleanly when N_GSM
is not built, /dev/ptmx is missing, or it lacks the capability to attach
the ldisc.
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Assisted-by: Claude:claude-opus-4-8
---
tools/testing/selftests/tty/.gitignore | 1 +
tools/testing/selftests/tty/Makefile | 2 +-
tools/testing/selftests/tty/config | 1 +
tools/testing/selftests/tty/tty_n_gsm_test.c | 344 +++++++++++++++++++
4 files changed, 347 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/tty/tty_n_gsm_test.c
diff --git a/tools/testing/selftests/tty/.gitignore b/tools/testing/selftests/tty/.gitignore
index 2453685d2..e3fcee15e 100644
--- a/tools/testing/selftests/tty/.gitignore
+++ b/tools/testing/selftests/tty/.gitignore
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
tty_tiocsti_test
tty_tstamp_update
+tty_n_gsm_test
diff --git a/tools/testing/selftests/tty/Makefile b/tools/testing/selftests/tty/Makefile
index 7f6fbe5a0..ae546d0d4 100644
--- a/tools/testing/selftests/tty/Makefile
+++ b/tools/testing/selftests/tty/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS = -O2 -Wall
-TEST_GEN_PROGS := tty_tstamp_update tty_tiocsti_test
+TEST_GEN_PROGS := tty_tstamp_update tty_tiocsti_test tty_n_gsm_test
LDLIBS += -lcap
include ../lib.mk
diff --git a/tools/testing/selftests/tty/config b/tools/testing/selftests/tty/config
index c6373aba6..66a5ffc9e 100644
--- a/tools/testing/selftests/tty/config
+++ b/tools/testing/selftests/tty/config
@@ -1 +1,2 @@
CONFIG_LEGACY_TIOCSTI=y
+CONFIG_N_GSM=y
diff --git a/tools/testing/selftests/tty/tty_n_gsm_test.c b/tools/testing/selftests/tty/tty_n_gsm_test.c
new file mode 100644
index 000000000..064231512
--- /dev/null
+++ b/tools/testing/selftests/tty/tty_n_gsm_test.c
@@ -0,0 +1,344 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * n_gsm line discipline test
+ *
+ * Exercise the n_gsm (GSM 07.10 mux) control paths over a pty, so the driver
+ * can be regression-tested without the real modem hardware. The test attaches
+ * the ldisc, configures the mux, opens DLCI 0, drives a control frame through
+ * the receive path (reaching gsm_control_reply()) and reconfigures, which
+ * tears the mux down and frees the DLCI. It is a functional coverage test of
+ * the receive and teardown paths, not a reproducer for any specific race.
+ *
+ * The frame encoding follows 3GPP TS 07.10 (a.k.a. 27.010), basic option.
+ *
+ * Requires CONFIG_N_GSM and CAP_NET_ADMIN to attach the ldisc.
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <poll.h>
+#include <time.h>
+#include <termios.h>
+#include <sys/ioctl.h>
+#include <linux/tty.h>
+#include <linux/gsmmux.h>
+
+#include "../kselftest_harness.h"
+
+#ifndef N_GSM0710
+#define N_GSM0710 21
+#endif
+
+/*
+ * GSM 07.10 basic option framing. Field encodings below are from
+ * 3GPP TS 07.10 (a.k.a. 27.010): the basic-option flag (section 5.2.6.1),
+ * the address field with EA/C-R/DLCI bits (5.2.1.2, command vs response
+ * C/R from table 1), the control field codings (5.2.1.3, table 2), the
+ * length field (5.2.1.5), and the control-channel message types (5.4.6.3).
+ */
+#define GSM0_SOF 0xf9 /* basic-option flag, 1001 1111 */
+#define ADDR_DLCI0 0x03 /* EA=1, C/R=1, DLCI=0 (command) */
+#define ADDR_DLCI0_RSP 0x01 /* EA=1, C/R=0, DLCI=0 (response) */
+#define ADDR_DLCI1 0x07 /* EA=1, C/R=1, DLCI=1 (command) */
+#define GSM_PF 0x10 /* poll/final bit */
+#define CTRL_SABM (0x2f | GSM_PF) /* SABM command */
+#define CTRL_UA (0x63 | GSM_PF) /* UA, the SABM acknowledgment */
+#define CTRL_UIH 0xef /* UIH command/response */
+#define INIT_FCS 0xff
+#define CMD_TEST_EA 0x23 /* (CMD_TEST << 1) | EA, type 5.4.6.3.4 */
+
+#define MAX_FRAME 64
+
+/* Reversed CRC-8 table (poly 0x07), from 3GPP TS 07.10 annex B.3.5. */
+static const unsigned char gsm_fcs8[256] = {
+0x00, 0x91, 0xe3, 0x72, 0x07, 0x96, 0xe4, 0x75, 0x0e, 0x9f, 0xed, 0x7c, 0x09, 0x98, 0xea, 0x7b,
+0x1c, 0x8d, 0xff, 0x6e, 0x1b, 0x8a, 0xf8, 0x69, 0x12, 0x83, 0xf1, 0x60, 0x15, 0x84, 0xf6, 0x67,
+0x38, 0xa9, 0xdb, 0x4a, 0x3f, 0xae, 0xdc, 0x4d, 0x36, 0xa7, 0xd5, 0x44, 0x31, 0xa0, 0xd2, 0x43,
+0x24, 0xb5, 0xc7, 0x56, 0x23, 0xb2, 0xc0, 0x51, 0x2a, 0xbb, 0xc9, 0x58, 0x2d, 0xbc, 0xce, 0x5f,
+0x70, 0xe1, 0x93, 0x02, 0x77, 0xe6, 0x94, 0x05, 0x7e, 0xef, 0x9d, 0x0c, 0x79, 0xe8, 0x9a, 0x0b,
+0x6c, 0xfd, 0x8f, 0x1e, 0x6b, 0xfa, 0x88, 0x19, 0x62, 0xf3, 0x81, 0x10, 0x65, 0xf4, 0x86, 0x17,
+0x48, 0xd9, 0xab, 0x3a, 0x4f, 0xde, 0xac, 0x3d, 0x46, 0xd7, 0xa5, 0x34, 0x41, 0xd0, 0xa2, 0x33,
+0x54, 0xc5, 0xb7, 0x26, 0x53, 0xc2, 0xb0, 0x21, 0x5a, 0xcb, 0xb9, 0x28, 0x5d, 0xcc, 0xbe, 0x2f,
+0xe0, 0x71, 0x03, 0x92, 0xe7, 0x76, 0x04, 0x95, 0xee, 0x7f, 0x0d, 0x9c, 0xe9, 0x78, 0x0a, 0x9b,
+0xfc, 0x6d, 0x1f, 0x8e, 0xfb, 0x6a, 0x18, 0x89, 0xf2, 0x63, 0x11, 0x80, 0xf5, 0x64, 0x16, 0x87,
+0xd8, 0x49, 0x3b, 0xaa, 0xdf, 0x4e, 0x3c, 0xad, 0xd6, 0x47, 0x35, 0xa4, 0xd1, 0x40, 0x32, 0xa3,
+0xc4, 0x55, 0x27, 0xb6, 0xc3, 0x52, 0x20, 0xb1, 0xca, 0x5b, 0x29, 0xb8, 0xcd, 0x5c, 0x2e, 0xbf,
+0x90, 0x01, 0x73, 0xe2, 0x97, 0x06, 0x74, 0xe5, 0x9e, 0x0f, 0x7d, 0xec, 0x99, 0x08, 0x7a, 0xeb,
+0x8c, 0x1d, 0x6f, 0xfe, 0x8b, 0x1a, 0x68, 0xf9, 0x82, 0x13, 0x61, 0xf0, 0x85, 0x14, 0x66, 0xf7,
+0xa8, 0x39, 0x4b, 0xda, 0xaf, 0x3e, 0x4c, 0xdd, 0xa6, 0x37, 0x45, 0xd4, 0xa1, 0x30, 0x42, 0xd3,
+0xb4, 0x25, 0x57, 0xc6, 0xb3, 0x22, 0x50, 0xc1, 0xba, 0x2b, 0x59, 0xc8, 0xbd, 0x2c, 0x5e, 0xcf,
+};
+
+static unsigned char fcs_header(const unsigned char *p, int n)
+{
+ unsigned char fcs = INIT_FCS;
+ int i;
+
+ for (i = 0; i < n; i++)
+ fcs = gsm_fcs8[fcs ^ p[i]];
+ return 0xff - fcs;
+}
+
+/*
+ * Build a GSM0 frame: SOF addr ctrl len [data] FCS SOF.
+ * Returns the frame length, or -1 if it would not fit in MAX_FRAME.
+ */
+static int build_frame(unsigned char *out, unsigned char addr,
+ unsigned char ctrl, const unsigned char *data, int dlen)
+{
+ unsigned char hdr[3] = { addr, ctrl, (unsigned char)((dlen << 1) | 1) };
+ int i = 0, j;
+
+ if (dlen < 0 || dlen + 6 > MAX_FRAME)
+ return -1;
+
+ out[i++] = GSM0_SOF;
+ out[i++] = addr;
+ out[i++] = ctrl;
+ out[i++] = (unsigned char)((dlen << 1) | 1);
+ for (j = 0; j < dlen; j++)
+ out[i++] = data[j];
+ out[i++] = fcs_header(hdr, 3);
+ out[i++] = GSM0_SOF;
+ return i;
+}
+
+static int gsm_setconf(int fd, int mtu)
+{
+ struct gsm_config c;
+
+ memset(&c, 0, sizeof(c));
+ c.adaption = 1;
+ c.encapsulation = 0; /* basic option framing */
+ c.initiator = 0; /* responder: the peer (master side) drives DLCI 0 */
+ c.mru = 64;
+ c.mtu = mtu;
+ c.i = 1; /* UIH frames */
+ c.k = 2; /* window size */
+ /* Short timers and a single retry so open/close handshakes and the
+ * teardown complete quickly within the test.
+ */
+ c.t1 = 1;
+ c.t2 = 1;
+ c.n2 = 1;
+ return ioctl(fd, GSMIOC_SETCONF, &c);
+}
+
+/*
+ * Open a pty pair with a raw master and the n_gsm ldisc on the slave.
+ * Returns 0 and fills *mfd (master) / *sfd (slave/ldisc) on success, or
+ * -errno otherwise.
+ */
+static int gsm_open(int *mfd, int *sfd)
+{
+ int ldisc = N_GSM0710;
+ char sname[128];
+ struct termios tio;
+ int m, s, e;
+
+ m = open("/dev/ptmx", O_RDWR | O_NOCTTY);
+ if (m < 0)
+ return -errno;
+ if (grantpt(m) || unlockpt(m) || ptsname_r(m, sname, sizeof(sname))) {
+ e = errno;
+ close(m);
+ return -e;
+ }
+ s = open(sname, O_RDWR | O_NOCTTY);
+ if (s < 0) {
+ e = errno;
+ close(m);
+ return -e;
+ }
+ if (tcgetattr(m, &tio) == 0) {
+ cfmakeraw(&tio);
+ tcsetattr(m, TCSANOW, &tio);
+ }
+ if (ioctl(s, TIOCSETD, &ldisc) < 0) {
+ e = errno;
+ close(s);
+ close(m);
+ return -e;
+ }
+ *mfd = m;
+ *sfd = s;
+ return 0;
+}
+
+static long now_ms(void)
+{
+ struct timespec ts;
+
+ clock_gettime(CLOCK_MONOTONIC, &ts);
+ return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
+}
+
+/*
+ * Wait until the mux sends a frame on DLCI 0 with the given address and
+ * control field, e.g. the UA that acknowledges SABM (addr 0x03), or the
+ * CMD_TEST reply (response addr 0x01). Returns 1 if a matching frame header
+ * (SOF, addr, ctrl) is seen, 0 on timeout. Matching the SOF + address +
+ * control sequence (rather than a lone control byte) avoids false hits on
+ * FCS or payload bytes that happen to equal the control value.
+ */
+static int wait_for_frame(int mfd, unsigned char addr, unsigned char ctrl,
+ int timeout_ms)
+{
+ struct pollfd pfd = { .fd = mfd, .events = POLLIN };
+ long deadline = now_ms() + timeout_ms;
+ unsigned char buf[256];
+ int left;
+
+ while ((left = deadline - now_ms()) > 0) {
+ int ret, n, i;
+
+ ret = poll(&pfd, 1, left);
+ if (ret < 0) {
+ if (errno == EINTR)
+ continue;
+ return 0;
+ }
+ if (ret == 0 || !(pfd.revents & POLLIN))
+ continue;
+
+ n = read(mfd, buf, sizeof(buf));
+ if (n <= 0)
+ continue;
+ for (i = 0; i + 2 < n; i++) {
+ if (buf[i] == GSM0_SOF && buf[i + 1] == addr &&
+ buf[i + 2] == ctrl)
+ return 1;
+ }
+ }
+ return 0;
+}
+
+FIXTURE(n_gsm)
+{
+ int mfd; /* pty master */
+ int sfd; /* pty slave, carrying the n_gsm ldisc */
+};
+
+FIXTURE_SETUP(n_gsm)
+{
+ int ret;
+
+ /* So FIXTURE_TEARDOWN does not close fd 0 if setup bails early. */
+ self->mfd = -1;
+ self->sfd = -1;
+
+ ret = gsm_open(&self->mfd, &self->sfd);
+
+ if (ret == -EPERM || ret == -EACCES)
+ SKIP(return, "need CAP_NET_ADMIN to attach n_gsm ldisc");
+ if (ret == -EINVAL || ret == -ENODEV)
+ SKIP(return, "CONFIG_N_GSM not enabled");
+ if (ret == -ENOENT)
+ SKIP(return, "no pty support (/dev/ptmx missing)");
+ ASSERT_EQ(ret, 0)
+ TH_LOG("gsm_open failed: %d", ret);
+}
+
+FIXTURE_TEARDOWN(n_gsm)
+{
+ if (self->sfd >= 0)
+ close(self->sfd);
+ if (self->mfd >= 0)
+ close(self->mfd);
+}
+
+/*
+ * Configure the mux, open DLCI 0 and push a control frame through the receive
+ * path, then reconfigure to tear the mux down. This needs no hardware and
+ * verifies the n_gsm receive/teardown paths are reachable and do not crash.
+ */
+TEST_F(n_gsm, basic)
+{
+ unsigned char sabm[MAX_FRAME], cmd_test[MAX_FRAME];
+ /*
+ * CMD_TEST control command: cmd byte = (CMD_TEST << 1) | EA, then a
+ * length-EA byte (0x01) meaning zero bytes of test data.
+ */
+ unsigned char payload[2] = { CMD_TEST_EA, 0x01 };
+ int slen, tlen;
+
+ /* Activate the mux; this allocates DLCI 0. */
+ ASSERT_EQ(gsm_setconf(self->sfd, 64), 0)
+ TH_LOG("GSMIOC_SETCONF failed: %m");
+
+ slen = build_frame(sabm, ADDR_DLCI0, CTRL_SABM, NULL, 0);
+ tlen = build_frame(cmd_test, ADDR_DLCI0, CTRL_UIH, payload, sizeof(payload));
+ ASSERT_GT(slen, 0);
+ ASSERT_GT(tlen, 0);
+
+ /* Open DLCI 0 and wait for the UA reply confirming it reached OPEN. */
+ ASSERT_EQ(write(self->mfd, sabm, slen), slen);
+ ASSERT_EQ(wait_for_frame(self->mfd, ADDR_DLCI0, CTRL_UA, 1000), 1)
+ TH_LOG("DLCI 0 did not open (no UA reply)");
+
+ /*
+ * Drive a CMD_TEST control frame; the receive path reaches
+ * gsm_control_reply(), which sends a CMD_TEST reply back on the
+ * response address. Wait for that reply so we know the frame was
+ * processed, rather than sleeping.
+ */
+ ASSERT_EQ(write(self->mfd, cmd_test, tlen), tlen);
+ EXPECT_EQ(wait_for_frame(self->mfd, ADDR_DLCI0_RSP, CTRL_UIH, 1000), 1)
+ TH_LOG("no CMD_TEST reply seen");
+
+ /* Reconfigure: tears the mux down and frees DLCI 0. */
+ EXPECT_EQ(gsm_setconf(self->sfd, 127), 0);
+}
+
+/*
+ * Configure the mux and read the configuration back with GSMIOC_GETCONF,
+ * checking the value round-trips.
+ */
+TEST_F(n_gsm, getconf)
+{
+ struct gsm_config c;
+
+ ASSERT_EQ(gsm_setconf(self->sfd, 64), 0)
+ TH_LOG("GSMIOC_SETCONF failed: %m");
+
+ memset(&c, 0, sizeof(c));
+ ASSERT_EQ(ioctl(self->sfd, GSMIOC_GETCONF, &c), 0)
+ TH_LOG("GSMIOC_GETCONF failed: %m");
+ EXPECT_EQ(c.mtu, 64u);
+}
+
+/*
+ * Open DLCI 0, then open a data channel (DLCI 1) with SABM and check the mux
+ * acknowledges it with a UA. This exercises gsm_dlci_alloc() and the data DLCI
+ * open path, not just the control channel.
+ */
+TEST_F(n_gsm, data_dlci)
+{
+ unsigned char sabm[MAX_FRAME];
+ int slen;
+
+ ASSERT_EQ(gsm_setconf(self->sfd, 64), 0)
+ TH_LOG("GSMIOC_SETCONF failed: %m");
+
+ slen = build_frame(sabm, ADDR_DLCI0, CTRL_SABM, NULL, 0);
+ ASSERT_GT(slen, 0);
+ ASSERT_EQ(write(self->mfd, sabm, slen), slen);
+ ASSERT_EQ(wait_for_frame(self->mfd, ADDR_DLCI0, CTRL_UA, 1000), 1)
+ TH_LOG("DLCI 0 did not open");
+
+ /* Open DLCI 1 (a data channel) and wait for its UA. */
+ slen = build_frame(sabm, ADDR_DLCI1, CTRL_SABM, NULL, 0);
+ ASSERT_GT(slen, 0);
+ ASSERT_EQ(write(self->mfd, sabm, slen), slen);
+ EXPECT_EQ(wait_for_frame(self->mfd, ADDR_DLCI1, CTRL_UA, 1000), 1)
+ TH_LOG("DLCI 1 did not open (no UA reply)");
+
+ /* Tear the mux down. */
+ EXPECT_EQ(gsm_setconf(self->sfd, 127), 0);
+}
+
+TEST_HARNESS_MAIN
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/2] tty: n_gsm: fix use-after-free in gsm_queue() control frame dispatch
From: Weiming Shi @ 2026-06-16 17:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Shuah Khan
Cc: Starke, Daniel, Xiang Mei, linux-serial, linux-kselftest,
linux-kernel, Weiming Shi, stable
In-Reply-To: <20260616173240.3665059-1-bestswngs@gmail.com>
The receive worker (flush_to_ldisc -> gsmld_receive_buf -> gsm0_receive/
gsm1_receive -> gsm_queue) reads gsm->dlci[address] and dispatches the
frame via dlci->data() without holding gsm->mutex. The control handlers
reached through dlci->data() then re-read gsm->dlci[]: gsm_control_reply()
re-reads gsm->dlci[0], while gsm_control_modem() (MSC), gsm_control_rls()
(RLS) and gsm_control_negotiation() (PN) re-read gsm->dlci[addr] for the
DLCI named in the command - a different channel from the one the frame
was addressed to.
Concurrently GSMIOC_SETCONF -> gsm_config() -> gsm_cleanup_mux() takes
gsm->mutex and releases every DLCI via gsm_dlci_release() -> dlci_put().
When the last reference is dropped the destructor gsm_dlci_free() clears
gsm->dlci[addr] and frees the object. If the worker dereferences one of
those DLCIs while it is being freed, it touches freed memory.
A peer that drives DLCI 0 control frames (e.g. CMD_TEST) while the mux
owner reconfigures the line discipline with GSMIOC_SETCONF can therefore
trigger a use-after-free:
BUG: KASAN: slab-use-after-free in gsm_control_reply.isra.0
Read of size 8 at addr ffff888029ae9000 by task kworker/u16:2/46
Workqueue: events_unbound flush_to_ldisc
Call Trace:
gsm_control_reply.isra.0 (drivers/tty/n_gsm.c:1494)
gsm_dlci_command (drivers/tty/n_gsm.c:2477)
gsmld_receive_buf (drivers/tty/n_gsm.c:3616)
tty_ldisc_receive_buf (drivers/tty/tty_buffer.c:398)
tty_port_default_receive_buf (drivers/tty/tty_port.c:37)
flush_to_ldisc (drivers/tty/tty_buffer.c:502)
process_one_work
worker_thread
kthread
Freed by task 5110:
kfree
gsm_cleanup_mux (drivers/tty/n_gsm.c:3161)
gsmld_ioctl (drivers/tty/n_gsm.c:3415)
tty_ioctl
Pin each DLCI across the dereference with its existing tty_port reference.
gsm_dlci_open_get() looks gsm->dlci[addr] up under gsm->mutex and, if
present, takes a dlci_get() reference before dropping the mutex; the
caller releases it with gsm_dlci_unget() once it is done. While the
reference is held the kref cannot reach zero, so gsm_dlci_free() cannot
run: the object stays live and gsm->dlci[addr] is not cleared. gsm_queue()
pins the addressed DLCI for the UI/UIH dispatch, and gsm_control_modem(),
gsm_control_rls() and gsm_control_negotiation() each pin the DLCI they
operate on.
The reference is taken only under the mutex, around the lookup; the mutex
is released before dlci->data() and before the data-path work
(gsm_process_modem(), tty_flip_buffer_push(), gsm_data_queue(), ...), so
the receive/transmit path is not serialised by gsm->mutex and its timing
is unaffected.
Because a pinned DLCI can outlive the gsm_cleanup_mux() that released it,
a subsequent GSMIOC_SETCONF may re-create a DLCI at the same address
before the worker drops its reference. Make gsm_dlci_free() clear the
slot only if it still points at the DLCI being freed, so the late
destructor cannot wipe a freshly installed DLCI:
cmpxchg(&dlci->gsm->dlci[dlci->addr], dlci, NULL);
Attaching the n_gsm line discipline requires CAP_NET_ADMIN (gsmld_open()
uses capable(), not ns_capable()), so this is a local denial of service
for a privileged mux owner whose control channel is driven by an
untrusted peer on the serial link while it reconfigures; harden the
receive path regardless.
Fixes: e1eaea46bb40 ("tty: n_gsm line discipline")
Cc: stable@vger.kernel.org
Reported-by: Xiang Mei <xmei5@asu.edu>
Link: https://lore.kernel.org/all/DJ7OKN8EMAK8.22CE0B8NZXD73@gmail.com/
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
drivers/tty/n_gsm.c | 105 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 96 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c13e050de..e1ab3a08f 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -453,6 +453,8 @@ static const u8 gsm_fcs8[256] = {
#define GOOD_FCS 0xCF
static void gsm_dlci_close(struct gsm_dlci *dlci);
+static struct gsm_dlci *gsm_dlci_open_get(struct gsm_mux *gsm, unsigned int addr);
+static void gsm_dlci_unget(struct gsm_dlci *dlci);
static int gsmld_output(struct gsm_mux *gsm, u8 *data, int len);
static int gsm_modem_update(struct gsm_dlci *dlci, u8 brk);
static struct gsm_msg *gsm_data_alloc(struct gsm_mux *gsm, u8 addr, int len,
@@ -1694,10 +1696,8 @@ static void gsm_control_modem(struct gsm_mux *gsm, const u8 *data, int clen)
return;
addr >>= 1;
- /* Closed port, or invalid ? */
- if (addr == 0 || addr >= NUM_DLCI || gsm->dlci[addr] == NULL)
+ if (addr == 0)
return;
- dlci = gsm->dlci[addr];
/* Must be at least one byte following the EA */
if ((cl - len) < 1)
@@ -1711,12 +1711,23 @@ static void gsm_control_modem(struct gsm_mux *gsm, const u8 *data, int clen)
if (len < 1)
return;
+ /*
+ * Pin the addressed DLCI across the dereference: a concurrent
+ * GSMIOC_SETCONF -> gsm_cleanup_mux() can free it otherwise. Pinning
+ * (not gsm->mutex over the whole handler) keeps the data path lock
+ * free.
+ */
+ dlci = gsm_dlci_open_get(gsm, addr);
+ if (dlci == NULL)
+ return;
+
tty = tty_port_tty_get(&dlci->port);
gsm_process_modem(tty, dlci, modem, cl);
if (tty) {
tty_wakeup(tty);
tty_kref_put(tty);
}
+ gsm_dlci_unget(dlci);
gsm_control_reply(gsm, CMD_MSC, data, clen);
}
@@ -1746,15 +1757,26 @@ static void gsm_control_negotiation(struct gsm_mux *gsm, unsigned int cr,
/* Invalid DLCI? */
params = (struct gsm_dlci_param_bits *)data;
addr = FIELD_GET(PN_D_FIELD_DLCI, params->d_bits);
- if (addr == 0 || addr >= NUM_DLCI || !gsm->dlci[addr]) {
+ if (addr == 0) {
+ gsm->open_error++;
+ return;
+ }
+
+ /*
+ * Pin the addressed DLCI across the negotiation; see gsm_control_modem()
+ * for why. Unlike MSC/RLS this DLCI need not be open, so pin first and
+ * check the state afterwards.
+ */
+ dlci = gsm_dlci_open_get(gsm, addr);
+ if (dlci == NULL) {
gsm->open_error++;
return;
}
- dlci = gsm->dlci[addr];
/* Too late for parameter negotiation? */
if ((!cr && dlci->state == DLCI_OPENING) || dlci->state == DLCI_OPEN) {
gsm->open_error++;
+ gsm_dlci_unget(dlci);
return;
}
@@ -1765,6 +1787,7 @@ static void gsm_control_negotiation(struct gsm_mux *gsm, unsigned int cr,
pr_info("%s PN failed\n", __func__);
gsm->open_error++;
gsm_dlci_close(dlci);
+ gsm_dlci_unget(dlci);
return;
}
@@ -1785,6 +1808,7 @@ static void gsm_control_negotiation(struct gsm_mux *gsm, unsigned int cr,
pr_info("%s PN in invalid state\n", __func__);
gsm->open_error++;
}
+ gsm_dlci_unget(dlci);
}
/**
@@ -1800,6 +1824,7 @@ static void gsm_control_negotiation(struct gsm_mux *gsm, unsigned int cr,
static void gsm_control_rls(struct gsm_mux *gsm, const u8 *data, int clen)
{
+ struct gsm_dlci *dlci;
struct tty_port *port;
unsigned int addr = 0;
u8 bits;
@@ -1816,15 +1841,21 @@ static void gsm_control_rls(struct gsm_mux *gsm, const u8 *data, int clen)
if (len <= 0)
return;
addr >>= 1;
- /* Closed port, or invalid ? */
- if (addr == 0 || addr >= NUM_DLCI || gsm->dlci[addr] == NULL)
+ if (addr == 0)
return;
/* No error ? */
bits = *dp;
if ((bits & 1) == 0)
return;
- port = &gsm->dlci[addr]->port;
+ /*
+ * Pin the addressed DLCI across the dereference; see gsm_control_modem()
+ * for why. gsm_cleanup_mux() can free it concurrently otherwise.
+ */
+ dlci = gsm_dlci_open_get(gsm, addr);
+ if (dlci == NULL)
+ return;
+ port = &dlci->port;
if (bits & 2)
tty_insert_flip_char(port, 0, TTY_OVERRUN);
@@ -1835,6 +1866,7 @@ static void gsm_control_rls(struct gsm_mux *gsm, const u8 *data, int clen)
tty_flip_buffer_push(port);
+ gsm_dlci_unget(dlci);
gsm_control_reply(gsm, CMD_RLS, data, clen);
}
@@ -2694,7 +2726,14 @@ static void gsm_dlci_free(struct tty_port *port)
struct gsm_dlci *dlci = container_of(port, struct gsm_dlci, port);
timer_shutdown_sync(&dlci->t1);
- dlci->gsm->dlci[dlci->addr] = NULL;
+ /*
+ * Only clear the slot if it still points at us. A receive worker can
+ * pin this DLCI across gsm_queue() dispatch with dlci_get(); if a
+ * concurrent GSMIOC_SETCONF tears the mux down and re-creates a DLCI
+ * at the same address before the worker drops its reference, the slot
+ * already refers to the new DLCI and must not be cleared here.
+ */
+ cmpxchg(&dlci->gsm->dlci[dlci->addr], dlci, NULL);
kfifo_free(&dlci->fifo);
while ((dlci->skb = skb_dequeue(&dlci->skb_list)))
dev_kfree_skb(dlci->skb);
@@ -2711,6 +2750,42 @@ static inline void dlci_put(struct gsm_dlci *dlci)
tty_port_put(&dlci->port);
}
+/**
+ * gsm_dlci_open_get - look up a DLCI and pin it
+ * @gsm: GSM mux
+ * @addr: DLCI address
+ *
+ * Look up gsm->dlci[addr] under gsm->mutex and, if present, take a
+ * tty_port reference so it cannot be freed while a control-frame handler
+ * dereferences it. A concurrent GSMIOC_SETCONF -> gsm_cleanup_mux()
+ * releases DLCIs under the same mutex, so the lookup and the pin are
+ * atomic with respect to the teardown. Returns the pinned DLCI or NULL.
+ * The caller must release it with gsm_dlci_unget(). Callers that require
+ * a particular state must check dlci->state themselves.
+ */
+static struct gsm_dlci *gsm_dlci_open_get(struct gsm_mux *gsm, unsigned int addr)
+{
+ struct gsm_dlci *dlci;
+
+ if (addr >= NUM_DLCI)
+ return NULL;
+ mutex_lock(&gsm->mutex);
+ dlci = gsm->dlci[addr];
+ if (dlci != NULL)
+ dlci_get(dlci);
+ mutex_unlock(&gsm->mutex);
+ return dlci;
+}
+
+/**
+ * gsm_dlci_unget - drop a reference from gsm_dlci_open_get()
+ * @dlci: DLCI to release
+ */
+static void gsm_dlci_unget(struct gsm_dlci *dlci)
+{
+ dlci_put(dlci);
+}
+
static void gsm_destroy_network(struct gsm_dlci *dlci);
/**
@@ -2839,11 +2914,23 @@ static void gsm_queue(struct gsm_mux *gsm)
case UI|PF:
case UIH:
case UIH|PF:
+ /*
+ * Pin the DLCI so a concurrent gsm_cleanup_mux() cannot free
+ * it while dlci->data() and the handlers it reaches use it.
+ * The mutex is dropped before the dispatch, so the data path
+ * is not serialised.
+ */
+ mutex_lock(&gsm->mutex);
+ dlci = gsm->dlci[address];
if (dlci == NULL || dlci->state != DLCI_OPEN) {
+ mutex_unlock(&gsm->mutex);
gsm_response(gsm, address, DM|PF);
return;
}
+ dlci_get(dlci);
+ mutex_unlock(&gsm->mutex);
dlci->data(dlci, gsm->buf, gsm->len);
+ dlci_put(dlci);
break;
default:
goto invalid;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/2] tty: n_gsm: fix gsm_queue() UAF and add a base regression test
From: Weiming Shi @ 2026-06-16 17:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Shuah Khan
Cc: Starke, Daniel, Xiang Mei, linux-serial, linux-kselftest,
linux-kernel, Weiming Shi
The receive worker walks gsm->dlci[] without gsm->mutex while a
concurrent GSMIOC_SETCONF -> gsm_cleanup_mux() frees the DLCIs, so the
control handlers can dereference a freed gsm_dlci. v1's NULL check only
narrowed the window; v2 fixes the use-after-free itself.
The fix pins each DLCI the dispatch dereferences with its existing
tty_port reference (option 2), so the data path stays lock-free. See the
patch 1 commit message for details, including why the late destructor
uses cmpxchg() so it cannot wipe a re-created mux (Daniel's teardown
concern).
Changes since v1:
- Fix the UAF by reference-pinning instead of a NULL check in the
handlers; no gsm->mutex in the data path (Greg, Daniel).
- Pin every DLCI the dispatch touches, not just the addressed one:
MSC/RLS/PN operate on gsm->dlci[k] named in the payload.
- Add a base selftest (patch 2), as Greg asked.
Verification (KASAN, panic_on_warn=1): the originally reported splat is
the gsm_control_reply() / CMD_TEST path (see the Link in patch 1). A
reproducer targeting the MSC handler crashes the unpatched kernel and
survives 270 race rounds on v2. The selftest passes on both the clean
and patched kernel (pass:3 fail:0 skip:0).
Weiming Shi (2):
tty: n_gsm: fix use-after-free in gsm_queue() control frame dispatch
selftests: tty: add base regression test for n_gsm line discipline
drivers/tty/n_gsm.c | 105 +++++-
tools/testing/selftests/tty/.gitignore | 1 +
tools/testing/selftests/tty/Makefile | 2 +-
tools/testing/selftests/tty/config | 1 +
tools/testing/selftests/tty/tty_n_gsm_test.c | 344 +++++++++++++++++++
5 files changed, 443 insertions(+), 10 deletions(-)
create mode 100644 tools/testing/selftests/tty/tty_n_gsm_test.c
--
2.43.0
^ permalink raw reply
* Re: [PATCH v3 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes
From: Rob Herring (Arm) @ 2026-06-16 17:28 UTC (permalink / raw)
To: Tapio Reijonen
Cc: linux-serial, Conor Dooley, Hugo Villeneuve, Jiri Slaby,
linux-kernel, Greg Kroah-Hartman, devicetree, Krzysztof Kozlowski
In-Reply-To: <20260615-b4-max310x-rs485-dt-v3-2-7e79f064bdd7@vaisala.com>
On Mon, 15 Jun 2026 10:27:36 +0000, Tapio Reijonen wrote:
> The MAX310x is a family of one- (max3107, max3108), two- (max3109) and
> four-channel (max14830) UARTs. The binding pulls in
> /schemas/serial/rs485.yaml at the chip level, describing a single set of
> RS-485 properties - enough for the single-channel parts, but a
> multi-channel chip can wire RS-485 differently on each channel.
>
> Split the binding per compatible:
>
> - single-channel parts (max3107, max3108): the chip node is itself the
> serial port and carries the RS-485 properties, as before;
>
> - multi-channel parts (max3109, max14830): the chip node is only a
> container and is no longer a serial node; each channel is a "serial@N"
> subnode that carries the standard serial.yaml/rs485.yaml properties
> (and may host a serial slave device). max3109 has channels 0-1,
> max14830 has 0-3.
>
> This avoids a chip node that is simultaneously a serial node and the
> parent of serial nodes. The driver still reads chip-level RS-485 for
> single-channel and legacy device trees, so existing users are unaffected.
>
> Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
> ---
> .../devicetree/bindings/serial/maxim,max310x.yaml | 92 +++++++++++++++++++++-
> 1 file changed, 90 insertions(+), 2 deletions(-)
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3] scsi: ufs: sysfs: Add HS_GEAR6 string in power_info/gear sysfs output
From: Bart Van Assche @ 2026-06-16 13:37 UTC (permalink / raw)
To: Himanshu Batra, Alim Akhtar, Avri Altman
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
linux-kernel, linux-serial, vamshigajjela, manugautam
In-Reply-To: <20260616100121.548759-1-himanshubatra@google.com>
On 6/16/26 3:01 AM, Himanshu Batra wrote:
> In power_info/gear sysfs, currently it supports output only till gear 5.
> If operating mode is gear 6, it outputs "UNKNOWN".
> Add support for HS_GEAR6 string in sysfs output when operating mode
> is gear 6.
Some general advice:
- New versions of a patch should be posted as a new email thread instead
of as a reply to an existing email conversation. Replies to an
existing email conversation tend to get overlooked.
- At least 24 hours should elapse before a new version of a patch is
posted. Otherwise reviewers who are in another time zone don't have
the chance to reply. For large patch series, more time should elapse
between reposts (e.g. one week).
- The "scsi:" prefix is no longer used for UFS kernel patches.
Since the code changes look good to me:
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply
* [PATCH v3] scsi: ufs: sysfs: Add HS_GEAR6 string in power_info/gear sysfs output
From: Himanshu Batra @ 2026-06-16 10:01 UTC (permalink / raw)
To: Alim Akhtar, Avri Altman, Bart Van Assche
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
linux-kernel, linux-serial, vamshigajjela, manugautam,
Himanshu Batra
In-Reply-To: <2026061659-enjoyer-boogeyman-25c0@gregkh>
In power_info/gear sysfs, currently it supports output only till gear 5.
If operating mode is gear 6, it outputs "UNKNOWN".
Add support for HS_GEAR6 string in sysfs output when operating mode
is gear 6.
Signed-off-by: Himanshu Batra <himanshubatra@google.com>
---
Changes in v3:
- Fixed the identity alignment issue: updated both the 'From:' line
and 'Signed-off-by:' tag to use my full real name ("Himanshu Batra")
Changes in v2:
- A slightly better comment.
drivers/ufs/core/ufs-sysfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 99af3c73f1af..d1f5041fc3c8 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -54,6 +54,7 @@ static const char *ufs_hs_gear_to_string(enum ufs_hs_gear_tag gear)
case UFS_HS_G3: return "HS_GEAR3";
case UFS_HS_G4: return "HS_GEAR4";
case UFS_HS_G5: return "HS_GEAR5";
+ case UFS_HS_G6: return "HS_GEAR6";
default: return "UNKNOWN";
}
}
--
2.54.0.1189.g8c84645362-goog
^ permalink raw reply related
* Re: [PATCH v2] scsi: ufs: sysfs: Add HS_GEAR6 string in power_info/gear sysfs output
From: Greg KH @ 2026-06-16 9:44 UTC (permalink / raw)
To: himanshubatra
Cc: Alim Akhtar, Avri Altman, Bart Van Assche, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, linux-kernel, linux-serial,
vamshigajjela, manugautam
In-Reply-To: <20260616083124.267262-1-himanshubatra@google.com>
On Tue, Jun 16, 2026 at 02:01:24PM +0530, himanshubatra wrote:
> In power_info/gear sysfs, currently it supports output only till gear 5.
> If operating mode is gear 6, it outputs "UNKNOWN".
> Add support for HS_GEAR6 string in sysfs output when operating mode
> is gear 6.
>
> Signed-off-by: himanshubatra <himanshubatra@google.com>
> ---
>
> Changes in v2:
> - A slightly better comment.
>
> drivers/ufs/core/ufs-sysfs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
> index 99af3c73f1af..d1f5041fc3c8 100644
> --- a/drivers/ufs/core/ufs-sysfs.c
> +++ b/drivers/ufs/core/ufs-sysfs.c
> @@ -54,6 +54,7 @@ static const char *ufs_hs_gear_to_string(enum ufs_hs_gear_tag gear)
> case UFS_HS_G3: return "HS_GEAR3";
> case UFS_HS_G4: return "HS_GEAR4";
> case UFS_HS_G5: return "HS_GEAR5";
> + case UFS_HS_G6: return "HS_GEAR6";
> default: return "UNKNOWN";
> }
> }
> --
> 2.54.0.1189.g8c84645362-goog
>
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- It looks like you did not use your "real" name for the patch on either
the Signed-off-by: line, or the From: line (both of which have to
match). Please read the kernel file,
Documentation/process/submitting-patches.rst for how to do this
correctly.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* [PATCH v2] scsi: ufs: sysfs: Add HS_GEAR6 string in power_info/gear sysfs output
From: himanshubatra @ 2026-06-16 8:31 UTC (permalink / raw)
To: Alim Akhtar, Avri Altman, Bart Van Assche
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
linux-kernel, linux-serial, vamshigajjela, manugautam,
himanshubatra
In-Reply-To: <CAEif7DR3KUdhPww-q_Fn6gR6L=V5nrD3EyLn3vi+hWLmS3eU6g@mail.gmail.com>
In power_info/gear sysfs, currently it supports output only till gear 5.
If operating mode is gear 6, it outputs "UNKNOWN".
Add support for HS_GEAR6 string in sysfs output when operating mode
is gear 6.
Signed-off-by: himanshubatra <himanshubatra@google.com>
---
Changes in v2:
- A slightly better comment.
drivers/ufs/core/ufs-sysfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 99af3c73f1af..d1f5041fc3c8 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -54,6 +54,7 @@ static const char *ufs_hs_gear_to_string(enum ufs_hs_gear_tag gear)
case UFS_HS_G3: return "HS_GEAR3";
case UFS_HS_G4: return "HS_GEAR4";
case UFS_HS_G5: return "HS_GEAR5";
+ case UFS_HS_G6: return "HS_GEAR6";
default: return "UNKNOWN";
}
}
--
2.54.0.1189.g8c84645362-goog
^ permalink raw reply related
* Re: [PATCH] ufs: Add HS_GEAR6 string in power_info/gear sysfs output
From: Himanshu Batra @ 2026-06-16 8:27 UTC (permalink / raw)
To: VAMSHI GAJJELA
Cc: Alim Akhtar, Avri Altman, Bart Van Assche, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, linux-kernel, linux-serial,
manugautam
In-Reply-To: <CAMTSyjpR455jn8pMFe43ozp1b0Jj9Vy9dKuD8LrcbMCY-cT+zA@mail.gmail.com>
On Tue, Jun 16, 2026 at 9:52 AM VAMSHI GAJJELA <vamshigajjela@google.com> wrote:
>
> scsi: ufs: sysfs: Add ....
>
Done in v2
> On Mon, Jun 15, 2026 at 7:56 PM himanshubatra <himanshubatra@google.com> wrote:
> >
> > In power_info/gear sysfs, currently it supports output only till gear 5.
> > If operating mode is gear 6, it is giving output as "UNKNOWN".
> it outputs "UNKNOWN"
Done in v2
> > Add support for HS_GEAR6 string in sysfs output when operating mode
> > is gear 6.
> >
> > Signed-off-by: himanshubatra <himanshubatra@google.com>
> > ---
> > drivers/ufs/core/ufs-sysfs.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
> > index 99af3c73f1af..d1f5041fc3c8 100644
> > --- a/drivers/ufs/core/ufs-sysfs.c
> > +++ b/drivers/ufs/core/ufs-sysfs.c
> > @@ -54,6 +54,7 @@ static const char *ufs_hs_gear_to_string(enum ufs_hs_gear_tag gear)
> > case UFS_HS_G3: return "HS_GEAR3";
> > case UFS_HS_G4: return "HS_GEAR4";
> > case UFS_HS_G5: return "HS_GEAR5";
> > + case UFS_HS_G6: return "HS_GEAR6";
> > default: return "UNKNOWN";
> > }
> > }
> > --
> > 2.54.0.1189.g8c84645362-goog
> >
^ permalink raw reply
* Re: [PATCH v2 6.12.y 00/10] serial: 8250_dw: backport BUSY deassert series
From: Ilpo Järvinen @ 2026-06-16 7:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Ionut Nechita (Wind River), stable, Andy Shevchenko, wander,
chris.friesen, linux-serial
In-Reply-To: <2026061641-cozy-creatable-9a9e@gregkh>
[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]
On Tue, 16 Jun 2026, Greg Kroah-Hartman wrote:
> On Wed, May 13, 2026 at 01:16:31PM +0300, Ilpo Järvinen wrote:
> > On Wed, 13 May 2026, Ionut Nechita (Wind River) wrote:
> >
> > > From: Ionut Nechita <ionut.nechita@windriver.com>
> > >
> > > Hi Greg, Ilpo,
> > >
> > > This is v2 of the 8250_dw BUSY deassert backport to 6.12.y,
> > > addressing Ilpo's review feedback on v1.
> >
> > FYI, this came up yesterday related to guard()s vs unlock variants:
> >
> > https://lore.kernel.org/linux-serial/cover.1778592805.git.jnilo@free.fr/
>
> Yeah, I'm not going to take these now, I'd like to see a lot more
> testing and some actual reasons why this is needed in this tree. So far
> windriver's track-record for backports is really low/bad so I'll just
> drop them from my review queue right now.
To be fair, it was me who strongly suggested that the backport should do
the guard() conversion as well, which contained the bug linked above (and
the cause there too, was my handiwork), while they initially kept using
open-coded lock/unlock variants which would not have been subject to that
problem.
But yes, you guys are the stable maintainers so what to take is your
decision.
--
i.
^ permalink raw reply
* Re: 8250_dw system pause due to IRQ load
From: Greg KH @ 2026-06-16 7:52 UTC (permalink / raw)
To: Craig McQueen; +Cc: linux-serial
In-Reply-To: <19ecf63b8bf.3059f90c1678338.4949518025835217958@mcqueen.au>
On Tue, Jun 16, 2026 at 05:44:39PM +1000, Craig McQueen wrote:
> I previously wrote:
>
> > I have a Rockchip RK3328 based embedded Linux system, using the 8250_dw driver (device tree "snps,dw-apb-uart") for serial console and other serial ports. I'm using Yocto scarthgap with kernel v6.6.123.
> >
> > It is talking to a microprocessor via a serial protocol at 921600 bps. Multiple times per hour, I see the serial protocol TX pause for 100 to 4500 ms. Usually the whole Linux system pauses during this time (realtime and monotonic clocks don't tick). mpstat shows high irq load. /proc/interrupts shows the 8250_dw interrupt count is going significantly higher during this time.
> >
> > I'm also seeing complete system lock-ups occur every 1 to 72 hours, with no diagnostic information shown in the kernel serial console output.
> >
> > Are there any known issues with the 8250_dw interrupt handler causing high CPU load, that I should try backporting to kernel v6.6?
> >
> > I've written some kernel drivers, but I have no experience debugging interrupt handler issues, especially when it's an issue that prevents the kernel doing console output. I would appreciate any advice on kernel facilities that are suitable to debug this type of bug.
>
> I have been able to diagnose serial TX pauses, using trace_printk() in the interrupt handler. The cause of TX pauses is many repeated `UART_IIR_RX_TIMEOUT` interrupts. The serial device appears to randomly get out of this state.
>
> I see the 8250_dw interrupt handler has a work-around to stop these `UART_IIR_RX_TIMEOUT` interrupts when the FIFO is empty. But it has only been enabled for non-DMA mode. But for the Rockchip RK3328, the serial device is configured for DMA mode. But in our usage, we're still seeing this issue randomly appear.
>
> I have modified the 8250_dw interrupt handler to do the work-around even in DMA mode. This seems to resolve the repeated `UART_IIR_RX_TIMEOUT` interrupts, and eliminate the TX pauses.
>
> My testing shows that this doesn't fix my other problem, of complete system lock-ups. I don't yet know if that is also 8250_dw related.
Note, many changes have happened in this driver since 6.6.y, can you try
the 7.1 release to see if it has been resolved there?
thanks,
greg k-h
^ permalink raw reply
* Re:8250_dw system pause due to IRQ load
From: Craig McQueen @ 2026-06-16 7:44 UTC (permalink / raw)
To: linux-serial
In-Reply-To: <19eba567d94.1f5854744141363.4614464714670279886@mcqueen.au>
I previously wrote:
> I have a Rockchip RK3328 based embedded Linux system, using the 8250_dw driver (device tree "snps,dw-apb-uart") for serial console and other serial ports. I'm using Yocto scarthgap with kernel v6.6.123.
>
> It is talking to a microprocessor via a serial protocol at 921600 bps. Multiple times per hour, I see the serial protocol TX pause for 100 to 4500 ms. Usually the whole Linux system pauses during this time (realtime and monotonic clocks don't tick). mpstat shows high irq load. /proc/interrupts shows the 8250_dw interrupt count is going significantly higher during this time.
>
> I'm also seeing complete system lock-ups occur every 1 to 72 hours, with no diagnostic information shown in the kernel serial console output.
>
> Are there any known issues with the 8250_dw interrupt handler causing high CPU load, that I should try backporting to kernel v6.6?
>
> I've written some kernel drivers, but I have no experience debugging interrupt handler issues, especially when it's an issue that prevents the kernel doing console output. I would appreciate any advice on kernel facilities that are suitable to debug this type of bug.
I have been able to diagnose serial TX pauses, using trace_printk() in the interrupt handler. The cause of TX pauses is many repeated `UART_IIR_RX_TIMEOUT` interrupts. The serial device appears to randomly get out of this state.
I see the 8250_dw interrupt handler has a work-around to stop these `UART_IIR_RX_TIMEOUT` interrupts when the FIFO is empty. But it has only been enabled for non-DMA mode. But for the Rockchip RK3328, the serial device is configured for DMA mode. But in our usage, we're still seeing this issue randomly appear.
I have modified the 8250_dw interrupt handler to do the work-around even in DMA mode. This seems to resolve the repeated `UART_IIR_RX_TIMEOUT` interrupts, and eliminate the TX pauses.
My testing shows that this doesn't fix my other problem, of complete system lock-ups. I don't yet know if that is also 8250_dw related.
--
Craig McQueen
^ permalink raw reply
* Re: [PATCH] ufs: Add HS_GEAR6 string in power_info/gear sysfs output
From: VAMSHI GAJJELA @ 2026-06-16 4:22 UTC (permalink / raw)
To: himanshubatra
Cc: Alim Akhtar, Avri Altman, Bart Van Assche, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, linux-kernel, linux-serial,
manugautam
In-Reply-To: <20260615142605.2795757-1-himanshubatra@google.com>
scsi: ufs: sysfs: Add ....
On Mon, Jun 15, 2026 at 7:56 PM himanshubatra <himanshubatra@google.com> wrote:
>
> In power_info/gear sysfs, currently it supports output only till gear 5.
> If operating mode is gear 6, it is giving output as "UNKNOWN".
it outputs "UNKNOWN"
> Add support for HS_GEAR6 string in sysfs output when operating mode
> is gear 6.
>
> Signed-off-by: himanshubatra <himanshubatra@google.com>
> ---
> drivers/ufs/core/ufs-sysfs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
> index 99af3c73f1af..d1f5041fc3c8 100644
> --- a/drivers/ufs/core/ufs-sysfs.c
> +++ b/drivers/ufs/core/ufs-sysfs.c
> @@ -54,6 +54,7 @@ static const char *ufs_hs_gear_to_string(enum ufs_hs_gear_tag gear)
> case UFS_HS_G3: return "HS_GEAR3";
> case UFS_HS_G4: return "HS_GEAR4";
> case UFS_HS_G5: return "HS_GEAR5";
> + case UFS_HS_G6: return "HS_GEAR6";
> default: return "UNKNOWN";
> }
> }
> --
> 2.54.0.1189.g8c84645362-goog
>
^ permalink raw reply
* Re: [PATCH v2 6.12.y 00/10] serial: 8250_dw: backport BUSY deassert series
From: Greg Kroah-Hartman @ 2026-06-16 3:43 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Ionut Nechita (Wind River), stable, Andy Shevchenko, wander,
chris.friesen, linux-serial
In-Reply-To: <c9f3545f-4522-4b78-7398-b364a4033a77@linux.intel.com>
On Wed, May 13, 2026 at 01:16:31PM +0300, Ilpo Järvinen wrote:
> On Wed, 13 May 2026, Ionut Nechita (Wind River) wrote:
>
> > From: Ionut Nechita <ionut.nechita@windriver.com>
> >
> > Hi Greg, Ilpo,
> >
> > This is v2 of the 8250_dw BUSY deassert backport to 6.12.y,
> > addressing Ilpo's review feedback on v1.
>
> FYI, this came up yesterday related to guard()s vs unlock variants:
>
> https://lore.kernel.org/linux-serial/cover.1778592805.git.jnilo@free.fr/
Yeah, I'm not going to take these now, I'd like to see a lot more
testing and some actual reasons why this is needed in this tree. So far
windriver's track-record for backports is really low/bad so I'll just
drop them from my review queue right now.
thanks,
greg k-h
^ permalink raw reply
* [PATCH] ufs: Add HS_GEAR6 string in power_info/gear sysfs output
From: himanshubatra @ 2026-06-15 14:26 UTC (permalink / raw)
To: Alim Akhtar, Avri Altman, Bart Van Assche
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
linux-kernel, linux-serial, vamshigajjela, manugautam,
himanshubatra
In power_info/gear sysfs, currently it supports output only till gear 5.
If operating mode is gear 6, it is giving output as "UNKNOWN".
Add support for HS_GEAR6 string in sysfs output when operating mode
is gear 6.
Signed-off-by: himanshubatra <himanshubatra@google.com>
---
drivers/ufs/core/ufs-sysfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 99af3c73f1af..d1f5041fc3c8 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -54,6 +54,7 @@ static const char *ufs_hs_gear_to_string(enum ufs_hs_gear_tag gear)
case UFS_HS_G3: return "HS_GEAR3";
case UFS_HS_G4: return "HS_GEAR4";
case UFS_HS_G5: return "HS_GEAR5";
+ case UFS_HS_G6: return "HS_GEAR6";
default: return "UNKNOWN";
}
}
--
2.54.0.1189.g8c84645362-goog
^ permalink raw reply related
* [PATCH v5 2/2] serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
From: Praveen Talari @ 2026-06-15 14:16 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Greg Kroah-Hartman, Jiri Slaby, konrad.dybcio
Cc: Praveen Talari, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, mukesh.savaliya, aniket.randive, chandana.chiluveru
In-Reply-To: <20260615-add-tracepoints-for-qcom-geni-serial-v5-0-2efa4c97e0e2@oss.qualcomm.com>
Add tracing to the Qualcomm GENI serial driver to improve runtime
observability.
Trace hooks are added at key points including termios and clock
configuration, manual control get/set, interrupt handling, and data
TX/RX paths.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v2->v3:
- Updated commit text(removed example as it was available on cover
letter).
---
drivers/tty/serial/qcom_geni_serial.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index d81b539cff7f..4b62e58d4918 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -7,6 +7,9 @@
/* Disable MMIO tracing to prevent excessive logging of unwanted MMIO traces */
#define __DISABLE_TRACE_MMIO__
+#define CREATE_TRACE_POINTS
+#include <trace/events/qcom_geni_serial.h>
+
#include <linux/clk.h>
#include <linux/console.h>
#include <linux/io.h>
@@ -226,7 +229,7 @@ static void qcom_geni_serial_config_port(struct uart_port *uport, int cfg_flags)
static unsigned int qcom_geni_serial_get_mctrl(struct uart_port *uport)
{
unsigned int mctrl = TIOCM_DSR | TIOCM_CAR;
- u32 geni_ios;
+ u32 geni_ios = 0;
if (uart_console(uport)) {
mctrl |= TIOCM_CTS;
@@ -236,6 +239,8 @@ static unsigned int qcom_geni_serial_get_mctrl(struct uart_port *uport)
mctrl |= TIOCM_CTS;
}
+ trace_geni_serial_get_mctrl(uport->dev, mctrl, geni_ios);
+
return mctrl;
}
@@ -254,6 +259,8 @@ static void qcom_geni_serial_set_mctrl(struct uart_port *uport,
if (port->manual_flow && !(mctrl & TIOCM_RTS) && !uport->suspended)
uart_manual_rfr = UART_MANUAL_RFR_EN | UART_RFR_NOT_READY;
writel(uart_manual_rfr, uport->membase + SE_UART_MANUAL_RFR);
+
+ trace_geni_serial_set_mctrl(uport->dev, mctrl, uart_manual_rfr);
}
static const char *qcom_geni_serial_get_type(struct uart_port *uport)
@@ -684,6 +691,8 @@ static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
xmit_size = kfifo_out_linear_ptr(&tport->xmit_fifo, &tail,
UART_XMIT_SIZE);
+ trace_geni_serial_tx_data(uport->dev, tail, xmit_size);
+
qcom_geni_set_rs485_mode(uport, SER_RS485_RTS_ON_SEND);
qcom_geni_serial_setup_tx(uport, xmit_size);
@@ -910,8 +919,10 @@ static void qcom_geni_serial_handle_rx_dma(struct uart_port *uport, bool drop)
return;
}
- if (!drop)
+ if (!drop) {
+ trace_geni_serial_rx_data(uport->dev, port->rx_buf, rx_in);
handle_rx_uart(uport, rx_in);
+ }
ret = geni_se_rx_dma_prep(&port->se, port->rx_buf,
DMA_RX_BUF_SIZE,
@@ -1082,6 +1093,10 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
geni_status = readl(uport->membase + SE_GENI_STATUS);
dma = readl(uport->membase + SE_GENI_DMA_MODE_EN);
m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
+
+ trace_geni_serial_irq(uport->dev, m_irq_status, s_irq_status,
+ dma_tx_status, dma_rx_status);
+
writel(m_irq_status, uport->membase + SE_GENI_M_IRQ_CLEAR);
writel(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
writel(dma_tx_status, uport->membase + SE_DMA_TX_IRQ_CLR);
@@ -1294,8 +1309,8 @@ static int geni_serial_set_rate(struct uart_port *uport, unsigned int baud)
return -EINVAL;
}
- dev_dbg(port->se.dev, "desired_rate = %u, clk_rate = %lu, clk_div = %u, clk_idx = %u\n",
- baud * sampling_rate, clk_rate, clk_div, clk_idx);
+ trace_geni_serial_clk_cfg(uport->dev, baud * sampling_rate, clk_rate,
+ clk_div, clk_idx);
uport->uartclk = clk_rate;
port->clk_rate = clk_rate;
@@ -1455,6 +1470,10 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
writel(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN);
writel(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN);
writel(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN);
+
+ trace_geni_serial_set_termios(uport->dev, baud, bits_per_char,
+ tx_trans_cfg, tx_parity_cfg, rx_trans_cfg,
+ rx_parity_cfg, stop_bit_len);
}
#ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE
--
2.34.1
^ permalink raw reply related
* [PATCH v5 1/2] serial: qcom-geni: trace: Drop redundant len field from geni_serial_data
From: Praveen Talari @ 2026-06-15 14:16 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Greg Kroah-Hartman, Jiri Slaby, konrad.dybcio
Cc: Praveen Talari, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, mukesh.savaliya, aniket.randive, chandana.chiluveru
In-Reply-To: <20260615-add-tracepoints-for-qcom-geni-serial-v5-0-2efa4c97e0e2@oss.qualcomm.com>
The dynamic array stored in the ring buffer already carries its own
length in the array metadata. There is no need to also store it as a
separate scalar field in the entry struct.
Drop __field(unsigned int, len) and the corresponding __entry->len
assignment, and use __get_dynamic_array_len(data) in the TP_printk for
both the len=%u format argument and the __print_hex() size argument.
This saves 4 bytes per event on the ring buffer.
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
include/trace/events/qcom_geni_serial.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/trace/events/qcom_geni_serial.h b/include/trace/events/qcom_geni_serial.h
index 417ec01f9fc8..e1aa551d525e 100644
--- a/include/trace/events/qcom_geni_serial.h
+++ b/include/trace/events/qcom_geni_serial.h
@@ -97,18 +97,17 @@ DECLARE_EVENT_CLASS(geni_serial_data,
TP_ARGS(dev, buf, len),
TP_STRUCT__entry(__string(name, dev_name(dev))
- __field(unsigned int, len)
__dynamic_array(u8, data, len)
),
TP_fast_assign(__assign_str(name);
- __entry->len = len;
memcpy(__get_dynamic_array(data), buf, len);
),
TP_printk("%s: len=%u data=%s",
- __get_str(name), __entry->len,
- __print_hex(__get_dynamic_array(data), __entry->len))
+ __get_str(name), __get_dynamic_array_len(data),
+ __print_hex(__get_dynamic_array(data),
+ __get_dynamic_array_len(data)))
);
DEFINE_EVENT(geni_serial_data, geni_serial_tx_data,
--
2.34.1
^ permalink raw reply related
* [PATCH v5 0/2] Add tracepoints support for Qualcomm GENI Serial drivers
From: Praveen Talari @ 2026-06-15 14:16 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Greg Kroah-Hartman, Jiri Slaby, konrad.dybcio
Cc: Praveen Talari, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, mukesh.savaliya, aniket.randive, chandana.chiluveru
Add tracepoints to the Qualcomm GENI (Generic Interface) serial driver.
These trace events enable runtime debugging and performance analysis of
UART operations.
The trace events cover UART termios configuration, clock setup, manual
control state, interrupt status, and actual transmitted/received data in
hexadecimal format.
Usage examples:
Enable all serial traces:
echo 1 > /sys/kernel/debug/tracing/events/qcom_geni_serial/enable
cat /sys/kernel/debug/tracing/trace_pipe
Example trace output:
2517.938432: geni_serial_clk_cfg: a94000.serial: desired_rate=1843200
clk_rate=7372800 clk_div=4 clk_idx=0
2517.938753: geni_serial_irq: a94000.serial: m_irq=0x88800000
s_irq=0x08000111 dma_tx=0x00000000 dma_rx=0x00000000
2517.938803: geni_serial_set_termios: a94000.serial: baud=115200 bpc=8
tx_trans=0x00000002 tx_par=0x00000000 rx_trans=0x00000000
rx_par=0x00000000 stop=0
2517.938807: geni_serial_set_mctrl: a94000.serial: mctrl=0x8006
uart_manual_rfr=0x00000000
2517.938818: geni_serial_get_mctrl: a94000.serial: mctrl=0x0160
geni_ios=0x00000001
2517.939165: geni_serial_irq: a94000.serial: m_irq=0x00400000
s_irq=0x00000000 dma_tx=0x00000000 dma_rx=0x00000000
2517.939592: geni_serial_tx_data: a94000.serial: tx_len=8 data=61 62 63
64 65 66 67 68
2517.940610: geni_serial_irq: a94000.serial: m_irq=0x00000001
s_irq=0x00000000 dma_tx=0x00000003 dma_rx=0x00000000
2517.942174: geni_serial_irq: a94000.serial: m_irq=0x08000000
s_irq=0x08000100 dma_tx=0x00000000 dma_rx=0x00000003
2517.942323: geni_serial_rx_data: a94000.serial: rx_len=8 data=61 62 63
64 65 66 67 68
2517.942680: geni_serial_set_mctrl: a94000.serial: mctrl=0x8000
uart_manual_rfr=0x80000002
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Changes in v5:
- Remove serial trace header patch since it was accepted and merged.
- Added a new patch to fix for remove unwanted variable len.
- Link to v4: https://lore.kernel.org/all/20260526-add-tracepoints-for-qcom-geni-serial-v4-0-e94fbaec0232@oss.qualcomm.com
Changes in v4:
- Rebased patch(02/02) on latest linux-next.
- Link to v3: https://lore.kernel.org/all/20260518-add-tracepoints-for-qcom-geni-serial-v3-0-b4addb151376@oss.qualcomm.com
Changes in v3:
- Removed \n from geni_serial_tx_data and geni_serial_rx_data events.
- Resolved aligment issues in geni_serial_data, geni_serial_tx_data and
geni_serial_rx_data events.
- Link to v2: https://lore.kernel.org/r/20260512-add-tracepoints-for-qcom-geni-serial-v2-0-a5726421b3af@oss.qualcomm.com
Changes in v2:
- removed multiple trace events for TX/RX events, instead used
DECLARE_EVENT_CLASS and DEFINE_EVENT.
- Link to v1: https://lore.kernel.org/r/20260506-add-tracepoints-for-qcom-geni-serial-v1-0-544b22612e08@oss.qualcomm.com
To: Steven Rostedt <rostedt@goodmis.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jiri Slaby <jirislaby@kernel.org>
To: konrad.dybcio@oss.qualcomm.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: mukesh.savaliya@oss.qualcomm.com
Cc: aniket.randive@oss.qualcomm.com
Cc: chandana.chiluveru@oss.qualcomm.com
---
Praveen Talari (2):
serial: qcom-geni: trace: Drop redundant len field from geni_serial_data
serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
drivers/tty/serial/qcom_geni_serial.c | 27 +++++++++++++++++++++++----
include/trace/events/qcom_geni_serial.h | 7 +++----
2 files changed, 26 insertions(+), 8 deletions(-)
---
base-commit: c425609d6ac4012c8bbf01ec2e10e801b1923a7b
change-id: 20260427-add-tracepoints-for-qcom-geni-serial-948777218b7b
Best regards,
--
Praveen Talari <praveen.talari@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v4 1/2] serial: qcom-geni: trace: Add tracepoint support for Qualcomm GENI serial
From: Greg Kroah-Hartman @ 2026-06-15 13:31 UTC (permalink / raw)
To: Praveen Talari
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Jiri Slaby,
konrad.dybcio, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, mukesh.savaliya, aniket.randive, chandana.chiluveru
In-Reply-To: <945821ea-5065-4e20-a1f9-32f7c9adb66a@oss.qualcomm.com>
On Mon, Jun 15, 2026 at 06:09:17PM +0530, Praveen Talari wrote:
> Hi
>
> On 15-06-2026 16:12, Greg Kroah-Hartman wrote:
> > On Mon, Jun 15, 2026 at 03:26:25PM +0530, Praveen Talari wrote:
> > > HI Steven,
> > >
> > > On 29-05-2026 19:44, Steven Rostedt wrote:
> > > > On Tue, 26 May 2026 23:07:39 +0530
> > > > Praveen Talari <praveen.talari@oss.qualcomm.com> wrote:
> > > >
> > > > > +DECLARE_EVENT_CLASS(geni_serial_data,
> > > > > + TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
> > > > > + TP_ARGS(dev, buf, len),
> > > > > +
> > > > > + TP_STRUCT__entry(__string(name, dev_name(dev))
> > > > > + __field(unsigned int, len)
> > > > > + __dynamic_array(u8, data, len)
> > > > > + ),
> > > > > +
> > > > > + TP_fast_assign(__assign_str(name);
> > > > > + __entry->len = len;
> > > > > + memcpy(__get_dynamic_array(data), buf, len);
> > > > > + ),
> > > > > +
> > > > > + TP_printk("%s: len=%u data=%s",
> > > > > + __get_str(name), __entry->len,
> > > > > + __print_hex(__get_dynamic_array(data), __entry->len))
> > > > > +);
> > > > No need to save the length of the dynamic array in __entry->len because
> > > > it's already saved in the metadata of the dynamic array that is stored
> > > > on the buffer. Instead you can have:
> > > >
> > > > DECLARE_EVENT_CLASS(geni_serial_data,
> > > > TP_PROTO(struct device *dev, const u8 *buf, unsigned int len),
> > > > TP_ARGS(dev, buf, len),
> > > >
> > > > TP_STRUCT__entry(__string(name, dev_name(dev))
> > > > __dynamic_array(u8, data, len)
> > > > ),
> > > >
> > > > TP_fast_assign(__assign_str(name);
> > > > memcpy(__get_dynamic_array(data), buf, len);
> > > > ),
> > > >
> > > > TP_printk("%s: len=%u data=%s",
> > > > __get_str(name), __entry->len,
> > > > __print_hex(__get_dynamic_array(data),
> > > > __get_dynamic_array_len(data)))
> > > > );
> > > >
> > > > That will save you 4 bytes per event on the ring buffer. And a few
> > > > cycles not having to store the redundant information.
> > > This patch has already been accepted and is available in linux-next.
> > Great, can you send a fixup for it? Or want me to revert this instead?
>
> can i add fix patch in same series by removing original patch(0/1)?
We can never rewrite a public git tree. So either revert and redo it as
a new patch, or send a fix for this one, your choice.
thanks,
greg k-h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox