tty: serial: qcom_geni_serial: Add 51.2MHz frequency support

Commit Message

Satya Priya May 29, 2020, 10:14 a.m. UTC

To support BT use case over UART at baud rate of 3.2 Mbps,
we need SE clocks to run at 51.2MHz frequency. Previously this
frequency was not available in clk src, so, we were requesting
for 102.4 MHz and dividing it internally by 2 to get 51.2MHz.

As now 51.2MHz frequency is made available in clk src,
adding this frequency to UART frequency table.

We will save significant amount of power, if 51.2 is used
because it belongs to LowSVS range whereas 102.4 fall into
Nominal category.

Signed-off-by: satya priya <[e-mail]>
---

Note: This depend on clk patch https://patchwork.kernel.org/patch/11554073/

 drivers/tty/serial/qcom_geni_serial.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Stephen Boyd May 30, 2020, 7:09 p.m. UTC | #1

Quoting satya priya (2020-05-29 03:14:42)
> To support BT use case over UART at baud rate of 3.2 Mbps,
> we need SE clocks to run at 51.2MHz frequency. Previously this
> frequency was not available in clk src, so, we were requesting
> for 102.4 MHz and dividing it internally by 2 to get 51.2MHz.
> 
> As now 51.2MHz frequency is made available in clk src,
> adding this frequency to UART frequency table.
> 
> We will save significant amount of power, if 51.2 is used
> because it belongs to LowSVS range whereas 102.4 fall into
> Nominal category.
> 
> Signed-off-by: satya priya <[e-mail]>

Great commit text! Maybe point to the commit that adds it to the
frequency table in the gcc clk driver instead of the patchwork link.

> ---
> 
> Note: This depend on clk patch https://patchwork.kernel.org/patch/11554073/
> 
>  drivers/tty/serial/qcom_geni_serial.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 6119090..168e1c0 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -141,9 +141,10 @@ static void qcom_geni_serial_stop_rx(struct uart_port *uport);
>  static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop);
>  
>  static const unsigned long root_freq[] = {7372800, 14745600, 19200000, 29491200,
> -                                       32000000, 48000000, 64000000, 80000000,
> -                                       96000000, 100000000, 102400000,
> -                                       112000000, 120000000, 128000000};
> +                                       32000000, 48000000, 51200000, 64000000,
> +                                       80000000, 96000000, 100000000,
> +                                       102400000, 112000000, 120000000,
> +                                       128000000};

Will this break sdm845? That clk frequency table hasn't been updated to
add 51.2 MHz.

Furthermore, it would be nice to get rid of this table and use
clk_round_rate() to find a frequency that will work with the requested
baud rate. Can we do that instead? That would make it work regardless of
what the clk driver supports for the particular SoC. Presumably we can
just call clk_round_rate() and then make sure it is evenly divisible by
the requested rate and then it will be mostly the same as before.

Or if we need to we can keep multiplying the rate 10 or 20 times and
test with clk_round_rate() each time and then give up if we don't find a
frequency that will work. The divider value looks like it is 12 bits
wide so there are 4095 possible dividers. If we need to loop through all
possible dividers then it may make sense to register a clk in this
driver and have it call divider_round_rate() to find the closest rate to
the desired rate. That would avoid reinventing a bunch of code that we
already have to implement clk dividers.

And one more thing, I see that this driver doesn't use DFS. Instead it
relies on the clk_set_rate() call to change the qup clk frequency. We
could support DFS by adding a driver specific member to struct
clk_rate_request that can be used to communicate back extra info to the
child clk. The idea is that the DFS clk (the qup uart one) can round the
rate and jam in the DFS index that corresponds to the rate into the new
member. Then the clk implemented in this serial driver can stash away
that index into some table that maps frequency of parent to DFS index
and then look up the DFS index during clk_set_rate() based on the parent
rate the clk_op is called with to program the DFS value in the uart
registers in addition to the divider.

---8<---
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 6119090ce045..7d147be997e5 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -140,11 +140,6 @@ static unsigned int qcom_geni_serial_tx_empty(struct uart_port *port);
 static void qcom_geni_serial_stop_rx(struct uart_port *uport);
 static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop);
 
-static const unsigned long root_freq[] = {7372800, 14745600, 19200000, 29491200,
-					32000000, 48000000, 64000000, 80000000,
-					96000000, 100000000, 102400000,
-					112000000, 120000000, 128000000};
-
 #define to_dev_port(ptr, member) \
 		container_of(ptr, struct qcom_geni_serial_port, member)
 
@@ -900,30 +895,22 @@ static int qcom_geni_serial_startup(struct uart_port *uport)
 	return 0;
 }
 
-static unsigned long get_clk_cfg(unsigned long clk_freq)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(root_freq); i++) {
-		if (!(root_freq[i] % clk_freq))
-			return root_freq[i];
-	}
-	return 0;
-}
-
-static unsigned long get_clk_div_rate(unsigned int baud,
+static unsigned long get_clk_div_rate(const struct geni_se *se,
+			unsigned int baud,
 			unsigned int sampling_rate, unsigned int *clk_div)
 {
 	unsigned long ser_clk;
 	unsigned long desired_clk;
+	long actual_clk;
 
 	desired_clk = baud * sampling_rate;
-	ser_clk = get_clk_cfg(desired_clk);
-	if (!ser_clk) {
+	actual_clk = clk_round_rate(se->clk, desired_clk);
+	if (actual_clk % desired_clk != 0) {
 		pr_err("%s: Can't find matching DFS entry for baud %d\n",
 								__func__, baud);
-		return ser_clk;
+		return 0;
 	}
+	ser_clk = actual_clk;
 
 	*clk_div = ser_clk / desired_clk;
 	return ser_clk;
@@ -956,7 +943,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
 	if (GENI_SE_VERSION_MAJOR(ver) >= 2 && GENI_SE_VERSION_MINOR(ver) >= 5)
 		sampling_rate /= 2;
 
-	clk_rate = get_clk_div_rate(baud, sampling_rate, &clk_div);
+	clk_rate = get_clk_div_rate(&port->se, baud, sampling_rate, &clk_div);
 	if (!clk_rate)
 		goto out_restart_rx;

Patch

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 6119090..168e1c0 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -141,9 +141,10 @@  static void qcom_geni_serial_stop_rx(struct uart_port *uport);
 static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop);
 
 static const unsigned long root_freq[] = {7372800, 14745600, 19200000, 29491200,
-					32000000, 48000000, 64000000, 80000000,
-					96000000, 100000000, 102400000,
-					112000000, 120000000, 128000000};
+					32000000, 48000000, 51200000, 64000000,
+					80000000, 96000000, 100000000,
+					102400000, 112000000, 120000000,
+					128000000};
 
 #define to_dev_port(ptr, member) \
 		container_of(ptr, struct qcom_geni_serial_port, member)