> > 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. > > I am taking a stab at my_console_write() as well. I have something that works > well > for me, but i am unsure if I have broken KGDB/XMON support. In my new > my_console_write() > I must know the length of the TX buffer in the TXBD. The TX buffer length > when the console > is fully operational is TX_BUF_SIZE. Before that, in serial_console_setup, > the length > is 4 bytes. But what is the TX buffer length before serial_console_setup has > run(that's > when KGB/XMON is active)? > > Jocke
Here is a version that fixes my_console_write as well. The KGDB/XMON question above is still open. Currently TX buffer length assumption is >=4 Anyone intressted? 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 17 Sep 2003 13:33:47 -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); @@ -2261,10 +2270,11 @@ { struct serial_state *ser; ser_info_t *info; - unsigned i; + unsigned i, c, cr_missing, max_tx_size; volatile cbd_t *bdp, *bdbase; volatile smc_uart_t *up; volatile u_char *cp; + unsigned long flags; ser = rs_table + idx; @@ -2273,40 +2283,41 @@ * we simply use the single buffer allocated. */ if ((info = (ser_info_t *)ser->info) != NULL) { - bdp = info->tx_cur; bdbase = info->tx_bd_base; - } - else { - /* Pointer to UART in parameter ram. - */ + max_tx_size = TX_BUF_SIZE; + } else { + /* Pointer to UART in parameter ram. */ up = (smc_uart_t *)&cpmp->cp_dparam[ser->port]; - - /* Get the address of the host memory buffer. - */ - bdp = bdbase = (cbd_t *)&cpmp->cp_dpmem[up->smc_tbase]; + /* Get the address of the host memory buffer.*/ info = &consinfo; + info->tx_bd_base = (cbd_t *)bdbase = (cbd_t *)&cpmp->cp_dpmem[up->smc_tbase]; + info->tx_cur = (cbd_t *)bdbase; + max_tx_size = 4; /* Early serial acces has this */ } - /* - * We need to gracefully shut down the transmitter, disable - * interrupts, then send our bytes out. - */ + cr_missing = 0; + while (1){ + c = MIN(max_tx_size, count); + if (c <= 0) + break; - /* - * Now, do each character. This is not as bad as it looks - * since this is a holding FIFO and not a transmitting FIFO. - * We could add the complexity of filling the entire transmit - * buffer, but we would just wait longer between accesses...... - */ - for (i = 0; i < count; i++, s++) { + local_irq_save(flags); + bdp = info->tx_cur; + bdbase = info->tx_bd_base; + if (bdp->cbd_sc & BD_SC_WRAP) + info->tx_cur = (cbd_t *)bdbase; + else + info->tx_cur = (cbd_t *)(bdp+1); + local_irq_restore(flags); + /* Wait for transmitter fifo to empty. * Ready indicates output is ready, and xmt is doing * that, not that it is ready for us to send. */ while (bdp->cbd_sc & BD_SC_READY); - /* Send the character out. + /* Send the characters out. * If the buffer address is in the CPM DPRAM, don't * convert it. */ @@ -2314,55 +2325,32 @@ cp = (u_char *)(bdp->cbd_bufaddr); else cp = info->tx_va_base + ((bdp - info->tx_bd_base) * TX_BUF_SIZE); - *cp = *s; - - bdp->cbd_datlen = 1; - bdp->cbd_sc |= BD_SC_READY; - - if (bdp->cbd_sc & BD_SC_WRAP) - bdp = bdbase; - else - bdp++; - - /* if a LF, also do CR... */ - if (*s == 10) { - while (bdp->cbd_sc & BD_SC_READY); - - /* This 'if' below will never be true, but a few - * people argued with me that it was a "bug by - * inspection" that is was easier to add the code - * than continue the discussion. The only time - * the buffer address is in DPRAM is during early - * use by kgdb/xmon, which format their own packets - * and we never get here. -- Dan - */ - if ((uint)(bdp->cbd_bufaddr) > (uint)IMAP_ADDR) - cp = (u_char *)(bdp->cbd_bufaddr); - else - cp = info->tx_va_base + ((bdp - info->tx_bd_base) * TX_BUF_SIZE); - *cp = 13; - bdp->cbd_datlen = 1; - bdp->cbd_sc |= BD_SC_READY; - if (bdp->cbd_sc & BD_SC_WRAP) { - bdp = bdbase; - } - else { - bdp++; - } + i=1; /* Keeps track of consumed TX buffer space */ + if (cr_missing) { + /* Previus loop didn't have room for the CR, insert it first in this */ + *cp++ = '\r'; + i++; } - } - - /* - * Finally, Wait for transmitter & holding register to empty - * and restore the IER - */ - while (bdp->cbd_sc & BD_SC_READY); + cr_missing = 0; + for (; i <= c; i++) { + if ((*cp++ = *s++) != '\n') + continue; /* Copy bytes until a NewLine is found */ + /* NewLine found, see if there is space in the TX buffer to add a CR */ + if (i < max_tx_size) { + *cp++ = '\r'; /* yes, there is space to add a CR */ + i++; + } else + cr_missing = 1; /* No space in the TX buffer, + rember it so it can be inserted in the next loop */ + } + count -= (c-cr_missing); + bdp->cbd_datlen = i-1; + bdp->cbd_sc |= BD_SC_READY; - if (info) - info->tx_cur = (cbd_t *)bdp; + } + /* while (bdp->cbd_sc & BD_SC_READY); is this really needed? */ } - static void serial_console_write(struct console *c, const char *s, unsigned count) { @@ -3073,10 +3061,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/