On 11/02/2007 10:34 AM, Jesper Nilsson wrote:
> Resubmission of the new and improved serial driver for CRISv10,
> this time with both patches in the same mail and with hopefully
> useful subject.
> 
> - Removed CVS tags.
> - Removed defines and comments for CRIS_BUF_SIZE and TTY_THRESHOLD_THROTTLE
>   (no longer used).
> - Merge of CRISv10 from Axis internal CVS.
[...]
>  #ifdef SERIAL_DEBUG_IO
> @@ -1440,12 +1094,11 @@ e100_rts(struct e100_serial *info, int set)
>  {
>  #ifndef CONFIG_SVINTO_SIM
>       unsigned long flags;
> -     save_flags(flags);
> -     cli();
> +     local_irq_save(flags);

Is this enough? Don't you need also spin lock (i.e. spin_lock_irqsave())?

>       info->rx_ctrl &= ~E100_RTS_MASK;
>       info->rx_ctrl |= (set ? 0 : E100_RTS_MASK);  /* RTS is active low */
>       info->port[REG_REC_CTRL] = info->rx_ctrl;
> -     restore_flags(flags);
> +     local_irq_restore(flags);
>  #ifdef SERIAL_DEBUG_IO
>       printk("ser%i rts %i\n", info->line, set);
>  #endif

[...]

> @@ -4434,7 +3941,7 @@ block_til_ready(struct tty_struct *tty, struct file * 
> filp,
>       if (tty_hung_up_p(filp) ||
>           (info->flags & ASYNC_CLOSING)) {
>               if (info->flags & ASYNC_CLOSING)
> -                     interruptible_sleep_on(&info->close_wait);
> +                     wait_event_interruptible(info->close_wait, 0);

Aiee, this is nonsense, 0 will never be 1, only signal will stop this, use
completion instead.

>  #ifdef SERIAL_DO_RESTART
>               if (info->flags & ASYNC_HUP_NOTIFY)
>                       return -EAGAIN;
> @@ -4472,21 +3979,19 @@ block_til_ready(struct tty_struct *tty, struct file * 
> filp,
>       printk("block_til_ready before block: ttyS%d, count = %d\n",
>              info->line, info->count);
>  #endif
> -     save_flags(flags);
> -     cli();
> +     local_irq_save(flags);
>       if (!tty_hung_up_p(filp)) {
>               extra_count++;
>               info->count--;
>       }
> -     restore_flags(flags);
> +     local_irq_restore(flags);
>       info->blocked_open++;
>       while (1) {
> -             save_flags(flags);
> -             cli();
> +             local_irq_save(flags);
>               /* assert RTS and DTR */
>               e100_rts(info, 1);
>               e100_dtr(info, 1);
> -             restore_flags(flags);
> +             local_irq_restore(flags);
>               set_current_state(TASK_INTERRUPTIBLE);
>               if (tty_hung_up_p(filp) ||
>                   !(info->flags & ASYNC_INITIALIZED)) {
> @@ -4538,9 +4043,9 @@ rs_open(struct tty_struct *tty, struct file * filp)
>       struct e100_serial      *info;
>       int                     retval, line;
>       unsigned long           page;
> +     int                     allocated_resources = 0;
>  
>       /* find which port we want to open */
> -
>       line = tty->index;
>  
>       if (line < 0 || line >= NR_PORTS)
> @@ -4581,7 +4086,7 @@ rs_open(struct tty_struct *tty, struct file * filp)
>       if (tty_hung_up_p(filp) ||
>           (info->flags & ASYNC_CLOSING)) {
>               if (info->flags & ASYNC_CLOSING)
> -                     interruptible_sleep_on(&info->close_wait);
> +                     wait_event_interruptible(info->close_wait, 0);

also here.

>  #ifdef SERIAL_DO_RESTART
>               return ((info->flags & ASYNC_HUP_NOTIFY) ?
>                       -EAGAIN : -ERESTARTSYS);
> @@ -4591,19 +4096,118 @@ rs_open(struct tty_struct *tty, struct file * filp)
>       }
>  
>       /*
> +      * If DMA is enabled try to allocate the irq's.
> +      */
> +     if (info->count == 1) {
> +             allocated_resources = 1;
> +             if (info->dma_in_enabled) {
> +                     if (request_irq(info->dma_in_irq_nbr,
> +                                     rec_interrupt,
> +                                     info->dma_in_irq_flags,
> +                                     info->dma_in_irq_description,
> +                                     info)) {
> +                             printk(KERN_WARNING "DMA irq '%s' busy; "
> +                                     "falling back to non-DMA mode\n",
> +                                     info->dma_in_irq_description);
> +                             /* Make sure we never try to use DMA in */
> +                             /* for the port again. */
> +                             info->dma_in_enabled = 0;
> +                     } else if (cris_request_dma(info->dma_in_nbr,
> +                                     info->dma_in_irq_description,
> +                                     DMA_VERBOSE_ON_ERROR,
> +                                     info->dma_owner)) {
> +                             free_irq(info->dma_in_irq_nbr, info);
> +                             printk(KERN_WARNING "DMA '%s' busy; "
> +                                     "falling back to non-DMA mode\n",
> +                                     info->dma_in_irq_description);
> +                             /* Make sure we never try to use DMA in */
> +                             /* for the port again. */
> +                             info->dma_in_enabled = 0;
> +                     }
> +#ifdef SERIAL_DEBUG_OPEN
> +                     else
> +                             printk(KERN_DEBUG "DMA irq '%s' allocated\n",
> +                                     info->dma_in_irq_description);
> +#endif
> +             }
> +             if (info->dma_out_enabled) {
> +                     if (request_irq(info->dma_out_irq_nbr,
> +                                            tr_interrupt,
> +                                            info->dma_out_irq_flags,
> +                                            info->dma_out_irq_description,
> +                                            info)) {
> +                             printk(KERN_WARNING "DMA irq '%s' busy; "
> +                                     "falling back to non-DMA mode\n",
> +                                     info->dma_out_irq_description);
> +                             /* Make sure we never try to use DMA out */
> +                             /* for the port again. */
> +                             info->dma_out_enabled = 0;
> +                     } else if (cris_request_dma(info->dma_out_nbr,
> +                                          info->dma_out_irq_description,
> +                                          DMA_VERBOSE_ON_ERROR,
> +                                          info->dma_owner)) {
> +                             free_irq(info->dma_out_irq_nbr, info);
> +                             printk(KERN_WARNING "DMA '%s' busy; "
> +                                     "falling back to non-DMA mode\n",
> +                                     info->dma_out_irq_description);
> +                             /* Make sure we never try to use DMA out */
> +                             /* for the port again. */
> +                             info->dma_out_enabled = 0;
> +                     }
> +#ifdef SERIAL_DEBUG_OPEN
> +                     else
> +                             printk(KERN_DEBUG "DMA irq '%s' allocated\n",
> +                                     info->dma_out_irq_description);
> +#endif
> +             }
> +     }
> +
> +     /*
>        * Start up the serial port
>        */
>  
>       retval = startup(info);
> -     if (retval)
> +     if (retval) {
> +             if (allocated_resources) {
> +                     if (info->dma_out_enabled) {
> +                             cris_free_dma(info->dma_out_nbr,
> +                                     info->dma_out_irq_description);
> +                             free_irq(info->dma_out_irq_nbr,
> +                                      info);
> +                     }
> +                     if (info->dma_in_enabled) {
> +                             cris_free_dma(info->dma_in_nbr,
> +                                     info->dma_in_irq_description);
> +                             free_irq(info->dma_in_irq_nbr,
> +                                      info);
> +                     }
> +             }

You maybe want to create a function for this deinit invoked from more places in
the new code.

> +             /* FIXME Decrease count info->count here too? */
>               return retval;
>  
> +     }
> +
> +
>       retval = block_til_ready(tty, filp, info);
>       if (retval) {
>  #ifdef SERIAL_DEBUG_OPEN
>               printk("rs_open returning after block_til_ready with %d\n",
>                      retval);
>  #endif
> +             if (allocated_resources) {
> +                     if (info->dma_out_enabled) {
> +                             cris_free_dma(info->dma_out_nbr,
> +                                     info->dma_out_irq_description);
> +                             free_irq(info->dma_out_irq_nbr,
> +                                     info);
> +                     }
> +                     if (info->dma_in_enabled) {
> +                             cris_free_dma(info->dma_in_nbr,
> +                                     info->dma_in_irq_description);
> +                             free_irq(info->dma_in_irq_nbr,
> +                                     info);
> +                     }
> +             }

...

>               return retval;
>       }
>  
> @@ -4793,6 +4397,8 @@ static const struct tty_operations rs_ops = {
>       .send_xchar = rs_send_xchar,
>       .wait_until_sent = rs_wait_until_sent,
>       .read_proc = rs_read_proc,
> +     .tiocmget = rs_tiocmget,
> +     .tiocmset = rs_tiocmset
>  };
>  
>  static int __init
> @@ -4812,7 +4418,26 @@ rs_init(void)
>  #if !defined(CONFIG_ETRAX_SERIAL_FAST_TIMER)
>       init_timer(&flush_timer);
>       flush_timer.function = timed_flush_handler;

Side note, this should be setup_timer without accessing .function.

> -     mod_timer(&flush_timer, jiffies + CONFIG_ETRAX_SERIAL_RX_TIMEOUT_TICKS);
> +     mod_timer(&flush_timer, jiffies + 5);
> +#endif
> +
> +#if defined(CONFIG_ETRAX_RS485)
> +#if defined(CONFIG_ETRAX_RS485_ON_PA)
> +     if (cris_io_interface_allocate_pins(if_ser0, 'a', rs485_pa_bit,
> +                     rs485_pa_bit)) {
> +             printk(KERN_CRIT "ETRAX100LX serial: Could not allocate "
> +                     "RS485 pin\n");
> +             return -EBUSY;
> +     }
> +#endif
> +#if defined(CONFIG_ETRAX_RS485_ON_PORT_G)
> +     if (cris_io_interface_allocate_pins(if_ser0, 'g', rs485_pa_bit,
> +                     rs485_port_g_bit)) {
> +             printk(KERN_CRIT "ETRAX100LX serial: Could not allocate "
> +                     "RS485 pin\n");
> +             return -EBUSY;
> +     }
> +#endif
>  #endif
>  
>       /* Initialize the tty_driver structure */
> @@ -4839,6 +4464,16 @@ rs_init(void)
>       /* do some initializing for the separate ports */
>  
>       for (i = 0, info = rs_table; i < NR_PORTS; i++,info++) {
> +             if (info->enabled) {
> +                     if (cris_request_io_interface(info->io_if,
> +                                     info->io_if_description)) {
> +                             printk(KERN_CRIT "ETRAX100LX async serial: "
> +                                     "Could not allocate IO pins for "
> +                                     "%s, port %d\n",
> +                                     info->io_if_description, i);
> +                             info->enabled = 0;
> +                     }
> +             }
>               info->uses_dma_in = 0;
>               info->uses_dma_out = 0;
>               info->line = i;
> @@ -4872,7 +4507,7 @@ rs_init(void)
>               info->rs485.delay_rts_before_send = 0;
>               info->rs485.enabled = 0;
>  #endif
> -             INIT_WORK(&info->work, do_softint, info);
> +             INIT_WORK(&info->work, do_softint);
>  
>               if (info->enabled) {
>                       printk(KERN_INFO "%s%d at 0x%x is a builtin UART with 
> DMA\n",
> @@ -4890,64 +4525,17 @@ rs_init(void)
>  #endif
>  
>  #ifndef CONFIG_SVINTO_SIM
> +#ifndef CONFIG_ETRAX_KGDB
>       /* Not needed in simulator.  May only complicate stuff. */
>       /* hook the irq's for DMA channel 6 and 7, serial output and input, and 
> some more... */
>  
> -     if (request_irq(SERIAL_IRQ_NBR, ser_interrupt, IRQF_SHARED | 
> IRQF_DISABLED, "serial ", NULL))
> -             panic("irq8");
> +     if (request_irq(SERIAL_IRQ_NBR, ser_interrupt,
> +                     IRQF_SHARED | IRQF_DISABLED, "serial ", driver))
> +             panic("%s: Failed to request irq8", __FUNCTION__);

Is the panic needed here? Can't the cris architecture live without the driver?

>  
> -#ifdef CONFIG_ETRAX_SERIAL_PORT0
> -#ifdef CONFIG_ETRAX_SERIAL_PORT0_DMA6_OUT
> -     if (request_irq(SER0_DMA_TX_IRQ_NBR, tr_interrupt, IRQF_DISABLED, 
> "serial 0 dma tr", NULL))
> -             panic("irq22");
> -#endif
> -#ifdef CONFIG_ETRAX_SERIAL_PORT0_DMA7_IN
> -     if (request_irq(SER0_DMA_RX_IRQ_NBR, rec_interrupt, IRQF_DISABLED, 
> "serial 0 dma rec", NULL))
> -             panic("irq23");
> -#endif
> -#endif
> -
> -#ifdef CONFIG_ETRAX_SERIAL_PORT1
> -#ifdef CONFIG_ETRAX_SERIAL_PORT1_DMA8_OUT
> -     if (request_irq(SER1_DMA_TX_IRQ_NBR, tr_interrupt, IRQF_DISABLED, 
> "serial 1 dma tr", NULL))
> -             panic("irq24");
> -#endif
> -#ifdef CONFIG_ETRAX_SERIAL_PORT1_DMA9_IN
> -     if (request_irq(SER1_DMA_RX_IRQ_NBR, rec_interrupt, IRQF_DISABLED, 
> "serial 1 dma rec", NULL))
> -             panic("irq25");
> -#endif
> -#endif
> -#ifdef CONFIG_ETRAX_SERIAL_PORT2
> -     /* DMA Shared with par0 (and SCSI0 and ATA) */
> -#ifdef CONFIG_ETRAX_SERIAL_PORT2_DMA2_OUT
> -     if (request_irq(SER2_DMA_TX_IRQ_NBR, tr_interrupt, IRQF_SHARED | 
> IRQF_DISABLED, "serial 2 dma tr", NULL))
> -             panic("irq18");
> -#endif
> -#ifdef CONFIG_ETRAX_SERIAL_PORT2_DMA3_IN
> -     if (request_irq(SER2_DMA_RX_IRQ_NBR, rec_interrupt, IRQF_SHARED | 
> IRQF_DISABLED, "serial 2 dma rec", NULL))
> -             panic("irq19");
> -#endif
> -#endif
> -#ifdef CONFIG_ETRAX_SERIAL_PORT3
> -     /* DMA Shared with par1 (and SCSI1 and Extern DMA 0) */
> -#ifdef CONFIG_ETRAX_SERIAL_PORT3_DMA4_OUT
> -     if (request_irq(SER3_DMA_TX_IRQ_NBR, tr_interrupt, IRQF_SHARED | 
> IRQF_DISABLED, "serial 3 dma tr", NULL))
> -             panic("irq20");
> -#endif
> -#ifdef CONFIG_ETRAX_SERIAL_PORT3_DMA5_IN
> -     if (request_irq(SER3_DMA_RX_IRQ_NBR, rec_interrupt, IRQF_SHARED | 
> IRQF_DISABLED, "serial 3 dma rec", NULL))
> -             panic("irq21");
> -#endif
> -#endif
> -
> -#ifdef CONFIG_ETRAX_SERIAL_FLUSH_DMA_FAST
> -     if (request_irq(TIMER1_IRQ_NBR, timeout_interrupt, IRQF_SHARED | 
> IRQF_DISABLED,
> -                    "fast serial dma timeout", NULL)) {
> -             printk(KERN_CRIT "err: timer1 irq\n");
> -     }
>  #endif
>  #endif /* CONFIG_SVINTO_SIM */
> -     debug_write_function = rs_debug_write_function;
> +
>       return 0;
>  }
>  
> --- /dev/null 2007-10-06 06:38:38.348072750 +0200
> +++ linux-2.6.23/drivers/serial/crisv10.h     2007-11-01 16:39:35.000000000 
> +0100
> @@ -0,0 +1,151 @@
> +/*
> + * serial.h: Arch-dep definitions for the Etrax100 serial driver.
> + *
> + * Copyright (C) 1998-2007 Axis Communications AB
> + */
> +
> +#ifndef _ETRAX_SERIAL_H
> +#define _ETRAX_SERIAL_H
> +
> +#include <linux/circ_buf.h>
> +#include <asm/termios.h>
> +#include <asm/dma.h>
> +#include <asm/arch/io_interface_mux.h>
> +
> +/* Software state per channel */
> +
> +#ifdef __KERNEL__
> +/*
> + * This is our internal structure for each serial port's state.
> + *
> + * Many fields are paralleled by the structure used by the serial_struct
> + * structure.
> + *
> + * For definitions of the flags field, see tty.h
> + */
> +
> +#define SERIAL_RECV_DESCRIPTORS 8
> +
> +struct etrax_recv_buffer {
> +     struct etrax_recv_buffer *next;
> +     unsigned short length;
> +     unsigned char error;
> +     unsigned char pad;
> +
> +     unsigned char buffer[0];
> +};
> +
> +struct e100_serial {
> +     int baud;
> +     volatile u8     *port;  /* R_SERIALx_CTRL */
> +     u32             irq;    /* bitnr in R_IRQ_MASK2 for dmaX_descr */
> +
> +     /* Output registers */
> +     volatile u8 *oclrintradr;       /* adr to R_DMA_CHx_CLR_INTR */
> +     volatile u32 *ofirstadr;        /* adr to R_DMA_CHx_FIRST */
> +     volatile u8 *ocmdadr;           /* adr to R_DMA_CHx_CMD */
> +     const volatile u8 *ostatusadr;  /* adr to R_DMA_CHx_STATUS */
> +
> +     /* Input registers */
> +     volatile u8 *iclrintradr;       /* adr to R_DMA_CHx_CLR_INTR */
> +     volatile u32 *ifirstadr;        /* adr to R_DMA_CHx_FIRST */
> +     volatile u8 *icmdadr;           /* adr to R_DMA_CHx_CMD */
> +     volatile u32 *idescradr;        /* adr to R_DMA_CHx_DESCR */
> +
> +     int flags;      /* defined in tty.h */
> +
> +     u8 rx_ctrl;     /* shadow for R_SERIALx_REC_CTRL */
> +     u8 tx_ctrl;     /* shadow for R_SERIALx_TR_CTRL */
> +     u8 iseteop;     /* bit number for R_SET_EOP for the input dma */
> +     int enabled;    /* Set to 1 if the port is enabled in HW config */
> +
> +     u8 dma_out_enabled:1;   /* Set to 1 if DMA should be used */
> +     u8 dma_in_enabled:1;    /* Set to 1 if DMA should be used */

bitfileds generate ugly code.

> +
> +     /* end of fields defined in rs_table[] in .c-file */
> +     int             dma_owner;
> +     unsigned int    dma_in_nbr;
> +     unsigned int    dma_out_nbr;
> +     unsigned int    dma_in_irq_nbr;
> +     unsigned int    dma_out_irq_nbr;
> +     unsigned long   dma_in_irq_flags;
> +     unsigned long   dma_out_irq_flags;
> +     char            *dma_in_irq_description;
> +     char            *dma_out_irq_description;
> +
> +     enum cris_io_interface io_if;
> +     char            *io_if_description;
> +
> +     u8              uses_dma_in;  /* Set to 1 if DMA is used */
> +     u8              uses_dma_out; /* Set to 1 if DMA is used */
> +     u8              forced_eop;   /* a fifo eop has been forced */
> +     int                     baud_base;     /* For special baudrates */
> +     int                     custom_divisor; /* For special baudrates */
> +     struct etrax_dma_descr  tr_descr;
> +     struct etrax_dma_descr  rec_descr[SERIAL_RECV_DESCRIPTORS];
> +     int                     cur_rec_descr;
> +
> +     volatile int            tr_running; /* 1 if output is running */

What's the volatile for here? atomic_t?

> +
> +     struct tty_struct       *tty;
> +     int                     read_status_mask;
> +     int                     ignore_status_mask;
> +     int                     x_char; /* xon/xoff character */
> +     int                     close_delay;
> +     unsigned short          closing_wait;
> +     unsigned short          closing_wait2;
> +     unsigned long           event;
> +     unsigned long           last_active;
> +     int                     line;
> +     int                     type;  /* PORT_ETRAX */
> +     int                     count;      /* # of fd on device */
> +     int                     blocked_open; /* # of blocked opens */
> +     struct circ_buf         xmit;
> +     struct etrax_recv_buffer *first_recv_buffer;
> +     struct etrax_recv_buffer *last_recv_buffer;
> +     unsigned int            recv_cnt;
> +     unsigned int            max_recv_cnt;
> +
> +     struct work_struct      work;
> +     struct async_icount     icount;   /* error-statistics etc.*/
> +     struct ktermios         normal_termios;
> +     struct ktermios         callout_termios;
> +#ifdef DECLARE_WAITQUEUE
> +     wait_queue_head_t       open_wait;
> +     wait_queue_head_t       close_wait;
> +#else
> +     struct wait_queue       *open_wait;
> +     struct wait_queue       *close_wait;
> +#endif

We know, we have it, don't we?

> +
> +     unsigned long char_time_usec;       /* The time for 1 char, in usecs */
> +     unsigned long flush_time_usec;      /* How often we should flush */
> +     unsigned long last_tx_active_usec;  /* Last tx usec in the jiffies */
> +     unsigned long last_tx_active;       /* Last tx time in jiffies */
> +     unsigned long last_rx_active_usec;  /* Last rx usec in the jiffies */
> +     unsigned long last_rx_active;       /* Last rx time in jiffies */
> +
> +     int break_detected_cnt;
> +     int errorcode;
> +
> +#ifdef CONFIG_ETRAX_RS485
> +     struct rs485_control    rs485;  /* RS-485 support */
> +#endif
> +};
> +
> +/* this PORT is not in the standard serial.h. it's not actually used for
> + * anything since we only have one type of async serial-port anyway in this
> + * system.
> + */
> +
> +#define PORT_ETRAX 1
> +
> +/*
> + * Events are used to schedule things to happen at timer-interrupt
> + * time, instead of at rs interrupt time.
> + */
> +#define RS_EVENT_WRITE_WAKEUP        0
> +
> +#endif /* __KERNEL__ */
> +
> +#endif /* !_ETRAX_SERIAL_H */
> 
> /^JN - Jesper Nilsson


-- 
Jiri Slaby ([EMAIL PROTECTED])
Faculty of Informatics, Masaryk University
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to