Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
On Fri, Mar 11, 2005 at 05:34:18PM -0500, Wen Xiong wrote:
> diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c
> linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c
> --- linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c 1969-12-31
> 18:00:00.0 -0600
> +++ linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c 2005-03-11
> 16:26:47.442988256 -0600
> +/*
> + * neo_param()
> + * Send any/all changes to the line to the UART.
> + */
> +static void neo_param(struct jsm_channel *ch)
> +{
> + u8 lcr = 0;
> + u8 uart_lcr = 0;
> + u8 ier = 0;
> + u32 baud = 9600;
> + int quot = 0;
> + struct jsm_board *bd;
> +
> + bd = ch->ch_bd;
> + if (!bd)
> + return;
> +
> + /*
> + * If baud rate is zero, flush queues, and set mval to drop DTR.
> + */
The modem signal side of this is already handled for you.
> + const u64 bauds[4][16] = {
> + {
> + 0, 50, 75, 110,
> + 134,150,200,300,
> + 600,1200, 1800, 2400,
> + 4800, 9600, 19200, 38400 },
> + {
> + 0, 57600, 115200, 230400,
> + 460800, 150,200,921600,
> + 600,1200, 1800, 2400,
> + 4800, 9600, 19200, 38400 },
> + {
> + 0, 57600, 76800, 115200,
> + 131657, 153600, 230400, 460800,
> + 921600, 1200, 1800, 2400,
> + 4800, 9600, 19200, 38400 },
> + {
> + 0, 57600, 115200, 230400,
> + 460800, 150,200,921600,
> + 600,1200, 1800, 2400,
> + 4800, 9600, 19200, 38400 }
> + };
> +
> + baud = C_BAUD(ch->uart_port.info->tty) & 0xff;
> +
> + if (ch->ch_c_cflag & CBAUDEX)
> + iindex = 1;
This is buggy. You're making invalid assumptions about the
given baud rate flags in the termios. Use the helper functions
provided please.
--
Russell King
Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
-
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/
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
On Fri, Mar 11, 2005 at 05:34:18PM -0500, Wen Xiong wrote:
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c
linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c
--- linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c 1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c 2005-03-11
16:26:47.442988256 -0600
+/*
+ * neo_param()
+ * Send any/all changes to the line to the UART.
+ */
+static void neo_param(struct jsm_channel *ch)
+{
+ u8 lcr = 0;
+ u8 uart_lcr = 0;
+ u8 ier = 0;
+ u32 baud = 9600;
+ int quot = 0;
+ struct jsm_board *bd;
+
+ bd = ch-ch_bd;
+ if (!bd)
+ return;
+
+ /*
+ * If baud rate is zero, flush queues, and set mval to drop DTR.
+ */
The modem signal side of this is already handled for you.
+ const u64 bauds[4][16] = {
+ {
+ 0, 50, 75, 110,
+ 134,150,200,300,
+ 600,1200, 1800, 2400,
+ 4800, 9600, 19200, 38400 },
+ {
+ 0, 57600, 115200, 230400,
+ 460800, 150,200,921600,
+ 600,1200, 1800, 2400,
+ 4800, 9600, 19200, 38400 },
+ {
+ 0, 57600, 76800, 115200,
+ 131657, 153600, 230400, 460800,
+ 921600, 1200, 1800, 2400,
+ 4800, 9600, 19200, 38400 },
+ {
+ 0, 57600, 115200, 230400,
+ 460800, 150,200,921600,
+ 600,1200, 1800, 2400,
+ 4800, 9600, 19200, 38400 }
+ };
+
+ baud = C_BAUD(ch-uart_port.info-tty) 0xff;
+
+ if (ch-ch_c_cflag CBAUDEX)
+ iindex = 1;
This is buggy. You're making invalid assumptions about the
given baud rate flags in the termios. Use the helper functions
provided please.
--
Russell King
Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
-
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/
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
Arjan van de Ven wrote:
Jeff pointed out several PCI posting errors last time. Before we used
udelay and now we changed to readb/readl instead of udelay this time.
But we only used PCI posting when we think maybe delay there.
So we have to do PCI posting on every writeb?
not every
Do you have some rules for
doing PCI posting while writeb? depends on what kind of registers?
basically you need to do posting flushes after every write for which you
assume later on the hardware received the write. If you do 5 writes in a
row you don't need 5 flushes. If you do a read later always anyway you
don't need any flushes.
On the other side of the spectrum: if you do something to the hardware
and then wait for some event, you do need to flush that stuff.
So at minimum you want to make sure that at any point where you leave
the driver (to userspace or tty layer or ...) all writes have been
flushed. And at all points where you are going to wait for hardware
events.
Based on above all points, I audited the whole drvier for PCI posting
issues.
Just added several PCI posting while the driver has interfaces with tty
layer.
Thanks for your comments.
wendy
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c
linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c
--- linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c 1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c 2005-03-11
16:26:47.442988256 -0600
@@ -0,0 +1,1402 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau <[EMAIL PROTECTED]>
+ * Wendy Xiong <[EMAIL PROTECTED]>
+ *
+ ***/
+#include/* For udelay */
+#include /* For the various UART offsets */
+#include
+#include
+#include
+
+#include "jsm.h" /* Driver main header file */
+
+static u32 jsm_offset_table[8] = { 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40,
0x80 };
+
+static void neo_set_cts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(>ch_neo_uart->ier);
+ u8 efr = readb(>ch_neo_uart->efr);
+
+ jsm_printk(PARAM, INFO, >ch_bd->pci_dev, "Setting CTSFLOW\n");
+
+ /* Turn on auto CTS flow control */
+ ier |= (UART_17158_IER_CTSDSR);
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_CTSDSR);
+
+ /* Turn off auto Xon flow control */
+ efr &= ~(UART_17158_EFR_IXON);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, >ch_neo_uart->efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, >ch_neo_uart->efr);
+
+ /* Turn on table D, with 8 char hi/low watermarks */
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_4DELAY),
>ch_neo_uart->fctr);
+
+ /* Feed the UART our trigger levels */
+ writeb(8, >ch_neo_uart->tfifo);
+ ch->ch_t_tlevel = 8;
+
+ writeb(ier, >ch_neo_uart->ier);
+}
+
+static void neo_set_rts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(>ch_neo_uart->ier);
+ u8 efr = readb(>ch_neo_uart->efr);
+
+ jsm_printk(PARAM, INFO, >ch_bd->pci_dev, "Setting RTSFLOW\n");
+
+ /* Turn on auto RTS flow control */
+ ier |= (UART_17158_IER_RTSDTR);
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_RTSDTR);
+
+ /* Turn off auto Xoff flow control */
+ ier &= ~(UART_17158_IER_XOFF);
+ efr &= ~(UART_17158_EFR_IXOFF);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, >ch_neo_uart->efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, >ch_neo_uart->efr);
+
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_4DELAY),
>ch_neo_uart->fctr);
+ ch->ch_r_watermark = 4;
+
+ writeb(56, >ch_neo_uart->rfifo);
+ ch->ch_r_tlevel = 56;
+
+ writeb(ier, >ch_neo_uart->ier);
+
+ /*
+* From the Neo UART spec sheet:
+* The auto RTS/DTR function must be started by asserting
+* RTS/DTR# output pin (MCR bit-0 or 1 to logic 1 after
+
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
> Jeff pointed out several PCI posting errors last time. Before we used > udelay and now we changed to readb/readl instead of udelay this time. > But we only used PCI posting when we think maybe delay there. > So we have to do PCI posting on every writeb? not every > Do you have some rules for > doing PCI posting while writeb? depends on what kind of registers? basically you need to do posting flushes after every write for which you assume later on the hardware received the write. If you do 5 writes in a row you don't need 5 flushes. If you do a read later always anyway you don't need any flushes. On the other side of the spectrum: if you do something to the hardware and then wait for some event, you do need to flush that stuff. So at minimum you want to make sure that at any point where you leave the driver (to userspace or tty layer or ...) all writes have been flushed. And at all points where you are going to wait for hardware events. - 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/
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
Arjan van de Ven wrote:
On Fri, 2005-03-11 at 10:38 -0500, Wen Xiong wrote:
+static void neo_set_cts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(>ch_neo_uart->ier);
+ u8 efr = readb(>ch_neo_uart->efr);
+
...
+
+ writeb(ier, >ch_neo_uart->ier);
+}
Hi,
have you ever audited this driver for PCI posting errors? On very first
sight it looks like the driver doesn't do this correctly but it might
just be very subtle...
Jeff pointed out several PCI posting errors last time. Before we used
udelay and now we changed to readb/readl instead of udelay this time.
But we only used PCI posting when we think maybe delay there.
So we have to do PCI posting on every writeb? Do you have some rules for
doing PCI posting while writeb? depends on what kind of registers?
Thanks,
wendy
-
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/
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
On Fri, 2005-03-11 at 10:38 -0500, Wen Xiong wrote:
> +static void neo_set_cts_flow_control(struct jsm_channel *ch)
> +{
> + u8 ier = readb(>ch_neo_uart->ier);
> + u8 efr = readb(>ch_neo_uart->efr);
> +
...
> +
> + writeb(ier, >ch_neo_uart->ier);
> +}
Hi,
have you ever audited this driver for PCI posting errors? On very first
sight it looks like the driver doesn't do this correctly but it might
just be very subtle...
-
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/
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
The third patch for jsm device driver.
Signed-off-by: Wen Xiong <[EMAIL PROTECTED]>
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c
linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c
--- linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c 1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c 2005-03-11
09:07:53.902968424 -0600
@@ -0,0 +1,1402 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau <[EMAIL PROTECTED]>
+ * Wendy Xiong <[EMAIL PROTECTED]>
+ *
+ ***/
+#include/* For udelay */
+#include /* For the various UART offsets */
+#include
+#include
+#include
+
+#include "jsm.h" /* Driver main header file */
+
+static u32 jsm_offset_table[8] = { 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40,
0x80 };
+
+static void neo_set_cts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(>ch_neo_uart->ier);
+ u8 efr = readb(>ch_neo_uart->efr);
+
+ jsm_printk(PARAM, INFO, >ch_bd->pci_dev, "Setting CTSFLOW\n");
+
+ /* Turn on auto CTS flow control */
+ ier |= (UART_17158_IER_CTSDSR);
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_CTSDSR);
+
+ /* Turn off auto Xon flow control */
+ efr &= ~(UART_17158_EFR_IXON);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, >ch_neo_uart->efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, >ch_neo_uart->efr);
+
+ /* Turn on table D, with 8 char hi/low watermarks */
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_4DELAY),
>ch_neo_uart->fctr);
+
+ /* Feed the UART our trigger levels */
+ writeb(8, >ch_neo_uart->tfifo);
+ ch->ch_t_tlevel = 8;
+
+ writeb(ier, >ch_neo_uart->ier);
+}
+
+static void neo_set_rts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(>ch_neo_uart->ier);
+ u8 efr = readb(>ch_neo_uart->efr);
+
+ jsm_printk(PARAM, INFO, >ch_bd->pci_dev, "Setting RTSFLOW\n");
+
+ /* Turn on auto RTS flow control */
+ ier |= (UART_17158_IER_RTSDTR);
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_RTSDTR);
+
+ /* Turn off auto Xoff flow control */
+ ier &= ~(UART_17158_IER_XOFF);
+ efr &= ~(UART_17158_EFR_IXOFF);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, >ch_neo_uart->efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, >ch_neo_uart->efr);
+
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_4DELAY),
>ch_neo_uart->fctr);
+ ch->ch_r_watermark = 4;
+
+ writeb(56, >ch_neo_uart->rfifo);
+ ch->ch_r_tlevel = 56;
+
+ writeb(ier, >ch_neo_uart->ier);
+
+ /*
+* From the Neo UART spec sheet:
+* The auto RTS/DTR function must be started by asserting
+* RTS/DTR# output pin (MCR bit-0 or 1 to logic 1 after
+* it is enabled.
+*/
+ ch->ch_mostat |= (UART_MCR_RTS);
+}
+
+
+static void neo_set_ixon_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(>ch_neo_uart->ier);
+ u8 efr = readb(>ch_neo_uart->efr);
+
+ jsm_printk(PARAM, INFO, >ch_bd->pci_dev, "Setting IXON FLOW\n");
+
+ /* Turn off auto CTS flow control */
+ ier &= ~(UART_17158_IER_CTSDSR);
+ efr &= ~(UART_17158_EFR_CTSDSR);
+
+ /* Turn on auto Xon flow control */
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXON);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, >ch_neo_uart->efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, >ch_neo_uart->efr);
+
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_8DELAY),
>ch_neo_uart->fctr);
+ ch->ch_r_watermark = 4;
+
+ writeb(32, >ch_neo_uart->rfifo);
+ ch->ch_r_tlevel = 32;
+
+ /* Tell UART what start/stop chars it should be looking for */
+ writeb(ch->ch_startc, >ch_neo_uart->xonchar1);
+ writeb(0,
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
The third patch for jsm device driver.
Signed-off-by: Wen Xiong [EMAIL PROTECTED]
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c
linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c
--- linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c 1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c 2005-03-11
09:07:53.902968424 -0600
@@ -0,0 +1,1402 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau [EMAIL PROTECTED]
+ * Wendy Xiong [EMAIL PROTECTED]
+ *
+ ***/
+#include linux/delay.h /* For udelay */
+#include linux/serial_reg.h /* For the various UART offsets */
+#include linux/tty.h
+#include linux/pci.h
+#include asm/io.h
+
+#include jsm.h /* Driver main header file */
+
+static u32 jsm_offset_table[8] = { 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40,
0x80 };
+
+static void neo_set_cts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(ch-ch_neo_uart-ier);
+ u8 efr = readb(ch-ch_neo_uart-efr);
+
+ jsm_printk(PARAM, INFO, ch-ch_bd-pci_dev, Setting CTSFLOW\n);
+
+ /* Turn on auto CTS flow control */
+ ier |= (UART_17158_IER_CTSDSR);
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_CTSDSR);
+
+ /* Turn off auto Xon flow control */
+ efr = ~(UART_17158_EFR_IXON);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, ch-ch_neo_uart-efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, ch-ch_neo_uart-efr);
+
+ /* Turn on table D, with 8 char hi/low watermarks */
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_4DELAY),
ch-ch_neo_uart-fctr);
+
+ /* Feed the UART our trigger levels */
+ writeb(8, ch-ch_neo_uart-tfifo);
+ ch-ch_t_tlevel = 8;
+
+ writeb(ier, ch-ch_neo_uart-ier);
+}
+
+static void neo_set_rts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(ch-ch_neo_uart-ier);
+ u8 efr = readb(ch-ch_neo_uart-efr);
+
+ jsm_printk(PARAM, INFO, ch-ch_bd-pci_dev, Setting RTSFLOW\n);
+
+ /* Turn on auto RTS flow control */
+ ier |= (UART_17158_IER_RTSDTR);
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_RTSDTR);
+
+ /* Turn off auto Xoff flow control */
+ ier = ~(UART_17158_IER_XOFF);
+ efr = ~(UART_17158_EFR_IXOFF);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, ch-ch_neo_uart-efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, ch-ch_neo_uart-efr);
+
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_4DELAY),
ch-ch_neo_uart-fctr);
+ ch-ch_r_watermark = 4;
+
+ writeb(56, ch-ch_neo_uart-rfifo);
+ ch-ch_r_tlevel = 56;
+
+ writeb(ier, ch-ch_neo_uart-ier);
+
+ /*
+* From the Neo UART spec sheet:
+* The auto RTS/DTR function must be started by asserting
+* RTS/DTR# output pin (MCR bit-0 or 1 to logic 1 after
+* it is enabled.
+*/
+ ch-ch_mostat |= (UART_MCR_RTS);
+}
+
+
+static void neo_set_ixon_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(ch-ch_neo_uart-ier);
+ u8 efr = readb(ch-ch_neo_uart-efr);
+
+ jsm_printk(PARAM, INFO, ch-ch_bd-pci_dev, Setting IXON FLOW\n);
+
+ /* Turn off auto CTS flow control */
+ ier = ~(UART_17158_IER_CTSDSR);
+ efr = ~(UART_17158_EFR_CTSDSR);
+
+ /* Turn on auto Xon flow control */
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXON);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, ch-ch_neo_uart-efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, ch-ch_neo_uart-efr);
+
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_8DELAY),
ch-ch_neo_uart-fctr);
+ ch-ch_r_watermark = 4;
+
+ writeb(32, ch-ch_neo_uart-rfifo);
+ ch-ch_r_tlevel = 32;
+
+ /* Tell UART what start/stop chars it should be looking for */
+
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
On Fri, 2005-03-11 at 10:38 -0500, Wen Xiong wrote:
+static void neo_set_cts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(ch-ch_neo_uart-ier);
+ u8 efr = readb(ch-ch_neo_uart-efr);
+
...
+
+ writeb(ier, ch-ch_neo_uart-ier);
+}
Hi,
have you ever audited this driver for PCI posting errors? On very first
sight it looks like the driver doesn't do this correctly but it might
just be very subtle...
-
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/
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
Arjan van de Ven wrote:
On Fri, 2005-03-11 at 10:38 -0500, Wen Xiong wrote:
+static void neo_set_cts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(ch-ch_neo_uart-ier);
+ u8 efr = readb(ch-ch_neo_uart-efr);
+
...
+
+ writeb(ier, ch-ch_neo_uart-ier);
+}
Hi,
have you ever audited this driver for PCI posting errors? On very first
sight it looks like the driver doesn't do this correctly but it might
just be very subtle...
Jeff pointed out several PCI posting errors last time. Before we used
udelay and now we changed to readb/readl instead of udelay this time.
But we only used PCI posting when we think maybe delay there.
So we have to do PCI posting on every writeb? Do you have some rules for
doing PCI posting while writeb? depends on what kind of registers?
Thanks,
wendy
-
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/
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
Jeff pointed out several PCI posting errors last time. Before we used udelay and now we changed to readb/readl instead of udelay this time. But we only used PCI posting when we think maybe delay there. So we have to do PCI posting on every writeb? not every Do you have some rules for doing PCI posting while writeb? depends on what kind of registers? basically you need to do posting flushes after every write for which you assume later on the hardware received the write. If you do 5 writes in a row you don't need 5 flushes. If you do a read later always anyway you don't need any flushes. On the other side of the spectrum: if you do something to the hardware and then wait for some event, you do need to flush that stuff. So at minimum you want to make sure that at any point where you leave the driver (to userspace or tty layer or ...) all writes have been flushed. And at all points where you are going to wait for hardware events. - 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/
Re: [ patch 3/5] drivers/serial/jsm: new serial device driver
Arjan van de Ven wrote:
Jeff pointed out several PCI posting errors last time. Before we used
udelay and now we changed to readb/readl instead of udelay this time.
But we only used PCI posting when we think maybe delay there.
So we have to do PCI posting on every writeb?
not every
Do you have some rules for
doing PCI posting while writeb? depends on what kind of registers?
basically you need to do posting flushes after every write for which you
assume later on the hardware received the write. If you do 5 writes in a
row you don't need 5 flushes. If you do a read later always anyway you
don't need any flushes.
On the other side of the spectrum: if you do something to the hardware
and then wait for some event, you do need to flush that stuff.
So at minimum you want to make sure that at any point where you leave
the driver (to userspace or tty layer or ...) all writes have been
flushed. And at all points where you are going to wait for hardware
events.
Based on above all points, I audited the whole drvier for PCI posting
issues.
Just added several PCI posting while the driver has interfaces with tty
layer.
Thanks for your comments.
wendy
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c
linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c
--- linux-2.6.11.org/drivers/serial/jsm/jsm_neo.c 1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_neo.c 2005-03-11
16:26:47.442988256 -0600
@@ -0,0 +1,1402 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau [EMAIL PROTECTED]
+ * Wendy Xiong [EMAIL PROTECTED]
+ *
+ ***/
+#include linux/delay.h /* For udelay */
+#include linux/serial_reg.h /* For the various UART offsets */
+#include linux/tty.h
+#include linux/pci.h
+#include asm/io.h
+
+#include jsm.h /* Driver main header file */
+
+static u32 jsm_offset_table[8] = { 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40,
0x80 };
+
+static void neo_set_cts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(ch-ch_neo_uart-ier);
+ u8 efr = readb(ch-ch_neo_uart-efr);
+
+ jsm_printk(PARAM, INFO, ch-ch_bd-pci_dev, Setting CTSFLOW\n);
+
+ /* Turn on auto CTS flow control */
+ ier |= (UART_17158_IER_CTSDSR);
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_CTSDSR);
+
+ /* Turn off auto Xon flow control */
+ efr = ~(UART_17158_EFR_IXON);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, ch-ch_neo_uart-efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, ch-ch_neo_uart-efr);
+
+ /* Turn on table D, with 8 char hi/low watermarks */
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_4DELAY),
ch-ch_neo_uart-fctr);
+
+ /* Feed the UART our trigger levels */
+ writeb(8, ch-ch_neo_uart-tfifo);
+ ch-ch_t_tlevel = 8;
+
+ writeb(ier, ch-ch_neo_uart-ier);
+}
+
+static void neo_set_rts_flow_control(struct jsm_channel *ch)
+{
+ u8 ier = readb(ch-ch_neo_uart-ier);
+ u8 efr = readb(ch-ch_neo_uart-efr);
+
+ jsm_printk(PARAM, INFO, ch-ch_bd-pci_dev, Setting RTSFLOW\n);
+
+ /* Turn on auto RTS flow control */
+ ier |= (UART_17158_IER_RTSDTR);
+ efr |= (UART_17158_EFR_ECB | UART_17158_EFR_RTSDTR);
+
+ /* Turn off auto Xoff flow control */
+ ier = ~(UART_17158_IER_XOFF);
+ efr = ~(UART_17158_EFR_IXOFF);
+
+ /* Why? Becuz Exar's spec says we have to zero it out before setting it
*/
+ writeb(0, ch-ch_neo_uart-efr);
+
+ /* Turn on UART enhanced bits */
+ writeb(efr, ch-ch_neo_uart-efr);
+
+ writeb((UART_17158_FCTR_TRGD | UART_17158_FCTR_RTS_4DELAY),
ch-ch_neo_uart-fctr);
+ ch-ch_r_watermark = 4;
+
+ writeb(56, ch-ch_neo_uart-rfifo);
+ ch-ch_r_tlevel = 56;
+
+ writeb(ier, ch-ch_neo_uart-ier);
+
+ /*
+* From the Neo UART spec sheet:
+* The auto RTS/DTR function must be started by asserting
+
