Skip to contentSkip to conversation

seria: sirf: only use lookup table to set baudrate when ioclk=150MHz

PATCH Superseded Barry Song 4 participants 4 messages Dec 25, 2012 – Dec 31, 2012 TTY layer and serial drivers

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Barry Song

author
#0 lore raw

PATCH 1/1 seria: sirf: only use lookup table to set baudrate when ioclk=150MHz

From: Barry Song <[email protected]>

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.

---
 drivers/tty/serial/sirfsoc_uart.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)
drivers/tty/serial/sirfsoc_uart.c +13 -5
index 142217c..fe28b1f 100644
@@ -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))
Signature · 6 lines
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 [email protected]/blog
#1 lore raw

PATCH 1/1 seria: sirf: only use lookup table to set baudrate when ioclk=150MHz

Hello.

Quoted 18 lines · Barry Song

From: Barry Song <[email protected]>

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.

---
 drivers/tty/serial/sirfsoc_uart.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)
drivers/tty/serial/sirfsoc_uart.c +0 -0
index 142217c..fe28b1f 100644

drivers/tty/serial/sirfsoc_uart.c static void sirfsoc_uart_set_termios(struct uart_port *port, · line 378

Quoted 5 lines
drivers/tty/serial/sirfsoc_uart.c +1 -1
@@ -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).

drivers/tty/serial/sirfsoc_uart.c static void sirfsoc_uart_set_termios(struct uart_port *port, · line 386

Quoted 8 lines
drivers/tty/serial/sirfsoc_uart.c +5 -0
+	BUG_ON(IS_ERR(clk));
+
+	ioclk_rate = clk_get_rate(clk);
+	clk_put(clk);
+
 	switch (termios->c_cflag & CSIZE) {
 	default:
 	case CS8:

WBR, Sergei

#2 lore raw

PATCH 1/1 seria: sirf: only use lookup table to set baudrate when ioclk=150MHz

In reply to Barry Song

drivers/tty/serial/sirfsoc_uart.c static void sirfsoc_uart_set_termios(struct uart_port *port, · line 379

Quoted 7 lines · Barry Song
drivers/tty/serial/sirfsoc_uart.c +2 -1
@@ -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.

#3 lore raw

PATCH 1/1 seria: sirf: only use lookup table to set baudrate when ioclk=150MHz

2012/12/26, Russell King - ARM Linux <[email protected]>:

Quoted 19 lines · Barry Song

drivers/tty/serial/sirfsoc_uart.c static void sirfsoc_uart_set_termios(struct uart_port · line 380

Quoted 8 lines
drivers/tty/serial/sirfsoc_uart.c +2 -1
@@ -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.

Quoted 4 lines

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