serial: Add driver for the Altera JTAG UART
in [Kernel]
|
Prev: kmod-replace-call_usermodehelper_pipe-with-use-of-umh-init-function- and-resolve-limit.cleanup
Next: tracing: return accurate value for print_graph_prologue
From: Alan Cox on 25 Feb 2010 10:30 > +static void altera_jtaguart_set_termios(struct uart_port *port, > + struct ktermios *termios, > + struct ktermios *old) > +{ > +} Erm no .. it may be wrong in the code you are copying but please don't further that. The requirement is that the termios handed back reflects the actual settings not the requested ones. Use tty_termios_copy_hw() to copy the old termios hardware settings back so that the caller sees it cannot set them. > + > +static void altera_jtaguart_rx_chars(struct altera_jtaguart *pp) > +{ > + struct uart_port *port = &pp->port; > + unsigned char ch, flag; > + unsigned long status; > + > + while ((status = readl(port->membase + ALTERA_JTAGUART_DATA_REG)) & > + ALTERA_JTAGUART_DATA_RVALID_MSK) { > + ch = status & ALTERA_JTAGUART_DATA_DATA_MSK; > + flag = TTY_NORMAL; > + port->icount.rx++; > + > + if (uart_handle_sysrq_char(port, ch)) > + continue; > + uart_insert_char(port, 0, 0, ch, flag); > + } > + > + tty_flip_buffer_push(port->state->port.tty); port.tty needs to be protected here - you might race an unplug/hangup as far as I can see - I think you need to take the lock over a bigger area [I'm working on switching this to krefs in all the drivers currently so that will end up cleaner] Basically fine other than that. The proposed UART number clashes with some other pending patches but that will get sorted out when they get merged -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Peter Korsgaard on 25 Feb 2010 12:00 >>>>> "Tobias" == Tobias Klauser <tklauser(a)distanz.ch> writes: Tobias> Add an UART driver for the JTAG UART component available as a SOPC Tobias> (System on Programmable Chip) component for Altera FPGAs. Tobias> Signed-off-by: Tobias Klauser <tklauser(a)distanz.ch> Tobias> +config SERIAL_ALTERA_JTAGUART Tobias> + bool "Altera JTAG UART support" Tobias> + depends on EMBEDDED Don't depend on EMBEDDED - There's embedded devices not using CONFIG_EMBEDDED and kernel configs for PCs using it. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Tobias Klauser on 26 Feb 2010 04:10 On 2010-02-25 at 16:26:22 +0100, Alan Cox <alan(a)lxorguk.ukuu.org.uk> wrote: > > +static void altera_jtaguart_set_termios(struct uart_port *port, > > + struct ktermios *termios, > > + struct ktermios *old) > > +{ > > +} > > Erm no .. it may be wrong in the code you are copying but please don't > further that. The requirement is that the termios handed back reflects > the actual settings not the requested ones. > > Use tty_termios_copy_hw() to copy the old termios hardware settings back > so that the caller sees it cannot set them. Would the following be sufficient? if (old) tty_termios_copy_hw(termios, old); > > +static void altera_jtaguart_rx_chars(struct altera_jtaguart *pp) > > +{ > > + struct uart_port *port = &pp->port; > > + unsigned char ch, flag; > > + unsigned long status; > > + > > + while ((status = readl(port->membase + ALTERA_JTAGUART_DATA_REG)) & > > + ALTERA_JTAGUART_DATA_RVALID_MSK) { > > + ch = status & ALTERA_JTAGUART_DATA_DATA_MSK; > > + flag = TTY_NORMAL; > > + port->icount.rx++; > > + > > + if (uart_handle_sysrq_char(port, ch)) > > + continue; > > + uart_insert_char(port, 0, 0, ch, flag); > > + } > > + > > + tty_flip_buffer_push(port->state->port.tty); > > port.tty needs to be protected here - you might race an unplug/hangup as > far as I can see - I think you need to take the lock over a bigger area So should we take port->lock in altera_jtaguart_interrupt already, something like the following? spin_lock(&port->lock); if (isr & ALTERA_JTAGUART_CONTROL_RE_MSK) altera_jtaguart_rx_chars(pp); if (isr & ALTERA_JTAGUART_CONTROL_WE_MSK) altera_jtaguart_tx_chars(pp); spin_unlock(&port->lock); And of course remove the lock/unlock from altera_jtaguart_tx_chars. > [I'm working on switching this to krefs in all the drivers currently so > that will end up cleaner] > > Basically fine other than that. The proposed UART number clashes with > some other pending patches but that will get sorted out when they get > merged Thanks a lot for your review. Tobias -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 26 Feb 2010 06:30 > > Use tty_termios_copy_hw() to copy the old termios hardware settings back > > so that the caller sees it cannot set them. > > Would the following be sufficient? > > if (old) > tty_termios_copy_hw(termios, old); That will do the trick yes. > spin_lock(&port->lock); > > if (isr & ALTERA_JTAGUART_CONTROL_RE_MSK) > altera_jtaguart_rx_chars(pp); > if (isr & ALTERA_JTAGUART_CONTROL_WE_MSK) > altera_jtaguart_tx_chars(pp); > > spin_unlock(&port->lock); Yes - that stops ->tty becoming NULL while your handler is running Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on 12 Mar 2010 15:50
On Fri, 5 Mar 2010 17:52:22 +0100 Tobias Klauser <tklauser(a)distanz.ch> wrote: > Add an UART driver for the JTAG UART component available as a SOPC > (System on Programmable Chip) component for Altera FPGAs. > > ... > > --- a/drivers/serial/Kconfig > +++ b/drivers/serial/Kconfig > @@ -1490,4 +1490,25 @@ config SERIAL_GRLIB_GAISLER_APBUART_CONSOLE > help > Support for running a console on the GRLIB APBUART > > +config SERIAL_ALTERA_JTAGUART > + bool "Altera JTAG UART support" > + select SERIAL_CORE > + help > + This driver supports the Altera JTAG UART port. > + > +config SERIAL_ALTERA_JTAGUART_CONSOLE > + bool "Altera JTAG UART console support" > + depends on SERIAL_ALTERA_JTAGUART > + select SERIAL_CORE_CONSOLE > + help > + Enable a Altera JTAG UART port to be the system console. > + > +config SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS > + bool "Bypass output when no connection" > + depends on SERIAL_ALTERA_JTAGUART_CONSOLE > + select SERIAL_CORE_CONSOLE > + help > + Bypass console output and keep going even if there is no > + JTAG terminal connection with the host. So this driver will be available on all CPU architectures. I'm guessing that the hardware _isn't_ available on all CPU architectures? Maybe that's wrong. If this hardware is only available on certain CPUs then we face tradeoffs. By making it universally available, the code gets better compile tesst coverage and that can detect problems (usually minor ones). otoh, it can increase your maintenance load a bit, and adds a risk that people will ship unusable kernel modules and will see increased build times. I have no particular opinion either way. > endmenu > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile > index 5548fe7..6228b6e 100644 > --- a/drivers/serial/Makefile > +++ b/drivers/serial/Makefile > @@ -82,3 +82,4 @@ obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o > obj-$(CONFIG_SERIAL_QE) += ucc_uart.o > obj-$(CONFIG_SERIAL_TIMBERDALE) += timbuart.o > obj-$(CONFIG_SERIAL_GRLIB_GAISLER_APBUART) += apbuart.o > +obj-$(CONFIG_SERIAL_ALTERA_JTAGUART) += altera_jtaguart.o > diff --git a/drivers/serial/altera_jtaguart.c b/drivers/serial/altera_jtaguart.c > new file mode 100644 > index 0000000..f9b49b5 > --- /dev/null > +++ b/drivers/serial/altera_jtaguart.c > @@ -0,0 +1,504 @@ > +/* > + * altera_jtaguart.c -- Altera JTAG UART driver > + * > + * Based on mcf.c -- Freescale ColdFire UART driver > + * > + * (C) Copyright 2003-2007, Greg Ungerer <gerg(a)snapgear.com> > + * (C) Copyright 2008, Thomas Chou <thomas(a)wytron.com.tw> > + * (C) Copyright 2010, Tobias Klauser <tklauser(a)distanz.ch> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/module.h> > +#include <linux/console.h> > +#include <linux/tty.h> > +#include <linux/tty_flip.h> > +#include <linux/serial.h> > +#include <linux/serial_core.h> > +#include <linux/platform_device.h> > +#include <linux/io.h> > +#include <linux/altera_jtaguart.h> Does it make sense to put altera_jtaguart.h into include/linux? Could we put it in drivers/serial/? > > ... > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |