Messages in this thread | | | Date | Mon, 11 May 2020 12:08:36 +0200 | From | Krzysztof Kozlowski <> | Subject | Re: [PATCH] tty: serial: samsung: Correct clock selection logic |
| |
On Fri, May 08, 2020 at 06:34:33PM -0700, Jonathan Bakker wrote: > Some variants of the samsung tty driver can pick which clock > to use for their baud rate generation. In the DT conversion, > a default clock was selected to be used if a specific one wasn't > assigned and then a comparison of which clock rate worked better > was done. Unfortunately, the comparison was implemented in such > a way that only the default clock was ever actually compared. > Fix this by iterating through all possible clocks, except when a > specific clock has already been picked via clk_sel (which is > only possible via board files). > > Signed-off-by: Jonathan Bakker <xc-racer2@live.ca> > --- > drivers/tty/serial/samsung_tty.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index 73f951d65b93..9d2b4be44209 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -1281,14 +1281,14 @@ static unsigned int s3c24xx_serial_getclk(struct s3c24xx_uart_port *ourport, > struct s3c24xx_uart_info *info = ourport->info; > struct clk *clk; > unsigned long rate; > - unsigned int cnt, baud, quot, clk_sel, best_quot = 0; > + unsigned int cnt, baud, quot, best_quot = 0; > char clkname[MAX_CLK_NAME_LENGTH]; > int calc_deviation, deviation = (1 << 30) - 1; > > - clk_sel = (ourport->cfg->clk_sel) ? ourport->cfg->clk_sel : > - ourport->info->def_clk_sel; > for (cnt = 0; cnt < info->num_clks; cnt++) { > - if (!(clk_sel & (1 << cnt))) > + /* Keep selected clock if provided */
Makes sense and good catch.
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
I wonder about the s3c24xx_serial_enable_baudclk() which has similar pattern - is there testing only def_clk_sel on purpose?
Best regards, Krzysztof
> + if (ourport->cfg->clk_sel && > + !(ourport->cfg->clk_sel & (1 << cnt))) > continue; > > sprintf(clkname, "clk_uart_baud%d", cnt); > -- > 2.20.1 >
| |