linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tty: serial: msm: Support more bauds
@ 2016-03-25 21:35 Stephen Boyd
  2016-03-29  9:56 ` Srinivas Kandagatla
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Stephen Boyd @ 2016-03-25 21:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-arm, linux-kernel, linux-serial, linux-arm-msm,
	Ivan T. Ivanov, Srinivas Kandagatla, Andy Gross,
	Matthew McClintock

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" <iivanov.xz@gmail.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Andy Gross <andy.gross@linaro.org>
Cc: Matthew McClintock <mmcclint@codeaurora.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 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;
-- 
2.8.0.rc4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] tty: serial: msm: Support more bauds
  2016-03-25 21:35 [PATCH] tty: serial: msm: Support more bauds Stephen Boyd
@ 2016-03-29  9:56 ` Srinivas Kandagatla
  2016-03-29 15:59 ` Bjorn Andersson
  2016-04-18 20:09 ` Andy Gross
  2 siblings, 0 replies; 5+ messages in thread
From: Srinivas Kandagatla @ 2016-03-29  9:56 UTC (permalink / raw)
  To: Stephen Boyd, Greg Kroah-Hartman
  Cc: linux-arm, linux-kernel, linux-serial, linux-arm-msm,
	Ivan T. Ivanov, Andy Gross, Matthew McClintock



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" <iivanov.xz@gmail.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: Andy Gross <andy.gross@linaro.org>
> Cc: Matthew McClintock <mmcclint@codeaurora.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

tested on DB410c and DB600c

Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tty: serial: msm: Support more bauds
  2016-03-25 21:35 [PATCH] tty: serial: msm: Support more bauds Stephen Boyd
  2016-03-29  9:56 ` Srinivas Kandagatla
@ 2016-03-29 15:59 ` Bjorn Andersson
  2016-04-18 20:09 ` Andy Gross
  2 siblings, 0 replies; 5+ messages in thread
From: Bjorn Andersson @ 2016-03-29 15:59 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm, linux-kernel, linux-serial,
	linux-arm-msm, Ivan T. Ivanov, Srinivas Kandagatla, Andy Gross,
	Matthew McClintock

On Fri 25 Mar 14:35 PDT 2016, 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" <iivanov.xz@gmail.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: Andy Gross <andy.gross@linaro.org>
> Cc: Matthew McClintock <mmcclint@codeaurora.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tty: serial: msm: Support more bauds
  2016-03-25 21:35 [PATCH] tty: serial: msm: Support more bauds Stephen Boyd
  2016-03-29  9:56 ` Srinivas Kandagatla
  2016-03-29 15:59 ` Bjorn Andersson
@ 2016-04-18 20:09 ` Andy Gross
  2 siblings, 0 replies; 5+ messages in thread
From: Andy Gross @ 2016-04-18 20:09 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Greg Kroah-Hartman, linux-arm, linux-kernel, linux-serial,
	linux-arm-msm, Ivan T. Ivanov, Srinivas Kandagatla,
	Matthew McClintock

On Fri, Mar 25, 2016 at 02:35:49PM -0700, Stephen Boyd wrote:

<snip>

> Fixes: 850b37a71bde ("tty: serial: msm: Remove 115.2 Kbps maximum baud rate limitation")
> Cc: "Ivan T. Ivanov" <iivanov.xz@gmail.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: Andy Gross <andy.gross@linaro.org>
> Cc: Matthew McClintock <mmcclint@codeaurora.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

Acked-by: Andy Gross <andy.gross@linaro.org>

<snip>

Greg,

Can you pick this one up?

Regards,

Andy

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] tty: serial: msm: Support more bauds
@ 2016-04-06 15:44 cprundea
  0 siblings, 0 replies; 5+ messages in thread
From: cprundea @ 2016-04-06 15:44 UTC (permalink / raw)
  To: Stephen Boyd, Greg Kroah-Hartman
  Cc: linux-arm, linux-kernel, linux-serial, linux-arm-msm,
	Ivan T. Ivanov, Srinivas Kandagatla, Andy Gross, mmcclint

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" <iivanov.xz@gmail.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: Andy Gross <andy.gross@linaro.org>
> Cc: Matthew McClintock <mmcclint@codeaurora.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

tested on DK04

Tested-by: Cristian Prundeanu <cprundea@codeaurora.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-04-18 20:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-25 21:35 [PATCH] tty: serial: msm: Support more bauds Stephen Boyd
2016-03-29  9:56 ` Srinivas Kandagatla
2016-03-29 15:59 ` Bjorn Andersson
2016-04-18 20:09 ` Andy Gross
2016-04-06 15:44 cprundea

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).