Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756540AbcC2J44 (ORCPT ); Tue, 29 Mar 2016 05:56:56 -0400 Received: from mail-wm0-f53.google.com ([74.125.82.53]:33345 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756266AbcC2J4w (ORCPT ); Tue, 29 Mar 2016 05:56:52 -0400 Subject: Re: [PATCH] tty: serial: msm: Support more bauds To: Stephen Boyd , Greg Kroah-Hartman References: <1458941749-5705-1-git-send-email-stephen.boyd@linaro.org> Cc: linux-arm@lists.infradead.org, linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, linux-arm-msm@vger.kernel.org, "Ivan T. Ivanov" , Andy Gross , Matthew McClintock From: Srinivas Kandagatla Message-ID: <56FA5160.9070607@linaro.org> Date: Tue, 29 Mar 2016 10:56:48 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1458941749-5705-1-git-send-email-stephen.boyd@linaro.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6742 Lines: 193 On 25/03/16 21:35, Stephen Boyd wrote: > The msm_find_best_baud() function is written with the assumption > that the port->uartclk rate is fixed to a particular rate at boot > time, but now this driver changes that clk rate at runtime when > the baud is changed. > > The way the hardware works is that an input clk rate comes from > the clk controller into the uart hw block. That rate is typically > 1843200 or 3686400 Hz. That rate can then be divided by an > internal divider in the hw block to achieve a particular baud on > the serial wire. msm_find_best_baud() is looking for that divider > value. > > A few things are wrong with the way the code is written. First, > it assumes that the maximum baud that the uart can support if the > clk rate is fixed at boot is 460800, which would correspond to an > input clk rate of 230400 * 16 == 3686400 Hz. Except some devices > have a boot rate of 1843200 Hz or max baud of 115200, so > achieving 230400 on those devices doesn't work at all because we > don't increase the clk rate unless max baud is 460800. > > Second, we can't achieve bauds higher than 460800 that require > anything besides a divisor of 1, because we always call > msm_find_best_baud() with a fixed port->uartclk rate that will > eventually be changed after we calculate the divisor. So if we > need to get a baud of 500000, we'll just multiply that by 16 and > hope that the clk can give us 500000 * 16 == 8000000 Hz, which it > typically can't do. To really achieve 500000 baud, we need to get > an input clk rate of 24000000 Hz and then divide that by 3 inside > the uart hardware. > > Finally, we return success for bauds even when we can't actually > achieve them. This means that when the user asks for 500000 baud, > we actually get 921600 right now, but the user doesn't know that. > > Fix all of this by searching through the divisor and clk rate > space with a combination of clk_round_rate() and baud > calculations, keeping track of the best clk rate and divisor we > find if we can't get an exact match. Typically we can get an > exact match with a divisor of 1, but sometimes we need to keep > track and try more frequencies. On my msm8916 device, this > results in all standard bauds in baud_table being supported > except for 1800, 576000, 1152000, and 4000000. > > Fixes: 850b37a71bde ("tty: serial: msm: Remove 115.2 Kbps maximum baud rate limitation") > Cc: "Ivan T. Ivanov" > Cc: Srinivas Kandagatla > Cc: Andy Gross > Cc: Matthew McClintock > Signed-off-by: Stephen Boyd tested on DB410c and DB600c Tested-by: Srinivas Kandagatla > --- > drivers/tty/serial/msm_serial.c | 99 +++++++++++++++++++++++++++-------------- > 1 file changed, 66 insertions(+), 33 deletions(-) > > diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c > index 96d3ce8dc2dc..3d28be85bd46 100644 > --- a/drivers/tty/serial/msm_serial.c > +++ b/drivers/tty/serial/msm_serial.c > @@ -861,37 +861,72 @@ struct msm_baud_map { > }; > > static const struct msm_baud_map * > -msm_find_best_baud(struct uart_port *port, unsigned int baud) > +msm_find_best_baud(struct uart_port *port, unsigned int baud, > + unsigned long *rate) > { > - unsigned int i, divisor; > - const struct msm_baud_map *entry; > + struct msm_port *msm_port = UART_TO_MSM(port); > + unsigned int divisor, result; > + unsigned long target, old, best_rate = 0, diff, best_diff = ULONG_MAX; > + const struct msm_baud_map *entry, *end, *best; > static const struct msm_baud_map table[] = { > - { 1536, 0x00, 1 }, > - { 768, 0x11, 1 }, > - { 384, 0x22, 1 }, > - { 192, 0x33, 1 }, > - { 96, 0x44, 1 }, > - { 48, 0x55, 1 }, > - { 32, 0x66, 1 }, > - { 24, 0x77, 1 }, > - { 16, 0x88, 1 }, > - { 12, 0x99, 6 }, > - { 8, 0xaa, 6 }, > - { 6, 0xbb, 6 }, > - { 4, 0xcc, 6 }, > - { 3, 0xdd, 8 }, > - { 2, 0xee, 16 }, > { 1, 0xff, 31 }, > - { 0, 0xff, 31 }, > + { 2, 0xee, 16 }, > + { 3, 0xdd, 8 }, > + { 4, 0xcc, 6 }, > + { 6, 0xbb, 6 }, > + { 8, 0xaa, 6 }, > + { 12, 0x99, 6 }, > + { 16, 0x88, 1 }, > + { 24, 0x77, 1 }, > + { 32, 0x66, 1 }, > + { 48, 0x55, 1 }, > + { 96, 0x44, 1 }, > + { 192, 0x33, 1 }, > + { 384, 0x22, 1 }, > + { 768, 0x11, 1 }, > + { 1536, 0x00, 1 }, > }; > > - divisor = uart_get_divisor(port, baud); > + best = table; /* Default to smallest divider */ > + target = clk_round_rate(msm_port->clk, 16 * baud); > + divisor = DIV_ROUND_CLOSEST(target, 16 * baud); > + > + end = table + ARRAY_SIZE(table); > + entry = table; > + while (entry < end) { > + if (entry->divisor <= divisor) { > + result = target / entry->divisor / 16; > + diff = abs(result - baud); > + > + /* Keep track of best entry */ > + if (diff < best_diff) { > + best_diff = diff; > + best = entry; > + best_rate = target; > + } > > - for (i = 0, entry = table; i < ARRAY_SIZE(table); i++, entry++) > - if (entry->divisor <= divisor) > - break; > + if (result == baud) > + break; > + } else if (entry->divisor > divisor) { > + old = target; > + target = clk_round_rate(msm_port->clk, old + 1); > + /* > + * The rate didn't get any faster so we can't do > + * better at dividing it down > + */ > + if (target == old) > + break; > + > + /* Start the divisor search over at this new rate */ > + entry = table; > + divisor = DIV_ROUND_CLOSEST(target, 16 * baud); > + continue; > + } > + entry++; > + } > > - return entry; /* Default to smallest divider */ > + *rate = best_rate; > + return best; > } > > static int msm_set_baud_rate(struct uart_port *port, unsigned int baud, > @@ -900,22 +935,20 @@ static int msm_set_baud_rate(struct uart_port *port, unsigned int baud, > unsigned int rxstale, watermark, mask; > struct msm_port *msm_port = UART_TO_MSM(port); > const struct msm_baud_map *entry; > - unsigned long flags; > - > - entry = msm_find_best_baud(port, baud); > - > - msm_write(port, entry->code, UART_CSR); > - > - if (baud > 460800) > - port->uartclk = baud * 16; > + unsigned long flags, rate; > > flags = *saved_flags; > spin_unlock_irqrestore(&port->lock, flags); > > - clk_set_rate(msm_port->clk, port->uartclk); > + entry = msm_find_best_baud(port, baud, &rate); > + clk_set_rate(msm_port->clk, rate); > + baud = rate / 16 / entry->divisor; > > spin_lock_irqsave(&port->lock, flags); > *saved_flags = flags; > + port->uartclk = rate; > + > + msm_write(port, entry->code, UART_CSR); > > /* RX stale watermark */ > rxstale = entry->rxstale; >