Skip to contentSkip to conversation

serial: 8250_dw: Fix runtime PM and use _DEFINE_DEV_PM_OPS

PATCH New Artem Shimko 1 participant 3 messages Nov 4, 2025

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover serial: 8250_dw: Fix runtime PM and use _DEFINE_DEV_PM_OPS

Dear maintainers and reviewers,

This patch series enhances runtime power management error handling in the 8250_dw driver and updates the PM ops initialization to modern kernel standards.

The first patch adds proper error checking for clock enable operations in runtime_resume, ensuring that failures are properly propagated rather than silently ignored.

The second patch reorganizes the runtime PM initialization sequence to simplify error handling and prevent potential suspend during device probe.

Thank you for your consideration.

--
Best regards,
Artem Shimko

---

ChangeLog:
v1: https://lore.kernel.org/all/[email protected]/T/#u
v2:
* Split combined changes into separate logical commits
* Added detailed commit messages explaining the changes

Artem Shimko (2):
serial: 8250_dw: handle clock enable errors in runtime_resume
serial: 8250_dw: fix runtime PM initialization sequence

 drivers/tty/serial/8250/8250_dw.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)
Signature · 1 line
2.43.0
#1 lore raw

PATCH v2 1/2 serial: 8250_dw: handle clock enable errors in runtime_resume

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.

---
 drivers/tty/serial/8250/8250_dw.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/tty/serial/8250/8250_dw.c +9 -2
index 710ae4d40aec..0ff500965c10 100644
@@ -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;
 }
Signature · 1 line
2.43.0
#2 lore raw

PATCH v2 2/2 serial: 8250_dw: fix runtime PM initialization sequence

In reply to Artem Shimko

Move pm_runtime_set_active() call earlier in probe to simplify error handling and add proper error checking to ensure the device is marked as active before any runtime PM operations can occur.

Additionally, replace the const struct dev_pm_ops declaration with _DEFINE_DEV_PM_OPS macro for better consistency with modern kernel PM patterns.

---
 drivers/tty/serial/8250/8250_dw.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/tty/serial/8250/8250_dw.c +7 -5
index 0ff500965c10..0c0a9fc97fe3 100644
@@ -643,6 +643,10 @@ static int dw8250_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	err = pm_runtime_set_active(dev);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to set the runtime suspend as active\n");
+
 	data->uart_16550_compatible = device_property_read_bool(dev, "snps,uart-16550-compatible");
 
 	data->pdata = device_get_match_data(p->dev);
@@ -685,7 +689,6 @@ static int dw8250_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, data);
 
-	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
 	return 0;
@@ -757,10 +760,9 @@ static int dw8250_runtime_resume(struct device *dev)
 	return 0;
 }
 
-static const struct dev_pm_ops dw8250_pm_ops = {
-	SYSTEM_SLEEP_PM_OPS(dw8250_suspend, dw8250_resume)
-	RUNTIME_PM_OPS(dw8250_runtime_suspend, dw8250_runtime_resume, NULL)
-};
+static _DEFINE_DEV_PM_OPS(dw8250_pm_ops, dw8250_suspend, dw8250_resume,
+			  dw8250_runtime_suspend, dw8250_runtime_resume,
+			  NULL);
 
 static const struct dw8250_platform_data dw8250_dw_apb = {
 	.usr_reg = DW_UART_USR,
Signature · 1 line
2.43.0