drivers/tty/serial/qcom_geni_serial.c | 176 +++----------------------- 1 file changed, 16 insertions(+), 160 deletions(-)
Revert commit 1afa70632c39 ("serial: qcom-geni: Enable PM runtime for
serial driver") and its dependent commit 86fa39dd6fb7 ("serial:
qcom-geni: Enable Serial on SA8255p Qualcomm platforms") because the
first one causes regression - hang task on Qualcomm RB1 board (QRB2210)
and unable to use serial at all during normal boot:
INFO: task kworker/u16:0:12 blocked for more than 42 seconds.
Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:kworker/u16:0 state:D stack:0 pid:12 tgid:12 ppid:2 task_flags:0x4208060 flags:0x00000010
Workqueue: async async_run_entry_fn
Call trace:
__switch_to+0xe8/0x1a0 (T)
__schedule+0x290/0x7c0
schedule+0x34/0x118
rpm_resume+0x14c/0x66c
rpm_resume+0x2a4/0x66c
rpm_resume+0x2a4/0x66c
rpm_resume+0x2a4/0x66c
__pm_runtime_resume+0x50/0x9c
__driver_probe_device+0x58/0x120
driver_probe_device+0x3c/0x154
__driver_attach_async_helper+0x4c/0xc0
async_run_entry_fn+0x34/0xe0
process_one_work+0x148/0x290
worker_thread+0x2c4/0x3e0
kthread+0x118/0x1c0
ret_from_fork+0x10/0x20
The issue was reported on 12th of August and was ignored by author of
commits introducing issue for two weeks. Only after complaining author
produced a fix which did not work, so if original commits cannot be
reliably fixed for 5 weeks, they obviously are buggy and need to be
dropped.
Fixes: 1afa70632c39 ("serial: qcom-geni: Enable PM runtime for serial driver")
Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
Closes: https://lore.kernel.org/all/DC0D53ZTNOBU.E8LSD5E5Z8TX@linaro.org/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. Correct reference to cause (proper commit) in the commit msg.
---
drivers/tty/serial/qcom_geni_serial.c | 176 +++-----------------------
1 file changed, 16 insertions(+), 160 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 0fdda3a1e70b..7c5befe5490d 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -14,7 +14,6 @@
#include <linux/irq.h>
#include <linux/module.h>
#include <linux/of.h>
-#include <linux/pm_domain.h>
#include <linux/pm_opp.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
@@ -102,16 +101,10 @@
#define DMA_RX_BUF_SIZE 2048
static DEFINE_IDA(port_ida);
-#define DOMAIN_IDX_POWER 0
-#define DOMAIN_IDX_PERF 1
struct qcom_geni_device_data {
bool console;
enum geni_se_xfer_mode mode;
- struct dev_pm_domain_attach_data pd_data;
- int (*resources_init)(struct uart_port *uport);
- int (*set_rate)(struct uart_port *uport, unsigned int baud);
- int (*power_state)(struct uart_port *uport, bool state);
};
struct qcom_geni_private_data {
@@ -149,7 +142,6 @@ struct qcom_geni_serial_port {
struct qcom_geni_private_data private_data;
const struct qcom_geni_device_data *dev_data;
- struct dev_pm_domain_list *pd_list;
};
static const struct uart_ops qcom_geni_console_pops;
@@ -1301,42 +1293,6 @@ static int geni_serial_set_rate(struct uart_port *uport, unsigned int baud)
return 0;
}
-static int geni_serial_set_level(struct uart_port *uport, unsigned int baud)
-{
- struct qcom_geni_serial_port *port = to_dev_port(uport);
- struct device *perf_dev = port->pd_list->pd_devs[DOMAIN_IDX_PERF];
-
- /*
- * The performance protocol sets UART communication
- * speeds by selecting different performance levels
- * through the OPP framework.
- *
- * Supported perf levels for baudrates in firmware are below
- * +---------------------+--------------------+
- * | Perf level value | Baudrate values |
- * +---------------------+--------------------+
- * | 300 | 300 |
- * | 1200 | 1200 |
- * | 2400 | 2400 |
- * | 4800 | 4800 |
- * | 9600 | 9600 |
- * | 19200 | 19200 |
- * | 38400 | 38400 |
- * | 57600 | 57600 |
- * | 115200 | 115200 |
- * | 230400 | 230400 |
- * | 460800 | 460800 |
- * | 921600 | 921600 |
- * | 2000000 | 2000000 |
- * | 3000000 | 3000000 |
- * | 3200000 | 3200000 |
- * | 4000000 | 4000000 |
- * +---------------------+--------------------+
- */
-
- return dev_pm_opp_set_level(perf_dev, baud);
-}
-
static void qcom_geni_serial_set_termios(struct uart_port *uport,
struct ktermios *termios,
const struct ktermios *old)
@@ -1355,7 +1311,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
/* baud rate */
baud = uart_get_baud_rate(uport, termios, old, 300, 8000000);
- ret = port->dev_data->set_rate(uport, baud);
+ ret = geni_serial_set_rate(uport, baud);
if (ret)
return;
@@ -1642,27 +1598,8 @@ static int geni_serial_resources_off(struct uart_port *uport)
return 0;
}
-static int geni_serial_resource_state(struct uart_port *uport, bool power_on)
+static int geni_serial_resource_init(struct qcom_geni_serial_port *port)
{
- return power_on ? geni_serial_resources_on(uport) : geni_serial_resources_off(uport);
-}
-
-static int geni_serial_pwr_init(struct uart_port *uport)
-{
- struct qcom_geni_serial_port *port = to_dev_port(uport);
- int ret;
-
- ret = dev_pm_domain_attach_list(port->se.dev,
- &port->dev_data->pd_data, &port->pd_list);
- if (ret <= 0)
- return -EINVAL;
-
- return 0;
-}
-
-static int geni_serial_resource_init(struct uart_port *uport)
-{
- struct qcom_geni_serial_port *port = to_dev_port(uport);
int ret;
port->se.clk = devm_clk_get(port->se.dev, "se");
@@ -1707,10 +1644,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
old_state = UART_PM_STATE_OFF;
if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF)
- pm_runtime_resume_and_get(uport->dev);
+ geni_serial_resources_on(uport);
else if (new_state == UART_PM_STATE_OFF &&
old_state == UART_PM_STATE_ON)
- pm_runtime_put_sync(uport->dev);
+ geni_serial_resources_off(uport);
}
@@ -1813,16 +1750,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
port->se.dev = &pdev->dev;
port->se.wrapper = dev_get_drvdata(pdev->dev.parent);
- ret = port->dev_data->resources_init(uport);
+ ret = geni_serial_resource_init(port);
if (ret)
return ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- ret = -EINVAL;
- goto error;
- }
-
+ if (!res)
+ return -EINVAL;
uport->mapbase = res->start;
uport->rs485_config = qcom_geni_rs485_config;
@@ -1834,26 +1768,19 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
if (!data->console) {
port->rx_buf = devm_kzalloc(uport->dev,
DMA_RX_BUF_SIZE, GFP_KERNEL);
- if (!port->rx_buf) {
- ret = -ENOMEM;
- goto error;
- }
+ if (!port->rx_buf)
+ return -ENOMEM;
}
port->name = devm_kasprintf(uport->dev, GFP_KERNEL,
"qcom_geni_serial_%s%d",
uart_console(uport) ? "console" : "uart", uport->line);
- if (!port->name) {
- ret = -ENOMEM;
- goto error;
- }
+ if (!port->name)
+ return -ENOMEM;
irq = platform_get_irq(pdev, 0);
- if (irq < 0) {
- ret = irq;
- goto error;
- }
-
+ if (irq < 0)
+ return irq;
uport->irq = irq;
uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
@@ -1875,18 +1802,16 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
IRQF_TRIGGER_HIGH, port->name, uport);
if (ret) {
dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret);
- goto error;
+ return ret;
}
ret = uart_get_rs485_mode(uport);
if (ret)
return ret;
- devm_pm_runtime_enable(port->se.dev);
-
ret = uart_add_one_port(drv, uport);
if (ret)
- goto error;
+ return ret;
if (port->wakeup_irq > 0) {
device_init_wakeup(&pdev->dev, true);
@@ -1896,15 +1821,11 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
device_init_wakeup(&pdev->dev, false);
ida_free(&port_ida, uport->line);
uart_remove_one_port(drv, uport);
- goto error;
+ return ret;
}
}
return 0;
-
-error:
- dev_pm_domain_detach_list(port->pd_list);
- return ret;
}
static void qcom_geni_serial_remove(struct platform_device *pdev)
@@ -1917,31 +1838,6 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
device_init_wakeup(&pdev->dev, false);
ida_free(&port_ida, uport->line);
uart_remove_one_port(drv, &port->uport);
- dev_pm_domain_detach_list(port->pd_list);
-}
-
-static int __maybe_unused qcom_geni_serial_runtime_suspend(struct device *dev)
-{
- struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
- struct uart_port *uport = &port->uport;
- int ret = 0;
-
- if (port->dev_data->power_state)
- ret = port->dev_data->power_state(uport, false);
-
- return ret;
-}
-
-static int __maybe_unused qcom_geni_serial_runtime_resume(struct device *dev)
-{
- struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
- struct uart_port *uport = &port->uport;
- int ret = 0;
-
- if (port->dev_data->power_state)
- ret = port->dev_data->power_state(uport, true);
-
- return ret;
}
static int qcom_geni_serial_suspend(struct device *dev)
@@ -1979,46 +1875,14 @@ static int qcom_geni_serial_resume(struct device *dev)
static const struct qcom_geni_device_data qcom_geni_console_data = {
.console = true,
.mode = GENI_SE_FIFO,
- .resources_init = geni_serial_resource_init,
- .set_rate = geni_serial_set_rate,
- .power_state = geni_serial_resource_state,
};
static const struct qcom_geni_device_data qcom_geni_uart_data = {
.console = false,
.mode = GENI_SE_DMA,
- .resources_init = geni_serial_resource_init,
- .set_rate = geni_serial_set_rate,
- .power_state = geni_serial_resource_state,
-};
-
-static const struct qcom_geni_device_data sa8255p_qcom_geni_console_data = {
- .console = true,
- .mode = GENI_SE_FIFO,
- .pd_data = {
- .pd_flags = PD_FLAG_DEV_LINK_ON,
- .pd_names = (const char*[]) { "power", "perf" },
- .num_pd_names = 2,
- },
- .resources_init = geni_serial_pwr_init,
- .set_rate = geni_serial_set_level,
-};
-
-static const struct qcom_geni_device_data sa8255p_qcom_geni_uart_data = {
- .console = false,
- .mode = GENI_SE_DMA,
- .pd_data = {
- .pd_flags = PD_FLAG_DEV_LINK_ON,
- .pd_names = (const char*[]) { "power", "perf" },
- .num_pd_names = 2,
- },
- .resources_init = geni_serial_pwr_init,
- .set_rate = geni_serial_set_level,
};
static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
- SET_RUNTIME_PM_OPS(qcom_geni_serial_runtime_suspend,
- qcom_geni_serial_runtime_resume, NULL)
SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_suspend, qcom_geni_serial_resume)
};
@@ -2027,18 +1891,10 @@ static const struct of_device_id qcom_geni_serial_match_table[] = {
.compatible = "qcom,geni-debug-uart",
.data = &qcom_geni_console_data,
},
- {
- .compatible = "qcom,sa8255p-geni-debug-uart",
- .data = &sa8255p_qcom_geni_console_data,
- },
{
.compatible = "qcom,geni-uart",
.data = &qcom_geni_uart_data,
},
- {
- .compatible = "qcom,sa8255p-geni-uart",
- .data = &sa8255p_qcom_geni_uart_data,
- },
{}
};
MODULE_DEVICE_TABLE(of, qcom_geni_serial_match_table);
--
2.48.1On Wed Sep 17, 2025 at 2:04 AM BST, Krzysztof Kozlowski wrote:
> Revert commit 1afa70632c39 ("serial: qcom-geni: Enable PM runtime for
> serial driver") and its dependent commit 86fa39dd6fb7 ("serial:
> qcom-geni: Enable Serial on SA8255p Qualcomm platforms") because the
> first one causes regression - hang task on Qualcomm RB1 board (QRB2210)
> and unable to use serial at all during normal boot:
>
> INFO: task kworker/u16:0:12 blocked for more than 42 seconds.
> Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> task:kworker/u16:0 state:D stack:0 pid:12 tgid:12 ppid:2 task_flags:0x4208060 flags:0x00000010
> Workqueue: async async_run_entry_fn
> Call trace:
> __switch_to+0xe8/0x1a0 (T)
> __schedule+0x290/0x7c0
> schedule+0x34/0x118
> rpm_resume+0x14c/0x66c
> rpm_resume+0x2a4/0x66c
> rpm_resume+0x2a4/0x66c
> rpm_resume+0x2a4/0x66c
> __pm_runtime_resume+0x50/0x9c
> __driver_probe_device+0x58/0x120
> driver_probe_device+0x3c/0x154
> __driver_attach_async_helper+0x4c/0xc0
> async_run_entry_fn+0x34/0xe0
> process_one_work+0x148/0x290
> worker_thread+0x2c4/0x3e0
> kthread+0x118/0x1c0
> ret_from_fork+0x10/0x20
>
> The issue was reported on 12th of August and was ignored by author of
> commits introducing issue for two weeks. Only after complaining author
> produced a fix which did not work, so if original commits cannot be
> reliably fixed for 5 weeks, they obviously are buggy and need to be
> dropped.
>
> Fixes: 1afa70632c39 ("serial: qcom-geni: Enable PM runtime for serial driver")
> Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
> Closes: https://lore.kernel.org/all/DC0D53ZTNOBU.E8LSD5E5Z8TX@linaro.org/
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: Alexey Klimov <alexey.klimov@linaro.org>
Reviewed-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>
> Changes in v2:
> 1. Correct reference to cause (proper commit) in the commit msg.
> ---
> drivers/tty/serial/qcom_geni_serial.c | 176 +++-----------------------
> 1 file changed, 16 insertions(+), 160 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 0fdda3a1e70b..7c5befe5490d 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -14,7 +14,6 @@
> #include <linux/irq.h>
> #include <linux/module.h>
> #include <linux/of.h>
> -#include <linux/pm_domain.h>
> #include <linux/pm_opp.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> @@ -102,16 +101,10 @@
> #define DMA_RX_BUF_SIZE 2048
>
> static DEFINE_IDA(port_ida);
> -#define DOMAIN_IDX_POWER 0
> -#define DOMAIN_IDX_PERF 1
>
> struct qcom_geni_device_data {
> bool console;
> enum geni_se_xfer_mode mode;
> - struct dev_pm_domain_attach_data pd_data;
> - int (*resources_init)(struct uart_port *uport);
> - int (*set_rate)(struct uart_port *uport, unsigned int baud);
> - int (*power_state)(struct uart_port *uport, bool state);
> };
>
> struct qcom_geni_private_data {
> @@ -149,7 +142,6 @@ struct qcom_geni_serial_port {
>
> struct qcom_geni_private_data private_data;
> const struct qcom_geni_device_data *dev_data;
> - struct dev_pm_domain_list *pd_list;
> };
>
> static const struct uart_ops qcom_geni_console_pops;
> @@ -1301,42 +1293,6 @@ static int geni_serial_set_rate(struct uart_port *uport, unsigned int baud)
> return 0;
> }
>
> -static int geni_serial_set_level(struct uart_port *uport, unsigned int baud)
> -{
> - struct qcom_geni_serial_port *port = to_dev_port(uport);
> - struct device *perf_dev = port->pd_list->pd_devs[DOMAIN_IDX_PERF];
> -
> - /*
> - * The performance protocol sets UART communication
> - * speeds by selecting different performance levels
> - * through the OPP framework.
> - *
> - * Supported perf levels for baudrates in firmware are below
> - * +---------------------+--------------------+
> - * | Perf level value | Baudrate values |
> - * +---------------------+--------------------+
> - * | 300 | 300 |
> - * | 1200 | 1200 |
> - * | 2400 | 2400 |
> - * | 4800 | 4800 |
> - * | 9600 | 9600 |
> - * | 19200 | 19200 |
> - * | 38400 | 38400 |
> - * | 57600 | 57600 |
> - * | 115200 | 115200 |
> - * | 230400 | 230400 |
> - * | 460800 | 460800 |
> - * | 921600 | 921600 |
> - * | 2000000 | 2000000 |
> - * | 3000000 | 3000000 |
> - * | 3200000 | 3200000 |
> - * | 4000000 | 4000000 |
> - * +---------------------+--------------------+
> - */
> -
> - return dev_pm_opp_set_level(perf_dev, baud);
> -}
> -
> static void qcom_geni_serial_set_termios(struct uart_port *uport,
> struct ktermios *termios,
> const struct ktermios *old)
> @@ -1355,7 +1311,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
> /* baud rate */
> baud = uart_get_baud_rate(uport, termios, old, 300, 8000000);
>
> - ret = port->dev_data->set_rate(uport, baud);
> + ret = geni_serial_set_rate(uport, baud);
> if (ret)
> return;
>
> @@ -1642,27 +1598,8 @@ static int geni_serial_resources_off(struct uart_port *uport)
> return 0;
> }
>
> -static int geni_serial_resource_state(struct uart_port *uport, bool power_on)
> +static int geni_serial_resource_init(struct qcom_geni_serial_port *port)
> {
> - return power_on ? geni_serial_resources_on(uport) : geni_serial_resources_off(uport);
> -}
> -
> -static int geni_serial_pwr_init(struct uart_port *uport)
> -{
> - struct qcom_geni_serial_port *port = to_dev_port(uport);
> - int ret;
> -
> - ret = dev_pm_domain_attach_list(port->se.dev,
> - &port->dev_data->pd_data, &port->pd_list);
> - if (ret <= 0)
> - return -EINVAL;
> -
> - return 0;
> -}
> -
> -static int geni_serial_resource_init(struct uart_port *uport)
> -{
> - struct qcom_geni_serial_port *port = to_dev_port(uport);
> int ret;
>
> port->se.clk = devm_clk_get(port->se.dev, "se");
> @@ -1707,10 +1644,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
> old_state = UART_PM_STATE_OFF;
>
> if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF)
> - pm_runtime_resume_and_get(uport->dev);
> + geni_serial_resources_on(uport);
> else if (new_state == UART_PM_STATE_OFF &&
> old_state == UART_PM_STATE_ON)
> - pm_runtime_put_sync(uport->dev);
> + geni_serial_resources_off(uport);
>
> }
>
> @@ -1813,16 +1750,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> port->se.dev = &pdev->dev;
> port->se.wrapper = dev_get_drvdata(pdev->dev.parent);
>
> - ret = port->dev_data->resources_init(uport);
> + ret = geni_serial_resource_init(port);
> if (ret)
> return ret;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res) {
> - ret = -EINVAL;
> - goto error;
> - }
> -
> + if (!res)
> + return -EINVAL;
> uport->mapbase = res->start;
>
> uport->rs485_config = qcom_geni_rs485_config;
> @@ -1834,26 +1768,19 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> if (!data->console) {
> port->rx_buf = devm_kzalloc(uport->dev,
> DMA_RX_BUF_SIZE, GFP_KERNEL);
> - if (!port->rx_buf) {
> - ret = -ENOMEM;
> - goto error;
> - }
> + if (!port->rx_buf)
> + return -ENOMEM;
> }
>
> port->name = devm_kasprintf(uport->dev, GFP_KERNEL,
> "qcom_geni_serial_%s%d",
> uart_console(uport) ? "console" : "uart", uport->line);
> - if (!port->name) {
> - ret = -ENOMEM;
> - goto error;
> - }
> + if (!port->name)
> + return -ENOMEM;
>
> irq = platform_get_irq(pdev, 0);
> - if (irq < 0) {
> - ret = irq;
> - goto error;
> - }
> -
> + if (irq < 0)
> + return irq;
> uport->irq = irq;
> uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
>
> @@ -1875,18 +1802,16 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> IRQF_TRIGGER_HIGH, port->name, uport);
> if (ret) {
> dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret);
> - goto error;
> + return ret;
> }
>
> ret = uart_get_rs485_mode(uport);
> if (ret)
> return ret;
>
> - devm_pm_runtime_enable(port->se.dev);
> -
> ret = uart_add_one_port(drv, uport);
> if (ret)
> - goto error;
> + return ret;
>
> if (port->wakeup_irq > 0) {
> device_init_wakeup(&pdev->dev, true);
> @@ -1896,15 +1821,11 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> device_init_wakeup(&pdev->dev, false);
> ida_free(&port_ida, uport->line);
> uart_remove_one_port(drv, uport);
> - goto error;
> + return ret;
> }
> }
>
> return 0;
> -
> -error:
> - dev_pm_domain_detach_list(port->pd_list);
> - return ret;
> }
>
> static void qcom_geni_serial_remove(struct platform_device *pdev)
> @@ -1917,31 +1838,6 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
> device_init_wakeup(&pdev->dev, false);
> ida_free(&port_ida, uport->line);
> uart_remove_one_port(drv, &port->uport);
> - dev_pm_domain_detach_list(port->pd_list);
> -}
> -
> -static int __maybe_unused qcom_geni_serial_runtime_suspend(struct device *dev)
> -{
> - struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
> - struct uart_port *uport = &port->uport;
> - int ret = 0;
> -
> - if (port->dev_data->power_state)
> - ret = port->dev_data->power_state(uport, false);
> -
> - return ret;
> -}
> -
> -static int __maybe_unused qcom_geni_serial_runtime_resume(struct device *dev)
> -{
> - struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
> - struct uart_port *uport = &port->uport;
> - int ret = 0;
> -
> - if (port->dev_data->power_state)
> - ret = port->dev_data->power_state(uport, true);
> -
> - return ret;
> }
>
> static int qcom_geni_serial_suspend(struct device *dev)
> @@ -1979,46 +1875,14 @@ static int qcom_geni_serial_resume(struct device *dev)
> static const struct qcom_geni_device_data qcom_geni_console_data = {
> .console = true,
> .mode = GENI_SE_FIFO,
> - .resources_init = geni_serial_resource_init,
> - .set_rate = geni_serial_set_rate,
> - .power_state = geni_serial_resource_state,
> };
>
> static const struct qcom_geni_device_data qcom_geni_uart_data = {
> .console = false,
> .mode = GENI_SE_DMA,
> - .resources_init = geni_serial_resource_init,
> - .set_rate = geni_serial_set_rate,
> - .power_state = geni_serial_resource_state,
> -};
> -
> -static const struct qcom_geni_device_data sa8255p_qcom_geni_console_data = {
> - .console = true,
> - .mode = GENI_SE_FIFO,
> - .pd_data = {
> - .pd_flags = PD_FLAG_DEV_LINK_ON,
> - .pd_names = (const char*[]) { "power", "perf" },
> - .num_pd_names = 2,
> - },
> - .resources_init = geni_serial_pwr_init,
> - .set_rate = geni_serial_set_level,
> -};
> -
> -static const struct qcom_geni_device_data sa8255p_qcom_geni_uart_data = {
> - .console = false,
> - .mode = GENI_SE_DMA,
> - .pd_data = {
> - .pd_flags = PD_FLAG_DEV_LINK_ON,
> - .pd_names = (const char*[]) { "power", "perf" },
> - .num_pd_names = 2,
> - },
> - .resources_init = geni_serial_pwr_init,
> - .set_rate = geni_serial_set_level,
> };
>
> static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
> - SET_RUNTIME_PM_OPS(qcom_geni_serial_runtime_suspend,
> - qcom_geni_serial_runtime_resume, NULL)
> SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_suspend, qcom_geni_serial_resume)
> };
>
> @@ -2027,18 +1891,10 @@ static const struct of_device_id qcom_geni_serial_match_table[] = {
> .compatible = "qcom,geni-debug-uart",
> .data = &qcom_geni_console_data,
> },
> - {
> - .compatible = "qcom,sa8255p-geni-debug-uart",
> - .data = &sa8255p_qcom_geni_console_data,
> - },
> {
> .compatible = "qcom,geni-uart",
> .data = &qcom_geni_uart_data,
> },
> - {
> - .compatible = "qcom,sa8255p-geni-uart",
> - .data = &sa8255p_qcom_geni_uart_data,
> - },
> {}
> };
> MODULE_DEVICE_TABLE(of, qcom_geni_serial_match_table);
On 17/09/2025 02:04, Krzysztof Kozlowski wrote:
> Revert commit 1afa70632c39 ("serial: qcom-geni: Enable PM runtime for
> serial driver") and its dependent commit 86fa39dd6fb7 ("serial:
> qcom-geni: Enable Serial on SA8255p Qualcomm platforms") because the
> first one causes regression - hang task on Qualcomm RB1 board (QRB2210)
> and unable to use serial at all during normal boot:
>
> INFO: task kworker/u16:0:12 blocked for more than 42 seconds.
> Not tainted 6.17.0-rc1-00004-g53e760d89498 #9
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> task:kworker/u16:0 state:D stack:0 pid:12 tgid:12 ppid:2 task_flags:0x4208060 flags:0x00000010
> Workqueue: async async_run_entry_fn
> Call trace:
> __switch_to+0xe8/0x1a0 (T)
> __schedule+0x290/0x7c0
> schedule+0x34/0x118
> rpm_resume+0x14c/0x66c
> rpm_resume+0x2a4/0x66c
> rpm_resume+0x2a4/0x66c
> rpm_resume+0x2a4/0x66c
> __pm_runtime_resume+0x50/0x9c
> __driver_probe_device+0x58/0x120
> driver_probe_device+0x3c/0x154
> __driver_attach_async_helper+0x4c/0xc0
> async_run_entry_fn+0x34/0xe0
> process_one_work+0x148/0x290
> worker_thread+0x2c4/0x3e0
> kthread+0x118/0x1c0
> ret_from_fork+0x10/0x20
>
> The issue was reported on 12th of August and was ignored by author of
> commits introducing issue for two weeks. Only after complaining author
> produced a fix which did not work, so if original commits cannot be
> reliably fixed for 5 weeks, they obviously are buggy and need to be
> dropped.
>
> Fixes: 1afa70632c39 ("serial: qcom-geni: Enable PM runtime for serial driver")
> Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
> Closes: https://lore.kernel.org/all/DC0D53ZTNOBU.E8LSD5E5Z8TX@linaro.org/
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
>
> Changes in v2:
> 1. Correct reference to cause (proper commit) in the commit msg.
> ---
> drivers/tty/serial/qcom_geni_serial.c | 176 +++-----------------------
> 1 file changed, 16 insertions(+), 160 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 0fdda3a1e70b..7c5befe5490d 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -14,7 +14,6 @@
> #include <linux/irq.h>
> #include <linux/module.h>
> #include <linux/of.h>
> -#include <linux/pm_domain.h>
> #include <linux/pm_opp.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> @@ -102,16 +101,10 @@
> #define DMA_RX_BUF_SIZE 2048
>
> static DEFINE_IDA(port_ida);
> -#define DOMAIN_IDX_POWER 0
> -#define DOMAIN_IDX_PERF 1
>
> struct qcom_geni_device_data {
> bool console;
> enum geni_se_xfer_mode mode;
> - struct dev_pm_domain_attach_data pd_data;
> - int (*resources_init)(struct uart_port *uport);
> - int (*set_rate)(struct uart_port *uport, unsigned int baud);
> - int (*power_state)(struct uart_port *uport, bool state);
> };
>
> struct qcom_geni_private_data {
> @@ -149,7 +142,6 @@ struct qcom_geni_serial_port {
>
> struct qcom_geni_private_data private_data;
> const struct qcom_geni_device_data *dev_data;
> - struct dev_pm_domain_list *pd_list;
> };
>
> static const struct uart_ops qcom_geni_console_pops;
> @@ -1301,42 +1293,6 @@ static int geni_serial_set_rate(struct uart_port *uport, unsigned int baud)
> return 0;
> }
>
> -static int geni_serial_set_level(struct uart_port *uport, unsigned int baud)
> -{
> - struct qcom_geni_serial_port *port = to_dev_port(uport);
> - struct device *perf_dev = port->pd_list->pd_devs[DOMAIN_IDX_PERF];
> -
> - /*
> - * The performance protocol sets UART communication
> - * speeds by selecting different performance levels
> - * through the OPP framework.
> - *
> - * Supported perf levels for baudrates in firmware are below
> - * +---------------------+--------------------+
> - * | Perf level value | Baudrate values |
> - * +---------------------+--------------------+
> - * | 300 | 300 |
> - * | 1200 | 1200 |
> - * | 2400 | 2400 |
> - * | 4800 | 4800 |
> - * | 9600 | 9600 |
> - * | 19200 | 19200 |
> - * | 38400 | 38400 |
> - * | 57600 | 57600 |
> - * | 115200 | 115200 |
> - * | 230400 | 230400 |
> - * | 460800 | 460800 |
> - * | 921600 | 921600 |
> - * | 2000000 | 2000000 |
> - * | 3000000 | 3000000 |
> - * | 3200000 | 3200000 |
> - * | 4000000 | 4000000 |
> - * +---------------------+--------------------+
> - */
> -
> - return dev_pm_opp_set_level(perf_dev, baud);
> -}
> -
> static void qcom_geni_serial_set_termios(struct uart_port *uport,
> struct ktermios *termios,
> const struct ktermios *old)
> @@ -1355,7 +1311,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
> /* baud rate */
> baud = uart_get_baud_rate(uport, termios, old, 300, 8000000);
>
> - ret = port->dev_data->set_rate(uport, baud);
> + ret = geni_serial_set_rate(uport, baud);
> if (ret)
> return;
>
> @@ -1642,27 +1598,8 @@ static int geni_serial_resources_off(struct uart_port *uport)
> return 0;
> }
>
> -static int geni_serial_resource_state(struct uart_port *uport, bool power_on)
> +static int geni_serial_resource_init(struct qcom_geni_serial_port *port)
> {
> - return power_on ? geni_serial_resources_on(uport) : geni_serial_resources_off(uport);
> -}
> -
> -static int geni_serial_pwr_init(struct uart_port *uport)
> -{
> - struct qcom_geni_serial_port *port = to_dev_port(uport);
> - int ret;
> -
> - ret = dev_pm_domain_attach_list(port->se.dev,
> - &port->dev_data->pd_data, &port->pd_list);
> - if (ret <= 0)
> - return -EINVAL;
> -
> - return 0;
> -}
> -
> -static int geni_serial_resource_init(struct uart_port *uport)
> -{
> - struct qcom_geni_serial_port *port = to_dev_port(uport);
> int ret;
>
> port->se.clk = devm_clk_get(port->se.dev, "se");
> @@ -1707,10 +1644,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
> old_state = UART_PM_STATE_OFF;
>
> if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF)
> - pm_runtime_resume_and_get(uport->dev);
> + geni_serial_resources_on(uport);
> else if (new_state == UART_PM_STATE_OFF &&
> old_state == UART_PM_STATE_ON)
> - pm_runtime_put_sync(uport->dev);
> + geni_serial_resources_off(uport);
>
> }
>
> @@ -1813,16 +1750,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> port->se.dev = &pdev->dev;
> port->se.wrapper = dev_get_drvdata(pdev->dev.parent);
>
> - ret = port->dev_data->resources_init(uport);
> + ret = geni_serial_resource_init(port);
> if (ret)
> return ret;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res) {
> - ret = -EINVAL;
> - goto error;
> - }
> -
> + if (!res)
> + return -EINVAL;
> uport->mapbase = res->start;
>
> uport->rs485_config = qcom_geni_rs485_config;
> @@ -1834,26 +1768,19 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> if (!data->console) {
> port->rx_buf = devm_kzalloc(uport->dev,
> DMA_RX_BUF_SIZE, GFP_KERNEL);
> - if (!port->rx_buf) {
> - ret = -ENOMEM;
> - goto error;
> - }
> + if (!port->rx_buf)
> + return -ENOMEM;
> }
>
> port->name = devm_kasprintf(uport->dev, GFP_KERNEL,
> "qcom_geni_serial_%s%d",
> uart_console(uport) ? "console" : "uart", uport->line);
> - if (!port->name) {
> - ret = -ENOMEM;
> - goto error;
> - }
> + if (!port->name)
> + return -ENOMEM;
>
> irq = platform_get_irq(pdev, 0);
> - if (irq < 0) {
> - ret = irq;
> - goto error;
> - }
> -
> + if (irq < 0)
> + return irq;
> uport->irq = irq;
> uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
>
> @@ -1875,18 +1802,16 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> IRQF_TRIGGER_HIGH, port->name, uport);
> if (ret) {
> dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret);
> - goto error;
> + return ret;
> }
>
> ret = uart_get_rs485_mode(uport);
> if (ret)
> return ret;
>
> - devm_pm_runtime_enable(port->se.dev);
> -
> ret = uart_add_one_port(drv, uport);
> if (ret)
> - goto error;
> + return ret;
>
> if (port->wakeup_irq > 0) {
> device_init_wakeup(&pdev->dev, true);
> @@ -1896,15 +1821,11 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> device_init_wakeup(&pdev->dev, false);
> ida_free(&port_ida, uport->line);
> uart_remove_one_port(drv, uport);
> - goto error;
> + return ret;
> }
> }
>
> return 0;
> -
> -error:
> - dev_pm_domain_detach_list(port->pd_list);
> - return ret;
> }
>
> static void qcom_geni_serial_remove(struct platform_device *pdev)
> @@ -1917,31 +1838,6 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
> device_init_wakeup(&pdev->dev, false);
> ida_free(&port_ida, uport->line);
> uart_remove_one_port(drv, &port->uport);
> - dev_pm_domain_detach_list(port->pd_list);
> -}
> -
> -static int __maybe_unused qcom_geni_serial_runtime_suspend(struct device *dev)
> -{
> - struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
> - struct uart_port *uport = &port->uport;
> - int ret = 0;
> -
> - if (port->dev_data->power_state)
> - ret = port->dev_data->power_state(uport, false);
> -
> - return ret;
> -}
> -
> -static int __maybe_unused qcom_geni_serial_runtime_resume(struct device *dev)
> -{
> - struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
> - struct uart_port *uport = &port->uport;
> - int ret = 0;
> -
> - if (port->dev_data->power_state)
> - ret = port->dev_data->power_state(uport, true);
> -
> - return ret;
> }
>
> static int qcom_geni_serial_suspend(struct device *dev)
> @@ -1979,46 +1875,14 @@ static int qcom_geni_serial_resume(struct device *dev)
> static const struct qcom_geni_device_data qcom_geni_console_data = {
> .console = true,
> .mode = GENI_SE_FIFO,
> - .resources_init = geni_serial_resource_init,
> - .set_rate = geni_serial_set_rate,
> - .power_state = geni_serial_resource_state,
> };
>
> static const struct qcom_geni_device_data qcom_geni_uart_data = {
> .console = false,
> .mode = GENI_SE_DMA,
> - .resources_init = geni_serial_resource_init,
> - .set_rate = geni_serial_set_rate,
> - .power_state = geni_serial_resource_state,
> -};
> -
> -static const struct qcom_geni_device_data sa8255p_qcom_geni_console_data = {
> - .console = true,
> - .mode = GENI_SE_FIFO,
> - .pd_data = {
> - .pd_flags = PD_FLAG_DEV_LINK_ON,
> - .pd_names = (const char*[]) { "power", "perf" },
> - .num_pd_names = 2,
> - },
> - .resources_init = geni_serial_pwr_init,
> - .set_rate = geni_serial_set_level,
> -};
> -
> -static const struct qcom_geni_device_data sa8255p_qcom_geni_uart_data = {
> - .console = false,
> - .mode = GENI_SE_DMA,
> - .pd_data = {
> - .pd_flags = PD_FLAG_DEV_LINK_ON,
> - .pd_names = (const char*[]) { "power", "perf" },
> - .num_pd_names = 2,
> - },
> - .resources_init = geni_serial_pwr_init,
> - .set_rate = geni_serial_set_level,
> };
>
> static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
> - SET_RUNTIME_PM_OPS(qcom_geni_serial_runtime_suspend,
> - qcom_geni_serial_runtime_resume, NULL)
> SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_suspend, qcom_geni_serial_resume)
> };
>
> @@ -2027,18 +1891,10 @@ static const struct of_device_id qcom_geni_serial_match_table[] = {
> .compatible = "qcom,geni-debug-uart",
> .data = &qcom_geni_console_data,
> },
> - {
> - .compatible = "qcom,sa8255p-geni-debug-uart",
> - .data = &sa8255p_qcom_geni_console_data,
> - },
> {
> .compatible = "qcom,geni-uart",
> .data = &qcom_geni_uart_data,
> },
> - {
> - .compatible = "qcom,sa8255p-geni-uart",
> - .data = &sa8255p_qcom_geni_uart_data,
> - },
> {}
> };
> MODULE_DEVICE_TABLE(of, qcom_geni_serial_match_table);
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
© 2016 - 2025 Red Hat, Inc.