On 14. 07. 20, 8:24, Johnson CH Chen (陳昭勳) wrote:
> This driver supports tty functions for all of MOXA's NPort series
> with v5.0. Using this driver, host part can use tty to connect NPort
> device server by ethernet.
...
> Signed-off-by: Johnson Chen <[email protected]>
> Signed-off-by: Jason Chen <[email protected]>
> Signed-off-by: Danny Lin <[email protected]>
> Signed-off-by: Victor Yu <[email protected]>
> ---
...
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -259,6 +259,17 @@ config MOXA_SMARTIO
>         This driver can also be built as a module. The module will be called
>         mxser. If you want to do that, say M here.
>  
> +config MOXA_NPORT_REAL_TTY
> +     tristate "Moxa NPort Real TTY support v5.0"

MOXA_SMARTIO is lexicographically after MOXA_NPORT_REAL_TTY. So move
this before MOXA_SMARTIO.

> +     help
> +       Say Y here if you have a Moxa NPort serial device server.
> +
> +       The purpose of this driver is to map NPort serial port to host tty
> +       port. Using this driver, you can use NPort serial port as local tty 
> port.
> +
> +       This driver can also be built as a module. The module will be called
> +       npreal2 by setting M.
> +
>  config SYNCLINK
>       tristate "Microgate SyncLink card support"
>       depends on SERIAL_NONSTANDARD && PCI && ISA_DMA_API
> diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
> index 020b1cd9294f..6d07985d6962 100644
> --- a/drivers/tty/Makefile
> +++ b/drivers/tty/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_CYCLADES)              += cyclades.o
>  obj-$(CONFIG_ISI)            += isicom.o
>  obj-$(CONFIG_MOXA_INTELLIO)  += moxa.o
>  obj-$(CONFIG_MOXA_SMARTIO)   += mxser.o
> +obj-$(CONFIG_MOXA_NPORT_REAL_TTY) += npreal2.o

The same.

>  obj-$(CONFIG_NOZOMI)         += nozomi.o
>  obj-$(CONFIG_NULL_TTY)               += ttynull.o
>  obj-$(CONFIG_ROCKETPORT)     += rocket.o
> diff --git a/drivers/tty/npreal2.c b/drivers/tty/npreal2.c
> new file mode 100644
> index 000000000000..65c773420755
> --- /dev/null
> +++ b/drivers/tty/npreal2.c
> @@ -0,0 +1,3042 @@
...
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/fcntl.h>
> +#include <linux/version.h>

What do you need version.h for? Are you sure, you need all these headers?

> +#include <linux/init.h>
> +#include <linux/ioport.h>
> +#include <linux/interrupt.h>
> +#include <linux/major.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/ptrace.h>
> +#include <linux/poll.h>
> +#include <linux/proc_fs.h>
> +#include <linux/uaccess.h>
> +#include <linux/serial.h>
> +#include <linux/serial_reg.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/signal.h>
> +#include <linux/sched.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/timer.h>
> +#include "npreal2.h"
> +
> +static int ttymajor = NPREALMAJOR;

You want dynamic major anyway. So kill this all.

> +static int verbose = 1;
> +
> +MODULE_AUTHOR("<[email protected]>");
> +MODULE_DESCRIPTION("MOXA Async/NPort Server Family Real TTY Driver");
> +module_param(ttymajor, int, 0);
> +module_param(verbose, int, 0644);
> +MODULE_VERSION(NPREAL_VERSION);
> +MODULE_LICENSE("GPL");
> +
> +struct server_setting_struct {
> +     int32_t server_type;
> +     int32_t disable_fifo;
> +};
> +
> +struct npreal_struct {
> +     struct tty_port ttyPort;
> +     struct work_struct tqueue;
> +     struct work_struct process_flip_tqueue;
> +     struct ktermios normal_termios;
> +     struct ktermios callout_termios;

callout in 2020?

> +     /* kernel counters for the 4 input interrupts */
> +     struct async_icount icount;
> +     struct semaphore rx_semaphore;

semaphores in new code? You have to explain why are you not using
simpler and faset mutexes.

> +     struct nd_struct *net_node;
> +     struct tty_struct *tty;
> +     struct pid *session;
> +     struct pid *pgrp;

Why does a driver need to care about pgrp? And session? You should kill
all these set, but unused fields. Note that you should also use fields
from struct tty_port instead of the duplicates here.

> +     wait_queue_head_t open_wait;
> +     wait_queue_head_t close_wait;
> +     wait_queue_head_t delta_msr_wait;
> +     unsigned long baud_base;
> +     unsigned long event;
> +     unsigned short closing_wait;
> +     int port;
> +     int flags;
> +     int type;  /* UART type */
> +     int xmit_fifo_size;
> +     int custom_divisor;
> +     int x_char; /* xon/xoff character */
> +     int close_delay;
> +     int modem_control; /* Modem control register */
> +     int modem_status;  /* Line status */
> +     int count; /* # of fd on device */
> +     int xmit_head;
> +     int xmit_tail;
> +     int xmit_cnt;
> +     unsigned char *xmit_buf;

ringbuf (as Greg suggests) or kfifo.

> +     /*
> +      * We use spin_lock_irqsave instead of semaphonre here.

"semaphonre"?

> +      * Reason: When we use pppd to dialout via Real TTY driver,
> +      * some driver functions, such as npreal_write(), would be
> +      * invoked under interrpute mode which causes warning in

"interrpute"?

> +      * down/up tx_semaphore.
> +      */
> +     spinlock_t tx_lock;
> +};
> +
> +struct nd_struct {
> +     struct semaphore cmd_semaphore;
> +     struct proc_dir_entry *node_entry;
> +     struct npreal_struct *tty_node;
> +     struct semaphore semaphore;
> +     wait_queue_head_t initialize_wait;
> +     wait_queue_head_t select_in_wait;
> +     wait_queue_head_t select_out_wait;
> +     wait_queue_head_t select_ex_wait;
> +     wait_queue_head_t cmd_rsp_wait;
> +     int32_t server_type;
> +     int do_session_recovery_len;
> +     int cmd_rsp_flag;
> +     int tx_ready;
> +     int rx_ready;
> +     int cmd_ready;
> +     int wait_oqueue_responsed;
> +     int oqueue;
> +     int rsp_length;
> +     unsigned long flag;
> +     unsigned char cmd_buffer[84];
> +     unsigned char rsp_buffer[84];
> +};
> +
> +static const struct proc_ops npreal_net_fops;
> +static const struct tty_operations mpvar_ops;
> +static struct proc_dir_entry *npvar_proc_root;
> +static struct tty_driver *npvar_sdriver;
> +static struct npreal_struct *npvar_table;
> +static struct nd_struct *npvar_net_nodes;

Could you reorder the code, so that you don't need these forward
declarations?

> +static void npreal_do_softint(struct work_struct *work)
> +{

Well, this is the old way of doing things.

> +     struct npreal_struct *info = container_of(work, struct npreal_struct, 
> tqueue);
> +     struct tty_struct *tty;
> +
> +     if (!info)
> +             return;
> +
> +     tty = info->tty;
> +     if (tty) {
> +             if (test_and_clear_bit(NPREAL_EVENT_TXLOW, &info->event)) {

Do you ever set that flag?

> +                     if ((tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
> +                             tty->ldisc->ops->write_wakeup)
> +                             (tty->ldisc->ops->write_wakeup)(tty);
> +                     wake_up_interruptible(&tty->write_wait);
> +             }
> +
> +             if (test_and_clear_bit(NPREAL_EVENT_HANGUP, &info->event)) {
> +                     /* Do it when entering npreal_hangup() */
> +                     tty_hangup(tty);

Have you checked what tty_hangup actually does? Drop this whole function.

> +             }
> +     }
> +}
> +
> +/**
> + * npreal_flush_to_ldisc() - Read data from tty device to line discipline
> + * @tty: pointer for struct tty_struct
> + * @filp: pointer for struct file
> + *
> + * This routine is called out of the software interrupt to flush data
> + * from the flip buffer to the line discipline.
> + *
> + */
> +
> +static void npreal_flush_to_ldisc(struct work_struct *work)
> +{

Ugh, the same as above, drop this and call flip directly.

> +     struct npreal_struct *info = container_of(work, struct npreal_struct, 
> process_flip_tqueue);
> +     struct tty_struct *tty;
> +     struct tty_port *port;
> +
> +     if (info == NULL)
> +             return;
> +
> +     tty = info->tty;
> +     if (tty == NULL)
> +             return;
> +
> +     port = tty->port;
> +     tty_flip_buffer_push(port);
> +}
> +
> +static inline void npreal_check_modem_status(struct npreal_struct *info, int 
> status)
> +{
> +     int is_dcd_changed = 0;
> +
> +     if ((info->modem_status & UART_MSR_DSR) != (status & UART_MSR_DSR))
> +             info->icount.dsr++;
> +     if ((info->modem_status & UART_MSR_DCD) != (status & UART_MSR_DCD)) {
> +             info->icount.dcd++;
> +             is_dcd_changed = 1;
> +     }
> +
> +     if ((info->modem_status & UART_MSR_CTS) != (status & UART_MSR_CTS))
> +             info->icount.cts++;
> +
> +     info->modem_status = status;
> +     wake_up_interruptible(&info->delta_msr_wait);
> +
> +     if ((info->flags & ASYNC_CHECK_CD) && (is_dcd_changed)) {
> +             if (status & UART_MSR_DCD) {
> +                     wake_up_interruptible(&info->open_wait);
> +             } else {
> +                     set_bit(NPREAL_EVENT_HANGUP, &info->event);
> +                     schedule_work(&info->tqueue);
> +             }
> +     }
> +}
> +
> +static int npreal_wait_and_set_command(struct nd_struct *nd, char 
> command_set, char command)
> +{
> +     unsigned long et;
> +
> +     if ((command_set != NPREAL_LOCAL_COMMAND_SET) &&
> +             ((nd->flag & NPREAL_NET_DO_SESSION_RECOVERY) ||
> +             (nd->flag & NPREAL_NET_NODE_DISCONNECTED))) {
> +
> +             if (nd->flag & NPREAL_NET_DO_SESSION_RECOVERY)
> +                     return -EAGAIN;
> +
> +             return -EIO;
> +     }
> +
> +     down(&nd->cmd_semaphore);
> +     nd->cmd_rsp_flag = 0;
> +     up(&nd->cmd_semaphore);
> +
> +     et = jiffies + NPREAL_CMD_TIMEOUT;
> +     while (1) {
> +             down(&nd->cmd_semaphore);
> +             if (!(nd->cmd_buffer[0] == 0 || ((jiffies - et >= 0) ||
> +                     signal_pending(current)))) {
> +                     up(&nd->cmd_semaphore);
> +                     current->state = TASK_INTERRUPTIBLE;
> +                     schedule_timeout(1);

This is very bad.

This calls for wait_event_interruptible or alike.  "jiffies - et >= 0"
is broken in any case. time_after() is your friend.

> +             } else {
> +                     nd->cmd_buffer[0] = command_set;
> +                     nd->cmd_buffer[1] = command;
> +                     up(&nd->cmd_semaphore);
> +                     return 0;
> +             }
> +     }
> +}
> +
> +static int npreal_wait_command_completed(struct nd_struct *nd, char 
> command_set, char command,
> +                                             long timeout, char *rsp_buf, 
> int *rsp_len)
> +{
> +     long st = 0, tmp = 0;
> +
> +     if ((command_set != NPREAL_LOCAL_COMMAND_SET) &&
> +             ((nd->flag & NPREAL_NET_DO_SESSION_RECOVERY) ||
> +             (nd->flag & NPREAL_NET_NODE_DISCONNECTED))) {
> +
> +             if (nd->flag & NPREAL_NET_DO_SESSION_RECOVERY)
> +                     return -EAGAIN;
> +             else
> +                     return -EIO;
> +     }
> +
> +     if (*rsp_len <= 0)
> +             return -EIO;
> +
> +     while (1) {
> +             down(&nd->cmd_semaphore);
> +
> +             if ((nd->rsp_length) && (nd->rsp_buffer[0] == command_set) &&
> +                                     (nd->rsp_buffer[1] == command)) {

You should break the loop here and do the processing below after the
loop. Making thuse the loop minimalistic.

> +                     if (nd->rsp_length > *rsp_len)
> +                             return -1;
> +
> +                     *rsp_len = nd->rsp_length;
> +                     memcpy(rsp_buf, nd->rsp_buffer, *rsp_len);
> +                     nd->rsp_length = 0;
> +                     up(&nd->cmd_semaphore);
> +                     return 0;
> +
> +             } else if (timeout > 0) {
> +                     up(&nd->cmd_semaphore);
> +                     if (signal_pending(current))
> +                             return -EIO;
> +
> +                     st = jiffies;
> +                     if (wait_event_interruptible_timeout(nd->cmd_rsp_wait,
> +                                                     nd->cmd_rsp_flag == 1, 
> timeout) != 0) {
> +                             down(&nd->cmd_semaphore);
> +                             nd->cmd_rsp_flag = 0;
> +                             up(&nd->cmd_semaphore);
> +                     }
> +
> +                     tmp = abs((long)jiffies - (long)st);
> +
> +                     if (tmp >= timeout)
> +                             timeout = 0;
> +                     else
> +                             timeout -= tmp;

wait_event_interruptible_timeout already returns what you compute here
in a complicated way, IIUC.

> +             } else {
> +                     up(&nd->cmd_semaphore);
> +                     return -ETIME;
> +             }
> +     }
> +}
> +
> +static int npreal_set_unused_command_done(struct nd_struct *nd, char 
> *rsp_buf, int *rsp_len)
> +{
> +     npreal_wait_and_set_command(nd, NPREAL_LOCAL_COMMAND_SET, 
> LOCAL_CMD_TTY_UNUSED);
> +     nd->cmd_buffer[2] = 0;
> +     nd->cmd_ready = 1;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +
> +     return npreal_wait_command_completed(nd, NPREAL_LOCAL_COMMAND_SET, 
> LOCAL_CMD_TTY_UNUSED,
> +                                             NPREAL_CMD_TIMEOUT, rsp_buf, 
> rsp_len);
> +}
> +
> +static int npreal_set_used_command_done(struct nd_struct *nd, char *rsp_buf, 
> int *rsp_len)
> +{
> +     nd->cmd_buffer[0] = 0;
> +     npreal_wait_and_set_command(nd, NPREAL_LOCAL_COMMAND_SET, 
> LOCAL_CMD_TTY_USED);
> +     nd->cmd_buffer[2] = 0;
> +     nd->cmd_ready = 1;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +
> +     return npreal_wait_command_completed(nd, NPREAL_LOCAL_COMMAND_SET, 
> LOCAL_CMD_TTY_USED,
> +                                             NPREAL_CMD_TIMEOUT, rsp_buf, 
> rsp_len);
> +}
> +
> +static int npreal_set_tx_fifo_command_done(struct npreal_struct *info, 
> struct nd_struct *nd,
> +                                                             char *rsp_buf, 
> int *rsp_len)
> +{
> +     int ret;
> +
> +     ret = npreal_wait_and_set_command(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_TX_FIFO);
> +     if (ret < 0)
> +             return ret;
> +
> +     nd->cmd_buffer[2] = 1;
> +     nd->cmd_buffer[3] = info->xmit_fifo_size;
> +     nd->cmd_ready = 1;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +
> +     *rsp_len = RSP_BUFFER_SIZE;
> +     ret = npreal_wait_command_completed(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_TX_FIFO,
> +                                             NPREAL_CMD_TIMEOUT, rsp_buf, 
> rsp_len);
> +     if (ret)
> +             return -EIO;
> +
> +     return 0;
> +}
> +
> +static int npreal_set_port_command_done(struct npreal_struct *info, struct 
> nd_struct *nd,
> +                                     struct ktermios *termio, char *rsp_buf, 
> int *rsp_len,
> +                                     int32_t mode, int32_t baud, int 
> baudIndex)
> +{
> +     int ret;
> +
> +     ret = npreal_wait_and_set_command(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_PORT_INIT);
> +     if (ret < 0)
> +             return ret;
> +
> +     nd->cmd_buffer[2] = 8;
> +     nd->cmd_buffer[3] = baudIndex;
> +     nd->cmd_buffer[4] = mode;
> +
> +     if (info->modem_control & UART_MCR_DTR)
> +             nd->cmd_buffer[5] = 1;
> +     else
> +             nd->cmd_buffer[5] = 0;

Or simply:
nd->cmd_buffer[5] = !!(info->modem_control & UART_MCR_DTR);
In all of them below:

> +     if (info->modem_control & UART_MCR_RTS)
> +             nd->cmd_buffer[6] = 1;
> +     else
> +             nd->cmd_buffer[6] = 0;
> +
> +     if (termio->c_cflag & CRTSCTS) {
> +             nd->cmd_buffer[7] = 1;
> +             nd->cmd_buffer[8] = 1;
> +     } else {
> +             nd->cmd_buffer[7] = 0;
> +             nd->cmd_buffer[8] = 0;
> +     }
> +
> +     if (termio->c_iflag & IXON)
> +             nd->cmd_buffer[9] = 1;
> +     else
> +             nd->cmd_buffer[9] = 0;
> +
> +     if (termio->c_iflag & IXOFF)
> +             nd->cmd_buffer[10] = 1;
> +     else
> +             nd->cmd_buffer[10] = 0;

What is this cmd_buffer good for actually? Only to let the user know?
Then -- drop it.

> +     nd->cmd_ready = 1;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +
> +     ret = npreal_wait_command_completed(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_PORT_INIT,
> +                                     NPREAL_CMD_TIMEOUT, rsp_buf, rsp_len);
> +     if (ret)
> +             return -EIO;
> +
> +     if ((*rsp_len != 6) || (rsp_buf[2] != 3))
> +             return -EIO;
> +
> +     return 0;
> +}
...
> +static int npreal_set_generic_command_done(struct npreal_struct *info, int 
> cmd)
> +{
> +     struct nd_struct *nd;
> +     char rsp_buffer[RSP_BUFFER_SIZE];
> +     int rsp_length = RSP_BUFFER_SIZE;
> +
> +     nd = info->net_node;
> +     if (!nd)
> +             return -EIO;
> +
> +     if (npreal_wait_and_set_command(nd, NPREAL_ASPP_COMMAND_SET, cmd) < 0)
> +             return -EIO;
> +
> +     nd->cmd_buffer[2] = 0;
> +     nd->cmd_ready = 1;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +     if (npreal_wait_command_completed(nd, NPREAL_ASPP_COMMAND_SET, cmd, 
> NPREAL_CMD_TIMEOUT,
> +                                             rsp_buffer, &rsp_length))
> +             return -EIO;
> +
> +     if ((rsp_length != 4) || (rsp_buffer[2] != 'O') || (rsp_buffer[3] != 
> 'K'))

Too many parentheses in all these functions.

> +             return -EIO;
> +
> +     return 0;
> +}
...
> +static void npreal_flush_buffer(struct tty_struct *tty)
> +{
> +     struct npreal_struct *info = (struct npreal_struct *)tty->driver_data;
> +     struct nd_struct *nd;
> +     char rsp_buffer[RSP_BUFFER_SIZE];
> +     int rsp_length = RSP_BUFFER_SIZE;
> +     unsigned long flags;
> +
> +     if (!info)
> +             return;
> +
> +     spin_lock_irqsave(&info->tx_lock, flags);
> +     info->xmit_tail = 0;
> +     info->xmit_head = 0;
> +     info->xmit_cnt = 0;
> +     spin_unlock_irqrestore(&info->tx_lock, flags);
> +     wake_up_interruptible(&tty->write_wait);
> +
> +     if ((tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) && 
> tty->ldisc->ops->write_wakeup)
> +             (tty->ldisc->ops->write_wakeup)(tty);

Why not tty_wakeup?

> +
> +     nd = info->net_node;
> +     if (!nd)
> +             return;
> +
> +     nd->tx_ready = 0;
> +     if (npreal_wait_and_set_command(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_FLUSH) < 0)
> +             return;
> +
> +     nd->cmd_buffer[2] = 1;
> +     nd->cmd_buffer[3] = ASPP_FLUSH_ALL_BUFFER;
> +     nd->cmd_ready = 1;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +
> +     npreal_wait_command_completed(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_FLUSH,
> +                                     NPREAL_CMD_TIMEOUT, rsp_buffer, 
> &rsp_length);
> +}
> +
> +static long npreal_wait_oqueue(struct npreal_struct *info, long timeout)
> +{
> +     struct nd_struct *nd;
> +     long st = 0, tmp = 0;
> +     uint32_t tout;
> +
> +     nd = info->net_node;
> +     if (!nd)
> +             return -EIO;
> +
> +     if (npreal_wait_and_set_command(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_WAIT_OQUEUE) < 0)
> +             return -EIO;
> +
> +     if (timeout < HZ / 10)
> +             timeout = HZ / 10;
> +
> +     st = jiffies;
> +
> +     if (timeout != MAX_SCHEDULE_TIMEOUT)
> +             tout = (uint32_t)timeout;
> +     else
> +             tout = 0x7FFFFFFF;
> +
> +     nd->cmd_buffer[2] = 4;
> +     memcpy(&nd->cmd_buffer[3], (void *)&tout, 4);
> +     nd->wait_oqueue_responsed = 0;
> +     nd->cmd_ready = 1;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +
> +     while (nd->cmd_ready == 1) {
> +             if (wait_event_interruptible_timeout(nd->cmd_rsp_wait, 
> nd->cmd_rsp_flag == 1,
> +                                                     timeout) != 0) {
> +                     down(&nd->cmd_semaphore);
> +                     nd->cmd_rsp_flag = 0;
> +                     up(&nd->cmd_semaphore);
> +             } else {
> +                     return -EIO;
> +             }
> +     }
> +
> +     nd->cmd_buffer[0] = 0;
> +     do {
> +             if (nd->wait_oqueue_responsed == 0) {
> +                     if (wait_event_interruptible_timeout(nd->cmd_rsp_wait,
> +                                                     nd->cmd_rsp_flag == 1, 
> timeout)) {
> +                             down(&nd->cmd_semaphore);
> +                             nd->cmd_rsp_flag = 0;
> +                             up(&nd->cmd_semaphore);
> +                     }
> +
> +                     tmp = abs((long)jiffies - (long)st);
> +                     if (tmp >= timeout)
> +                             timeout = 0;
> +                     else
> +                             timeout -= tmp;

Again this beast.

> +             } else {
> +                     return nd->oqueue;
> +             }
> +     } while (timeout > 0);
> +
> +     return -EIO;
> +}
...
> +static void npreal_port_init_baud(struct npreal_struct *info, struct 
> ktermios *termio,
> +                             struct ktermios *old_termios, int32_t 
> *baud_ret, int *index_ret)
> +{
> +     int baudIndex;
> +     int32_t baud;
> +
> +     switch (termio->c_cflag & (CBAUD | CBAUDEX)) {
> +     case B921600:
> +             baud = 921600L;

Why those L (long) suffixes when you assign to an int? Why is not the
int unsigned?

> +             baudIndex = ASPP_IOCTL_B921600;
> +             break;
> +
> +     case B460800:
> +             baud = 460800;
> +             baudIndex = ASPP_IOCTL_B460800;
> +             break;
> +
> +     case B230400:
> +             baud = 230400L;
> +             baudIndex = ASPP_IOCTL_B230400;
> +             break;
...
> +     default:
> +             baud = tty_termios_baud_rate(termio);
> +             baudIndex = 0xff;
> +     }
> +
> +#ifdef ASYNC_SPD_CUST
> +     if ((info->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST)
> +             baudIndex = 0xff;
> +#endif
> +
> +     if (baud > 921600L) {
> +             termio->c_cflag &= ~(CBAUD | CBAUDEX);
> +             termio->c_cflag |= old_termios->c_cflag & (CBAUD | CBAUDEX);
> +     }
> +
> +     *baud_ret = baud;
> +     *index_ret = baudIndex;
> +}
> +
> +static int npreal_port_init(struct npreal_struct *info, struct ktermios 
> *old_termios)
> +{
> +     struct ktermios *termio;
> +     struct nd_struct *nd;
> +     int rsp_length = RSP_BUFFER_SIZE;
> +     int baudIndex, modem_status;
> +     int ret;
> +     int32_t baud, mode;
> +     char rsp_buffer[RSP_BUFFER_SIZE];
> +
> +     nd = info->net_node;
> +     if (!info->tty || !nd)
> +             return -EIO;
> +
> +     termio = &(info->tty->termios);
> +     mode = npreal_port_init_mode(termio);
> +     npreal_port_init_baud(info, termio, old_termios, &baud, &baudIndex);
> +     ret = npreal_set_port_command_done(info, nd, termio, rsp_buffer, 
> &rsp_length, mode, baud,
> +                                             baudIndex);
> +     if (ret < 0)
> +             return ret;
> +
> +     modem_status = 0;
> +     if (((unsigned char)rsp_buffer[3] == 0xff) && ((unsigned 
> char)rsp_buffer[4] == 0xff) &&
> +             ((unsigned char)rsp_buffer[5] == 0xff)) {
> +             termio->c_cflag &= ~(CBAUD | CBAUDEX);
> +             termio->c_cflag |= old_termios->c_cflag & (CBAUD | CBAUDEX);
> +     } else {
> +             if (rsp_buffer[3])
> +                     modem_status |= UART_MSR_DSR;
> +             if (rsp_buffer[4])
> +                     modem_status |= UART_MSR_CTS;
> +             if (rsp_buffer[5])
> +                     modem_status |= UART_MSR_DCD;
> +     }
> +
> +     npreal_check_modem_status(info, modem_status);
> +
> +     if ((baudIndex == 0xff) && (baud != 0)) {
> +             ret = npreal_set_baud_command_done(info, nd, rsp_buffer, 
> &rsp_length, baud);
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     if (termio->c_iflag & (IXON | IXOFF)) {
> +             ret = npreal_set_xonxoff_command_done(info, termio, nd, 
> rsp_buffer, &rsp_length);
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     if (termio->c_cflag & CLOCAL)
> +             info->flags &= ~ASYNC_CHECK_CD;
> +     else
> +             info->flags |= ASYNC_CHECK_CD;
> +
> +     if (!info->tty)
> +             return -EIO;
> +
> +     return 0;
> +}
> +
> +static void npreal_init_net_node(struct nd_struct *net_node, struct 
> npreal_struct *tty_node,
> +                                                             struct 
> proc_dir_entry *de)
> +{
> +     net_node->tty_node = tty_node;
> +     net_node->node_entry = de;
> +     net_node->cmd_rsp_flag = 0;
> +     net_node->flag = 0;
> +
> +     sema_init(&net_node->cmd_semaphore, 1);
> +     sema_init(&net_node->semaphore, 1);
> +
> +     init_waitqueue_head(&net_node->initialize_wait);
> +     init_waitqueue_head(&net_node->select_in_wait);
> +     init_waitqueue_head(&net_node->select_out_wait);
> +     init_waitqueue_head(&net_node->select_ex_wait);
> +     init_waitqueue_head(&net_node->cmd_rsp_wait);
> +}
> +
> +static void npreal_init_tty_node(struct npreal_struct *tty_node, struct 
> nd_struct *net_node, int i)
> +{
> +     tty_node->net_node = net_node;
> +     tty_node->port = i;
> +     tty_node->type = PORT_16550A;
> +     tty_node->flags = 0;
> +     tty_node->xmit_fifo_size = 16;
> +     tty_node->baud_base = 921600L;
> +     tty_node->close_delay = 5 * HZ / 10;
> +     tty_node->closing_wait = 30 * HZ;
> +     tty_node->normal_termios = npvar_sdriver->init_termios;
> +
> +     memset(&tty_node->icount, 0, sizeof(tty_node->icount));
> +     INIT_WORK(&tty_node->process_flip_tqueue, npreal_flush_to_ldisc);
> +     INIT_WORK(&tty_node->tqueue, npreal_do_softint);
> +     spin_lock_init(&tty_node->tx_lock);
> +     sema_init(&tty_node->rx_semaphore, 1);
> +
> +     init_waitqueue_head(&tty_node->open_wait);
> +     init_waitqueue_head(&tty_node->close_wait);
> +     init_waitqueue_head(&tty_node->delta_msr_wait);
> +}
> +
> +static int npreal_init(struct npreal_struct *tty_node, struct nd_struct 
> *net_node)
> +{
> +     struct proc_dir_entry *de;
> +     char buf[4];
> +     int i;
> +
> +     npvar_proc_root = proc_mkdir("npreal2", NULL);
> +     if (!npvar_proc_root)
> +             return -ENOMEM;
> +
> +     tty_node = &npvar_table[0];
> +     net_node = &npvar_net_nodes[0];
> +
> +     for (i = 0; i < NPREAL_PORTS; i++, tty_node++, net_node++) {
> +             sprintf(buf, "%d", i);
> +             de = proc_create_data(buf, 0666 | S_IFREG, npvar_proc_root, 
> &npreal_net_fops,
> +                                     (void *)net_node);
> +             if (!de)
> +                     return -ENOMEM;
> +
> +             npreal_init_net_node(net_node, tty_node, de);
> +             npreal_init_tty_node(tty_node, net_node, i);
> +     }
> +
> +     return 0;
> +}
> +
> +static int npreal_chars_in_buffer(struct tty_struct *tty)
> +{
> +     struct npreal_struct *info = (struct npreal_struct *)tty->driver_data;
> +
> +     if (!info)
> +             return -EIO;
> +
> +     return info->xmit_cnt;
> +}
> +
> +static int npreal_block_till_ready(struct tty_struct *tty, struct file *filp,
> +                                             struct npreal_struct *info)

Why are you not using tty_port_block_til_ready?

> +{
> +     DECLARE_WAITQUEUE(wait, current);
> +     struct nd_struct *nd;
> +     int do_clocal = 0;
> +     int retval = 0;
> +
> +     nd = info->net_node;
> +     if (!nd)
> +             return -EIO;
> +
> +     if ((filp->f_flags & O_NONBLOCK) || test_bit(TTY_IO_ERROR, 
> &tty->flags)) {
> +             info->flags |= ASYNC_NORMAL_ACTIVE;
> +             return 0;
> +     }
> +
> +     if (tty->termios.c_cflag & CLOCAL)
> +             do_clocal = 1;
> +
> +     add_wait_queue(&info->open_wait, &wait);
> +     while (1) {
> +             if (tty_hung_up_p(filp))
> +                     break;
> +             else if (info->flags & ASYNC_CLOSING) {
> +                     if (SERIAL_DO_RESTART && !(info->flags & 
> ASYNC_HUP_NOTIFY))
> +                             retval = -ERESTARTSYS;
> +                     else
> +                             retval = -EAGAIN;
> +                     break;
> +             }
> +
> +             if (!(info->flags & ASYNC_CLOSING) &&
> +                     (do_clocal || (info->modem_status & UART_MSR_DCD)))
> +                     break;
> +
> +             if (signal_pending(current)) {
> +                     retval = -EIO;
> +                     break;
> +             }
> +
> +             current->state = TASK_INTERRUPTIBLE;
> +             schedule();
> +     }
> +
> +     remove_wait_queue(&info->open_wait, &wait);
> +     if (!retval)
> +             info->flags |= ASYNC_NORMAL_ACTIVE;
> +
> +     return retval;
> +}
> +
> +static void set_common_xmit_fifo_size(struct npreal_struct *info, struct 
> nd_struct *nd)
> +{
> +     if (info->type == PORT_16550A) {
> +             if (nd->server_type == CN2500)
> +                     info->xmit_fifo_size = 64;
> +             else
> +                     info->xmit_fifo_size = 16;
> +     } else {
> +             info->xmit_fifo_size = 1;
> +     }
> +}
> +
> +static int npreal_port_shutdown(struct npreal_struct *info)
> +{
> +     struct nd_struct *nd;
> +     char rsp_buffer[RSP_BUFFER_SIZE];
> +     int rsp_length = RSP_BUFFER_SIZE;
> +
> +     nd = info->net_node;
> +     if (!nd)
> +             return -EIO;
> +
> +     npreal_disconnect(nd, rsp_buffer, &rsp_length);
> +     nd->flag &= ~NPREAL_NET_TTY_INUSED;
> +     return 0;
> +}
> +
> +static int npreal_get_serial_info(struct npreal_struct *info, struct 
> serial_struct *retinfo)
> +{
> +     struct serial_struct tmp;
> +
> +     if (!retinfo)
> +             return -EFAULT;
> +
> +     memset(&tmp, 0, sizeof(tmp));
> +     tmp.type = info->type;
> +     tmp.line = info->port;
> +     tmp.flags = info->flags;
> +     tmp.close_delay = info->close_delay;
> +     tmp.closing_wait = info->closing_wait;
> +     tmp.custom_divisor = info->custom_divisor;
> +     tmp.baud_base = info->baud_base;
> +     tmp.hub6 = 0;
> +
> +     if (copy_to_user(retinfo, &tmp, sizeof(*retinfo)))
> +             return -EFAULT;
> +     else
> +             return 0;
> +}
> +
> +static int npreal_set_serial_info(struct npreal_struct *info, struct 
> serial_struct *new_info)
> +{
> +     struct serial_struct new_serial;
> +     int rsp_length = RSP_BUFFER_SIZE;
> +     int retval = 0;
> +     unsigned int flags;
> +     char rsp_buffer[RSP_BUFFER_SIZE];
> +
> +
> +     if ((!new_info) || copy_from_user(&new_serial, new_info, 
> sizeof(new_serial)))
> +             return -EFAULT;
> +
> +     flags = info->flags & ASYNC_SPD_MASK;
> +
> +     if (!capable(CAP_SYS_ADMIN)) {
> +             if ((new_serial.close_delay != info->close_delay) ||
> +                     ((new_serial.flags & ~ASYNC_USR_MASK) != (info->flags & 
> ~ASYNC_USR_MASK)))
> +                     return -EPERM;
> +
> +             info->flags = ((info->flags & ~ASYNC_USR_MASK) |
> +                             (new_serial.flags & ASYNC_USR_MASK));
> +     } else {
> +             info->flags = ((info->flags & ~ASYNC_FLAGS) | (new_serial.flags 
> & ASYNC_FLAGS));
> +             info->close_delay = new_serial.close_delay * HZ / 100;
> +
> +             if (new_serial.closing_wait == ASYNC_CLOSING_WAIT_NONE)
> +                     info->closing_wait = ASYNC_CLOSING_WAIT_NONE;
> +             else
> +                     info->closing_wait = new_serial.closing_wait * HZ / 100;
> +     }
> +
> +     info->type = new_serial.type;
> +     set_common_xmit_fifo_size(info, info->net_node);
> +
> +     if (info->flags & ASYNC_INITIALIZED) {
> +             if (flags != (info->flags & ASYNC_SPD_MASK))
> +                     retval = npreal_port_init(info, 0);
> +
> +             if (info->net_node)
> +                     npreal_set_tx_fifo_command_done(info, info->net_node, 
> rsp_buffer,
> +                                                     &rsp_length);
> +
> +     }
> +
> +     info->custom_divisor = new_serial.custom_divisor;
> +
> +     if (info->custom_divisor == 0)
> +             info->baud_base = 921600L;
> +     else
> +             info->baud_base = new_serial.baud_base;
> +
> +     return retval;
> +}
> +
> +/**
> + * npreal_get_lsr_info() - get line status register info
> + *
> + * Let user call ioctl() to get info when the UART physically is emptied.
> + * On bus types like RS485, the transmitter must release the bus after
> + * transmitting. This must be done when the transmit shift register is
> + * empty, not be done when the transmit holding register is empty.
> + * This functionality allows an RS485 driver to be written in user space.
> + *
> + * Always return 0 when function is ended.
> + */
> +static int npreal_get_lsr_info(struct npreal_struct *info,
> +                             unsigned int *value)
> +{
> +     unsigned int result = 0;
> +
> +     if (npreal_wait_oqueue(info, 0) == 0)
> +             result = TIOCSER_TEMT;
> +
> +     put_user(result, value);
> +
> +     return 0;
> +}
> +
> +static int npreal_start_break(struct nd_struct *nd)
> +{
> +     char rsp_buffer[RSP_BUFFER_SIZE];
> +     int rsp_length = RSP_BUFFER_SIZE;
> +
> +     npreal_wait_and_set_command(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_START_BREAK);
> +     nd->cmd_buffer[2] = 0;
> +     nd->cmd_ready = 1;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +
> +     if (npreal_wait_command_completed(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_START_BREAK,
> +                                     NPREAL_CMD_TIMEOUT, rsp_buffer, 
> &rsp_length))
> +             return -EIO;
> +
> +     if ((rsp_length != 4) || (rsp_buffer[2] != 'O') || (rsp_buffer[3] != 
> 'K'))
> +             return -EIO;
> +
> +     return 0;
> +}
> +
> +static int npreal_stop_break(struct nd_struct *nd)
> +{
> +     char rsp_buffer[RSP_BUFFER_SIZE];
> +     int rsp_length = RSP_BUFFER_SIZE;
> +
> +     npreal_wait_and_set_command(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_STOP_BREAK);
> +     nd->cmd_buffer[2] = 0;
> +     nd->cmd_ready = 1;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +
> +     rsp_length = sizeof(rsp_buffer);
> +     if (npreal_wait_command_completed(nd, NPREAL_ASPP_COMMAND_SET, 
> ASPP_CMD_STOP_BREAK,
> +                                     NPREAL_CMD_TIMEOUT, rsp_buffer, 
> &rsp_length))
> +             return -EIO;
> +
> +     if (rsp_length != 4  || (rsp_buffer[2] != 'O') || (rsp_buffer[3] != 
> 'K'))
> +             return -EIO;
> +
> +     return 0;
> +}
> +
> +/**
> + * npreal_send_break() - Sends break characters out the serial port.
> + * @info: pointer for npread_struct
> + * @duration: the time during sending break signals
> + *
> + * This is called by npreal_ioctl() with case of TCSBRK or TCSBRKP
> + */
> +static void npreal_send_break(struct npreal_struct *info, unsigned int 
> duration)
> +{
> +     struct nd_struct *nd;
> +
> +     nd = info->net_node;
> +     if (!nd)
> +             return;
> +
> +     npreal_start_break(nd);
> +     current->state = TASK_INTERRUPTIBLE;
> +     schedule_timeout(duration);
> +     npreal_stop_break(nd);
> +}
> +
> +static void npreal_remove_proc_entry(struct proc_dir_entry *pde, int idx)
> +{
> +     char tmp[10];
> +
> +     if (!pde)
> +             return;
> +
> +     sprintf(tmp, "%d", idx);
> +
> +     if (idx == 404)
> +             remove_proc_entry("npreal2", NULL);
> +     else
> +             remove_proc_entry(tmp, npvar_proc_root);
> +}
> +
> +static void npreal_process_notify(struct nd_struct *nd, char *rsp_buffer, 
> int rsp_length)
> +{
> +     struct npreal_struct *info = nd->tty_node;
> +     int state;
> +
> +     if (!info || rsp_length != 5)
> +             return;
> +
> +     if (rsp_buffer[2] & ASPP_NOTIFY_MSR_CHG) {
> +             state = 0;
> +
> +             if (rsp_buffer[3] & 0x10)
> +                     state |= UART_MSR_CTS;
> +             if (rsp_buffer[3] & 0x20)
> +                     state |= UART_MSR_DSR;
> +             if (rsp_buffer[3] & 0x80)
> +                     state |= UART_MSR_DCD;
> +             npreal_check_modem_status(info, state);
> +     }
> +
> +     if (rsp_buffer[2] & ASPP_NOTIFY_BREAK) {
> +             struct tty_struct *tty;
> +
> +             down(&info->rx_semaphore);
> +             tty = info->tty;
> +             if (!tty || !info->ttyPort.low_latency) {
> +                     up(&info->rx_semaphore);
> +                     return;
> +             }
> +
> +             tty_insert_flip_char(&info->ttyPort, 0, TTY_BREAK);
> +             up(&info->rx_semaphore);
> +             info->icount.rx++;
> +             info->icount.brk++;
> +             schedule_work(&info->process_flip_tqueue);
> +
> +             if (info->flags & ASYNC_SAK)
> +                     do_SAK(info->tty);
> +     }
> +
> +     if (rsp_buffer[2] & ASPP_NOTIFY_PARITY)
> +             info->icount.parity++;
> +     if (rsp_buffer[2] & ASPP_NOTIFY_FRAMING)
> +             info->icount.frame++;
> +     if ((rsp_buffer[2] & ASPP_NOTIFY_SW_OVERRUN) || (rsp_buffer[2] & 
> ASPP_NOTIFY_HW_OVERRUN))
> +             info->icount.overrun++;
> +}
> +
> +static int32_t npreal_do_session_mode(struct ktermios *termio)
> +{
> +     int32_t mode;
> +
> +     mode = termio->c_cflag & CSIZE;
> +     switch (mode) {
> +     case CS5:
> +             mode = ASPP_IOCTL_BITS5;
> +             break;
> +
> +     case CS6:
> +             mode = ASPP_IOCTL_BITS6;
> +             break;
> +
> +     case CS7:
> +             mode = ASPP_IOCTL_BITS7;
> +             break;
> +
> +     case CS8:
> +             mode = ASPP_IOCTL_BITS8;
> +             break;
> +
> +     }
> +
> +     if (termio->c_cflag & CSTOPB)
> +             mode |= ASPP_IOCTL_STOP2;
> +     else
> +             mode |= ASPP_IOCTL_STOP1;
> +
> +     if (termio->c_cflag & PARENB) {
> +             if (termio->c_cflag & PARODD)
> +                     mode |= ASPP_IOCTL_ODD;
> +             else
> +                     mode |= ASPP_IOCTL_EVEN;
> +     } else {
> +             mode |= ASPP_IOCTL_NONE;
> +     }
> +
> +     return mode;
> +}
> +
> +static void npreal_do_session_baud(struct npreal_struct *info, struct 
> ktermios *termio,
> +                                                     int32_t *baud_ret, int 
> *baud_ind_ret)
> +{
> +     int32_t baud;
> +     int baudIndex;
> +
> +     switch (termio->c_cflag & (CBAUD | CBAUDEX)) {
> +     case B921600:
> +             baud = 921600L;
> +             baudIndex = ASPP_IOCTL_B921600;
> +             break;

Do I have deja-vu? Why all this duplicated code?

> +     case B460800:
> +             baud = 460800;
> +             baudIndex = ASPP_IOCTL_B460800;
> +             break;
...
> +static void npreal_do_session_buffer(struct nd_struct *nd, struct 
> npreal_struct *info,
> +                             struct ktermios *termio, int baud, int mode, 
> int baudIndex)
> +{
> +     nd->cmd_buffer[0] = NPREAL_ASPP_COMMAND_SET;
> +     nd->cmd_buffer[1] = ASPP_CMD_PORT_INIT;
> +     nd->cmd_buffer[2] = 8;
> +     nd->cmd_buffer[3] = baudIndex;   /* baud rate */
> +     nd->cmd_buffer[4] = mode;       /* mode */
> +
> +     /* line control */
> +     if (info->modem_control & UART_MCR_DTR)
> +             nd->cmd_buffer[5] = 1;
> +     else
> +             nd->cmd_buffer[5] = 0;

And this duplicated code?

> +     if (info->modem_control & UART_MCR_RTS)
> +             nd->cmd_buffer[6] = 1;
> +     else
> +             nd->cmd_buffer[6] = 0;
> +
> +     /* flow control */
> +     if ((info->flags & ASYNC_INITIALIZED) && (termio->c_cflag & CRTSCTS)) {
> +             nd->cmd_buffer[7] = 1;
> +             nd->cmd_buffer[8] = 1;
> +     } else {
> +             nd->cmd_buffer[7] = 0;
> +             nd->cmd_buffer[8] = 0;
> +     }
> +
> +     if (termio->c_iflag & IXON)
> +             nd->cmd_buffer[9] = 1;
> +     else
> +             nd->cmd_buffer[9] = 0;
> +     if (termio->c_iflag & IXOFF)
> +             nd->cmd_buffer[10] = 1;
> +     else
> +             nd->cmd_buffer[10] = 0;
> +
> +     if ((baudIndex == 0xff) && (baud != 0)) {
> +             nd->cmd_buffer[11] = ASPP_CMD_SETBAUD;
> +             nd->cmd_buffer[12] = 4;
> +
> +             if (((info->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST) && 
> info->custom_divisor)
> +                     baud = info->baud_base/info->custom_divisor;
> +
> +             memcpy(&nd->cmd_buffer[13], &baud, 4);
> +     }
> +
> +     if (termio->c_iflag & (IXON | IXOFF)) {
> +             nd->cmd_buffer[17] = ASPP_CMD_XONXOFF;
> +             nd->cmd_buffer[18] = 2;
> +             nd->cmd_buffer[19] = termio->c_cc[VSTART];
> +             nd->cmd_buffer[20] = termio->c_cc[VSTOP];
> +     }
> +
> +     nd->cmd_buffer[21] = ASPP_CMD_TX_FIFO;
> +     nd->cmd_buffer[22] = 1;
> +     nd->cmd_buffer[23] = info->xmit_fifo_size;
> +     nd->cmd_buffer[24] = ASPP_CMD_LINECTRL;
> +     nd->cmd_buffer[25] = 2;
> +
> +     if (info->modem_control & UART_MCR_DTR)
> +             nd->cmd_buffer[26] = 1;
> +     else
> +             nd->cmd_buffer[26] = 0;
> +     if (info->modem_control & UART_MCR_RTS)
> +             nd->cmd_buffer[27] = 1;
> +     else
> +             nd->cmd_buffer[27] = 0;
> +
> +     nd->do_session_recovery_len = 27 + 1;
> +}
> +
> +static void npreal_do_session_recovery(struct npreal_struct *info)
> +{
> +     struct tty_struct *tty;
> +     struct nd_struct *nd;
> +     struct ktermios *termio;
> +     int32_t baud, mode;
> +     int baudIndex;
> +
> +     tty = info->tty;
> +     nd = info->net_node;
> +
> +     if (!tty || !nd)
> +             return;
> +
> +     if (!(nd->flag & NPREAL_NET_NODE_OPENED) || !(nd->flag & 
> NPREAL_NET_NODE_CONNECTED))
> +             return;
> +
> +     if (info->flags & ASYNC_INITIALIZED) {
> +             termio = &(info->tty->termios);
> +     } else {
> +             termio = &info->normal_termios;
> +
> +             if (!termio)
> +                     return;
> +     }
> +
> +     down(&nd->cmd_semaphore);
> +     mode = npreal_do_session_mode(termio);
> +     npreal_do_session_baud(info, termio, &baud, &baudIndex);
> +     npreal_do_session_buffer(nd, info, termio, baud, mode, baudIndex);
> +     nd->flag |= NPREAL_NET_DO_SESSION_RECOVERY;
> +     nd->cmd_ready = 1;
> +     up(&nd->cmd_semaphore);
> +     /* waitqueue_active() is safe because nd->cmd_ready is in semaphore.*/
> +     if (waitqueue_active(&nd->select_ex_wait))
> +             wake_up_interruptible(&nd->select_ex_wait);
> +}
> +
> +static int npreal_startup_serial_port(struct npreal_struct *info, struct 
> nd_struct *nd)
> +{
> +     int ret;
> +
> +     nd->flag &= ~NPREAL_NET_DO_SESSION_RECOVERY;
> +     info->modem_status = 0;
> +     info->modem_control = 0;
> +
> +     if (info->tty->termios.c_cflag & CBAUD)
> +             info->modem_control = UART_MCR_DTR | UART_MCR_RTS;
> +
> +     ret = npreal_port_init(info, 0);
> +     if (ret != 0)
> +             return ret;
> +
> +     set_common_xmit_fifo_size(info, nd);
> +     return 0;
> +}
> +
> +static int npreal_startup_init(struct npreal_struct *info, struct nd_struct 
> *nd,
> +                                     struct tty_struct *tty, unsigned long 
> *page)
> +{
> +     DECLARE_WAITQUEUE(wait, current);
> +
> +     if (!nd)
> +             return -EIO;
> +
> +     add_wait_queue(&nd->initialize_wait, &wait);
> +     while (test_and_set_bit(NPREAL_NET_DO_INITIALIZE, &nd->flag)) {
> +             if (signal_pending(current))
> +                     break;
> +
> +             schedule();

Nah. wait_event and friends.

> +     }
> +
> +     remove_wait_queue(&nd->initialize_wait, &wait);
> +
> +     info->tty = tty;
> +
> +     if (info->flags & ASYNC_INITIALIZED) {
> +             clear_bit(NPREAL_NET_DO_INITIALIZE, &nd->flag);
> +             smp_mb(); /* use smp_mb() with waitqueue_active() */
> +             /* used waitqueue_active() is safe because smp_mb() is used */
> +             if (waitqueue_active(&nd->initialize_wait))
> +                     wake_up_interruptible(&nd->initialize_wait);
> +
> +             return 0;
> +     }
> +
> +     *page = __get_free_page(GFP_KERNEL);
> +     if (!(*page)) {
> +             clear_bit(NPREAL_NET_DO_INITIALIZE, &nd->flag);
> +             smp_mb(); /* use smp_mb() with waitqueue_active() */
> +             /* used waitqueue_active() is safe because smp_mb() is used */
> +             if (waitqueue_active(&nd->initialize_wait))
> +                     wake_up_interruptible(&nd->initialize_wait);
> +
> +             return -ENOMEM;
> +     }
> +
> +     return 1;
> +}
> +
> +static int npreal_startup_tty_usage(struct npreal_struct *info, struct 
> nd_struct *nd,
> +                                                     char *rsp_buf, int 
> *rsp_len)
> +{
> +     int ret;
> +
> +     ret = npreal_set_used_command_done(nd, rsp_buf, rsp_len);
> +     if (ret != 0) {
> +             npreal_set_unused_command_done(nd, rsp_buf, rsp_len);
> +             return ret;
> +     } else if (OFFLINE_POLLING) {
> +             if (!rsp_buf[2]) {
> +                     npreal_set_unused_command_done(nd, rsp_buf, rsp_len);
> +                     return ret;
> +             }
> +     }
> +
> +     nd->flag |= NPREAL_NET_TTY_INUSED;
> +     return 0;
> +}
> +
> +static int npreal_startup_disconnect(struct npreal_struct *info, struct 
> nd_struct *nd,
> +                                                             char *rsp_buf, 
> int *rsp_len)
> +{
> +     int ret;
> +
> +     nd->flag &= ~NPREAL_NET_NODE_DISCONNECTED;
> +
> +     ret = npreal_set_unused_command_done(nd, rsp_buf, rsp_len);
> +     if (ret != 0)
> +             nd->flag |= NPREAL_NET_NODE_DISCONNECTED;
> +     else
> +             nd->flag &= ~NPREAL_NET_TTY_INUSED;
> +
> +     return ret;
> +}
> +
> +static int npreal_startup(struct npreal_struct *info, struct file *filp,
> +                     struct tty_struct *tty)
> +{
> +     struct nd_struct *nd = info->net_node;
> +     unsigned long page;
> +     int rsp_length = RSP_BUFFER_SIZE;
> +     int cnt = 0, ret;
> +     char rsp_buffer[RSP_BUFFER_SIZE];
> +
> +     ret = npreal_startup_init(info, nd, tty, &page);
> +     if (ret < 1)
> +             return ret;
> +
> +     if (!(nd->flag & NPREAL_NET_TTY_INUSED)) {
> +             ret = npreal_startup_tty_usage(info, nd, rsp_buffer, 
> &rsp_length);
> +             if (ret)
> +                     goto startup_err;
> +     } else {
> +             if (nd->flag & NPREAL_NET_NODE_DISCONNECTED) {
> +                     npreal_startup_disconnect(info, nd, rsp_buffer, 
> &rsp_length);
> +                     goto startup_err;
> +             }
> +
> +             while ((nd->cmd_ready == 1) && (cnt++ < 10)) {
> +                     current->state = TASK_INTERRUPTIBLE;
> +                     schedule_timeout(HZ / 100);

All these random schedules needs to go.

> +             }
> +     }
> +
> +     ret = npreal_startup_serial_port(info, nd);
> +     if (ret)
> +             goto startup_err;
> +
> +     ret = npreal_set_tx_fifo_command_done(info, nd, rsp_buffer, 
> &rsp_length);
> +     if (ret)
> +             goto startup_err;
> +
> +     if (info->xmit_buf)
> +             free_page(page);
> +     else
> +             info->xmit_buf = (unsigned char *)page;
> +
> +     if (info->tty)
> +             test_and_clear_bit(TTY_IO_ERROR, &info->tty->flags);
> +
> +     info->xmit_tail = 0;
> +     info->xmit_head = 0;
> +     info->xmit_cnt = 0;
> +     info->flags |= ASYNC_INITIALIZED;
> +     clear_bit(NPREAL_NET_DO_INITIALIZE, &nd->flag);
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->initialize_wait))
> +             wake_up_interruptible(&nd->initialize_wait);
> +
> +     return 0;
> +
> +startup_err:
> +     npreal_disconnect(nd, rsp_buffer, &rsp_length);
> +     free_page(page);
> +     clear_bit(NPREAL_NET_DO_INITIALIZE, &nd->flag);
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->initialize_wait))
> +             wake_up_interruptible(&nd->initialize_wait);
> +
> +     return ret;
> +}
> +
> +static int npreal_open_startup(struct tty_struct *tty, struct npreal_struct 
> *info,
> +                                                             struct file 
> *filp)
> +{
> +     long jiff_th;
> +     int ret, retry;
> +
> +     retry = NPREAL_CMD_TRY - 1;
> +     while (retry) {
> +             /* For some circumstance, device may reset the connection
> +              * during the port opening. These code is to reopen the port
> +              * without telling application. Considering a real situation
> +              * of connection lost, we use -ETIME to exit the retry loop.
> +              */
> +             ret = npreal_startup(info, filp, tty);
> +             if (ret == 0)
> +                     break;
> +             else if (ret == -ETIME) {
> +                     pr_err("npreal_startup failed(%d)\n", ret);
> +                     return -EIO;
> +             }
> +
> +             jiff_th = (NPREAL_CMD_TRY-retry)*HZ/2;
> +             schedule_timeout_uninterruptible(jiff_th);

So msleep((NPREAL_CMD_TRY-retry)*500)?

> +             retry--;
> +             if (retry < 0) {
> +                     pr_err("npreal_startup failed\n");
> +                     return -EIO;
> +             }
> +     }
> +     return 0;
> +}
> +
> +/**
> + * npreal_throttle() - Notify the tty driver input buffer is full
> + * @tty: pointer for struct tty_struct
> + *
> + * This routine is called by the upper-layer tty layer to signal that
> + * incoming characters should be throttled. (tty driver buffer is full)
> + *
> + */
> +static void npreal_throttle(struct tty_struct *tty)
> +{
> +     struct npreal_struct *info = (struct npreal_struct *)tty->driver_data;
> +     struct nd_struct *nd;
> +
> +     if (!info)
> +             return;
> +
> +     nd = info->net_node;
> +     if (!nd)
> +             return;
> +
> +     nd->rx_ready = 0;
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->select_out_wait))

Why do you actully do these racy waitqueue_active checks all over the code?

> +             wake_up_interruptible(&nd->select_out_wait);
> +}
...
> +static void npreal_shutdown(struct npreal_struct *info)
> +{
> +     struct nd_struct *nd = info->net_node;
> +     unsigned long flags;
> +
> +     while (test_and_set_bit(NPREAL_NET_DO_INITIALIZE, &nd->flag)) {
> +             if (signal_pending(current))
> +                     break;
> +             current->state = TASK_INTERRUPTIBLE;
> +             schedule_timeout(HZ / 100);

You already know what, right?

> +     }
> +
> +     if (!(info->flags & ASYNC_INITIALIZED))
> +             goto done;
> +
> +     spin_lock_irqsave(&info->tx_lock, flags);
> +     if (info->xmit_buf) {
> +             free_page((unsigned long)info->xmit_buf);
> +             info->xmit_buf = 0;
> +     }
> +     spin_unlock_irqrestore(&info->tx_lock, flags);
> +
> +     if (info->tty) {
> +             set_bit(TTY_IO_ERROR, &info->tty->flags);
> +             npreal_unthrottle(info->tty);
> +     }
> +
> +     if (!info->tty || (info->tty->termios.c_cflag & HUPCL))
> +             info->modem_control &= ~(UART_MCR_DTR | UART_MCR_RTS);
> +
> +done:
> +     npreal_port_shutdown(info);
> +     info->flags &= ~(ASYNC_NORMAL_ACTIVE |
> +     ASYNC_INITIALIZED | ASYNC_CLOSING);
> +     down(&info->rx_semaphore);
> +     info->tty = 0;

0 is not a pointer.

> +     up(&info->rx_semaphore);
> +     clear_bit(NPREAL_NET_DO_INITIALIZE, &nd->flag);
> +     wake_up_interruptible(&info->open_wait);
> +     wake_up_interruptible(&info->close_wait);
> +     smp_mb(); /* use smp_mb() with waitqueue_active() */
> +     /* used waitqueue_active() is safe because smp_mb() is used */
> +     if (waitqueue_active(&nd->initialize_wait))
> +             wake_up_interruptible(&nd->initialize_wait);
> +}
...
> +static int npreal_open(struct tty_struct *tty, struct file *filp)
> +{

Why not to use tty_port_open?

> +}
...
> +static void npreal_close(struct tty_struct *tty, struct file *filp)
> +{

And tty_port_close?

> +}
...
> +static int npreal_write(struct tty_struct *tty, const unsigned char *buf, 
> int count)
> +{
> +     struct npreal_struct *info = (struct npreal_struct *)tty->driver_data;
> +     struct nd_struct *nd;
> +     unsigned long flags;
> +     int c, total = 0;
> +
> +     if ((!info) || !tty || !info->xmit_buf)
> +             return 0;
> +
> +     nd = info->net_node;
> +
> +     if (!nd)
> +             return 0;
> +     while (1) {
> +             c = min(count, min((int)(SERIAL_XMIT_SIZE - info->xmit_cnt - 1),
> +                                     (int)(SERIAL_XMIT_SIZE - 
> info->xmit_head)));

Casts here only mean you have wrong types somewhere. If it must be (not
in this case), use min_t.

> +             if (c <= 0)
> +                     break;
> +
> +             spin_lock_irqsave(&info->tx_lock, flags);
> +             memcpy(info->xmit_buf + info->xmit_head, buf, c);
> +             info->xmit_head = (info->xmit_head + c) & (SERIAL_XMIT_SIZE - 
> 1);

If you used modulo (which is what you want here), you need not decrement
by one, right?

> +             info->xmit_cnt += c;
> +             spin_unlock_irqrestore(&info->tx_lock, flags);
> +
> +             buf += c;
> +             count -= c;
> +             total += c;
> +     }
> +
> +     if (info->xmit_cnt) {
> +             nd->tx_ready = 1;
> +             smp_mb(); /* use smp_mb() with waitqueue_active() */
> +             /* used waitqueue_active() is safe because smp_mb() is used */
> +             if (waitqueue_active(&nd->select_in_wait))
> +                     wake_up_interruptible(&nd->select_in_wait);
> +     }
> +
> +     return total;
> +}
...
> +static int npreal_ioctl(struct tty_struct *tty, unsigned int cmd,
> +                     unsigned long arg)
> +{
> +     struct npreal_struct *info = (struct npreal_struct *)tty->driver_data;
> +     struct serial_icounter_struct *p_cuser; /* user space */
> +     unsigned long templ;
> +     int ret = 0;
> +
> +     if (!info)
> +             return -ENODEV;
> +
> +     if ((cmd != TIOCGSERIAL) && (cmd != TIOCMIWAIT) && (cmd != TIOCGICOUNT) 
> &&
> +             test_bit(TTY_IO_ERROR, &tty->flags))
> +             return -EIO;
> +
> +     switch (cmd) {
> +     case TCFLSH:

You should not handle ioctl like these in the driver, should you?

> +             ret = tty_check_change(tty);
> +             if (!ret) {
> +                     switch (arg) {
> +                     case TCIFLUSH:
> +                             if (tty->ldisc->ops->flush_buffer)
> +                                     tty->ldisc->ops->flush_buffer(tty);
> +                             break;
> +
> +                     case TCIOFLUSH:
> +                             if (tty->ldisc->ops->flush_buffer)
> +                                     tty->ldisc->ops->flush_buffer(tty);
> +                             npreal_flush_buffer(tty);
> +                             break;
> +
> +                     case TCOFLUSH:
> +                             npreal_flush_buffer(tty);
> +                             break;
> +
> +                     default:
> +                             ret = -EINVAL;
> +                     }
> +             }
> +             break;
...
> +static const struct tty_operations mpvar_ops = {
> +     .open               = npreal_open,
> +     .close              = npreal_close,
> +     .write              = npreal_write,
> +     .put_char           = npreal_put_char,
> +     .write_room         = npreal_write_room,
> +     .chars_in_buffer    = npreal_chars_in_buffer,
> +     .flush_buffer       = npreal_flush_buffer,
> +     .wait_until_sent    = npreal_wait_until_sent,
> +     .break_ctl          = npreal_break,
> +     .ioctl              = npreal_ioctl,
> +     .throttle           = npreal_throttle,
> +     .unthrottle         = npreal_unthrottle,
> +     .set_termios        = npreal_set_termios,
> +     .hangup             = npreal_hangup,
> +     .tiocmget           = npreal_tiocmget,
> +     .tiocmset           = npreal_tiocmset,
> +};
...
> +static ssize_t npreal_net_read(struct file *file, char *buf, size_t count, 
> loff_t *ppos)
> +{
> +     struct nd_struct *nd = file->private_data;
> +     struct npreal_struct *info;
> +     struct tty_struct *tty;
> +     ssize_t rtn = 0;
> +     int min_val;
> +     unsigned long flags;
> +
> +
> +     info = (struct npreal_struct *)nd->tty_node;
> +     tty = info->tty;
> +
> +     if (!nd || !info || !tty) {
> +             rtn = -ENXIO;
> +             return rtn;
> +     }
> +
> +     if (info->x_char) {
> +             rtn = 1;
> +             if (copy_to_user(buf, &info->x_char, rtn)) {

I.e. put_user.

...
> +static ssize_t npreal_net_write(struct file *file, const char *buf, size_t 
> count, loff_t *ppos)
> +{
> +     struct nd_struct *nd = file->private_data;
> +     struct npreal_struct *info;
> +     struct tty_struct *tty;
> +     unsigned char *k_buf = NULL;
> +     ssize_t rtn = 0;
> +     int cnt;
> +     unsigned long tmp;
> +
> +     if (!buf) {
> +             rtn = count;
> +             goto done;
> +     }
> +
> +     k_buf = kmalloc_array(count, sizeof(unsigned char), GFP_KERNEL);
> +     info = (struct npreal_struct *)nd->tty_node;
> +     tmp =  copy_from_user(k_buf, buf, count);

Is buf a user buffer? It is not marked as such. Ah, this is the proc
interface you should drop.

> +     if ((k_buf == NULL || tmp) || (!nd || !info) || (info->flags & 
> ASYNC_CLOSING)) {
> +             rtn = count;
> +             goto done;
> +     }



Overall, you shall deduplicate the code and use tty_port helpers
wherever possible. This will simplify the code a lot. I wonder where you
get from this submission with "3194 insertions".

thanks,
-- 
js

Reply via email to