Re: [PATCH v1 6/9] serial: qcom-geni: move resource control logic to separate functions

Jiri Slaby
2025-04-14T07:59:53Z

On 10. 04. 25, 19:40, Praveen Talari wrote:
> Supports use in PM system/runtime frameworks, helping to
> distinguish new resource control mechanisms and facilitate
> future modifications within the new API.
>
> The code that handles the actual enable or disable of resources
> like clock and ICC paths to a separate function
> (geni_serial_resources_on() and geni_serial_resources_off()) which
> enhances code readability.
>
> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 53 +++++++++++++++++++++------
> 1 file changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 889ce8961e0a..e341f5090ecc 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1572,6 +1572,42 @@ static struct uart_driver qcom_geni_uart_driver = {
> .nr = GENI_UART_PORTS,
> };
>
> +static int geni_serial_resources_off(struct uart_port *uport)
> +{
> + struct qcom_geni_serial_port *port = to_dev_port(uport);
> + int ret;
> +
> + dev_pm_opp_set_rate(uport->dev, 0);
> + ret = geni_se_resources_off(&port->se);
> + if (ret)
> + return ret;
> +
> + geni_icc_disable(&port->se);
> +
> + return ret;

This is a bit confusing (needs context). return 0 directly.

> +}
> +
> +static int geni_serial_resources_on(struct uart_port *uport)
> +{
> + struct qcom_geni_serial_port *port = to_dev_port(uport);
> + int ret;
> +
> + ret = geni_icc_enable(&port->se);
> + if (ret)
> + return ret;
> +
> + ret = geni_se_resources_on(&port->se);
> + if (ret) {
> + geni_icc_disable(&port->se);
> + return ret;
> + }
> +
> + if (port->clk_rate)
> + dev_pm_opp_set_rate(uport->dev, port->clk_rate);
> +
> + return ret;

Same here.

thanks,
--
js
suse labs