> > Jean-Denis Boyer wrote:
> >
> > > I have just tested your patch on 8260 and it fixes the problem when
> > > printk is called ...
> >
> > The use of printk is strictly a debug function. If it is necessary to see
> > it print out pretty, then use a different I/O path for the application or
> > dump out the log buffer separately. Serial ports don't guarantee any
> > transactional operation with multiple users, printk isn't any different
> > although it does the bad thing of blocking interrupts while it processes
> > the message.
> hmm, it does not block interrupts as is, does it? Only if you apply the patch?
>
> >
> > Adding this patch adds lots of interrupt latency and I would recommend
> > against it. It appears there is a bug that should be fixed to ensure
> > the proper synchronization of the BDs, but not with such large scale
> > blocking of interrupts.
> Agreed, but is there another mechanism but to disable interrupts? I can't
> think of any.
> It is possible to shrink the window with interrupts off dramatically.
>
> Jocke
> >
> > Thanks.
> >
> >
> > -- Dan
OK, here is my attempt to fix the 8xx uart driver. With this
patch I can no longer break the console from user space. Against fairly recent
linuxppc_devel snapshot. Please comment/apply.
Summary:
- uses local_irq_save()/local_irq_restore() around tx_cur accesses.
I haved tried to minimize the time between local_irq_save() and
local_irq_restore()
- Fixed a bug around copy_from_user() in rs_8xx_write().
- my_console_write() is still buggy. It takes a little more effort to fix
this function. It should be fixed since a spurious printk can screw/lock
your console.
- fixed 4 trivial warnings as well.
Jocke
Index: arch/ppc/8xx_io/uart.c
===================================================================
RCS file: /home/cvsadmin/cvsroot/kernel/linuxppc/arch/ppc/8xx_io/uart.c,v
retrieving revision 1.7
diff -u -r1.7 uart.c
--- arch/ppc/8xx_io/uart.c 13 Jun 2003 09:04:43 -0000 1.7
+++ arch/ppc/8xx_io/uart.c 16 Sep 2003 12:17:01 -0000
@@ -1068,6 +1068,7 @@
ser_info_t *info = (ser_info_t *)tty->driver_data;
volatile cbd_t *bdp;
unsigned char *cp;
+ unsigned long flags;
if (serial_paranoia_check(info, tty->device, "rs_put_char"))
return;
@@ -1075,7 +1076,16 @@
if (!tty)
return;
+ local_irq_save(flags);
bdp = info->tx_cur;
+ /* Get next BD.
+ */
+ if (bdp->cbd_sc & BD_SC_WRAP)
+ info->tx_cur = info->tx_bd_base;
+ else
+ info->tx_cur = (cbd_t *)bdp + 1;
+ local_irq_restore(flags);
+
while (bdp->cbd_sc & BD_SC_READY);
cp = info->tx_va_base + ((bdp - info->tx_bd_base) * TX_BUF_SIZE);
@@ -1083,14 +1093,6 @@
bdp->cbd_datlen = 1;
bdp->cbd_sc |= BD_SC_READY;
- /* Get next BD.
- */
- if (bdp->cbd_sc & BD_SC_WRAP)
- bdp = info->tx_bd_base;
- else
- bdp++;
-
- info->tx_cur = (cbd_t *)bdp;
}
static int rs_8xx_write(struct tty_struct * tty, int from_user,
@@ -1100,6 +1102,7 @@
ser_info_t *info = (ser_info_t *)tty->driver_data;
volatile cbd_t *bdp;
unsigned char *cp;
+ unsigned long flags;
#ifdef CONFIG_KGDB_CONSOLE
/* Try to let stub handle output. Returns true if it did. */
@@ -1113,44 +1116,46 @@
if (!tty)
return 0;
- bdp = info->tx_cur;
-
while (1) {
c = MIN(count, TX_BUF_SIZE);
if (c <= 0)
break;
+ local_irq_save(flags);
+ bdp = info->tx_cur;
if (bdp->cbd_sc & BD_SC_READY) {
info->flags |= TX_WAKEUP;
+ local_irq_restore(flags);
break;
}
cp = info->tx_va_base + ((bdp - info->tx_bd_base) *
TX_BUF_SIZE);
if (from_user) {
- if (copy_from_user((void *)cp, buf, c)) {
+ c -= copy_from_user((void *)cp, buf, c);
+ if (!c) {
if (!ret)
ret = -EFAULT;
+ local_irq_restore(flags);
break;
}
} else {
memcpy((void *)cp, buf, c);
}
-
+ /* Get next BD.
+ */
+ if (bdp->cbd_sc & BD_SC_WRAP)
+ info->tx_cur = info->tx_bd_base;
+ else
+ info->tx_cur = (cbd_t *)bdp + 1;
+ local_irq_restore(flags);
+
bdp->cbd_datlen = c;
bdp->cbd_sc |= BD_SC_READY;
buf += c;
count -= c;
ret += c;
-
- /* Get next BD.
- */
- if (bdp->cbd_sc & BD_SC_WRAP)
- bdp = info->tx_bd_base;
- else
- bdp++;
- info->tx_cur = (cbd_t *)bdp;
}
return ret;
}
@@ -1210,28 +1215,28 @@
{
volatile cbd_t *bdp;
unsigned char *cp;
+ unsigned long flags;
ser_info_t *info = (ser_info_t *)tty->driver_data;
if (serial_paranoia_check(info, tty->device, "rs_send_char"))
return;
+ local_irq_save(flags);
bdp = info->tx_cur;
+ /* Get next BD.
+ */
+ if (bdp->cbd_sc & BD_SC_WRAP)
+ info->tx_cur = info->tx_bd_base;
+ else
+ info->tx_cur = (cbd_t *)bdp + 1;
+ local_irq_restore(flags);
while (bdp->cbd_sc & BD_SC_READY);
cp = info->tx_va_base + ((bdp - info->tx_bd_base) * TX_BUF_SIZE);
*cp = ch;
bdp->cbd_datlen = 1;
bdp->cbd_sc |= BD_SC_READY;
-
- /* Get next BD.
- */
- if (bdp->cbd_sc & BD_SC_WRAP)
- bdp = info->tx_bd_base;
- else
- bdp++;
-
- info->tx_cur = (cbd_t *)bdp;
}
/*
@@ -1802,7 +1807,7 @@
static void rs_8xx_wait_until_sent(struct tty_struct *tty, int timeout)
{
ser_info_t *info = (ser_info_t *)tty->driver_data;
- unsigned long orig_jiffies, char_time;
+ unsigned long orig_jiffies, char_time, tst_res;
/*int lsr;*/
volatile cbd_t *bdp;
@@ -1837,6 +1842,7 @@
* are empty.
*/
do {
+ unsigned long flags;
#ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
printk("lsr = %d (jiff=%lu)...", lsr, jiffies);
#endif
@@ -1854,12 +1860,15 @@
* is the buffer is available. There are still characters
* in the CPM FIFO.
*/
+ local_irq_save(flags);
bdp = info->tx_cur;
if (bdp == info->tx_bd_base)
bdp += (TX_NUM_FIFO-1);
else
bdp--;
- } while (bdp->cbd_sc & BD_SC_READY);
+ tst_res = !!(bdp->cbd_sc & BD_SC_READY);
+ local_irq_restore(flags);
+ } while (tst_res);
current->state = TASK_RUNNING;
#ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
printk("lsr = %d (jiff=%lu)...done\n", lsr, jiffies);
@@ -3073,10 +3082,10 @@
bdp->cbd_bufaddr = iopa(mem_addr);
(bdp+1)->cbd_bufaddr = iopa(mem_addr+4);
- consinfo.rx_va_base = mem_addr;
- consinfo.rx_bd_base = bdp;
- consinfo.tx_va_base = mem_addr + 4;
- consinfo.tx_bd_base = bdp+1;
+ consinfo.rx_va_base = (unsigned char *) mem_addr;
+ consinfo.rx_bd_base = (cbd_t *) bdp;
+ consinfo.tx_va_base = (unsigned char *) (mem_addr + 4);
+ consinfo.tx_bd_base = (cbd_t *) (bdp+1);
/* For the receive, set empty and wrap.
* For transmit, set wrap.
** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/
