Hi, Alan,

Thanks for your comments!
Some question for them below,

On 2014年11月26日 20:33, One Thousand Gnomes wrote:
+                213 = /dev/ttySPX0             SPRD serial port 0
+                   ...
+                216 = /dev/ttySPX3             SPRD serial port 3
Please use dynamic allocation or give a very very good reason why you
can't
do we just leave the .major & .minor as NULL in struct uart_driver for dynamic allocation?
+config SERIAL_SPRD_NR
+        int "Maximum number of sprd serial ports"
+        depends on SERIAL_SPRD
+        default "4"
If you are doing dynamic allocation this should pretty much go away
I think you mean getting the number of uarts from dt and dynamically allocate their port structure?

+static int sprd_startup(struct uart_port *port)
+{
+       int ret = 0;
+       unsigned int ien, ctrl1;
+       struct sprd_uart_port *sp;
+
+       serial_out(port, SPRD_CTL2, ((THLD_TX_EMPTY << 8) | THLD_RX_FULL));
+
+       /* clear rx fifo */
+       while (serial_in(port, SPRD_STS1) & 0x00ff)
+               serial_in(port, SPRD_RXD);
+
+       /* clear tx fifo */
+       while (serial_in(port, SPRD_STS1) & 0xff00)
+               ;
Missing a cpu_relax and I would have thought a timeout on both of these.
We will add in V4


+static void sprd_set_termios(struct uart_port *port,
+                                   struct ktermios *termios,
+                                   struct ktermios *old)
+{
+       unsigned int baud, quot;
+       /* calculate parity */
+       lcr &= ~SPRD_LCR_PARITY;
+       if (termios->c_cflag & PARENB) {
+               lcr |= SPRD_LCR_PARITY_EN;
+               if (termios->c_cflag & PARODD)
+                       lcr |= SPRD_LCR_ODD_PAR;
+               else
+                       lcr |= SPRD_LCR_EVEN_PAR;
+       }
If you don't support mark/space parity then also clear CMSPAR in
termios->c_cflag.
just simply use code like "termios->c_cflag &= ~CMSPAR" ?

If you do then it ought to be handled above.

+
+       /* clock divider bit16~bit20 */
+       serial_out(port, SPRD_CLKD1, (quot & 0x1f0000) >> 16);
+       serial_out(port, SPRD_LCR, lcr);
+       fc |= 0x3e00 | THLD_RX_FULL;
+       serial_out(port, SPRD_CTL1, fc);
Also set the resulting baud back into the termios (see the 8250 termios
for an example)
you mean something like this part ?

/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);




+static int __init sprd_console_setup(struct console *co, char *options)
+{
+       struct uart_port *port;
+       int baud = 115200;
+       int bits = 8;
+       int parity = 'n';
+       int flow = 'n';
+
+       if (unlikely(co->index >= UART_NR_MAX || co->index < 0))
+               co->index = 0;
+
+       port = (struct uart_port *)sprd_port[co->index];
+       if (port == NULL) {
+               pr_info("srial port %d not yet initialized\n", co->index);
Typo
it will be fixed :)

Thanks,
Orson
Looks basically sound to me

Alan

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to