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

[Tobias Klauser <tklauser@distanz.ch>]

On 2010-03-24 at 12:05:27 +0100, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 24 Mar 2010 07:47:47 +0100 Tobias Klauser <tklauser@distanz.ch> wrote:
> > On 2010-03-23 at 22:54:59 +0100, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > On Fri,  5 Mar 2010 17:52:23 +0100
> > > Tobias Klauser <tklauser@distanz.ch> wrote:
> > > 
> > > > +	sigs |= (altera_uart_getppdcd(port->line) ? TIOCM_CD : 0);
> > > > +	sigs |= (altera_uart_getppdtr(port->line) ? TIOCM_DTR : 0);
> > > 
> > > 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'
> > 
> > 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?
> 
> 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.

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