Hi Stephen,
On 2020-05-31 00:39, Stephen Boyd wrote:
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 <skakit@xxxxxxxxxxxxxx>
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.
No, as the sampling rate in sdm845 is 32, we will not be requesting 51.2
over there.
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.
Okay.
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.
If i understand correctly, clk_round_rate gives the nearest possible
value to the desired clk rate, but i am not very clear about
divider_round_rate API, is it an alternate method to get nearest clk
rate? Also it has few parameters like prate, clk_hw etc which we are not
sure of. For now we are planning to post the change for clk_round_rate
API.
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.
okay will look into this.
---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;
Thanks,
Satya Priya
