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-rac...@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 <k...@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
>
