Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master
Hello. Andrew Morton wrote: + status = *(volatile u32 *)up->port.private_data; It distresses me that this patch uses a variable which this patch doesn't initialise anywhere. It isn't complete. I assume this gets passed via early_serial_setup(). Marc? The sub-driver code whch sets up this field shuld be included in the patch, no? Hardly so, this code (not a subdriver) resides under arch/mips/ I think. WBR, Sergei - 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] serial driver PMC MSP71xx, kernel linux-mips.git master
Hello. Andrew Morton wrote: + status = *(volatile u32 *)up-port.private_data; It distresses me that this patch uses a variable which this patch doesn't initialise anywhere. It isn't complete. I assume this gets passed via early_serial_setup(). Marc? The sub-driver code whch sets up this field shuld be included in the patch, no? Hardly so, this code (not a subdriver) resides under arch/mips/ I think. WBR, Sergei - 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] serial driver PMC MSP71xx, kernel linux-mips.git master
On Thu, 15 Feb 2007 13:26:29 -0600 Marc St-Jean <[EMAIL PROTECTED]> wrote: > + status = *(volatile u32 *)up->port.private_data; It distresses me that this patch uses a variable which this patch doesn't initialise anywhere. It isn't complete. The sub-driver code whch sets up this field shuld be included in the patch, no? - 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] serial driver PMC MSP71xx, kernel linux-mips.git master
Hello.
Marc St-Jean wrote:
There are three different fixes:
1. Fix for DesignWare APB THRE errata:
In brief, this is a non-standard 16550 in that the THRE interrupt
will not re-assert itself simply by disabling and re-enabling the
THRI bit in the IER, it is only re-enabled if a character is actually
sent out.
It appears that the "8250-uart-backup-timer.patch" in the "mm" tree also
fixes it so we have dropped our initial workaround.
This patch now needs to be applied on top of that "mm" patch.
This patch has hit the mainline kernel already.
2. Fix for Busy Detect on LCR write:
The DesignWare APB UART has a feature which causes a new Busy Detect
interrupt to be generated if it's busy when the LCR is written. This
fix saves the value of the LCR and rewrites it after clearing the
interrupt.
3. Workaround for interrupt/data concurrency issue:
The SoC needs to ensure that writes that can cause interrupts to be
cleared reach the UART before returning from the ISR. This fix reads
a non-destructive register on the UART so the read transaction
completion ensures the previously queued write transaction has
also completed.
The differences from the last attempt is dropping the call to
in_irq() and including more detailed description of the fixes.
The difference part would better be under the "---" cutoff line, along
with diffstat.
This way it gets auto-removed by quilt/git when applying the patch.
Signed-off-by: Marc St-Jean <[EMAIL PROTECTED]>
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..bfaacc5 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
return inb(up->port.iobase + 1);
case UPIO_MEM:
+ case UPIO_DWAPB:
return readb(up->port.membase + offset);
case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
static void
serial_out(struct uart_8250_port *up, int offset, int value)
{
+ /* Save the offset before it's remapped */
+ int save_offset = offset;
offset = map_8250_out_reg(up, offset) << up->port.regshift;
switch (up->port.iotype) {
I've just got an idea how to "beautify" this code -- rename the 'offset'
arg to something like reg, and then declare/init 'offset' as local variable.
Would certainly look better (and worth doing in all serial_* accessros).
@@ -359,6 +362,18 @@ #endif
writeb(value, up->port.membase + offset);
break;
+ case UPIO_DWAPB:
+ /* Save the LCR value so it can be re-written when a
+* Busy Detect interrupt occurs. */
+ if (save_offset == UART_LCR)
+ up->lcr = value;
+ writeb(value, up->port.membase + offset);
+ /* Read the IER to ensure any interrupt is cleared before
+* returning from ISR. */
+ if (save_offset == UART_TX || save_offset == UART_IER)
+ value = serial_in(up, UART_IER);
+ break;
+
default:
outb(value, up->port.iobase + offset);
}
@@ -2454,8 +2485,8 @@ int __init serial8250_start_console(stru
add_preferred_console("ttyS", line, options);
printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
- line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
- port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
+ line, port->iotype >= UPIO_MEM ? "MMIO" : "I/O port",
+ port->iotype >= UPIO_MEM ? (unsigned long) port->mapbase :
(unsigned long) port->iobase, options);
if (!(serial8250_console.flags & CON_ENABLED)) {
serial8250_console.flags &= ~CON_PRINTBUFFER;
I've remembered why I decided not to fix it: this code only gets called
from 8250__eraly.c which can't handle anything beyond UPIO_MEM anyway.
WBR, Sergei
-
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] serial driver PMC MSP71xx, kernel linux-mips.git master
Hello.
Marc St-Jean wrote:
There are three different fixes:
1. Fix for DesignWare APB THRE errata:
In brief, this is a non-standard 16550 in that the THRE interrupt
will not re-assert itself simply by disabling and re-enabling the
THRI bit in the IER, it is only re-enabled if a character is actually
sent out.
It appears that the 8250-uart-backup-timer.patch in the mm tree also
fixes it so we have dropped our initial workaround.
This patch now needs to be applied on top of that mm patch.
This patch has hit the mainline kernel already.
2. Fix for Busy Detect on LCR write:
The DesignWare APB UART has a feature which causes a new Busy Detect
interrupt to be generated if it's busy when the LCR is written. This
fix saves the value of the LCR and rewrites it after clearing the
interrupt.
3. Workaround for interrupt/data concurrency issue:
The SoC needs to ensure that writes that can cause interrupts to be
cleared reach the UART before returning from the ISR. This fix reads
a non-destructive register on the UART so the read transaction
completion ensures the previously queued write transaction has
also completed.
The differences from the last attempt is dropping the call to
in_irq() and including more detailed description of the fixes.
The difference part would better be under the --- cutoff line, along
with diffstat.
This way it gets auto-removed by quilt/git when applying the patch.
Signed-off-by: Marc St-Jean [EMAIL PROTECTED]
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..bfaacc5 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
return inb(up-port.iobase + 1);
case UPIO_MEM:
+ case UPIO_DWAPB:
return readb(up-port.membase + offset);
case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
static void
serial_out(struct uart_8250_port *up, int offset, int value)
{
+ /* Save the offset before it's remapped */
+ int save_offset = offset;
offset = map_8250_out_reg(up, offset) up-port.regshift;
switch (up-port.iotype) {
I've just got an idea how to beautify this code -- rename the 'offset'
arg to something like reg, and then declare/init 'offset' as local variable.
Would certainly look better (and worth doing in all serial_* accessros).
@@ -359,6 +362,18 @@ #endif
writeb(value, up-port.membase + offset);
break;
+ case UPIO_DWAPB:
+ /* Save the LCR value so it can be re-written when a
+* Busy Detect interrupt occurs. */
+ if (save_offset == UART_LCR)
+ up-lcr = value;
+ writeb(value, up-port.membase + offset);
+ /* Read the IER to ensure any interrupt is cleared before
+* returning from ISR. */
+ if (save_offset == UART_TX || save_offset == UART_IER)
+ value = serial_in(up, UART_IER);
+ break;
+
default:
outb(value, up-port.iobase + offset);
}
@@ -2454,8 +2485,8 @@ int __init serial8250_start_console(stru
add_preferred_console(ttyS, line, options);
printk(Adding console on ttyS%d at %s 0x%lx (options '%s')\n,
- line, port-iotype == UPIO_MEM ? MMIO : I/O port,
- port-iotype == UPIO_MEM ? (unsigned long) port-mapbase :
+ line, port-iotype = UPIO_MEM ? MMIO : I/O port,
+ port-iotype = UPIO_MEM ? (unsigned long) port-mapbase :
(unsigned long) port-iobase, options);
if (!(serial8250_console.flags CON_ENABLED)) {
serial8250_console.flags = ~CON_PRINTBUFFER;
I've remembered why I decided not to fix it: this code only gets called
from 8250__eraly.c which can't handle anything beyond UPIO_MEM anyway.
WBR, Sergei
-
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] serial driver PMC MSP71xx, kernel linux-mips.git master
On Thu, 15 Feb 2007 13:26:29 -0600 Marc St-Jean [EMAIL PROTECTED] wrote: + status = *(volatile u32 *)up-port.private_data; It distresses me that this patch uses a variable which this patch doesn't initialise anywhere. It isn't complete. The sub-driver code whch sets up this field shuld be included in the patch, no? - 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] serial driver PMC MSP71xx, kernel linux-mips.git master
Hello.
Marc St-Jean wrote:
Fourth attempt at the serial driver patch for the PMC-Sierra MSP71xx device.
There are three different fixes:
1. Fix for DesignWare THRE errata
- Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
tree also fixes it. This patch needs to be applied on top of "mm" patch.
2. Fix for Busy Detect on LCR write
- Changed the ordering of test in serial8250_interrupt().
- Combined test for UPIO_DWAPB with UPIO_MEM in serial8250_start_console().
- Renamed new uart_8250_port member to private_data.
3. Workaround for interrupt/data concurrency issue
- No changes since last patch.
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..b309c4c 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
return inb(up->port.iobase + 1);
case UPIO_MEM:
+ case UPIO_DWAPB:
return readb(up->port.membase + offset);
case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
static void
serial_out(struct uart_8250_port *up, int offset, int value)
{
+ /* Save the offset before it's remapped */
+ int save_offset = offset;
offset = map_8250_out_reg(up, offset) << up->port.regshift;
switch (up->port.iotype) {
@@ -359,6 +362,18 @@ #endif
writeb(value, up->port.membase + offset);
break;
+ case UPIO_DWAPB:
+ /* Save the LCR value so it can be re-written when a
+* Busy Detect interrupt occurs. */
+ if (save_offset == UART_LCR)
+ up->lcr = value;
+ writeb(value, up->port.membase + offset);
+ /* Read the IER to ensure any interrupt is cleared before
+* returning from ISR. */
+ if ((save_offset == UART_TX || save_offset == UART_IER) &&
in_irq())
+ value = serial_in(up, UART_IER);
+ break;
+
default:
outb(value, up->port.iobase + offset);
}
@@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
#ifdef CONFIG_SERIAL_8250_AU1X00
case UPIO_AU:
#endif
+ case UPIO_DWAPB:
serial_out(up, offset, value);
(void)serial_in(up, UART_LCR); /* safe, no side-effects */
break;
@@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
handled = 1;
end = NULL;
+ } else if (up->port.iotype == UPIO_DWAPB &&
+ (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
Worth aligning this line with the opening paren of if... but's that's
nitpicking. :-)
+ /* The DesignWare APB UART has an Busy Detect (0x07)
+* interrupt meaning an LCR write attempt occured while
the
+* UART was busy. The interrupt must be cleared by
reading
+* the UART status register (USR) and the LCR
re-written. */
+ unsigned int status;
+ status = *(volatile u32 *)up->port.private_data;
+ serial_out(up, UART_LCR, up->lcr);
+
+ handled = 1;
+
+ end = NULL;
} else if (end == NULL)
end = l;
return 0;
Still, shouldn't you be doing this in serial8250_timeout() also?
What IRQ numbers this UART is using, BTW?
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf23813..bd9711a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -276,6 +277,7 @@ #define UPF_USR_MASK((__force upf_t) (
struct device *dev; /* parent device */
unsigned char hub6; /* this should be in
the 8250 driver */
unsigned char unused[3];
+ void*private_data; /* generic
platform data pointer */
One tab to many before *private_data...
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 3c8a6aa..1c5ed7d 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -38,6 +38,8 @@ #define UART_IIR_THRI 0x02 /* Transmitt
#define UART_IIR_RDI 0x04 /* Receiver data interrupt */
#define UART_IIR_RLSI 0x06 /* Receiver line status interrupt */
+#define UART_IIR_BUSY 0x07 /* DesignWare APB Busy Detect */
+
#define UART_FCR 2 /* Out: FIFO Control Register */
#define UART_FCR_ENABLE_FIFO 0x01 /* Enable the FIFO */
#define UART_FCR_CLEAR_RCVR 0x02 /* Clear the RCVR FIFO */
Oops, your mailer went and did it again. :-)
WBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo
Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master
Hello.
Marc St-Jean wrote:
Fourth attempt at the serial driver patch for the PMC-Sierra MSP71xx device.
There are three different fixes:
1. Fix for DesignWare THRE errata
- Dropped our fix since the 8250-uart-backup-timer.patch in the mm
tree also fixes it. This patch needs to be applied on top of mm patch.
2. Fix for Busy Detect on LCR write
- Changed the ordering of test in serial8250_interrupt().
- Combined test for UPIO_DWAPB with UPIO_MEM in serial8250_start_console().
- Renamed new uart_8250_port member to private_data.
3. Workaround for interrupt/data concurrency issue
- No changes since last patch.
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..b309c4c 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
return inb(up-port.iobase + 1);
case UPIO_MEM:
+ case UPIO_DWAPB:
return readb(up-port.membase + offset);
case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
static void
serial_out(struct uart_8250_port *up, int offset, int value)
{
+ /* Save the offset before it's remapped */
+ int save_offset = offset;
offset = map_8250_out_reg(up, offset) up-port.regshift;
switch (up-port.iotype) {
@@ -359,6 +362,18 @@ #endif
writeb(value, up-port.membase + offset);
break;
+ case UPIO_DWAPB:
+ /* Save the LCR value so it can be re-written when a
+* Busy Detect interrupt occurs. */
+ if (save_offset == UART_LCR)
+ up-lcr = value;
+ writeb(value, up-port.membase + offset);
+ /* Read the IER to ensure any interrupt is cleared before
+* returning from ISR. */
+ if ((save_offset == UART_TX || save_offset == UART_IER)
in_irq())
+ value = serial_in(up, UART_IER);
+ break;
+
default:
outb(value, up-port.iobase + offset);
}
@@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
#ifdef CONFIG_SERIAL_8250_AU1X00
case UPIO_AU:
#endif
+ case UPIO_DWAPB:
serial_out(up, offset, value);
(void)serial_in(up, UART_LCR); /* safe, no side-effects */
break;
@@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
handled = 1;
end = NULL;
+ } else if (up-port.iotype == UPIO_DWAPB
+ (iir UART_IIR_BUSY) == UART_IIR_BUSY) {
Worth aligning this line with the opening paren of if... but's that's
nitpicking. :-)
+ /* The DesignWare APB UART has an Busy Detect (0x07)
+* interrupt meaning an LCR write attempt occured while
the
+* UART was busy. The interrupt must be cleared by
reading
+* the UART status register (USR) and the LCR
re-written. */
+ unsigned int status;
+ status = *(volatile u32 *)up-port.private_data;
+ serial_out(up, UART_LCR, up-lcr);
+
+ handled = 1;
+
+ end = NULL;
} else if (end == NULL)
end = l;
return 0;
Still, shouldn't you be doing this in serial8250_timeout() also?
What IRQ numbers this UART is using, BTW?
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf23813..bd9711a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -276,6 +277,7 @@ #define UPF_USR_MASK((__force upf_t) (
struct device *dev; /* parent device */
unsigned char hub6; /* this should be in
the 8250 driver */
unsigned char unused[3];
+ void*private_data; /* generic
platform data pointer */
One tab to many before *private_data...
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 3c8a6aa..1c5ed7d 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -38,6 +38,8 @@ #define UART_IIR_THRI 0x02 /* Transmitt
#define UART_IIR_RDI 0x04 /* Receiver data interrupt */
#define UART_IIR_RLSI 0x06 /* Receiver line status interrupt */
+#define UART_IIR_BUSY 0x07 /* DesignWare APB Busy Detect */
+
#define UART_FCR 2 /* Out: FIFO Control Register */
#define UART_FCR_ENABLE_FIFO 0x01 /* Enable the FIFO */
#define UART_FCR_CLEAR_RCVR 0x02 /* Clear the RCVR FIFO */
Oops, your mailer went and did it again. :-)
WBR, Sergei
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at
Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master
> unsigned char hub6; /* this should be in > the 8250 driver */ > unsigned char unused[3]; > + void*data; /* generic > platform data pointer */ > }; Convention is "private_data" > > /* > diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h > index 3c8a6aa..b3550cc 100644 > --- a/include/linux/serial_reg.h > +++ b/include/linux/serial_reg.h > @@ -37,6 +37,7 @@ #define UART_IIR_MSI0x00 /* Modem stat > #define UART_IIR_THRI 0x02 /* Transmitter holding register > empty */ > #define UART_IIR_RDI0x04 /* Receiver data interrupt */ > #define UART_IIR_RLSI 0x06 /* Receiver line status interrupt > */ > +#define UART_IIR_BUSY0x07 /* DesignWare APB Busy Detect */ Please move this down a line to break it from "official" values and call it DESIGNWARE_UART_IIR_BUSY, so it is obviously designware specific. Otherwise looks much less invasive and messy - 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] serial driver PMC MSP71xx, kernel linux-mips.git master
unsigned char hub6; /* this should be in the 8250 driver */ unsigned char unused[3]; + void*data; /* generic platform data pointer */ }; Convention is private_data /* diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h index 3c8a6aa..b3550cc 100644 --- a/include/linux/serial_reg.h +++ b/include/linux/serial_reg.h @@ -37,6 +37,7 @@ #define UART_IIR_MSI0x00 /* Modem stat #define UART_IIR_THRI 0x02 /* Transmitter holding register empty */ #define UART_IIR_RDI0x04 /* Receiver data interrupt */ #define UART_IIR_RLSI 0x06 /* Receiver line status interrupt */ +#define UART_IIR_BUSY0x07 /* DesignWare APB Busy Detect */ Please move this down a line to break it from official values and call it DESIGNWARE_UART_IIR_BUSY, so it is obviously designware specific. Otherwise looks much less invasive and messy - 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] serial driver PMC MSP71xx, kernel linux-mips.git master
Marc St-Jean wrote:
Here is my second attempt at the serial driver patch for the
PMC-Sierra MSP71xx device.
There are three different fixes:
1. Fix for THRE errata
- I verified the UART_BUG_TXEN fix does not help with this erratum.
- I left our current fix in until I get our platform booting on
2.6.20-rc4 to try the mm tree "8250-uart-backup-timer.patch".
Feel free to ignore for now.
2. Fix for Busy Detect on LCR write
- Moved to new UPIO_DWAPB iotype. Because the new type is a memory
mapped device and there are several tests for UPIO_MEM, this involved
updating serial_core.c and 8250_early.c in addition to 8250.c.
- I tried implementing this totally in serial_in as suggested, but
it can't be done because of bit overlap between UART_IIR_NO_INT and
UART_IIR_BUSY. Also there is no way to set the interrupt "handled = 1"
from serial_in.
3. Workaround for interrupt/data concurrency issue
- Moved to new UPIO_DWAPB iotype.
Index: linux_2_6/drivers/serial/8250.c
===
RCS file: linux_2_6/drivers/serial/8250.c,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 8250.c
--- linux_2_6/drivers/serial/8250.c 19 Oct 2006 21:00:58 - 1.1.1.7
+++ linux_2_6/drivers/serial/8250.c 24 Jan 2007 23:55:27 -
[...]
@@ -333,6 +334,8 @@
static void
serial_out(struct uart_8250_port *up, int offset, int value)
Your patch is clearly garbled again, something added an extra space to all
lines stating with space... :-/
{
+ /* Save the offset before it's remapped */
+ int save_offset = offset;
Is there real need to save this? What regshift equals for this UART?
offset = map_8250_out_reg(up, offset) << up->port.regshift;
switch (up->port.iotype) {
@@ -359,6 +362,19 @@
writeb(value, up->port.membase + offset);
break;
+ case UPIO_DWAPB:
+ /* Save the LCR value so it can be re-written when a
+* Busy Detect interrupt occurs. */
+ if (save_offset == UART_LCR)
+ up->lcr = value;
+ writeb(value, up->port.membase + offset);
+ /* Read the IER to ensure any interrupt is cleared before
+* returning from ISR. */
+ if ((save_offset == UART_TX || save_offset == UART_IER) &&
Not sure how an IER read ensures that...
+ in_irq())
I'd suggest to either indent this line right (start below 2ns paren of if
stmt) or keep on the same line.
+ value = serial_in(up, UART_IER);
+ break;
+
default:
outb(value, up->port.iobase + offset);
}
@@ -1016,6 +1032,17 @@
up->bugs |= UART_BUG_NOMSR;
#endif
+ /* Workaround:
+* The DesignWare SoC UART part has a bug for all
+* versions before 3.03a (2005-07-18)
+* In brief, this is a non-standard 16550 in that the THRE interrupt
+* will not re-assert itself simply by disabling and re-enabling the
+* THRI bit in the IER, it is only re-enabled if a character is actually
+* sent out.
+*/
+ if( up->port.flags & UPF_DW_THRE_BUG )
+ up->bugs |= UART_BUG_DWTHRE;
+
serial_outp(up, UART_LCR, save_lcr);
if (up->capabilities != uart_config[up->port.type].flags) {
@@ -1141,6 +1168,12 @@
iir = serial_in(up, UART_IIR);
if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT)
transmit_chars(up);
+ } else if (up->bugs & UART_BUG_DWTHRE) {
+ unsigned char lsr, iir;
+ lsr = serial_in(up, UART_LSR);
+ iir = serial_in(up, UART_IIR);
+ if (lsr & UART_LSR_THRE)
Why read IIR if you don't check it?
@@ -2352,9 +2402,12 @@
add_preferred_console("ttyS", line, options);
printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
- line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
- port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
- (unsigned long) port->iobase, options);
+ line,
+ (port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
+ ? "MMIO" : "I/O port",
+ (port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
+ ? (unsigned long) port->mapbase : (unsigned long)
port->iobase,
+ options);
if (!(serial8250_console.flags & CON_ENABLED)) {
serial8250_console.flags &= ~CON_PRINTBUFFER;
register_console(_console);
[...]
Index: linux_2_6/drivers/serial/8250_early.c
===
RCS file: linux_2_6/drivers/serial/8250_early.c,v
retrieving revision 1.1.1.3
diff
Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master
Marc St-Jean wrote:
Here is my second attempt at the serial driver patch for the
PMC-Sierra MSP71xx device.
There are three different fixes:
1. Fix for THRE errata
- I verified the UART_BUG_TXEN fix does not help with this erratum.
- I left our current fix in until I get our platform booting on
2.6.20-rc4 to try the mm tree 8250-uart-backup-timer.patch.
Feel free to ignore for now.
2. Fix for Busy Detect on LCR write
- Moved to new UPIO_DWAPB iotype. Because the new type is a memory
mapped device and there are several tests for UPIO_MEM, this involved
updating serial_core.c and 8250_early.c in addition to 8250.c.
- I tried implementing this totally in serial_in as suggested, but
it can't be done because of bit overlap between UART_IIR_NO_INT and
UART_IIR_BUSY. Also there is no way to set the interrupt handled = 1
from serial_in.
3. Workaround for interrupt/data concurrency issue
- Moved to new UPIO_DWAPB iotype.
Index: linux_2_6/drivers/serial/8250.c
===
RCS file: linux_2_6/drivers/serial/8250.c,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 8250.c
--- linux_2_6/drivers/serial/8250.c 19 Oct 2006 21:00:58 - 1.1.1.7
+++ linux_2_6/drivers/serial/8250.c 24 Jan 2007 23:55:27 -
[...]
@@ -333,6 +334,8 @@
static void
serial_out(struct uart_8250_port *up, int offset, int value)
Your patch is clearly garbled again, something added an extra space to all
lines stating with space... :-/
{
+ /* Save the offset before it's remapped */
+ int save_offset = offset;
Is there real need to save this? What regshift equals for this UART?
offset = map_8250_out_reg(up, offset) up-port.regshift;
switch (up-port.iotype) {
@@ -359,6 +362,19 @@
writeb(value, up-port.membase + offset);
break;
+ case UPIO_DWAPB:
+ /* Save the LCR value so it can be re-written when a
+* Busy Detect interrupt occurs. */
+ if (save_offset == UART_LCR)
+ up-lcr = value;
+ writeb(value, up-port.membase + offset);
+ /* Read the IER to ensure any interrupt is cleared before
+* returning from ISR. */
+ if ((save_offset == UART_TX || save_offset == UART_IER)
Not sure how an IER read ensures that...
+ in_irq())
I'd suggest to either indent this line right (start below 2ns paren of if
stmt) or keep on the same line.
+ value = serial_in(up, UART_IER);
+ break;
+
default:
outb(value, up-port.iobase + offset);
}
@@ -1016,6 +1032,17 @@
up-bugs |= UART_BUG_NOMSR;
#endif
+ /* Workaround:
+* The DesignWare SoC UART part has a bug for all
+* versions before 3.03a (2005-07-18)
+* In brief, this is a non-standard 16550 in that the THRE interrupt
+* will not re-assert itself simply by disabling and re-enabling the
+* THRI bit in the IER, it is only re-enabled if a character is actually
+* sent out.
+*/
+ if( up-port.flags UPF_DW_THRE_BUG )
+ up-bugs |= UART_BUG_DWTHRE;
+
serial_outp(up, UART_LCR, save_lcr);
if (up-capabilities != uart_config[up-port.type].flags) {
@@ -1141,6 +1168,12 @@
iir = serial_in(up, UART_IIR);
if (lsr UART_LSR_TEMT iir UART_IIR_NO_INT)
transmit_chars(up);
+ } else if (up-bugs UART_BUG_DWTHRE) {
+ unsigned char lsr, iir;
+ lsr = serial_in(up, UART_LSR);
+ iir = serial_in(up, UART_IIR);
+ if (lsr UART_LSR_THRE)
Why read IIR if you don't check it?
@@ -2352,9 +2402,12 @@
add_preferred_console(ttyS, line, options);
printk(Adding console on ttyS%d at %s 0x%lx (options '%s')\n,
- line, port-iotype == UPIO_MEM ? MMIO : I/O port,
- port-iotype == UPIO_MEM ? (unsigned long) port-mapbase :
- (unsigned long) port-iobase, options);
+ line,
+ (port-iotype == UPIO_MEM || port-iotype == UPIO_DWAPB)
+ ? MMIO : I/O port,
+ (port-iotype == UPIO_MEM || port-iotype == UPIO_DWAPB)
+ ? (unsigned long) port-mapbase : (unsigned long)
port-iobase,
+ options);
if (!(serial8250_console.flags CON_ENABLED)) {
serial8250_console.flags = ~CON_PRINTBUFFER;
register_console(serial8250_console);
[...]
Index: linux_2_6/drivers/serial/8250_early.c
===
RCS file: linux_2_6/drivers/serial/8250_early.c,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 8250_early.c
---
Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master
> There are three different fixes:
> 1. Fix for THRE errata
That should be handled anyway. The current code actually spots this and
uses a backup timer for dodgy UARTS
> 2. Fix for Busy Detect on LCR write
> 3. Workaround for interrupt/data concurrency issue
> case UPIO_MEM:
> +#ifdef CONFIG_PMC_MSP
> + /* Save the LCR value so it can be re-written when a
> + * Busy Detect interrupt occurs. */
> + if (dwapb_offset == UART_LCR)
> + up->dwapb_lcr = value;
> +#endif
> writeb(value, up->port.membase + offset);
> +#ifdef CONFIG_PMC_MSP
> + /* Re-read the IER to ensure any interrupt disabling has
> + * completed before proceeding with ISR. */
> + if (dwapb_offset == UART_IER)
> + value = serial_in(up, dwapb_offset);
> +#endif
> break;
This I would hope you can hide in the platform specific
serial_in/serial_out functions. If you write the UART_LCR save it in
serial_out(), if you read IER etc.
> +#ifdef CONFIG_PMC_MSP
> + } else if ((iir & UART_IER_BUSY) == UART_IER_BUSY) {
> + /*
> + * The MSP (DesignWare APB UART) serial subsystem has a
> + * non-standard interrupt condition (0x7) which means
> + * that the LCR was written while the UART was busy, so
> + * the LCR was not actually written. It is cleared by
> + * reading the special non-standard extended UART status
> + * register.
Ditto... spot this case and whack it in your serial methods.
And we might want to add a void * for board specific insanity to the 8250
structures if we really have to so you can hang your brain damage
privately off that ?
-
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] serial driver PMC MSP71xx, kernel linux-mips.git master
There are three different fixes:
1. Fix for THRE errata
That should be handled anyway. The current code actually spots this and
uses a backup timer for dodgy UARTS
2. Fix for Busy Detect on LCR write
3. Workaround for interrupt/data concurrency issue
case UPIO_MEM:
+#ifdef CONFIG_PMC_MSP
+ /* Save the LCR value so it can be re-written when a
+ * Busy Detect interrupt occurs. */
+ if (dwapb_offset == UART_LCR)
+ up-dwapb_lcr = value;
+#endif
writeb(value, up-port.membase + offset);
+#ifdef CONFIG_PMC_MSP
+ /* Re-read the IER to ensure any interrupt disabling has
+ * completed before proceeding with ISR. */
+ if (dwapb_offset == UART_IER)
+ value = serial_in(up, dwapb_offset);
+#endif
break;
This I would hope you can hide in the platform specific
serial_in/serial_out functions. If you write the UART_LCR save it in
serial_out(), if you read IER etc.
+#ifdef CONFIG_PMC_MSP
+ } else if ((iir UART_IER_BUSY) == UART_IER_BUSY) {
+ /*
+ * The MSP (DesignWare APB UART) serial subsystem has a
+ * non-standard interrupt condition (0x7) which means
+ * that the LCR was written while the UART was busy, so
+ * the LCR was not actually written. It is cleared by
+ * reading the special non-standard extended UART status
+ * register.
Ditto... spot this case and whack it in your serial methods.
And we might want to add a void * for board specific insanity to the 8250
structures if we really have to so you can hang your brain damage
privately off that ?
-
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/
