| Message ID | 1408450488-4083-4-git-send-email-frans.klaver@xsens.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 19, 2014 at 02:14:47PM +0200, Frans Klaver wrote: > At 3.6Mbaud, with slightly over 2Mbit/s data coming in, we see 1600 uart > rx buffer overflows within 30 seconds. Threading the interrupt handling reduces > this to about 170 overflows in 10 minutes. Can you try Sebastian Siewior's patches for 8250_omap and 8250 dma support ? That should help you a lot. > In practice this therefore reduces the need for hardware flow control, > meaning the sending side doesn't have to buffer as much either. > > Signed-off-by: Frans Klaver <frans.klaver@xsens.com> > --- > drivers/tty/serial/omap-serial.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c > index 14a0167..57664b9 100644 > --- a/drivers/tty/serial/omap-serial.c > +++ b/drivers/tty/serial/omap-serial.c > @@ -575,6 +575,21 @@ static void serial_omap_rdi(struct uart_omap_port *up, unsigned int lsr) > } > > /** > + * serial_omap_fast_irq() - schedule interrupt handling > + */ > +static irqreturn_t serial_omap_fast_irq(int irq, void *dev_id) > +{ > + struct uart_omap_port *up = dev_id; > + unsigned int iir = serial_in(up, UART_IIR); > + > + if (iir & UART_IIR_NO_INT) > + return IRQ_NONE; > + > + disable_irq_nosync(up->port.irq); NAK. Either use IRQF_ONESHOT or actually mask the IRQs at the device's registers (basically clearing IER).
On Tue, Aug 19, 2014 at 01:57:02PM -0500, Felipe Balbi wrote: > On Tue, Aug 19, 2014 at 02:14:47PM +0200, Frans Klaver wrote: > > At 3.6Mbaud, with slightly over 2Mbit/s data coming in, we see 1600 uart > > rx buffer overflows within 30 seconds. Threading the interrupt handling reduces > > this to about 170 overflows in 10 minutes. > > Can you try Sebastian Siewior's patches for 8250_omap and 8250 dma > support ? That should help you a lot. I'll have a look at that series. Thanks for pointing it out. > > drivers/tty/serial/omap-serial.c | 31 ++++++++++++++++++++++++------- > > 1 file changed, 24 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c > > index 14a0167..57664b9 100644 > > --- a/drivers/tty/serial/omap-serial.c > > +++ b/drivers/tty/serial/omap-serial.c > > @@ -575,6 +575,21 @@ static void serial_omap_rdi(struct uart_omap_port *up, unsigned int lsr) > > } > > > > /** > > + * serial_omap_fast_irq() - schedule interrupt handling > > + */ > > +static irqreturn_t serial_omap_fast_irq(int irq, void *dev_id) > > +{ > > + struct uart_omap_port *up = dev_id; > > + unsigned int iir = serial_in(up, UART_IIR); > > + > > + if (iir & UART_IIR_NO_INT) > > + return IRQ_NONE; > > + > > + disable_irq_nosync(up->port.irq); > > NAK. Either use IRQF_ONESHOT or actually mask the IRQs at the device's > registers (basically clearing IER). Given the work that's been done on the 8250 based driver, what are the short-term chances omap-serial will be dropped? I'd be happy to improve here if it makes sense to do so. If the 8250 based driver is going to replace omap-serial anyway on the short term, I don't see the point of further developing this patch. The others would still make sense in my opinion. Thanks, Frans
On Wed, Aug 20, 2014 at 08:40:28AM +0200, Frans Klaver wrote: > On Tue, Aug 19, 2014 at 01:57:02PM -0500, Felipe Balbi wrote: > > On Tue, Aug 19, 2014 at 02:14:47PM +0200, Frans Klaver wrote: > > > At 3.6Mbaud, with slightly over 2Mbit/s data coming in, we see 1600 uart > > > rx buffer overflows within 30 seconds. Threading the interrupt handling reduces > > > this to about 170 overflows in 10 minutes. > > > > Can you try Sebastian Siewior's patches for 8250_omap and 8250 dma > > support ? That should help you a lot. > > I'll have a look at that series. Thanks for pointing it out. > > > > drivers/tty/serial/omap-serial.c | 31 ++++++++++++++++++++++++------- > > > 1 file changed, 24 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c > > > index 14a0167..57664b9 100644 > > > --- a/drivers/tty/serial/omap-serial.c > > > +++ b/drivers/tty/serial/omap-serial.c > > > @@ -575,6 +575,21 @@ static void serial_omap_rdi(struct uart_omap_port *up, unsigned int lsr) > > > } > > > > > > /** > > > + * serial_omap_fast_irq() - schedule interrupt handling > > > + */ > > > +static irqreturn_t serial_omap_fast_irq(int irq, void *dev_id) > > > +{ > > > + struct uart_omap_port *up = dev_id; > > > + unsigned int iir = serial_in(up, UART_IIR); > > > + > > > + if (iir & UART_IIR_NO_INT) > > > + return IRQ_NONE; > > > + > > > + disable_irq_nosync(up->port.irq); > > > > NAK. Either use IRQF_ONESHOT or actually mask the IRQs at the device's > > registers (basically clearing IER). > > Given the work that's been done on the 8250 based driver, what are the > short-term chances omap-serial will be dropped? I'd be happy to improve > here if it makes sense to do so. If the 8250 based driver is going to > replace omap-serial anyway on the short term, I don't see the point of > further developing this patch. The others would still make sense in my > opinion. if we find a way to maintain the same ttyO* naming scheme and make it coexist with ttyS* naming, then we could drop as soon as 8250_omap is ready for prime time.
On Wed, Aug 20, 2014 at 11:06:05AM -0500, Felipe Balbi wrote: > On Wed, Aug 20, 2014 at 08:40:28AM +0200, Frans Klaver wrote: > > On Tue, Aug 19, 2014 at 01:57:02PM -0500, Felipe Balbi wrote: > > > On Tue, Aug 19, 2014 at 02:14:47PM +0200, Frans Klaver wrote: > > > > At 3.6Mbaud, with slightly over 2Mbit/s data coming in, we see 1600 uart > > > > rx buffer overflows within 30 seconds. Threading the interrupt handling reduces > > > > this to about 170 overflows in 10 minutes. > > > > > > Can you try Sebastian Siewior's patches for 8250_omap and 8250 dma > > > support ? That should help you a lot. > > > > I'll have a look at that series. Thanks for pointing it out. I have had a look at Sebastian's series, but I'm not convinced yet that we should use it. So far the serial console seems to be missing data every now and then. I haven't gotten around to testing our high traffic application yet. > > > > drivers/tty/serial/omap-serial.c | 31 ++++++++++++++++++++++++------- > > > > 1 file changed, 24 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c > > > > index 14a0167..57664b9 100644 > > > > --- a/drivers/tty/serial/omap-serial.c > > > > +++ b/drivers/tty/serial/omap-serial.c > > > > @@ -575,6 +575,21 @@ static void serial_omap_rdi(struct uart_omap_port *up, unsigned int lsr) > > > > } > > > > > > > > /** > > > > + * serial_omap_fast_irq() - schedule interrupt handling > > > > + */ > > > > +static irqreturn_t serial_omap_fast_irq(int irq, void *dev_id) > > > > +{ > > > > + struct uart_omap_port *up = dev_id; > > > > + unsigned int iir = serial_in(up, UART_IIR); > > > > + > > > > + if (iir & UART_IIR_NO_INT) > > > > + return IRQ_NONE; > > > > + > > > > + disable_irq_nosync(up->port.irq); > > > > > > NAK. Either use IRQF_ONESHOT or actually mask the IRQs at the device's > > > registers (basically clearing IER). > > > > Given the work that's been done on the 8250 based driver, what are the > > short-term chances omap-serial will be dropped? I'd be happy to improve > > here if it makes sense to do so. If the 8250 based driver is going to > > replace omap-serial anyway on the short term, I don't see the point of > > further developing this patch. The others would still make sense in my > > opinion. > > if we find a way to maintain the same ttyO* naming scheme and make it > coexist with ttyS* naming, then we could drop as soon as 8250_omap is > ready for prime time. With that in mind, for now I'll be betting on two horses. I'll cook up a new version using oneshot. Thanks, Frans
Hi, On Thu, Aug 21, 2014 at 11:41:17PM +0200, Frans Klaver wrote: > On Wed, Aug 20, 2014 at 11:06:05AM -0500, Felipe Balbi wrote: > > On Wed, Aug 20, 2014 at 08:40:28AM +0200, Frans Klaver wrote: > > > On Tue, Aug 19, 2014 at 01:57:02PM -0500, Felipe Balbi wrote: > > > > On Tue, Aug 19, 2014 at 02:14:47PM +0200, Frans Klaver wrote: > > > > > At 3.6Mbaud, with slightly over 2Mbit/s data coming in, we see 1600 uart > > > > > rx buffer overflows within 30 seconds. Threading the interrupt handling reduces > > > > > this to about 170 overflows in 10 minutes. > > > > > > > > Can you try Sebastian Siewior's patches for 8250_omap and 8250 dma > > > > support ? That should help you a lot. > > > > > > I'll have a look at that series. Thanks for pointing it out. > > I have had a look at Sebastian's series, but I'm not convinced > yet that we should use it. So far the serial console seems to be > missing data every now and then. I haven't gotten around to testing our > high traffic application yet. as any new driver, there are bugs to be fixed. How about reporting the ones you found together with a way of reproducing them so they can be fixed ? cheers
On August 21, 2014 11:48:40 PM CEST, Felipe Balbi <balbi@ti.com> wrote: >Hi, > >On Thu, Aug 21, 2014 at 11:41:17PM +0200, Frans Klaver wrote: >> On Wed, Aug 20, 2014 at 11:06:05AM -0500, Felipe Balbi wrote: >> > On Wed, Aug 20, 2014 at 08:40:28AM +0200, Frans Klaver wrote: >> > > On Tue, Aug 19, 2014 at 01:57:02PM -0500, Felipe Balbi wrote: >> > > > On Tue, Aug 19, 2014 at 02:14:47PM +0200, Frans Klaver wrote: >> > > > > At 3.6Mbaud, with slightly over 2Mbit/s data coming in, we >see 1600 uart >> > > > > rx buffer overflows within 30 seconds. Threading the >interrupt handling reduces >> > > > > this to about 170 overflows in 10 minutes. >> > > > >> > > > Can you try Sebastian Siewior's patches for 8250_omap and 8250 >dma >> > > > support ? That should help you a lot. >> > > >> > > I'll have a look at that series. Thanks for pointing it out. >> >> I have had a look at Sebastian's series, but I'm not convinced >> yet that we should use it. So far the serial console seems to be >> missing data every now and then. I haven't gotten around to testing >our >> high traffic application yet. > >as any new driver, there are bugs to be fixed. How about reporting the >ones you found together with a way of reproducing them so they can be >fixed ? Yes, definitely. Cheers, Frans
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index 14a0167..57664b9 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -575,6 +575,21 @@ static void serial_omap_rdi(struct uart_omap_port *up, unsigned int lsr) } /** + * serial_omap_fast_irq() - schedule interrupt handling + */ +static irqreturn_t serial_omap_fast_irq(int irq, void *dev_id) +{ + struct uart_omap_port *up = dev_id; + unsigned int iir = serial_in(up, UART_IIR); + + if (iir & UART_IIR_NO_INT) + return IRQ_NONE; + + disable_irq_nosync(up->port.irq); + return IRQ_WAKE_THREAD; +} + +/** * serial_omap_irq() - This handles the interrupt from one port * @irq: uart port irq number * @dev_id: uart port info @@ -584,7 +599,6 @@ static irqreturn_t serial_omap_irq(int irq, void *dev_id) struct uart_omap_port *up = dev_id; unsigned int iir, lsr; unsigned int type; - irqreturn_t ret = IRQ_NONE; int max_count = 256; spin_lock(&up->port.lock); @@ -595,7 +609,6 @@ static irqreturn_t serial_omap_irq(int irq, void *dev_id) if (iir & UART_IIR_NO_INT) break; - ret = IRQ_HANDLED; lsr = serial_in(up, UART_LSR); /* extract IRQ type from IIR register */ @@ -630,11 +643,13 @@ static irqreturn_t serial_omap_irq(int irq, void *dev_id) tty_flip_buffer_push(&up->port.state->port); + enable_irq(up->port.irq); + pm_runtime_mark_last_busy(up->dev); pm_runtime_put_autosuspend(up->dev); up->port_activity = jiffies; - return ret; + return IRQ_HANDLED; } static unsigned int serial_omap_tx_empty(struct uart_port *port) @@ -731,15 +746,17 @@ static int serial_omap_startup(struct uart_port *port) /* * Allocate the IRQ */ - retval = request_irq(up->port.irq, serial_omap_irq, up->port.irqflags, - up->name, up); + retval = request_threaded_irq(up->port.irq, serial_omap_fast_irq, + serial_omap_irq, up->port.irqflags, + up->name, up); if (retval) return retval; /* Optional wake-up IRQ */ if (up->wakeirq) { - retval = request_irq(up->wakeirq, serial_omap_irq, - up->port.irqflags, up->name, up); + retval = request_threaded_irq(up->wakeirq, serial_omap_fast_irq, + serial_omap_irq, up->port.irqflags, + up->name, up); if (retval) { free_irq(up->port.irq, up); return retval;
At 3.6Mbaud, with slightly over 2Mbit/s data coming in, we see 1600 uart rx buffer overflows within 30 seconds. Threading the interrupt handling reduces this to about 170 overflows in 10 minutes. In practice this therefore reduces the need for hardware flow control, meaning the sending side doesn't have to buffer as much either. Signed-off-by: Frans Klaver <frans.klaver@xsens.com> --- drivers/tty/serial/omap-serial.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)