2008-06-03 02:20:30

by gyang

[permalink] [raw]
Subject: Re: Blackfin Serial Driver: Enable IR function when user application (irattach /dev/ttyBFx -s) call TIOCSETD ioctl with line discipline N_IRDA

Hi, Alan,

Have the set_ldisc operator been put into main tree?

As current development cycle will end in few weeks, while rc4 kernel
still can't be compiled successfully because of this issue.

BRs,
Graf


?? 2008-05-04?յ? 16:56 +0100??Alan Coxд????
> > Oh, I got the compile failure now:
> > --
> > CC drivers/serial/bfin_5xx.o
> > drivers/serial/bfin_5xx.c: In function 'bfin_serial_init':
> > drivers/serial/bfin_5xx.c:1281: error: 'struct tty_driver' has no
> > member named 'set_ldisc'
> > make[2]: *** [drivers/serial/bfin_5xx.o] Error 1
> > make[1]: *** [drivers/serial] Error 2
> > make: *** [drivers] Error 2
> > --
> >
> > Obviously, it is no way to change code like this for passing compiling:
>
> I've added the uart set_ldisc operator to my tree. I thought Andrew had
> it as well, but if not I'll push it again Tuesday (Monday is a holiday
> here)
>
> Alan
> --
> "Alan, I'm getting a bit worried about you."
> -- Linus Torvalds


2008-06-03 14:34:38

by Alan

[permalink] [raw]
Subject: [PATCH] serial_core: uart_set_ldisc (Was Re: Blackfin Serial Driver: Enable IR function when user application (irattach /dev/ttyBFx -s) call TIOCSETD ioctl with line discipline N_IRDA)

On Tue, 03 Jun 2008 10:18:46 +0800
gyang <[email protected]> wrote:

> Hi, Alan,
>
> Have the set_ldisc operator been put into main tree?

It's been in for ages. The Uart level one seems to have gotten stuck. Let
me poke that again
--

The tty layer provides a callback that is used when the line discipline
is changed. Some hardware uses this to configure hardware specific
features such as IrDA mode on serial ports. Unfortunately the serial
layer does not provide this feature or pass it down to drivers.

Blackfin used to hack around this by rewriting the tty ops, but those are
now properly shared and const so the hack fails. Instead provide the
proper operations.

Linus: This change plus a follow up from the Blackfin guys is needed to
avoid blackfin losing features in this release.

Signed-off-by: Alan Cox <[email protected]>

diff -u --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.26-rc2/drivers/serial/serial_core.c linux-2.6.26-rc2/drivers/serial/serial_core.c
--- linux.vanilla-2.6.26-rc2/drivers/serial/serial_core.c 2008-05-12 12:09:06.000000000 +0100
+++ linux-2.6.26-rc2/drivers/serial/serial_core.c 2008-06-03 15:09:52.000000000 +0100
@@ -1165,6 +1166,15 @@
return ret;
}

+static void uart_set_ldisc(struct tty_struct *tty, int ldisc)
+{
+ struct uart_state *state = tty->driver_data;
+ struct uart_port *port = state->port;
+
+ if (port->ops->set_ldisc)
+ port->ops->set_ldisc(port);
+}
+
static void uart_set_termios(struct tty_struct *tty,
struct ktermios *old_termios)
{
@@ -2285,6 +2298,7 @@
.unthrottle = uart_unthrottle,
.send_xchar = uart_send_xchar,
.set_termios = uart_set_termios,
+ .set_ldisc = uart_set_ldisc,
.stop = uart_stop,
.start = uart_start,
.hangup = uart_hangup,
diff -u --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.26-rc2/include/linux/serial_core.h linux-2.6.26-rc2/include/linux/serial_core.h
--- linux.vanilla-2.6.26-rc2/include/linux/serial_core.h 2008-05-12 12:09:08.000000000 +0100
+++ linux-2.6.26-rc2/include/linux/serial_core.h 2008-06-03 15:08:28.000000000 +0100
@@ -192,6 +192,7 @@
void (*shutdown)(struct uart_port *);
void (*set_termios)(struct uart_port *, struct ktermios *new,
struct ktermios *old);
+ void (*set_ldisc)(struct uart_port *);
void (*pm)(struct uart_port *, unsigned int state,
unsigned int oldstate);
int (*set_wake)(struct uart_port *, unsigned int state);

2008-06-04 16:01:16

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] serial_core: uart_set_ldisc (Was Re: Blackfin Serial Driver: Enable IR function when user application (irattach /dev/ttyBFx -s) call TIOCSETD ioctl with line discipline N_IRDA)



On Tue, 3 Jun 2008, Alan Cox wrote:
>
> Linus: This change plus a follow up from the Blackfin guys is needed to
> avoid blackfin losing features in this release.

Alan, I applied this as "obviously safe", since nobody sets the uart-level
set_ldisc thing yet... BUT!

I now get an annoying compiler warning, and the compiler is definitely
right. You have:

static void uart_set_ldisc(struct tty_struct *tty, int ldisc)
..
.set_ldisc = uart_set_ldisc,

Oops. Totally wrong function type. Because the tty-level one is

void (*set_ldisc)(struct tty_struct *tty);

in my tree, and as a result I suspect you sent me the wrong version of a
patch (perhaps based on -mm or -next).

I assume I should just remove the (unused) "int ldisc" argument, but would
like to get confirmation first.

Linus

--
drivers/serial/serial_core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 951a75e..c9b64e7 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1165,7 +1165,7 @@ out:
return ret;
}

-static void uart_set_ldisc(struct tty_struct *tty, int ldisc)
+static void uart_set_ldisc(struct tty_struct *tty)
{
struct uart_state *state = tty->driver_data;
struct uart_port *port = state->port;