From: neil.armstrong@linaro.org
To: Praveen Talari <quic_ptalari@quicinc.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>, Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konradybcio@kernel.org>,
Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
Stephen Boyd <sboyd@kernel.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
linux-pm@vger.kernel.org
Cc: psodagud@quicinc.com, djaggi@quicinc.com,
quic_msavaliy@quicinc.com, quic_vtanuku@quicinc.com,
quic_arandive@quicinc.com, quic_mnaresh@quicinc.com,
quic_shazhuss@quicinc.com
Subject: Re: [PATCH v3 9/9] serial: qcom-geni: Enable Serial on SA8255p Qualcomm platforms
Date: Fri, 2 May 2025 11:24:42 +0200 [thread overview]
Message-ID: <aef73cdc-d855-4f17-bd76-289b6e26cc14@linaro.org> (raw)
In-Reply-To: <20250502031018.1292-10-quic_ptalari@quicinc.com>
On 02/05/2025 05:10, Praveen Talari wrote:
> The Qualcomm automotive SA8255p SoC relies on firmware to configure
> platform resources, including clocks, interconnects and TLMM.
> The driver requests resources operations over SCMI using power
> and performance protocols.
>
> The SCMI power protocol enables or disables resources like clocks,
> interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
> such as resume/suspend, to control power states(on/off).
>
> The SCMI performance protocol manages UART baud rates, with each baud
> rate represented by a performance level. The driver uses the
> dev_pm_opp_set_level() API to request the desired baud rate by
> specifying the performance level.
>
> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 150 +++++++++++++++++++++++---
> 1 file changed, 135 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 9d698c354510..51036d5c8ea1 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -11,6 +11,7 @@
> #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>
> @@ -99,10 +100,16 @@
> #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 (*geni_serial_pwr_rsc_init)(struct uart_port *uport);
> + int (*geni_serial_set_rate)(struct uart_port *uport, unsigned long clk_freq);
> + int (*geni_serial_switch_power_state)(struct uart_port *uport, bool state);
The geni_serial_ is not needed here, so not need to use "pwr", just use the
original names:
resources_init
set_rate
power_state
Neil
> };
>
> struct qcom_geni_private_data {
> @@ -140,6 +147,7 @@ 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;
> @@ -1331,6 +1339,42 @@ static int geni_serial_set_rate(struct uart_port *uport, unsigned long baud)
> return 0;
> }
>
> +static int geni_serial_set_level(struct uart_port *uport, unsigned long 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)
> @@ -1349,7 +1393,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
> /* baud rate */
> baud = uart_get_baud_rate(uport, termios, old, 300, 4000000);
>
> - ret = geni_serial_set_rate(uport, baud);
> + ret = port->dev_data->geni_serial_set_rate(uport, baud);
> if (ret) {
> dev_err(port->se.dev,
> "%s: Failed to set baud:%u ret:%d\n",
> @@ -1640,8 +1684,27 @@ static int geni_serial_resources_on(struct uart_port *uport)
> return 0;
> }
>
> -static int geni_serial_resource_init(struct qcom_geni_serial_port *port)
> +static int geni_serial_resource_state(struct uart_port *uport, bool power_on)
> {
> + 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");
> @@ -1680,7 +1743,6 @@ static int geni_serial_resource_init(struct qcom_geni_serial_port *port)
> static void qcom_geni_serial_pm(struct uart_port *uport,
> unsigned int new_state, unsigned int old_state)
> {
> -
> /* If we've never been called, treat it as off */
> if (old_state == UART_PM_STATE_UNDEFINED)
> old_state = UART_PM_STATE_OFF;
> @@ -1774,13 +1836,16 @@ 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 = geni_serial_resource_init(port);
> + ret = port->dev_data->geni_serial_pwr_rsc_init(uport);
> if (ret)
> return ret;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res)
> - return -EINVAL;
> + if (!res) {
> + ret = -EINVAL;
> + goto error;
> + }
> +
> uport->mapbase = res->start;
>
> port->tx_fifo_depth = DEF_FIFO_DEPTH_WORDS;
> @@ -1790,19 +1855,26 @@ 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)
> - return -ENOMEM;
> + if (!port->rx_buf) {
> + ret = -ENOMEM;
> + goto error;
> + }
> }
>
> port->name = devm_kasprintf(uport->dev, GFP_KERNEL,
> "qcom_geni_serial_%s%d",
> uart_console(uport) ? "console" : "uart", uport->line);
> - if (!port->name)
> - return -ENOMEM;
> + if (!port->name) {
> + ret = -ENOMEM;
> + goto error;
> + }
>
> irq = platform_get_irq(pdev, 0);
> - if (irq < 0)
> - return irq;
> + if (irq < 0) {
> + ret = irq;
> + goto error;
> + }
> +
> uport->irq = irq;
> uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
>
> @@ -1824,7 +1896,7 @@ 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);
> - return ret;
> + goto error;
> }
>
> pm_runtime_enable(port->se.dev);
> @@ -1849,6 +1921,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>
> error:
> pm_runtime_disable(port->se.dev);
> + dev_pm_domain_detach_list(port->pd_list);
> return ret;
> }
>
> @@ -1863,22 +1936,31 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
> ida_free(&port_ida, uport->line);
> pm_runtime_disable(port->se.dev);
> uart_remove_one_port(drv, &port->uport);
> + dev_pm_domain_detach_list(port->pd_list);
> }
>
> static int 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;
>
> - return geni_serial_resources_off(uport);
> + if (port->dev_data->geni_serial_switch_power_state)
> + ret = port->dev_data->geni_serial_switch_power_state(uport, false);
> +
> + return ret;
> };
>
> static int 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->geni_serial_switch_power_state)
> + ret = port->dev_data->geni_serial_switch_power_state(uport, true);
>
> - return geni_serial_resources_on(uport);
> + return ret;
> };
>
> static int qcom_geni_serial_suspend(struct device *dev)
> @@ -1916,11 +1998,41 @@ 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,
> + .geni_serial_pwr_rsc_init = geni_serial_resource_init,
> + .geni_serial_set_rate = geni_serial_set_rate,
> + .geni_serial_switch_power_state = geni_serial_resource_state,
> };
>
> static const struct qcom_geni_device_data qcom_geni_uart_data = {
> .console = false,
> .mode = GENI_SE_DMA,
> + .geni_serial_pwr_rsc_init = geni_serial_resource_init,
> + .geni_serial_set_rate = geni_serial_set_rate,
> + .geni_serial_switch_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,
> + },
> + .geni_serial_pwr_rsc_init = geni_serial_pwr_init,
> + .geni_serial_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,
> + },
> + .geni_serial_pwr_rsc_init = geni_serial_pwr_init,
> + .geni_serial_set_rate = geni_serial_set_level,
> };
>
> static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
> @@ -1934,10 +2046,18 @@ 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);
prev parent reply other threads:[~2025-05-02 9:24 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-02 3:10 [PATCH v3 0/9] Enable QUPs and Serial on SA8255p Qualcomm platforms Praveen Talari
2025-05-02 3:10 ` [PATCH v3 1/9] opp: add new helper API dev_pm_opp_set_level() Praveen Talari
2025-05-02 3:49 ` Trilok Soni
2025-05-02 5:37 ` Viresh Kumar
2025-05-02 7:37 ` Praveen Talari
2025-05-02 7:45 ` Viresh Kumar
2025-05-02 8:01 ` Praveen Talari
2025-05-02 8:14 ` Viresh Kumar
2025-05-02 14:01 ` Praveen Talari
2025-05-02 14:11 ` Viresh Kumar
2025-05-02 14:39 ` Praveen Talari
2025-05-02 3:10 ` [PATCH v3 2/9] dt-bindings: serial: describe SA8255p Praveen Talari
2025-05-02 3:49 ` Trilok Soni
2025-05-02 6:36 ` Krzysztof Kozlowski
2025-05-02 3:10 ` [PATCH v3 3/9] dt-bindings: qcom: geni-se: " Praveen Talari
2025-05-02 6:37 ` Krzysztof Kozlowski
2025-05-02 8:07 ` Praveen Talari
2025-05-02 3:10 ` [PATCH v3 4/9] soc: qcom: geni-se: Enable QUPs on SA8255p Qualcomm platforms Praveen Talari
2025-05-02 9:28 ` neil.armstrong
2025-05-07 5:49 ` kernel test robot
2025-05-02 3:10 ` [PATCH v3 5/9] serial: qcom-geni: move resource initialization to separate function Praveen Talari
2025-05-02 3:10 ` [PATCH v3 6/9] serial: qcom-geni: move resource control logic to separate functions Praveen Talari
2025-05-02 3:10 ` [PATCH v3 7/9] serial: qcom-geni: move clock-rate logic to separate function Praveen Talari
2025-05-02 3:10 ` [PATCH v3 8/9] serial: qcom-geni: Enable PM runtime for serial driver Praveen Talari
2025-05-07 7:58 ` kernel test robot
2025-05-02 3:10 ` [PATCH v3 9/9] serial: qcom-geni: Enable Serial on SA8255p Qualcomm platforms Praveen Talari
2025-05-02 9:24 ` neil.armstrong [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aef73cdc-d855-4f17-bd76-289b6e26cc14@linaro.org \
--to=neil.armstrong@linaro.org \
--cc=andersson@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=djaggi@quicinc.com \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=konradybcio@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=nm@ti.com \
--cc=psodagud@quicinc.com \
--cc=quic_arandive@quicinc.com \
--cc=quic_mnaresh@quicinc.com \
--cc=quic_msavaliy@quicinc.com \
--cc=quic_ptalari@quicinc.com \
--cc=quic_shazhuss@quicinc.com \
--cc=quic_vtanuku@quicinc.com \
--cc=rafael@kernel.org \
--cc=robh@kernel.org \
--cc=sboyd@kernel.org \
--cc=vireshk@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox