lkml.org 
[lkml]   [2011]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] tty/serial: add support for Xilinx PS UART
On Tue, 19 Apr 2011 14:14:52 -0600
John Linn <john.linn@xilinx.com> wrote:

> The Xilinx PS Uart is used on the new ARM based SoC. This
> UART is not compatible with others such that a seperate
> driver is required.

Joyous. I wish people would standardise.

> + 213 = /dev/ttyPS0 Xilinx PS serial port 0
> + 214 = /dev/ttyPS1 Xilinx PS serial port 1
> + 215 = /dev/ttyPS2 Xilinx PS serial port 2
> + 216 = /dev/ttyPS3 Xilinx PS serial port 3

Is there a specific reason you need fixed minor numbers ? If not please
use a dynamic range and keep Linus happy.

> +/**
> + * xuartps_isr - Interrupt handler
> + * @irq: Irq number
> + * @dev_id: Id of the port
> + *
> + * Returns IRQHANDLED
> + **/
> +static irqreturn_t xuartps_isr(int irq, void *dev_id)
> +{
> + struct uart_port *port = (struct uart_port *)dev_id;
> + struct tty_struct *tty = port->state->port.tty;
> + unsigned long flags;
> + unsigned int isrstatus, numbytes;
> + unsigned int data;
> + char status = TTY_NORMAL;
> +
> + spin_lock_irqsave(&port->lock, flags);

The ttys are refcounting now and your locking subtly wrong if you want to
avoid it (you lookup port-> stuff before you lock it)

The best way to do this is

tty = tty_port_tty_get(&port->state->port);

/* Returns a tty with reference or NULL */

Do stuff

tty_kref_put(tty);



> +static void xuartps_set_termios(struct uart_port *port,
> + struct ktermios *termios, struct ktermios *old)
> +{

> + /* Min baud rate = 6bps and Max Baud Rate is 10Mbps for 100Mhz clk */
> + baud = uart_get_baud_rate(port, termios, old, 0, 460800);
> + xuartps_set_baud_rate(port, baud);

So why pass 460800 ?

> + if (termios->c_iflag & INPCK)
> + port->read_status_mask |= XUARTPS_IXR_PARITY |
> + XUARTPS_IXR_FRAMING;
> +
> + if (termios->c_iflag & IGNPAR)
> + port->ignore_status_mask |= XUARTPS_IXR_PARITY |
> + XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN;
> +
> + /* ignore all characters if CREAD is not set */
> + if ((termios->c_cflag & CREAD) == 0)
> + port->ignore_status_mask |= XUARTPS_IXR_RXTRIG |
> + XUARTPS_IXR_TOUT | XUARTPS_IXR_PARITY |
> + XUARTPS_IXR_FRAMING | XUARTPS_IXR_OVERRUN;
> +
> + mode_reg = xuartps_readl(XUARTPS_MR_OFFSET);
> +
> + /* Handling Data Size */
> + switch (termios->c_cflag & CSIZE) {
> + case CS6:
> + cval |= XUARTPS_MR_CHARLEN_6_BIT;
> + break;
> + case CS7:
> + cval |= XUARTPS_MR_CHARLEN_7_BIT;
> + break;
> + default:
> + case CS8:
> + cval |= XUARTPS_MR_CHARLEN_8_BIT;
> + break;

You need to clear/set appropriately any flags for hardware requests you
cannot meet. So your default should be

termios->c_cflag &= ~CSIZE;
termios->c_cflag |= CS8;

to rewrite CS5

And set the baud rate (see 8250.c for an example). Note that the helper
functions know about mapping slight errors so if you are asked for 9600
and the hardware does 9575 it will report B9600 as you'd expect not do
something crazy.

> + xuartps_set_baud_rate(port, 9600);

This seems a bit odd in the startup or is there a hardware need to do
this and then fix the rate ? Ditto some of the others


Otherwise looks fine.

Alan


\
 
 \ /
  Last update: 2011-04-19 23:17    [W:0.093 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site