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. 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;
