Re: [PATCHv3 2/2] serial: Add driver for the Altera UART

Previous thread: [git pull] vfs part 2 by Al Viro on Friday, March 5, 2010 - 9:29 am. (2 messages)

Next thread: [PATCH] MTD: drop +x perms on C source files by Mike Frysinger on Friday, March 5, 2010 - 10:00 am. (2 messages)
From: Tobias Klauser
Date: Friday, March 5, 2010 - 9:52 am

This is the third version of the patchset to add the serial drivers for the
Altera UARTs. I made the corrections suggested by Alan Cox on top of the second
version submitted on 01 Mar 2010.

Changes from v2 to v3:

 - altera_uart: Copy old termios settings back and set baud rate
 - altera_uart: Remove board specific DTR/DCD macros

Changes from v1 to v2:

 - altera_jtaguart: Implement altera_jtaguart_set_termios
 - altera_jtaguart: Protect port.tty in altera_jtaguart_rx_chars
 - altera_uart, altera_jtaguart: Don't depend on EMBEDDED
 - altera_jtaguart: Correct check for co->index
 - altera_uart: Correct check for co->index

Thanks,
Tobias
--

From: Tobias Klauser
Date: Friday, March 5, 2010 - 9:52 am

Add an UART driver for the UART component available as a SOPC (System on
Programmable Chip) component for Altera FPGAs.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/serial/Kconfig       |   31 +++
 drivers/serial/Makefile      |    1 +
 drivers/serial/altera_uart.c |  573 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/altera_uart.h  |   14 +
 include/linux/serial_core.h  |    1 +
 5 files changed, 620 insertions(+), 0 deletions(-)
 create mode 100644 drivers/serial/altera_uart.c
 create mode 100644 include/linux/altera_uart.h

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 7e67283..eaae8bb 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -1511,4 +1511,35 @@ config SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS
 	  Bypass console output and keep going even if there is no
 	  JTAG terminal connection with the host.
 
+config SERIAL_ALTERA_UART
+	bool "Altera UART support"
+	select SERIAL_CORE
+	help
+	  This driver supports the Altera softcore UART port.
+
+config SERIAL_ALTERA_UART_MAXPORTS
+	int "Maximum number of Altera UART ports"
+	depends on SERIAL_ALTERA_UART
+	default 4
+	help
+	  This setting lets you define the maximum number of the Altera
+	  UART ports. The usual default varies from board to board, and
+	  this setting is a way of catering for that.
+
+config SERIAL_ALTERA_UART_BAUDRATE
+	int "Default baudrate for Altera UART ports"
+	depends on SERIAL_ALTERA_UART
+	default 115200
+	help
+	  This setting lets you define what the default baudrate is for the
+	  Altera UART ports. The usual default varies from board to board,
+	  and this setting is a way of catering for that.
+
+config SERIAL_ALTERA_UART_CONSOLE
+	bool "Altera UART console support"
+	depends on SERIAL_ALTERA_UART
+	select SERIAL_CORE_CONSOLE
+	help
+	  Enable a Altera UART port to be the system console.
+
 endmenu
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 6228b6e..39007e6 100644
--- ...
From: Andrew Morton
Date: Tuesday, March 23, 2010 - 2:54 pm

On Fri,  5 Mar 2010 17:52:23 +0100

We seem to be missing a few things here.

drivers/serial/altera_uart.c: In function 'altera_uart_get_mctrl':
drivers/serial/altera_uart.c:100: error: implicit declaration of function 'altera_uart_getppdcd'
drivers/serial/altera_uart.c:101: error: implicit declaration of function 'altera_uart_getppdtr'
drivers/serial/altera_uart.c: In function 'altera_uart_set_mctrl':
drivers/serial/altera_uart.c:114: error: implicit declaration of function 'altera_uart_setppdtr'                                                                                                                                                                              
--

From: Tobias Klauser
Date: Tuesday, March 23, 2010 - 11:47 pm

These should usually be declared in a board specific header. There were
compatibility macros in altera_uart.c which defined them to NOPs in case
the board header did not properly define them. But I remove them as per
request by Alan Cox (Message-ID: 20100301181920.3952c3e7@lxorguk.ukuu.org.uk).

Should we add them again (maybe to altera_uart.h)? Or would it be better
to define a config symbol which is set in the board specific Kconfig and
altera_uart depends on it?

Thanks
Tobias
--

From: Andrew Morton
Date: Wednesday, March 24, 2010 - 4:05 am

I guess the latter.

There should have been a real implementation of these in the patchset -
otherwise the code can't be used or tested.  Confused.

--

From: Tobias Klauser
Date: Wednesday, March 24, 2010 - 9:24 am

Sorry for the confusion.

The last patchset I submitted (with the functions removed from
altera_uart.c) was tested on a local branch, where I added the macros to
a global board specific header. I didn't include that one in the patch.

After looking at the code and it's history a bit closer (and also on the
nios2 specific part) I realised that this macro was probably added
because the driver was originally based on drivers/serial/mcf.c (the
macros are present there too).

Also there are currently no board configurations known to me that define
these macros. So I'd suggest to remove the usage of these macros
alltogether. We could still add them again (to the board specific part
and with the config option then) in case there will be a board
configuration implementing DTR/DCD lines on GPIOs.

Could anyone on nios2-dev verify that there are currently no such board
configurations?

I'd remove the usage of the macros then and post an updated patch.

  Tobias
--

From: Thomas Chou
Date: Wednesday, March 24, 2010 - 5:54 pm

Maybe we can add pointers to functions for the DCD/DTR in the struct 
altera_uart_platform_uart. Then board config file can define them if 
they implement these pins, NULL/0 otherwise.

struct altera_uart_platform_uart {
...
     int (*getppdcd)(...);    /* get DCD status */
...

};


static unsigned int altera_uart_get_mctrl(struct uart_port *port)
{
...
     if (port->getppdcd)
         sigs |= (port->getppdcd(...)) ? TIOCM_CD : 0);


- Thomas

--

From: Tobias Klauser
Date: Monday, March 29, 2010 - 6:29 am

Unfortunately we then need to define them in struct altera_uart too and
set them in the probe function using the platform data.

For me that seems like a bit too much overhead just to support the
possibility of someone defining these functions in the future. I'd
prefer to get rid of the macros now for the mainline submission and then
add it later if someone would actually use these DCD/DTR get/set
functions. Hope that's OK for you too, Thomas.

-- Tobias
--

From: Thomas Chou
Date: Monday, March 29, 2010 - 5:18 pm

Hi Tobias,

OK. Please drop them. That would be easier.

Best regards,
Thomas
--

From: Tobias Klauser
Date: Friday, March 5, 2010 - 9:52 am

Add an UART driver for the JTAG UART component available as a SOPC
(System on Programmable Chip) component for Altera FPGAs.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/serial/Kconfig           |   21 ++
 drivers/serial/Makefile          |    1 +
 drivers/serial/altera_jtaguart.c |  504 ++++++++++++++++++++++++++++++++++++++
 include/linux/altera_jtaguart.h  |   16 ++
 include/linux/serial_core.h      |    3 +
 5 files changed, 545 insertions(+), 0 deletions(-)
 create mode 100644 drivers/serial/altera_jtaguart.c
 create mode 100644 include/linux/altera_jtaguart.h

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 888a0ce..7e67283 100644
--- 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.
+
 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 ...
From: Andrew Morton
Date: Friday, March 12, 2010 - 1:48 pm

On Fri,  5 Mar 2010 17:52:22 +0100

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

Does it make sense to put altera_jtaguart.h into include/linux?  Could
--

From: Tobias Klauser
Date: Monday, March 15, 2010 - 3:20 am

In principle it is available on all CPU. I can think of a hardware
component with an Altera FPGA containing this UART. But at the moment no
such implementation is known to me and the UART is only usable on the
nios2 CPU architecture  (which is not in mainline yet - we're planning
to change that, submitting the arch-sepcific drivers is the first step

I'd like to have this universally available if no one objects. It'd be
nice to get better test coverage and I don't really mind the additional
maintanance load. AFAICS other UART driver are only available on one
single architecture too, so I hoped it would be fine to go the same way

It is stored in include/linux and not in drivers/serial because we need
to include it from arch/nios2 for the platform device setup. Including
<linux/altera_jtaguart.h> looks nicer than including
"../drivers/serial/altera_jtaguart.h". The same holds true for the
altera_uart driver.

Thanks a lot,
Tobias
--

From: waltergoossens
Date: Monday, March 15, 2010 - 3:24 am

----- Origineel bericht -----
Van: Tobias Klauser <tklauser@distanz.ch>
Datum: maandag, maart 15, 2010 10:21
Onderwerp: Re: [Nios2-dev] [PATCHv3 1/2] serial: Add driver for the Altera	JTAG UART
Aan: Andrew Morton <akpm@linux-foundation.org>
--

From: Thomas Chou
Date: Monday, March 15, 2010 - 5:47 pm

It is true that this driver is used mostly with nios2, soft-core 
coldfire and arm. But peripherals on FPGA can be made available to 
almost any CPU architecture. So I think it might make sense to let it 
There is only a platform data structure declaration in the header, which 
should be passed through platform device data to the driver. If we put 
it in drivers/serial/, then we will need to include 
"../../../drivers/serial/altera_jtaguart.h" .

I have written this driver in parallel with the altera_uart driver. We 
could have passed the data ( base, irq, freq ) using platform resources. 
But I don't know if there is a way to pass ( freq )? Please let me know 
if you have any suggestions.

- Thomas
--

Previous thread: [git pull] vfs part 2 by Al Viro on Friday, March 5, 2010 - 9:29 am. (2 messages)

Next thread: [PATCH] MTD: drop +x perms on C source files by Mike Frysinger on Friday, March 5, 2010 - 10:00 am. (2 messages)