diff mbox series

[v1,7/9] serial: qcom-geni: move clock-rate logic to separate function

Message ID 20250410174010.31588-8-quic_ptalari@quicinc.com
State New
Headers show
Series Enable QUPs and Serial on SA8255p Qualcomm platforms | expand

Commit Message

Praveen Talari April 10, 2025, 5:40 p.m. UTC
Facilitates future modifications within the new function,
leading to better readability and maintainability of the code.

Move the code that handles the actual logic of clock-rate
calculations to a separate function geni_serial_set_rate()
which enhances code readability.

Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 56 +++++++++++++++++----------
 1 file changed, 36 insertions(+), 20 deletions(-)

Comments

Jiri Slaby April 14, 2025, 8:01 a.m. UTC | #1
On 10. 04. 25, 19:40, Praveen Talari wrote:
> Facilitates future modifications within the new function,
> leading to better readability and maintainability of the code.
> 
> Move the code that handles the actual logic of clock-rate
> calculations to a separate function geni_serial_set_rate()
> which enhances code readability.
> 
> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
> ---
>   drivers/tty/serial/qcom_geni_serial.c | 56 +++++++++++++++++----------
>   1 file changed, 36 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index e341f5090ecc..25d16ac3f406 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
...
> @@ -1323,6 +1310,37 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
>   	port->se.icc_paths[CPU_TO_GENI].avg_bw = Bps_to_icc(baud);
>   	geni_icc_set_bw(&port->se);
>   
> +	writel(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG);
> +	writel(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG);
> +	return 0;

Did this pass checkpatch?

> +}
> +
> +static void qcom_geni_serial_set_termios(struct uart_port *uport,
> +					 struct ktermios *termios,
> +					 const struct ktermios *old)
> +{
> +	struct qcom_geni_serial_port *port = to_dev_port(uport);
> +	unsigned int baud;
> +	unsigned long timeout;
> +	u32 bits_per_char;
> +	u32 tx_trans_cfg;
> +	u32 tx_parity_cfg;
> +	u32 rx_trans_cfg;
> +	u32 rx_parity_cfg;
> +	u32 stop_bit_len;
> +	int ret = 0;
> +
> +	/* baud rate */
> +	baud = uart_get_baud_rate(uport, termios, old, 300, 4000000);
> +
> +	ret = geni_serial_set_rate(uport, baud);
> +	if (ret) {
> +		dev_err(port->se.dev,
> +			"%s: Failed to set  baud: %u  ret: %d\n",

Why the doubled spaces?

> +			__func__, baud, ret);
> +		return;
> +	}
> +
>   	/* parity */
>   	tx_trans_cfg = readl(uport->membase + SE_UART_TX_TRANS_CFG);
>   	tx_parity_cfg = readl(uport->membase + SE_UART_TX_PARITY_CFG);

thanks,
diff mbox series

Patch

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index e341f5090ecc..25d16ac3f406 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1267,27 +1267,14 @@  static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud,
 	return ser_clk;
 }
 
-static void qcom_geni_serial_set_termios(struct uart_port *uport,
-					 struct ktermios *termios,
-					 const struct ktermios *old)
+static int geni_serial_set_rate(struct uart_port *uport, unsigned long baud)
 {
-	unsigned int baud;
-	u32 bits_per_char;
-	u32 tx_trans_cfg;
-	u32 tx_parity_cfg;
-	u32 rx_trans_cfg;
-	u32 rx_parity_cfg;
-	u32 stop_bit_len;
-	unsigned int clk_div;
-	u32 ser_clk_cfg;
 	struct qcom_geni_serial_port *port = to_dev_port(uport);
 	unsigned long clk_rate;
-	u32 ver, sampling_rate;
 	unsigned int avg_bw_core;
-	unsigned long timeout;
-
-	/* baud rate */
-	baud = uart_get_baud_rate(uport, termios, old, 300, 4000000);
+	unsigned int clk_div;
+	u32 ver, sampling_rate;
+	u32 ser_clk_cfg;
 
 	sampling_rate = UART_OVERSAMPLING;
 	/* Sampling rate is halved for IP versions >= 2.5 */
@@ -1301,7 +1288,7 @@  static void qcom_geni_serial_set_termios(struct uart_port *uport,
 		dev_err(port->se.dev,
 			"Couldn't find suitable clock rate for %u\n",
 			baud * sampling_rate);
-		return;
+		return -EINVAL;
 	}
 
 	dev_dbg(port->se.dev, "desired_rate = %u, clk_rate = %lu, clk_div = %u\n",
@@ -1323,6 +1310,37 @@  static void qcom_geni_serial_set_termios(struct uart_port *uport,
 	port->se.icc_paths[CPU_TO_GENI].avg_bw = Bps_to_icc(baud);
 	geni_icc_set_bw(&port->se);
 
+	writel(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG);
+	writel(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG);
+	return 0;
+}
+
+static void qcom_geni_serial_set_termios(struct uart_port *uport,
+					 struct ktermios *termios,
+					 const struct ktermios *old)
+{
+	struct qcom_geni_serial_port *port = to_dev_port(uport);
+	unsigned int baud;
+	unsigned long timeout;
+	u32 bits_per_char;
+	u32 tx_trans_cfg;
+	u32 tx_parity_cfg;
+	u32 rx_trans_cfg;
+	u32 rx_parity_cfg;
+	u32 stop_bit_len;
+	int ret = 0;
+
+	/* baud rate */
+	baud = uart_get_baud_rate(uport, termios, old, 300, 4000000);
+
+	ret = geni_serial_set_rate(uport, baud);
+	if (ret) {
+		dev_err(port->se.dev,
+			"%s: Failed to set  baud: %u  ret: %d\n",
+			__func__, baud, ret);
+		return;
+	}
+
 	/* parity */
 	tx_trans_cfg = readl(uport->membase + SE_UART_TX_TRANS_CFG);
 	tx_parity_cfg = readl(uport->membase + SE_UART_TX_PARITY_CFG);
@@ -1390,8 +1408,6 @@  static void qcom_geni_serial_set_termios(struct uart_port *uport,
 	writel(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN);
 	writel(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN);
 	writel(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN);
-	writel(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG);
-	writel(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG);
 }
 
 #ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE