UART: add CSR SiRFprimaII SoC on-chip uart drivers
PATCH 1/1 UART: add CSR SiRFprimaII SoC on-chip uart drivers
From: Rong Wang <[email protected]>
SiRFprimaII is the latest generation application
processor from CSR?s multi-function SoC product
family.
The SoC support codes are in arch/arm/mach-prima2
from Linux mainline 3.0.
There are three dedicated UARTs in system. This
patch adds basic driver support for them.
It has used the newest pinmux subsystem from Linus Walleij.
--- drivers/tty/serial/Kconfig | 21 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/sirfsoc_uart.c | 740 +++++++++++++++++++++++++++++++++++++ drivers/tty/serial/sirfsoc_uart.h | 180 +++++++++ 4 files changed, 942 insertions(+), 0 deletions(-) create mode 100644 drivers/tty/serial/sirfsoc_uart.c create mode 100644 drivers/tty/serial/sirfsoc_uart.h
drivers/tty/serial/Kconfig +21 -0
@@ -528,6 +528,27 @@ config SERIAL_S5PV210 help Serial port support for Samsung's S5P Family of SoC's +config SERIAL_SIRFSOC + tristate "SiRF SoC Platform Serial port support" + depends on ARM && ARCH_PRIMA2 + select SERIAL_CORE + help + Support for the on-chip UART on the CSR SiRFprimaII series, + providing /dev/ttyS0, 1 and 2 (note, some machines may not + provide all of these ports, depending on how the serial port + pins are configured. + +config SERIAL_SIRFSOC_CONSOLE + bool "Support for console on SiRF SoC serial port" + depends on SERIAL_SIRFSOC=y + select SERIAL_CORE_CONSOLE + help + Even if you say Y here, the currently visible virtual console + (/dev/tty0) will still be used as the system console by default, but + you can alter that using a kernel command line option such as + "console=ttySx". (Try "man bootparam" or see the documentation of + your boot loader about how to pass options to the kernel at + boot time.) config SERIAL_MAX3100 tristate "MAX3100 support"
drivers/tty/serial/Makefile +1 -0
@@ -94,3 +94,4 @@ obj-$(CONFIG_SERIAL_MSM_SMD) += msm_smd_tty.o obj-$(CONFIG_SERIAL_MXS_AUART) += mxs-auart.o obj-$(CONFIG_SERIAL_LANTIQ) += lantiq.o obj-$(CONFIG_SERIAL_XILINX_PS_UART) += xilinx_uartps.o +obj-$(CONFIG_SERIAL_SIRFSOC) += sirfsoc_uart.o
drivers/tty/serial/sirfsoc_uart.c new file +740 -0
740 lines not on this page · raw message
drivers/tty/serial/sirfsoc_uart.h new file +180 -0
180 lines not on this page · raw message
Signature · 6 lines
1.7.1 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
PATCH 1/1 UART: add CSR SiRFprimaII SoC on-chip uart drivers
Looks basically ok but somewhat outdated for the tty layer. In particular it needs to be aware of the refcounting on tty objects and of the fact we allow arbitary baud rate handling
Quoted 1 line
+static struct
sirfsoc_baudrate_to_regvbaudrate_to_regv[] = {
const
drivers/tty/serial/sirfsoc_uart.c line 196
Quoted 6 lines
(quoted diff) +2
-0+static unsigned int +sirfsoc_uart_pio_rx_chars(struct uart_port *port, unsigned int
max_rx_count) +{drivers/tty/serial/sirfsoc_uart.c new file +3
-0+unsigned int ch, rx_count =0 ; +int temp; +struct tty_struct *tty = port->state->port.tty;
tty = tty_port_tty_get(&port->state->port);
[the newer tty code is refcounting, also tty can be NULL here and needs h handling accordingly]
drivers/tty/serial/sirfsoc_uart.c line 203
Quoted 9 lines
(quoted diff) +2
-0+ while (!(rd_regl(port, SIRFUART_RX_FIFO_STATUS) & +
SIRFUART_FIFOEMPTY_MASK(port))) {(quoted diff) +1
-0+ temp =
tty_buffer_request_room(port->state->port.tty, 1);drivers/tty/serial/sirfsoc_uart.c new file +4
-0+if (unlikely (temp ==0 )) { + port->icount.buf_overrun++; +break ; + }
You don't need to do this - just uart_insert_char. If we run out of mid layer buffering it's not a port overrun as such (ie a fifo exceeded) it's a system wide memory crunch and something pretty serious and bigger is going on.
drivers/tty/serial/sirfsoc_uart.c line 213
Quoted 2 lines
drivers/tty/serial/sirfsoc_uart.c new file +2
-0+ port->icount.rx += rx_count; +tty_flip_buffer_push (tty);
[Do these only if tty != NULL obviously)
and
tty_kref_put(tty);
drivers/tty/serial/sirfsoc_uart.c line 312
Quoted 4 lines
drivers/tty/serial/sirfsoc_uart.c new file +4
-0+static void sirfsoc_uart_set_termios (struct uart_port *port, +struct ktermios *termios, +struct ktermios *old) +{
If you don't support CMSPAR then clear the bit in the passed termios.
Quoted 6 lines
drivers/tty/serial/sirfsoc_uart.c new file +5
-0+for (ic =0 ; ic <SIRFUART_BAUD_RATE_SUPPORT_NR ; ic++) +if (baud_rate == baudrate_to_regv[ic].baud_rate) + clk_div_reg = baudrate_to_regv[ic].reg_val; +if (clk_div_reg ==0 ) + pr_err("SiRF UART: Cannot set Baud Rate (9600 ~4000000).\n");
The baud rate is not guaranteed to be one the Bxxxxx values, you should be computing it not using a table.
Also the pr_err means any user can fill the logs with junk.
The correct behaviour here is
compute the best timing for the baud rate requested compute the actual baud rate obtained
then report it back to the caller
if (tty_termios_baud_rate(termios)) tty_termios_encode_baud_rate(termios, baud. baud);
The above assuming you set the same input and output rate
drivers/tty/serial/sirfsoc_uart.c line 549
Quoted 2 lines
drivers/tty/serial/sirfsoc_uart.c new file +2
-0+static struct console sirfsoc_uart_console = { + .name ="ttyS" ,
ttyS is 8250 devces. Pick a new name for your own.
drivers/tty/serial/sirfsoc_uart.c line 572
Quoted 4 lines
drivers/tty/serial/sirfsoc_uart.c new file +3
-0+ .dev_name = SIRFSOC_UART_NAME, + .major = SIRFSOC_UART_MAJOR, + .minor = SIRFSOC_UART_MINOR,
Use dynamic allocations
drivers/tty/serial/sirfsoc_uart.h line 138
Quoted 3 lines
drivers/tty/serial/sirfsoc_uart.h new file +3
-0+#define SIRFSOC_UART_NAME "ttyS" +#define SIRFSOC_UART_MAJOR TTY_MAJOR +#define SIRFSOC_UART_MINOR 64
These values belong to an existing inerface, don't reuse the names like that, and please use dynamic allocation for the numbers
Quoted 3 lines
(quoted diff) +2
-0+#define uart_tx_port_tty_invalid(port) \ + (((port)->state == NULL) || ((port)->state->port.tty ==NULL))
This has no locking so little meaning - what guarantees it doesn't change under or after the call. I suspect you want to be checking and referencing the tty in one shot as in the example I gave above for the rx fix.
PATCH 1/1 UART: add CSR SiRFprimaII SoC on-chip uart drivers
Hi Alan,
Thanks for your quick feedback.
2011/10/13 Alan Cox <[email protected]>:
Quoted 7 lines
Looks basically ok but somewhat outdated for the tty layer. In particular it needs to be aware of the refcounting on tty objects and of the fact we allow arbitary baud rate handling
Quoted 1 line
+static struct
sirfsoc_baudrate_to_regvbaudrate_to_regv[] = {const
agree
Quoted 10 lines
Quoted 7 lines
(quoted diff) +2
-0+static unsigned int +sirfsoc_uart_pio_rx_chars(struct uart_port *port, unsigned int
max_rx_count) +{
+ ? ? unsigned int ch,rx_count= 0;
+ ? ? int temp;
+ ? ? structtty_struct*tty =port->state->port.tty;tty =
tty_port_tty_get(&port->state->port);
Quoted 2 lines
[the newer tty code is refcounting, also tty can be NULL here and needs h handling accordingly]
agree
Quoted 16 lines
Quoted 10 lines
+ ? ? while (!(rd_regl(port,
SIRFUART_RX_FIFO_STATUS) & +SIRFUART_FIFOEMPTY_MASK(port))) {
+ ? ? ? ? ? ? temp =tty_buffer_request_room(port->state->port.tty, 1);
+ ? ? ? ? ? ? if (unlikely(temp == 0)) {
+ ? ? ? ? ? ? ? ? ? ? port->icount.buf_overrun++;
+ ? ? ? ? ? ? ? ? ? ? break;
+ ? ? ? ? ? ? }You don't need to do this - just
uart_insert_char. If we run out of mid layer buffering it's not a port overrun as such (ie a fifo exceeded) it's a system wide memory crunch and something pretty serious and bigger is going on.
Quoted 8 lines
Quoted 2 lines
+ ? ?
port->icount.rx+=rx_count;
+ ? ?tty_flip_buffer_push(tty);[Do these only if tty != NULL obviously)
and
? ? ? ?tty_kref_put(tty);
agree
Quoted 8 lines
drivers/tty/serial/sirfsoc_uart.c line 312
Quoted 6 lines
drivers/tty/serial/sirfsoc_uart.c new file +4
-0+static void sirfsoc_uart_set_termios (struct uart_port *port, + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct ktermios *termios, + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct ktermios *old) +{If you don't support CMSPAR then clear the bit in the passed termios.
actually it is supported if you read codes again.
Quoted 10 lines
Quoted 7 lines
+ ? ? for (ic = 0; ic <
SIRFUART_BAUD_RATE_SUPPORT_NR; ic++)
+ ? ? ? ? ? ? if (baud_rate== baudrate_to_regv[ic].baud_rate)
+ ? ? ? ? ? ? ? ? ? ?clk_div_reg= baudrate_to_regv[ic].reg_val;
+ ? ? if (clk_div_reg== 0)
+ ? ? ? ? ? ?pr_err("SiRF UART: Cannot set Baud Rate (9600 ~ 4000000).\n");The baud rate is not guaranteed to be one the Bxxxxx values, you should be computing it not using a table.
Rong has changed it to contain both Bxxx table and non-Bxxx calculation. For Bxxx values, get set from table. otherwise, calculate.
Quoted 11 lines
Also the
pr_errmeans any user can fill the logs with junk.The correct behaviour here is
? ? ? ?compute the best timing for the baud rate requested ? ? ? ?compute the actual baud rate obtained
then report it back to the caller
? ? ? ?if (
tty_termios_baud_rate(termios))
? ? ? ? ? ? ? ?tty_termios_encode_baud_rate(termios, baud. baud);
agree
Quoted 9 lines
The above assuming you set the same input and output rate
Quoted 2 lines
+static struct console
sirfsoc_uart_console= {
+ ? ? .name ? ? ? ? ? = "ttyS",ttyS is 8250 devces. Pick a new name for your own.
ok. most devices have names like ttySQ, ttySA, ttySG... since our SoC is named SiRFprimaII, i'd like to have ttySI?
Quoted 9 lines
Quoted 5 lines
+ ? ? .dev_name ? ? ? =
SIRFSOC_UART_NAME,
+ ? ? .major ? ? ? ? ?=SIRFSOC_UART_MAJOR,
+ ? ? .minor ? ? ? ? ?=SIRFSOC_UART_MINOR,Use dynamic allocations
Quoted 6 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+#define SIRFSOC_UART_NAME ? ? ? ? ? ? ? ? ? ?"ttyS" +#define SIRFSOC_UART_MAJOR ? ? ? ? ? ? ? ? ? TTY_MAJOR +#define SIRFSOC_UART_MINOR ? ? ? ? ? ? ? ? ? 64These values belong to an existing inerface, don't reuse the names like that, and please use dynamic allocation for the numbers
agree
Quoted 9 lines
Quoted 4 lines
+#define
uart_tx_port_tty_invalid(port)? \
+ ? ? (((port)->state == NULL) || ((port)->state->port.tty == NULL))This has no locking so little meaning - what guarantees it doesn't change ?under or after the call. I suspect you want to be checking and referencing the tty in one shot as in the example I gave above for the rx fix.
agree.
-barry
PATCH 1/1 UART: add CSR SiRFprimaII SoC on-chip uart drivers
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
+static struct console
sirfsoc_uart_console= {
+ .name = "ttyS",ttyS is 8250 devces. Pick a new name for your own.
ok. most devices have names like ttySQ, ttySA, ttySG... since our SoC is named SiRFprimaII, i'd like to have ttySI?
If nobody else is using it for anything mainstream sure. Or ttySiRF0 .. etc might be more definitively unique
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 UART: add CSR SiRFprimaII SoC on-chip uart drivers
2011/10/13 Alan Cox <[email protected]>:
Quoted 10 lines
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
+static struct console
sirfsoc_uart_console= {
+ ? ? .name ? ? ? ? ? = "ttyS",ttyS is 8250 devces. Pick a new name for your own.
ok. most devices have names like ttySQ, ttySA, ttySG... since our SoC is named SiRFprimaII, i'd like to have ttySI?
If nobody else is using it for anything mainstream sure. Or ttySiRF0 .. etc might be more definitively unique
well. the later in v2.
Quoted 4 lines
Alan
-barry