Archive-only list for patches
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.18-5.10] serial: sprd: Return -EPROBE_DEFER when uart clock is not ready
@ 2025-12-10  6:34 Sasha Levin
  2025-12-10  6:34 ` [PATCH AUTOSEL 6.18-5.10] usb: xhci: limit run_graceperiod for only usb 3.0 devices Sasha Levin
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Sasha Levin @ 2025-12-10  6:34 UTC (permalink / raw)
  To: patches, stable
  Cc: Wenhua Lin, Cixi Geng, Greg Kroah-Hartman, Sasha Levin, orsonzhai

From: Wenhua Lin <Wenhua.Lin@unisoc.com>

[ Upstream commit 29e8a0c587e328ed458380a45d6028adf64d7487 ]

In sprd_clk_init(), when devm_clk_get() returns -EPROBE_DEFER
for either uart or source clock, we should propagate the
error instead of just warning and continuing with NULL clocks.

Currently the driver only emits a warning when clock acquisition
fails and proceeds with NULL clock pointers. This can lead to
issues later when the clocks are actually needed. More importantly,
when the clock provider is not ready yet and returns -EPROBE_DEFER,
we should return this error to allow deferred probing.

This change adds explicit checks for -EPROBE_DEFER after both:
1. devm_clk_get(uport->dev, uart)
2. devm_clk_get(uport->dev, source)

When -EPROBE_DEFER is encountered, the function now returns
-EPROBE_DEFER to let the driver framework retry probing
later when the clock dependencies are resolved.

Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
Link: https://patch.msgid.link/20251022030840.956589-1-Wenhua.Lin@unisoc.com
Reviewed-by: Cixi Geng <cixi.geng@linux.dev>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Commit Analysis: serial: sprd: Return -EPROBE_DEFER when uart clock
is not ready

### 1. COMMIT MESSAGE ANALYSIS

**Subject**: Fixes incorrect handling of `-EPROBE_DEFER` error from
`devm_clk_get()`

**Key observations**:
- No `Cc: stable@vger.kernel.org` tag present
- No `Fixes:` tag pointing to when the bug was introduced
- Has `Reviewed-by:` from Cixi Geng
- Accepted by Greg Kroah-Hartman (serial subsystem maintainer)

### 2. CODE CHANGE ANALYSIS

The change adds two checks in `sprd_clk_init()`:

```c
clk_uart = devm_clk_get(uport->dev, "uart");
if (IS_ERR(clk_uart)) {
+    if (PTR_ERR(clk_uart) == -EPROBE_DEFER)
+        return -EPROBE_DEFER;
    dev_warn(...);
    clk_uart = NULL;
}

clk_parent = devm_clk_get(uport->dev, "source");
if (IS_ERR(clk_parent)) {
+    if (PTR_ERR(clk_parent) == -EPROBE_DEFER)
+        return -EPROBE_DEFER;
    dev_warn(...);
    clk_parent = NULL;
}
```

**Technical bug mechanism**: When clock providers aren't ready yet,
`devm_clk_get()` returns `-EPROBE_DEFER`. The existing code ignores this
error, sets the clock pointer to NULL, and continues. This bypasses the
kernel's deferred probing mechanism which exists precisely to handle
this dependency ordering scenario.

**Existing pattern**: The function already has identical handling for
the "enable" clock (visible in the context lines). This fix makes the
handling consistent for all three clocks.

### 3. CLASSIFICATION

**Type**: Bug fix (not a feature addition)

This fixes incorrect error handling. The `-EPROBE_DEFER` mechanism is a
fundamental kernel feature for handling driver load order dependencies.
Not propagating this error is a bug.

### 4. SCOPE AND RISK ASSESSMENT

- **Lines changed**: 6 lines added (two 3-line checks)
- **Files touched**: 1 file (`drivers/tty/serial/sprd_serial.c`)
- **Complexity**: Very low - follows identical pattern already in the
  same function
- **Subsystem**: Hardware-specific serial driver for Spreadtrum/Unisoc
  UARTs

**Risk**: Very low
- Pattern is already established and proven in the same function
- Only affects error handling during probe
- No changes to normal operation when clocks are available
- Worst case: probe failure happens earlier/more explicitly

### 5. USER IMPACT

**Affected users**: Users of Spreadtrum/Unisoc UART hardware (embedded
devices, some Android phones)

**Severity**: Medium-High for affected users - without this fix, the
serial port may not work correctly on systems where clock providers load
after the serial driver. This is common in embedded systems with device
tree-based configurations.

**Real bug**: This is a practical issue in probe ordering scenarios. The
driver would proceed with NULL clocks instead of waiting for
dependencies to be ready.

### 6. STABILITY INDICATORS

- Reviewed-by tag indicates code review
- Maintainer accepted the change
- Simple, straightforward change following existing code patterns
- No complex logic introduced

### 7. DEPENDENCY CHECK

- **Dependencies**: None - self-contained fix
- **Code existence in stable**: The sprd_serial driver has existed since
  ~2015 (commit 3e1f2029a4b40), so it's present in all active stable
  trees

### SUMMARY

**What it fixes**: Incorrect handling of `-EPROBE_DEFER` from
`devm_clk_get()` for two clocks ("uart" and "source"), causing the
driver to proceed with NULL clocks instead of deferring probe when clock
providers aren't ready.

**Stable kernel criteria**:
- ✅ Obviously correct (follows identical pattern already in function)
- ✅ Fixes a real bug (broken deferred probing)
- ✅ Small and contained (6 lines, 1 file)
- ✅ No new features
- ✅ Low risk of regression

**Concerns**:
- No explicit `Cc: stable` tag (author/maintainer didn't flag for
  stable)
- Relatively niche driver (Spreadtrum hardware)

**Risk vs Benefit**: The fix is minimal risk (identical pattern to
existing code) and addresses a real bug that could leave serial hardware
non-functional on embedded systems. The benefit outweighs the minimal
risk.

The lack of `Cc: stable` tag is notable but not determinative - this is
a straightforward bug fix that meets all stable criteria. The fix is
small, obviously correct, and addresses a real probe ordering issue that
embedded system users could encounter.

**YES**

 drivers/tty/serial/sprd_serial.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 8c9366321f8e7..092755f356836 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -1133,6 +1133,9 @@ static int sprd_clk_init(struct uart_port *uport)
 
 	clk_uart = devm_clk_get(uport->dev, "uart");
 	if (IS_ERR(clk_uart)) {
+		if (PTR_ERR(clk_uart) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
 		dev_warn(uport->dev, "uart%d can't get uart clock\n",
 			 uport->line);
 		clk_uart = NULL;
@@ -1140,6 +1143,9 @@ static int sprd_clk_init(struct uart_port *uport)
 
 	clk_parent = devm_clk_get(uport->dev, "source");
 	if (IS_ERR(clk_parent)) {
+		if (PTR_ERR(clk_parent) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
 		dev_warn(uport->dev, "uart%d can't get source clock\n",
 			 uport->line);
 		clk_parent = NULL;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-12-10  6:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-10  6:34 [PATCH AUTOSEL 6.18-5.10] serial: sprd: Return -EPROBE_DEFER when uart clock is not ready Sasha Levin
2025-12-10  6:34 ` [PATCH AUTOSEL 6.18-5.10] usb: xhci: limit run_graceperiod for only usb 3.0 devices Sasha Levin
2025-12-10  6:34 ` [PATCH AUTOSEL 6.18-5.10] usb: typec: ucsi: Handle incorrect num_connectors capability Sasha Levin
2025-12-10  6:34 ` [PATCH AUTOSEL 6.18-6.1] iio: adc: ti_am335x_adc: Limit step_avg to valid range for gcc complains Sasha Levin
2025-12-10  6:34 ` [PATCH AUTOSEL 6.18-5.10] usbip: Fix locking bug in RT-enabled kernels Sasha Levin
2025-12-10  6:34 ` [PATCH AUTOSEL 6.18-6.17] usb: xhci: Don't unchain link TRBs on quirky HCs Sasha Levin
2025-12-10  6:34 ` [PATCH AUTOSEL 6.18-6.6] libperf cpumap: Fix perf_cpu_map__max for an empty/NULL map Sasha Levin
2025-12-10  6:34 ` [PATCH AUTOSEL 6.18-5.10] usb: usb-storage: No additional quirks need to be added to the EL-R12 optical drive Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox