[PATCH AUTOSEL 6.19] tty: vt/keyboard: Split apart vt_do_diacrit()

[PATCH AUTOSEL 6.19] tty: vt/keyboard: Split apart vt_do_diacrit()

[Sasha Levin]

From: Nathan Chancellor <nathan@kernel.org>

[ Upstream commit 0a76a17238f805b231d97b118232a5185bbb7a18 ]

After commit bfb24564b5fd ("tty: vt/keyboard: use __free()"), builds
using asm goto for put_user() and get_user() with a version of clang
older than 17 error with:

  drivers/tty/vt/keyboard.c:1709:7: error: cannot jump from this asm goto statement to one of its possible targets
                  if (put_user(asize, &a->kb_cnt))
                      ^
  ...
  arch/arm64/include/asm/uaccess.h:298:2: note: expanded from macro '__put_mem_asm'
          asm goto(                                                       \
          ^
  drivers/tty/vt/keyboard.c:1687:7: note: possible target of asm goto statement
                  if (put_user(asize, &a->kb_cnt))
                      ^
  ...
  arch/arm64/include/asm/uaccess.h:342:2: note: expanded from macro '__raw_put_user'
          __rpu_failed:                                                   \
          ^
  drivers/tty/vt/keyboard.c:1697:23: note: jump exits scope of variable with __attribute__((cleanup))
                  void __free(kfree) *buf = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacruc),
                                      ^
  drivers/tty/vt/keyboard.c:1671:33: note: jump bypasses initialization of variable with __attribute__((cleanup))
                  struct kbdiacr __free(kfree) *dia = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacr),
                                                ^

Prior to a fix to clang's scope checker in clang 17 [1], all labels in a
function were validated as potential targets of all asm gotos in a
function, regardless of whether they actually were a target of an asm
goto call, resulting in false positive errors about skipping over
variables marked with the cleanup attribute.

To workaround this error, split up the bodies of the case statements in
vt_do_diacrit() into their own functions so that the scope checker does
not trip up on the multiple instances of __free().

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202509091702.Oc7eCRDw-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202511241835.EA8lShgH-lkp@intel.com/
Link: https://github.com/llvm/llvm-project/commit/f023f5cdb2e6c19026f04a15b5a935c041835d14 [1]
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Link: https://patch.msgid.link/20251125-tty-vt-keyboard-wa-clang-scope-check-error-v1-1-f5a5ea55c578@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

The current code in this tree has the `__free()` annotations from
`bfb24564b5fd8`, confirming that the prerequisite commit is present and
this fix is needed.

### Scope and Risk Assessment

- **Files changed**: 1 (drivers/tty/vt/keyboard.c)
- **Nature of change**: Pure mechanical refactoring - moving code into
  separate functions
- **Logic changes**: None - the behavior is identical
- **Risk**: Very low. The code is simply moved into helper functions. No
  logic changes, no new paths, no changed semantics.
- **Lines changed**: The diff looks large (~170 lines removed and added)
  but it's entirely code movement, not new code.

### User Impact

- **Who is affected**: Anyone building the kernel with clang < 17 on
  arm64 (or potentially other architectures using asm goto for
  put_user/get_user)
- **Severity**: Build failure - the kernel **cannot be compiled** at all
  with these toolchain combinations
- **Practical impact**: Enterprise distributions and CI systems that use
  older clang versions would be completely blocked

### Stable Criteria Assessment

1. **Obviously correct and tested**: Yes - pure code movement, reported
   by kernel test robot, authored by Nathan Chancellor (clang/compiler
   expert), signed off by Greg KH
2. **Fixes a real bug**: Yes - build failure
3. **Important issue**: Yes - build failures prevent kernel compilation
   entirely
4. **Small and contained**: Yes - single file, pure refactoring
5. **No new features**: Correct - no new features
6. **Applies cleanly**: Should apply cleanly since the prerequisite
   `bfb24564b5fd8` must also be present

### Dependency

This commit depends on `bfb24564b5fd8` ("tty: vt/keyboard: use
__free()") being present in the stable tree. The fix is only needed if
`__free()` annotations were backported. If `bfb24564b5fd8` was
backported to a stable tree, then this fix **must** also be backported
to avoid build failures with older clang.

### Verification

- **git log master -- drivers/tty/vt/keyboard.c** confirmed
  `bfb24564b5fd8` is present in the tree
- **Read of keyboard.c lines 1650-1699** confirmed the `__free(kfree)`
  annotations are present in the current codebase, making this fix
  necessary
- **git show bfb24564b5fd8** confirmed this is the prerequisite that
  introduced the problematic `__free()` usage
- Commit message includes concrete error output from the kernel test
  robot, confirming the build failure is real and reproducible
- The commit references the specific clang fix (llvm-project commit
  f023f5cdb2e6c19) confirming this is a known compiler issue
- Author is Nathan Chancellor, the primary Linux kernel clang/LLVM
  maintainer - expert in this area
- Signed-off by Greg KH (stable tree maintainer)

### Conclusion

This is a textbook **build fix** - one of the explicitly allowed
categories for stable backporting. It fixes a real compilation failure
with clang < 17 on arm64, the change is purely mechanical (code movement
into helper functions with zero logic changes), the risk is extremely
low, and it's authored by the kernel's clang expert and signed off by
the stable maintainer. The only caveat is that it requires the
prerequisite commit `bfb24564b5fd8` to be present in the stable tree.

**YES**

 drivers/tty/vt/keyboard.c | 221 ++++++++++++++++++++------------------
 1 file changed, 115 insertions(+), 106 deletions(-)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index d65fc60dd7bed..3538d54d6a6ac 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -1649,134 +1649,143 @@ int __init kbd_init(void)
 
 /* Ioctl support code */
 
-/**
- *	vt_do_diacrit		-	diacritical table updates
- *	@cmd: ioctl request
- *	@udp: pointer to user data for ioctl
- *	@perm: permissions check computed by caller
- *
- *	Update the diacritical tables atomically and safely. Lock them
- *	against simultaneous keypresses
- */
-int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
+static int vt_do_kdgkbdiacr(void __user *udp)
 {
-	int asize;
-
-	switch (cmd) {
-	case KDGKBDIACR:
-	{
-		struct kbdiacrs __user *a = udp;
-		int i;
+	struct kbdiacrs __user *a = udp;
+	int i, asize;
 
-		struct kbdiacr __free(kfree) *dia = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacr),
-								  GFP_KERNEL);
-		if (!dia)
-			return -ENOMEM;
+	struct kbdiacr __free(kfree) *dia = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacr),
+							  GFP_KERNEL);
+	if (!dia)
+		return -ENOMEM;
 
-		/* Lock the diacriticals table, make a copy and then
-		   copy it after we unlock */
-		scoped_guard(spinlock_irqsave, &kbd_event_lock) {
-			asize = accent_table_size;
-			for (i = 0; i < asize; i++) {
-				dia[i].diacr = conv_uni_to_8bit(accent_table[i].diacr);
-				dia[i].base = conv_uni_to_8bit(accent_table[i].base);
-				dia[i].result = conv_uni_to_8bit(accent_table[i].result);
-			}
+	/* Lock the diacriticals table, make a copy and then
+	   copy it after we unlock */
+	scoped_guard(spinlock_irqsave, &kbd_event_lock) {
+		asize = accent_table_size;
+		for (i = 0; i < asize; i++) {
+			dia[i].diacr = conv_uni_to_8bit(accent_table[i].diacr);
+			dia[i].base = conv_uni_to_8bit(accent_table[i].base);
+			dia[i].result = conv_uni_to_8bit(accent_table[i].result);
 		}
-
-		if (put_user(asize, &a->kb_cnt))
-			return -EFAULT;
-		if (copy_to_user(a->kbdiacr, dia, asize * sizeof(struct kbdiacr)))
-			return -EFAULT;
-		return 0;
 	}
-	case KDGKBDIACRUC:
-	{
-		struct kbdiacrsuc __user *a = udp;
 
-		void __free(kfree) *buf = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacruc),
-							GFP_KERNEL);
-		if (buf == NULL)
-			return -ENOMEM;
+	if (put_user(asize, &a->kb_cnt))
+		return -EFAULT;
+	if (copy_to_user(a->kbdiacr, dia, asize * sizeof(struct kbdiacr)))
+		return -EFAULT;
+	return 0;
+}
 
-		/* Lock the diacriticals table, make a copy and then
-		   copy it after we unlock */
-		scoped_guard(spinlock_irqsave, &kbd_event_lock) {
-			asize = accent_table_size;
-			memcpy(buf, accent_table, asize * sizeof(struct kbdiacruc));
-		}
+static int vt_do_kdgkbdiacruc(void __user *udp)
+{
+	struct kbdiacrsuc __user *a = udp;
+	int asize;
 
-		if (put_user(asize, &a->kb_cnt))
-			return -EFAULT;
-		if (copy_to_user(a->kbdiacruc, buf, asize * sizeof(struct kbdiacruc)))
-			return -EFAULT;
+	void __free(kfree) *buf = kmalloc_array(MAX_DIACR, sizeof(struct kbdiacruc),
+						GFP_KERNEL);
+	if (buf == NULL)
+		return -ENOMEM;
 
-		return 0;
+	/* Lock the diacriticals table, make a copy and then
+	   copy it after we unlock */
+	scoped_guard(spinlock_irqsave, &kbd_event_lock) {
+		asize = accent_table_size;
+		memcpy(buf, accent_table, asize * sizeof(struct kbdiacruc));
 	}
 
-	case KDSKBDIACR:
-	{
-		struct kbdiacrs __user *a = udp;
-		struct kbdiacr __free(kfree) *dia = NULL;
-		unsigned int ct;
-		int i;
+	if (put_user(asize, &a->kb_cnt))
+		return -EFAULT;
+	if (copy_to_user(a->kbdiacruc, buf, asize * sizeof(struct kbdiacruc)))
+		return -EFAULT;
 
-		if (!perm)
-			return -EPERM;
-		if (get_user(ct, &a->kb_cnt))
-			return -EFAULT;
-		if (ct >= MAX_DIACR)
-			return -EINVAL;
+	return 0;
+}
 
-		if (ct) {
-			dia = memdup_array_user(a->kbdiacr,
-						ct, sizeof(struct kbdiacr));
-			if (IS_ERR(dia))
-				return PTR_ERR(dia);
-		}
+static int vt_do_kdskbdiacr(void __user *udp, int perm)
+{
+	struct kbdiacrs __user *a = udp;
+	struct kbdiacr __free(kfree) *dia = NULL;
+	unsigned int ct;
+	int i;
 
-		guard(spinlock_irqsave)(&kbd_event_lock);
-		accent_table_size = ct;
-		for (i = 0; i < ct; i++) {
-			accent_table[i].diacr =
-					conv_8bit_to_uni(dia[i].diacr);
-			accent_table[i].base =
-					conv_8bit_to_uni(dia[i].base);
-			accent_table[i].result =
-					conv_8bit_to_uni(dia[i].result);
-		}
+	if (!perm)
+		return -EPERM;
+	if (get_user(ct, &a->kb_cnt))
+		return -EFAULT;
+	if (ct >= MAX_DIACR)
+		return -EINVAL;
 
-		return 0;
+	if (ct) {
+		dia = memdup_array_user(a->kbdiacr,
+					ct, sizeof(struct kbdiacr));
+		if (IS_ERR(dia))
+			return PTR_ERR(dia);
 	}
 
-	case KDSKBDIACRUC:
-	{
-		struct kbdiacrsuc __user *a = udp;
-		unsigned int ct;
-		void __free(kfree) *buf = NULL;
+	guard(spinlock_irqsave)(&kbd_event_lock);
+	accent_table_size = ct;
+	for (i = 0; i < ct; i++) {
+		accent_table[i].diacr =
+				conv_8bit_to_uni(dia[i].diacr);
+		accent_table[i].base =
+				conv_8bit_to_uni(dia[i].base);
+		accent_table[i].result =
+				conv_8bit_to_uni(dia[i].result);
+	}
 
-		if (!perm)
-			return -EPERM;
+	return 0;
+}
 
-		if (get_user(ct, &a->kb_cnt))
-			return -EFAULT;
+static int vt_do_kdskbdiacruc(void __user *udp, int perm)
+{
+	struct kbdiacrsuc __user *a = udp;
+	unsigned int ct;
+	void __free(kfree) *buf = NULL;
 
-		if (ct >= MAX_DIACR)
-			return -EINVAL;
+	if (!perm)
+		return -EPERM;
 
-		if (ct) {
-			buf = memdup_array_user(a->kbdiacruc,
-						ct, sizeof(struct kbdiacruc));
-			if (IS_ERR(buf))
-				return PTR_ERR(buf);
-		}
-		guard(spinlock_irqsave)(&kbd_event_lock);
-		if (ct)
-			memcpy(accent_table, buf,
-					ct * sizeof(struct kbdiacruc));
-		accent_table_size = ct;
-		return 0;
+	if (get_user(ct, &a->kb_cnt))
+		return -EFAULT;
+
+	if (ct >= MAX_DIACR)
+		return -EINVAL;
+
+	if (ct) {
+		buf = memdup_array_user(a->kbdiacruc,
+					ct, sizeof(struct kbdiacruc));
+		if (IS_ERR(buf))
+			return PTR_ERR(buf);
 	}
+	guard(spinlock_irqsave)(&kbd_event_lock);
+	if (ct)
+		memcpy(accent_table, buf,
+				ct * sizeof(struct kbdiacruc));
+	accent_table_size = ct;
+	return 0;
+}
+
+/**
+ *	vt_do_diacrit		-	diacritical table updates
+ *	@cmd: ioctl request
+ *	@udp: pointer to user data for ioctl
+ *	@perm: permissions check computed by caller
+ *
+ *	Update the diacritical tables atomically and safely. Lock them
+ *	against simultaneous keypresses
+ */
+int vt_do_diacrit(unsigned int cmd, void __user *udp, int perm)
+{
+	switch (cmd) {
+	case KDGKBDIACR:
+		return vt_do_kdgkbdiacr(udp);
+	case KDGKBDIACRUC:
+		return vt_do_kdgkbdiacruc(udp);
+	case KDSKBDIACR:
+		return vt_do_kdskbdiacr(udp, perm);
+	case KDSKBDIACRUC:
+		return vt_do_kdskbdiacruc(udp, perm);
 	}
 	return 0;
 }
-- 
2.51.0

[PATCH AUTOSEL 6.19-5.10] serial: 8250_dw: handle clock enable errors in runtime_resume

[Sasha Levin]

From: Artem Shimko <a.shimko.dev@gmail.com>

[ Upstream commit d31228143a489ba6ba797896a07541ce06828c09 ]

Add error checking for clk_prepare_enable() calls in
dw8250_runtime_resume(). Currently if either clock fails to enable,
the function returns success while leaving clocks in inconsistent state.

This change implements comprehensive error handling by checking the return
values of both clk_prepare_enable() calls. If the second clock enable
operation fails after the first clock has already been successfully
enabled, the code now properly cleans up by disabling and unpreparing
the first clock before returning. The error code is then propagated to
the caller, ensuring that clock enable failures are properly reported
rather than being silently ignored.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
Link: https://patch.msgid.link/20251104145433.2316165-2-a.shimko.dev@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Classification

This is an **error handling fix** — a recognized category of bug fixes
for stable. The commit adds missing error checking for
`clk_prepare_enable()` return values in the runtime resume path.

## Bug Assessment

**What was broken:**
- `clk_prepare_enable()` can fail (e.g., clock hardware issues,
  regulator failure), returning a negative error code.
- The old code ignored these return values and always returned 0
  (success).
- This means the PM runtime framework would believe the device is active
  when clocks may not actually be enabled.

**Consequences of the bug:**
1. **Inconsistent clock state**: If `clk` fails after `pclk` succeeds,
   only one clock is enabled but the function claims success. On the
   next `runtime_suspend`, both clocks will be disabled/unprepared —
   leading to an unbalanced `clk_disable_unprepare()` on a clock that
   was never successfully enabled.
2. **Serial port malfunction**: Without clocks properly enabled, the
   UART hardware won't function, but the software stack thinks it's
   ready.
3. **Clock framework imbalance**: Unbalanced enable/disable calls can
   cause issues in the clock framework, potentially affecting other
   devices sharing the same clock tree.

**Bug existed since 2013** (commit `ffc3ae6dd925b6`) — over 12 years.

## Severity Assessment

**Moderate severity.** While `clk_prepare_enable()` failing in runtime
resume is not a common occurrence in normal operation, when it does
happen:
- The consequences are real (clock imbalance, non-functional hardware)
- The PM framework gets incorrect state information
- Other drivers in the same subsystem (fsl_lpuart, imx) properly check
  these return values, showing this is a known pattern

## Stable Kernel Criteria Check

1. **Obviously correct and tested**: Yes — the pattern is
   straightforward error checking, matching what other serial drivers
   already do. Merged by Greg Kroah-Hartman (the serial/stable
   maintainer).
2. **Fixes a real bug**: Yes — ignoring clock enable failures leaves
   hardware in inconsistent state and can cause unbalanced clock
   operations.
3. **Important issue**: Moderate — clock enable failures can cause
   device malfunction and clock framework inconsistency.
4. **Small and contained**: Yes — only ~10 lines changed in a single
   function, single file.
5. **No new features**: Correct — purely error handling.
6. **Applies cleanly**: The affected code has been stable since 2019
   (`a8afc193558a4`), so it should apply cleanly to all active stable
   trees.

## Risk Assessment

**Very low risk.** The change:
- Only adds error checking to an existing code path
- Only affects the failure case (when `clk_prepare_enable()` returns
  non-zero)
- The success path is unchanged
- Proper cleanup of `pclk` when `clk` fails prevents resource leaks
- Follows established patterns used by other serial drivers

## Verification

- **Git blame** confirmed the buggy code has been present since 2013
  (`ffc3ae6dd925b6` by Heikki Krogerus) with pclk added in 2014
  (`7d78cbefaa465`).
- **Code review** of other serial drivers (fsl_lpuart.c, imx.c)
  confirmed they properly check `clk_prepare_enable()` return values in
  runtime_resume, establishing this as an expected pattern.
- **8250_mtk.c** has the same unchecked pattern, confirming this is a
  real class of bugs in 8250 drivers.
- **Commit was merged by Greg Kroah-Hartman** (serial subsystem and
  stable maintainer), who linked it via patch.msgid.link.
- **dw8250_runtime_suspend** unconditionally calls
  `clk_disable_unprepare()` on both clocks — verified that if
  runtime_resume falsely reports success, the next suspend would call
  disable on a clock that wasn't successfully enabled, causing clock
  framework imbalance.
- **Could NOT verify** whether any user actually reported hitting this
  bug in practice — the commit has no Reported-by tag, suggesting this
  was found by code review rather than a user report.

## Conclusion

This is a legitimate bug fix that adds missing error handling for clock
enable operations in a runtime PM callback. The fix is small, surgical,
obviously correct, follows patterns established by other drivers in the
same subsystem, and carries essentially zero regression risk. While the
bug may not be frequently triggered in practice (clock enable failures
are relatively uncommon), when it does trigger, it causes real problems
(clock imbalance, non-functional hardware). The code has been present in
all stable trees since 2013-2014, so the fix is applicable broadly.

**YES**

 drivers/tty/serial/8250/8250_dw.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 27af83f0ff463..0f8207652efe6 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -741,11 +741,18 @@ static int dw8250_runtime_suspend(struct device *dev)
 
 static int dw8250_runtime_resume(struct device *dev)
 {
+	int ret;
 	struct dw8250_data *data = dev_get_drvdata(dev);
 
-	clk_prepare_enable(data->pclk);
+	ret = clk_prepare_enable(data->pclk);
+	if (ret)
+		return ret;
 
-	clk_prepare_enable(data->clk);
+	ret = clk_prepare_enable(data->clk);
+	if (ret) {
+		clk_disable_unprepare(data->pclk);
+		return ret;
+	}
 
 	return 0;
 }
-- 
2.51.0

[PATCH AUTOSEL 6.19] serial: rsci: Add set_rtrg() callback

[Sasha Levin]

From: Biju Das <biju.das.jz@bp.renesas.com>

[ Upstream commit b346e5d7dbf6696176417923c49838a1beb1d785 ]

The rtrg variable is populated in sci_init_single() for RZ/T2H. Add
set_rtrg() callback for setting the rtrg value.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Link: https://patch.msgid.link/20251129164325.209213-4-biju.das.jz@bp.renesas.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

`fifosize = 16`, so `fifosize > 1` is true, meaning the
`rx_fifo_trigger` sysfs attribute is also created. Both sysfs paths are
reachable and will cause NULL pointer dereferences without the
`set_rtrg` callback.

### Summary of Analysis

**What the commit does:** Adds a `set_rtrg()` callback to the RSCI
serial driver that programs the receive FIFO trigger level into the
hardware.

**What bug it fixes:** Without this callback, the `set_rtrg` function
pointer in `rsci_port_ops` is NULL. The shared SCI framework code in
`sh-sci.c` calls `s->ops->set_rtrg()` **without NULL checks** from
multiple paths:

1. **sysfs `rx_fifo_trigger` write** (line 1347): Created for RSCI
   because `fifosize=16 > 1`. Writing to it calls NULL `set_rtrg` →
   **kernel crash/panic**
2. **sysfs `rx_fifo_timeout` write** (line 1392): Explicitly created for
   `SCI_PORT_RSCI` at line 3921-3922. Writing a non-zero value calls
   NULL `set_rtrg` → **kernel crash/panic**
3. **Timer callback `rx_fifo_timer_fn`** (line 1322): Once a user writes
   to `rx_fifo_timeout`, the timer is set up and will fire, calling NULL
   `set_rtrg` → **kernel crash/panic**
4. **Interrupt handler path** (lines 1980-1982): If `rx_trigger > 1`
   (it's 15 for RSCI) and `rx_fifo_timeout > 0`, the interrupt handler
   calls NULL `rtrg_enabled` first → **kernel crash/panic**

The `rx_trigger` for RSCI is initialized to 15 (line 3333), and both
sysfs attributes are created, making these paths reachable from
userspace.

**Risk assessment:** LOW risk. The change adds a simple function that
reads a register, clamps a value, and writes it back. It only affects
the RSCI port type. The callback is registered in the existing ops
structure. No behavioral changes for any other port type.

**Scope:** Small - one new function (~15 lines) and one ops structure
entry.

**Stable criteria check:**
- Fixes a real bug: YES - NULL pointer dereference (kernel crash)
  reachable from sysfs
- Obviously correct: YES - straightforward register read/modify/write
- Small and contained: YES - ~15 lines of new code, 1 file
- No new features: The function itself enables correct operation of
  existing sysfs interfaces; the commit message frames it as "adding a
  callback" but it's actually fixing a NULL pointer dereference
- Tested: YES - has "Tested-by:" tag

**Note:** The commit is also missing a `rtrg_enabled` callback, which is
also called without NULL check at line 1981. This commit only adds
`set_rtrg`, not `rtrg_enabled`. However, `set_rtrg` alone fixes the most
immediate crash paths (sysfs writes and timer). The `rtrg_enabled` path
at line 1981 would still be a problem but only if both `rx_trigger > 1`
AND `rx_fifo_timeout > 0`, which requires explicit user action to set
the timeout.

### Verification

- **Verified** that `SCI_PORT_RSCI` sets `rx_trigger = 15` at sh-
  sci.c:3332-3333
- **Verified** that `rx_fifo_timeout` sysfs attribute is created for
  `SCI_PORT_RSCI` at sh-sci.c:3921-3922
- **Verified** that `rx_fifo_trigger` sysfs attribute is created when
  `fifosize > 1` at sh-sci.c:3916-3919 (RSCI fifosize=16 per rsci.c:420)
- **Verified** that `set_rtrg` is called without NULL checks at sh-sci.c
  lines 1322, 1347, 1349, 1392, 1517, 1955, 1982, 2661, 2666, 2668
- **Verified** that `rtrg_enabled` is called without NULL check at sh-
  sci.c:1981
- **Verified** that the RSCI `rsci_set_termios` at rsci.c:154-169 does
  NOT call the shared `sci_set_termios` (lines 2673+) so the set_termios
  path at lines 2659-2668 is not directly triggered for RSCI
- **Verified** that `rsci_port_ops` before this commit has no `set_rtrg`
  callback (it was not listed in the pre-patch ops structure)
- **Could NOT verify** whether a separate commit adds `rtrg_enabled` for
  RSCI (this commit only adds `set_rtrg`)

**YES**

 drivers/tty/serial/rsci.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/tty/serial/rsci.c b/drivers/tty/serial/rsci.c
index b3c48dc1e07db..0533a4bb1d03c 100644
--- a/drivers/tty/serial/rsci.c
+++ b/drivers/tty/serial/rsci.c
@@ -151,6 +151,22 @@ static void rsci_start_rx(struct uart_port *port)
 	rsci_serial_out(port, CCR0, ctrl);
 }
 
+static int rsci_scif_set_rtrg(struct uart_port *port, int rx_trig)
+{
+	u32 fcr = rsci_serial_in(port, FCR);
+
+	if (rx_trig >= port->fifosize)
+		rx_trig = port->fifosize - 1;
+	else if (rx_trig < 1)
+		rx_trig = 0;
+
+	fcr &= ~FCR_RTRG4_0;
+	fcr |= field_prep(FCR_RTRG4_0, rx_trig);
+	rsci_serial_out(port, FCR, fcr);
+
+	return rx_trig;
+}
+
 static void rsci_set_termios(struct uart_port *port, struct ktermios *termios,
 			     const struct ktermios *old)
 {
@@ -454,6 +470,7 @@ static const struct sci_port_ops rsci_port_ops = {
 	.poll_put_char		= rsci_poll_put_char,
 	.prepare_console_write	= rsci_prepare_console_write,
 	.suspend_regs_size	= rsci_suspend_regs_size,
+	.set_rtrg		= rsci_scif_set_rtrg,
 	.shutdown_complete	= rsci_shutdown_complete,
 };
 
-- 
2.51.0

[PATCH AUTOSEL 6.19-6.12] serial: 8250: 8250_omap.c: Add support for handling UART error conditions

[Sasha Levin]

From: Moteen Shah <m-shah@ti.com>

[ Upstream commit 623b07b370e9963122d167e04fdc1dc713ebfbaf ]

The DMA IRQ handler does not accounts for the overrun(OE) or any other
errors being reported by the IP before triggering a DMA transaction which
leads to the interrupts not being handled resulting into an IRQ storm.

The way to handle OE is to:
1. Reset the RX FIFO.
2. Read the UART_RESUME register, which clears the internal flag

Earlier, the driver issued DMA transations even in case of OE which shouldn't
be done according to the OE handling mechanism mentioned above, as we are
resetting the FIFO's, refer section: "12.1.6.4.8.1.3.6 Overrun During
Receive" [0].

[0] https://www.ti.com/lit/pdf/spruiu1

Signed-off-by: Moteen Shah <m-shah@ti.com>
Link: https://patch.msgid.link/20260112081829.63049-2-m-shah@ti.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Detailed Analysis

### 1. What the commit fixes

The commit fixes an **IRQ storm** on TI K3/AM654 SoCs when a UART
overrun error (OE) occurs during DMA-based UART communication. The
existing `am654_8250_handle_rx_dma()` function did not handle UART error
conditions at all:

- When an overrun occurs, the UART sets `UART_LSR_OE` in the line status
  register
- The existing code would try to start a DMA transaction despite the
  overrun condition
- Without reading the UART_OMAP_RESUME register and resetting the FIFO,
  the error interrupt flag stays set
- This causes an **interrupt storm** — the interrupt fires repeatedly
  because it was never properly acknowledged/cleared

This is a known hardware behavior documented in the TI reference manual
section "12.1.6.4.8.1.3.6 Overrun During Receive."

### 2. Does it meet stable kernel rules?

**Obviously correct**: Yes. The fix follows the TI reference manual's
prescribed overrun handling: reset RX FIFO, then read the RESUME
register. The additional error handling for FE/PE/BI follows standard
UART error clearing practices.

**Fixes a real bug**: Yes. An IRQ storm is a serious hardware-triggered
bug that can lock up the system or make it unresponsive. This has been a
known class of problems on K3 SoCs (see prior commits `b67e830d38fa9`
and `c128a1b0523b6` that fixed similar IRQ storms from different
causes).

**Important issue**: Yes. IRQ storms can cause:
- 100% CPU consumption in interrupt context
- System hangs or unresponsiveness
- Potential soft lockups / hard lockups

**Small and contained**: The change adds ~15 lines of new error handling
code, modifies 2-3 lines in the existing function, and adds one register
define. All changes are confined to a single file and a single driver.

**No new features**: Despite the subject saying "Add support", this is
really fixing missing error handling in an existing IRQ handler. It
doesn't add new functionality; it properly handles error conditions that
were being ignored.

### 3. Risk vs Benefit

**Risk**: LOW
- Changes are confined to the AM654/K3 DMA RX path only (guarded by
  `UART_HAS_EFR2` habit flag)
- The fix follows documented hardware procedures from TI's reference
  manual
- The new `am654_8250_handle_uart_errors()` function is straightforward:
  it clears error conditions by reading appropriate registers
- The condition `!(status & UART_LSR_OE)` prevents DMA on overrun, which
  is the correct behavior per the hardware documentation

**Benefit**: HIGH
- Prevents IRQ storms on K3/AM654 SoCs when UART overrun or other error
  conditions occur during DMA
- IRQ storms can make systems unusable
- This is particularly important for embedded/industrial use cases of
  AM654 SoCs

### 4. Dependencies

- Patch 2/2 in a series, but patch 1 ("Clear DMA RX running status only
  after DMA termination is done") appears independent
- The code depends on `serial8250_clear_and_reinit_fifos()` which has
  existed since early 8250 driver code
- The `am654_8250_handle_rx_dma()` function exists since commit
  `c26389f998a865` (v5.7 era), so it's present in all current stable
  trees
- The `UART_OMAP_RESUME` register define is new but it's just a constant
  (0x0B) — trivial

### 5. Concerns

The patch needs `UART_OMAP_RESUME` define which is added by this same
commit. This should apply cleanly as long as the define section hasn't
changed significantly. There may also be minor context conflicts due to
intermediate patches, but nothing fundamental.

The commit title "Add support for handling..." sounds like a feature
addition, but analysis shows it's a bug fix for missing error handling
that causes IRQ storms.

## Verification

- **git blame** confirmed `am654_8250_handle_rx_dma()` was introduced in
  commit `c26389f998a865` (2020, v5.7 era) — present in all current
  stable trees
- **git show `b67e830d38fa9`** confirmed prior IRQ storm fix on same K3
  SoCs (2021), demonstrating this is a known class of bugs
- **git show `c128a1b0523b6`** confirmed another IRQ storm fix related
  to Errata i2310 (2024), showing ongoing attention to this problem area
- **lore.kernel.org** confirmed this is patch 2/2, independent of patch
  1 (cover letter describes separate issues)
- **Grep** confirmed `serial8250_clear_and_reinit_fifos` is declared in
  `drivers/tty/serial/8250/8250.h` (available to the driver)
- **Grep** confirmed `UART_OMAP_RESUME` is not in the current codebase —
  it's introduced by this patch as a new define (0x0B register offset)
- **Read** of current `am654_8250_handle_rx_dma()` confirmed there is no
  error handling for OE/FE/PE/BI conditions — the bug exists
- **Unverified**: Whether stable trees 6.6.y or 6.1.y have any context
  conflicts that would prevent clean backport (likely minor if any)
- Greg Kroah-Hartman signed off on the commit, confirming it went
  through normal review

## Conclusion

This commit fixes a real, documented hardware bug (IRQ storm from
unhandled UART error conditions) on TI K3/AM654 SoCs. The fix is small,
contained, follows the hardware vendor's documented error handling
procedure, and addresses a serious issue (IRQ storms can make systems
unusable). The affected code (`am654_8250_handle_rx_dma`) has been in
stable trees since v5.7. This is consistent with the pattern of previous
IRQ storm fixes for this same hardware family (`b67e830d38fa9`,
`c128a1b0523b6`) that were both marked for stable backport.

**YES**

 drivers/tty/serial/8250/8250_omap.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 9e49ef48b851b..e26bae0a6488f 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -100,6 +100,9 @@
 #define OMAP_UART_REV_52 0x0502
 #define OMAP_UART_REV_63 0x0603
 
+/* Resume register */
+#define UART_OMAP_RESUME		0x0B
+
 /* Interrupt Enable Register 2 */
 #define UART_OMAP_IER2			0x1B
 #define UART_OMAP_IER2_RHR_IT_DIS	BIT(2)
@@ -119,7 +122,6 @@
 /* Timeout low and High */
 #define UART_OMAP_TO_L                 0x26
 #define UART_OMAP_TO_H                 0x27
-
 struct omap8250_priv {
 	void __iomem *membase;
 	int line;
@@ -1256,6 +1258,20 @@ static u16 omap_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir, u16 status
 	return status;
 }
 
+static void am654_8250_handle_uart_errors(struct uart_8250_port *up, u8 iir, u16 status)
+{
+	if (status & UART_LSR_OE) {
+		serial8250_clear_and_reinit_fifos(up);
+		serial_in(up, UART_LSR);
+		serial_in(up, UART_OMAP_RESUME);
+	} else {
+		if (status & (UART_LSR_FE | UART_LSR_PE | UART_LSR_BI))
+			serial_in(up, UART_RX);
+		if (iir & UART_IIR_XOFF)
+			serial_in(up, UART_IIR);
+	}
+}
+
 static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir,
 				     u16 status)
 {
@@ -1266,7 +1282,8 @@ static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir,
 	 * Queue a new transfer if FIFO has data.
 	 */
 	if ((status & (UART_LSR_DR | UART_LSR_BI)) &&
-	    (up->ier & UART_IER_RDI)) {
+	    (up->ier & UART_IER_RDI) && !(status & UART_LSR_OE)) {
+		am654_8250_handle_uart_errors(up, iir, status);
 		omap_8250_rx_dma(up);
 		serial_out(up, UART_OMAP_EFR2, UART_OMAP_EFR2_TIMEOUT_BEHAVE);
 	} else if ((iir & 0x3f) == UART_IIR_RX_TIMEOUT) {
@@ -1282,6 +1299,8 @@ static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir,
 		serial_out(up, UART_OMAP_EFR2, 0x0);
 		up->ier |= UART_IER_RLSI | UART_IER_RDI;
 		serial_out(up, UART_IER, up->ier);
+	} else {
+		am654_8250_handle_uart_errors(up, iir, status);
 	}
 }
 
-- 
2.51.0

[PATCH AUTOSEL 6.19-5.10] serial: 8250: 8250_omap.c: Clear DMA RX running status only after DMA termination is done

[Sasha Levin]

From: Moteen Shah <m-shah@ti.com>

[ Upstream commit a5fd8945a478ff9be14812693891d7c9b4185a50 ]

Clear rx_running flag only after DMA teardown polling completes. In the
previous implementation the flag was being cleared while hardware teardown
was still in progress, creating a mismatch between software state
(flag = 0, "ready") and hardware state (still terminating).

Signed-off-by: Moteen Shah <m-shah@ti.com>
Link: https://patch.msgid.link/20260112081829.63049-3-m-shah@ti.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## 3. Classification

This is a **bug fix** — specifically a **state synchronization/race
condition fix** between software state and hardware state. The
`rx_running` flag was being cleared prematurely, before DMA hardware
teardown completed, creating a window where software and hardware states
were inconsistent.

## 4. Scope and Risk Assessment

- **Lines changed**: 2 lines — one line removed, one line added (just
  moving `dma->rx_running = 0;`)
- **Files touched**: 1 file (drivers/tty/serial/8250/8250_omap.c)
- **Complexity**: Minimal — a single line is moved to a later position
  in the same function
- **Risk**: Very low. The change only affects the timing of when
  `rx_running` is cleared. The core logic is unchanged.
- **Potential concern**: The `count == 0` early return path (`goto out`)
  now skips clearing `rx_running`, which could theoretically leave the
  flag set. However, this is a minor edge case that likely doesn't occur
  in practice (DMA completion with zero data transfer).

## 5. User Impact

- **Who is affected**: Users of OMAP SoC serial ports with DMA (TI
  AM335x, AM437x, AM65x, etc.) — widely used in embedded/industrial
  systems
- **Severity if triggered**: The state mismatch could cause:
  - A new DMA being started while old hardware teardown is still in
    progress
  - Potential data corruption or missed serial data
  - Possible DMA engine errors or hangs
- **Likelihood**: Moderate — this would occur when DMA teardown takes
  time (in-flight bytes scenario) and another DMA operation is attempted
  during the teardown window

## 6. Stability Indicators

- Author is from TI (SoC vendor) — deep knowledge of the hardware
- Reviewed by another TI engineer (Kumar, Udit)
- Accepted by Greg Kroah-Hartman (serial subsystem maintainer)
- Small, obviously correct fix with clear rationale
- Self-contained — no dependencies on other patches in the series

## 7. Stable Kernel Criteria Assessment

| Criterion | Met? |
|-----------|------|
| Obviously correct and tested | Yes — simple line movement, accepted by
maintainer |
| Fixes a real bug | Yes — software/hardware state mismatch |
| Fixes important issue | Yes — potential data corruption/DMA errors |
| Small and contained | Yes — 2 lines in 1 file |
| No new features | Correct — pure bug fix |
| No new APIs | Correct |

## Verification

- **Code analysis**: Read `__dma_rx_do_complete()` at lines 916-971 to
  understand the full function flow and confirm the bug mechanism
  (premature flag clearing before teardown polling)
- **Caller analysis**: Verified `__dma_rx_do_complete` is called from
  `__dma_rx_complete` (line 991, DMA completion callback) and
  `omap_8250_rx_dma_flush` (line 1022, flush path)
- **Lock analysis**: Verified that `__dma_rx_complete` does NOT hold
  `rx_dma_lock` (only port lock at line 981), while
  `omap_8250_rx_dma_flush` does hold `rx_dma_lock` (line 1009). This
  confirms a real synchronization gap.
- **Race window**: After `__dma_rx_complete` calls
  `__dma_rx_do_complete`, it calls `omap_8250_rx_dma(p)` at line 998,
  which checks `rx_running`. With old code, `rx_running==0` was visible
  during teardown polling.
- **Self-contained**: Verified patch 2/3 (623b07b370e99) modifies
  different functions (`am654_8250_handle_rx_dma`,
  `am654_8250_handle_uart_errors`) and does not conflict with or depend
  on this patch
- **File history**: `8250_omap.c` DMA-RX callback added in commit
  0e31c8d173ab1 (2014-09-29), present in all active stable trees
- **Mailing list**: Found the patch on lore.kernel.org, reviewed by
  Kumar, Udit (TI), accepted by Greg KH
- **count==0 edge case**: Identified that with the patch, the `goto out`
  for `count==0` skips clearing `rx_running`. This is a minor concern
  but the count==0 case after DMA completion is unusual. This was NOT
  verified to be problematic in practice (unverified edge case concern).

## Conclusion

This is a small, surgical, obviously correct fix for a real state
synchronization bug in the OMAP 8250 serial DMA path. The `rx_running`
flag was cleared too early, before hardware DMA teardown completed,
creating a window where software and hardware state were inconsistent.
This could lead to DMA conflicts, data corruption, or hangs on OMAP/AM
series SoCs which are widely used in embedded systems.

The fix is minimal (moving one line), self-contained, has no
dependencies, was written by the SoC vendor (TI), reviewed by another TI
engineer, and accepted by the serial subsystem maintainer. It meets all
stable kernel criteria.

**YES**

 drivers/tty/serial/8250/8250_omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index e26bae0a6488f..272bc07c9a6b5 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -931,7 +931,6 @@ static void __dma_rx_do_complete(struct uart_8250_port *p)
 		goto out;
 
 	cookie = dma->rx_cookie;
-	dma->rx_running = 0;
 
 	/* Re-enable RX FIFO interrupt now that transfer is complete */
 	if (priv->habit & UART_HAS_RHR_IT_DIS) {
@@ -965,6 +964,7 @@ static void __dma_rx_do_complete(struct uart_8250_port *p)
 		goto out;
 	ret = tty_insert_flip_string(tty_port, dma->rx_buf, count);
 
+	dma->rx_running = 0;
 	p->port.icount.rx += ret;
 	p->port.icount.buf_overrun += count - ret;
 out:
-- 
2.51.0