linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] seria: sirf: only use lookup table to set baudrate when ioclk=150MHz
@ 2012-12-25 10:26 Barry Song
  2012-12-25 18:53 ` Sergei Shtylyov
  2012-12-26 15:04 ` Russell King - ARM Linux
  0 siblings, 2 replies; 4+ messages in thread
From: Barry Song @ 2012-12-25 10:26 UTC (permalink / raw)
  To: alan; +Cc: linux-serial, linux-arm-kernel, workgroup.linux, Barry Song

From: Barry Song <Baohua.Song@csr.com>

The fast lookup table to set baudrate is only right when ioclk
is 150MHz. for most platforms, ioclk is 150MHz, but some boards
might set ioclk to other frequency.

so re-calc the clk_div_reg when ioclk is not 150MHz.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/tty/serial/sirfsoc_uart.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index 142217c..fe28b1f 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -375,7 +375,12 @@ static void sirfsoc_uart_set_termios(struct uart_port *port,
 	int		threshold_div;
 	int		temp;
 
-	ioclk_rate = 150000000;
+	struct clk *clk = clk_get_sys("io", NULL);
+	BUG_ON(IS_ERR(clk));
+
+	ioclk_rate = clk_get_rate(clk);
+	clk_put(clk);
+
 	switch (termios->c_cflag & CSIZE) {
 	default:
 	case CS8:
@@ -431,10 +436,13 @@ static void sirfsoc_uart_set_termios(struct uart_port *port,
 			sirfsoc_uart_disable_ms(port);
 	}
 
-	/* common rate: fast calculation */
-	for (ic = 0; ic < SIRF_BAUD_RATE_SUPPORT_NR; ic++)
-		if (baud_rate == baudrate_to_regv[ic].baud_rate)
-			clk_div_reg = baudrate_to_regv[ic].reg_val;
+	if (ioclk_rate == 150000000) {
+		/* common rate: fast calculation */
+		for (ic = 0; ic < SIRF_BAUD_RATE_SUPPORT_NR; ic++)
+			if (baud_rate == baudrate_to_regv[ic].baud_rate)
+				clk_div_reg = baudrate_to_regv[ic].reg_val;
+	}
+
 	setted_baud = baud_rate;
 	/* arbitary rate setting */
 	if (unlikely(clk_div_reg == 0))
-- 
1.7.5.4



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

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

* Re: [PATCH] seria: sirf: only use lookup table to set baudrate when ioclk=150MHz
  2012-12-25 10:26 [PATCH] seria: sirf: only use lookup table to set baudrate when ioclk=150MHz Barry Song
@ 2012-12-25 18:53 ` Sergei Shtylyov
  2012-12-26 15:04 ` Russell King - ARM Linux
  1 sibling, 0 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2012-12-25 18:53 UTC (permalink / raw)
  To: Barry Song
  Cc: Barry Song, linux-serial, linux-arm-kernel, workgroup.linux, alan

Hello.

On 12/25/2012 01:26 PM, Barry Song wrote:

> From: Barry Song <Baohua.Song@csr.com>

> The fast lookup table to set baudrate is only right when ioclk
> is 150MHz. for most platforms, ioclk is 150MHz, but some boards
> might set ioclk to other frequency.

> so re-calc the clk_div_reg when ioclk is not 150MHz.

> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  drivers/tty/serial/sirfsoc_uart.c |   18 +++++++++++++-----
>  1 files changed, 13 insertions(+), 5 deletions(-)

> diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
> index 142217c..fe28b1f 100644
> --- a/drivers/tty/serial/sirfsoc_uart.c
> +++ b/drivers/tty/serial/sirfsoc_uart.c
> @@ -375,7 +375,12 @@ static void sirfsoc_uart_set_termios(struct uart_port *port,
>  	int		threshold_div;
>  	int		temp;
>  
> -	ioclk_rate = 150000000;
> +	struct clk *clk = clk_get_sys("io", NULL);

   Please keep the coding style and insert emoty line here; you can also remove
preceding empty line (not to break the declaration block).

> +	BUG_ON(IS_ERR(clk));
> +
> +	ioclk_rate = clk_get_rate(clk);
> +	clk_put(clk);
> +
>  	switch (termios->c_cflag & CSIZE) {
>  	default:
>  	case CS8:

WBR, Sergei

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

* Re: [PATCH] seria: sirf: only use lookup table to set baudrate when ioclk=150MHz
  2012-12-25 10:26 [PATCH] seria: sirf: only use lookup table to set baudrate when ioclk=150MHz Barry Song
  2012-12-25 18:53 ` Sergei Shtylyov
@ 2012-12-26 15:04 ` Russell King - ARM Linux
  2012-12-31  2:02   ` Barry Song
  1 sibling, 1 reply; 4+ messages in thread
From: Russell King - ARM Linux @ 2012-12-26 15:04 UTC (permalink / raw)
  To: Barry Song
  Cc: alan, workgroup.linux, linux-arm-kernel, linux-serial, Barry Song

On Tue, Dec 25, 2012 at 06:26:02PM +0800, Barry Song wrote:
> @@ -375,7 +375,12 @@ static void sirfsoc_uart_set_termios(struct uart_port *port,
>  	int		threshold_div;
>  	int		temp;
>  
> -	ioclk_rate = 150000000;
> +	struct clk *clk = clk_get_sys("io", NULL);
> +	BUG_ON(IS_ERR(clk));

No.  Really, no.  Stop using BUG_ON() as some kind of crappy assert().
BUG_ON() takes the entire kernel out when it fails.  There's absolutely
no need for this what so ever - especially here.

Get the clock at probe or port initialization time.  Save that pointer.
Only give it up when the port is torn down.  And treat it as any other
clock - prepare and enable it, and disable and unprepare it when you're
done with it.

And there's no need to use this clk_get_sys() crap in drivers.  Add the
necessary clkdev entries or deal with it in DT.  Absolutely do not use
clk_get_sys() in drivers; it's there for *PLATFORM* code to use when
there's no other possibility for them and NOT drivers.

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

* Re: [PATCH] seria: sirf: only use lookup table to set baudrate when ioclk=150MHz
  2012-12-26 15:04 ` Russell King - ARM Linux
@ 2012-12-31  2:02   ` Barry Song
  0 siblings, 0 replies; 4+ messages in thread
From: Barry Song @ 2012-12-31  2:02 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Barry Song, alan, workgroup.linux, linux-arm-kernel, linux-serial,
	Barry Song

2012/12/26, Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, Dec 25, 2012 at 06:26:02PM +0800, Barry Song wrote:
>> @@ -375,7 +375,12 @@ static void sirfsoc_uart_set_termios(struct uart_port
>> *port,
>>  	int		threshold_div;
>>  	int		temp;
>>
>> -	ioclk_rate = 150000000;
>> +	struct clk *clk = clk_get_sys("io", NULL);
>> +	BUG_ON(IS_ERR(clk));
>
> No.  Really, no.  Stop using BUG_ON() as some kind of crappy assert().
> BUG_ON() takes the entire kernel out when it fails.  There's absolutely
> no need for this what so ever - especially here.
>
> Get the clock at probe or port initialization time.  Save that pointer.
> Only give it up when the port is torn down.  And treat it as any other
> clock - prepare and enable it, and disable and unprepare it when you're
> done with it.
>
> And there's no need to use this clk_get_sys() crap in drivers.  Add the
> necessary clkdev entries or deal with it in DT.  Absolutely do not use
> clk_get_sys() in drivers; it's there for *PLATFORM* code to use when
> there's no other possibility for them and NOT drivers.

Thanks, Russell.
what made this happen is that clkio is used in io bus and by all of
dmac, nand, audio, uart, i2c, spi, usp, pwm, pulse as an input, so we
took it as a global/system clock. this way should be not good for the
real users.
as i have enabled "clk: prima2: enable dt-binding clkdev mapping"
https://patchwork.kernel.org/patch/1898811/
i might simply let uart node in DT takes the uart clk output of clk
controller as your suggestion.

-barry

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

end of thread, other threads:[~2012-12-31  2:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-25 10:26 [PATCH] seria: sirf: only use lookup table to set baudrate when ioclk=150MHz Barry Song
2012-12-25 18:53 ` Sergei Shtylyov
2012-12-26 15:04 ` Russell King - ARM Linux
2012-12-31  2:02   ` Barry Song

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