* [PATCH] serial: omap: fix the overrun case
@ 2012-09-21 10:22 Shubhrajyoti D
2012-09-21 11:00 ` Felipe Balbi
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Shubhrajyoti D @ 2012-09-21 10:22 UTC (permalink / raw)
To: linux-arm-kernel
Overrun also causes an internal flag to be set, which disables further
reception. Before the next frame can
be received, the MPU must:
? Reset the RX FIFO.
? clear the internal flag.
In the uart mode a dummy read is needed. Add the same.
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
- functional testing on omap4sdp
- Verified idle and suspend path hits off on beagle.
drivers/tty/serial/omap-serial.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index a0d4460..bc22a2b 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
{
unsigned int flag;
+ unsigned char ch = 0;
+
+ if (!(lsr & UART_LSR_BRK_ERROR_BITS))
+ return;
+
+ if (likely(lsr & UART_LSR_DR))
+ ch = serial_in(up, UART_RX);
up->port.icount.rx++;
flag = TTY_NORMAL;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] serial: omap: fix the overrun case
2012-09-21 10:22 [PATCH] serial: omap: fix the overrun case Shubhrajyoti D
@ 2012-09-21 11:00 ` Felipe Balbi
2012-09-21 11:16 ` Shubhrajyoti
2012-09-21 11:39 ` Poddar, Sourav
2012-09-21 11:18 ` Poddar, Sourav
2012-09-21 14:18 ` Kevin Hilman
2 siblings, 2 replies; 10+ messages in thread
From: Felipe Balbi @ 2012-09-21 11:00 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 21, 2012 at 03:52:56PM +0530, Shubhrajyoti D wrote:
> Overrun also causes an internal flag to be set, which disables further
> reception. Before the next frame can
> be received, the MPU must:
> ? Reset the RX FIFO.
> ? clear the internal flag.
>
> In the uart mode a dummy read is needed. Add the same.
Very nice patch but I think commit log can be a bit more verbose.
Please make the problem a little clearer. Why do we even get that
interrupt fired if BRK_ERROR_BITS aren't set ?
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> - functional testing on omap4sdp
> - Verified idle and suspend path hits off on beagle.
>
> drivers/tty/serial/omap-serial.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index a0d4460..bc22a2b 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
> static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
> {
> unsigned int flag;
> + unsigned char ch = 0;
> +
> + if (!(lsr & UART_LSR_BRK_ERROR_BITS))
> + return;
> +
> + if (likely(lsr & UART_LSR_DR))
> + ch = serial_in(up, UART_RX);
Maybe add a comment before this condition stating why this character
read is necessary ?
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120921/0dfb7196/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] serial: omap: fix the overrun case
2012-09-21 11:00 ` Felipe Balbi
@ 2012-09-21 11:16 ` Shubhrajyoti
2012-09-21 11:35 ` Felipe Balbi
2012-09-21 11:39 ` Poddar, Sourav
1 sibling, 1 reply; 10+ messages in thread
From: Shubhrajyoti @ 2012-09-21 11:16 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 21 September 2012 04:30 PM, Felipe Balbi wrote:
> On Fri, Sep 21, 2012 at 03:52:56PM +0530, Shubhrajyoti D wrote:
>> Overrun also causes an internal flag to be set, which disables further
>> reception. Before the next frame can
>> be received, the MPU must:
>> ? Reset the RX FIFO.
>> ? clear the internal flag.
>>
>> In the uart mode a dummy read is needed. Add the same.
> Very nice patch but I think commit log can be a bit more verbose.
ok
> Please make the problem a little clearer. Why do we even get that
> interrupt fired if BRK_ERROR_BITS aren't set ?
I did not get this point.
it it is ! BRK_ERROR_BITS I return.
If it is and there is data in the fifo then a read is done.
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>> ---
>> - functional testing on omap4sdp
>> - Verified idle and suspend path hits off on beagle.
>>
>> drivers/tty/serial/omap-serial.c | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index a0d4460..bc22a2b 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
>> static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
>> {
>> unsigned int flag;
>> + unsigned char ch = 0;
>> +
>> + if (!(lsr & UART_LSR_BRK_ERROR_BITS))
>> + return;
>> +
>> + if (likely(lsr & UART_LSR_DR))
>> + ch = serial_in(up, UART_RX);
> Maybe add a comment before this condition stating why this character
> read is necessary ?
OK.
>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] serial: omap: fix the overrun case
2012-09-21 11:16 ` Shubhrajyoti
@ 2012-09-21 11:35 ` Felipe Balbi
2012-09-21 11:48 ` Shubhrajyoti Datta
0 siblings, 1 reply; 10+ messages in thread
From: Felipe Balbi @ 2012-09-21 11:35 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 21, 2012 at 04:46:52PM +0530, Shubhrajyoti wrote:
> On Friday 21 September 2012 04:30 PM, Felipe Balbi wrote:
> > On Fri, Sep 21, 2012 at 03:52:56PM +0530, Shubhrajyoti D wrote:
> >> Overrun also causes an internal flag to be set, which disables further
> >> reception. Before the next frame can
> >> be received, the MPU must:
> >> ? Reset the RX FIFO.
> >> ? clear the internal flag.
> >>
> >> In the uart mode a dummy read is needed. Add the same.
> > Very nice patch but I think commit log can be a bit more verbose.
> ok
> > Please make the problem a little clearer. Why do we even get that
> > interrupt fired if BRK_ERROR_BITS aren't set ?
> I did not get this point.
>
> it it is ! BRK_ERROR_BITS I return.
That's what I mean. rlsi handler is basically taking care of those
bits... So how come we get RLSI IRQ when those bits aren't set ?
Meaning, you shouldn't ever need that check, right ? Ideally, whenever
that handler is called, it's because BRK_ERROR_BITS are set.
Maybe add a WARN_ONCE() kinda thing just to see if that will ever really
happen ??
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120921/2690f140/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] serial: omap: fix the overrun case
2012-09-21 11:35 ` Felipe Balbi
@ 2012-09-21 11:48 ` Shubhrajyoti Datta
0 siblings, 0 replies; 10+ messages in thread
From: Shubhrajyoti Datta @ 2012-09-21 11:48 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 21, 2012 at 5:05 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Fri, Sep 21, 2012 at 04:46:52PM +0530, Shubhrajyoti wrote:
>> On Friday 21 September 2012 04:30 PM, Felipe Balbi wrote:
>> > On Fri, Sep 21, 2012 at 03:52:56PM +0530, Shubhrajyoti D wrote:
[...]
>> it it is ! BRK_ERROR_BITS I return.
>
> That's what I mean. rlsi handler is basically taking care of those
> bits... So how come we get RLSI IRQ when those bits aren't set ?
>
> Meaning, you shouldn't ever need that check, right ? Ideally, whenever
> that handler is called, it's because BRK_ERROR_BITS are set.
>
> Maybe add a WARN_ONCE() kinda thing just to see if that will ever really
> happen ??
hmm yes. will get back.
>
> --
> balbi
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] serial: omap: fix the overrun case
2012-09-21 11:00 ` Felipe Balbi
2012-09-21 11:16 ` Shubhrajyoti
@ 2012-09-21 11:39 ` Poddar, Sourav
2012-09-21 12:07 ` Felipe Balbi
1 sibling, 1 reply; 10+ messages in thread
From: Poddar, Sourav @ 2012-09-21 11:39 UTC (permalink / raw)
To: linux-arm-kernel
Hi Felipe,
On Fri, Sep 21, 2012 at 4:30 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Fri, Sep 21, 2012 at 03:52:56PM +0530, Shubhrajyoti D wrote:
>> Overrun also causes an internal flag to be set, which disables further
>> reception. Before the next frame can
>> be received, the MPU must:
>> ? Reset the RX FIFO.
>> ? clear the internal flag.
>>
>> In the uart mode a dummy read is needed. Add the same.
>
> Very nice patch but I think commit log can be a bit more verbose.
>
> Please make the problem a little clearer. Why do we even get that
> interrupt fired if BRK_ERROR_BITS aren't set ?
>
According to LSR registers, there are few other bits like RX_FIFO_E( atleast 1
character in RX_FIFO or TX FIFO Empty), which might be the cause of an
interrupt. ?
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>> ---
>> - functional testing on omap4sdp
>> - Verified idle and suspend path hits off on beagle.
>>
>> drivers/tty/serial/omap-serial.c | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index a0d4460..bc22a2b 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
>> static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
>> {
>> unsigned int flag;
>> + unsigned char ch = 0;
>> +
>> + if (!(lsr & UART_LSR_BRK_ERROR_BITS))
>> + return;
>> +
>> + if (likely(lsr & UART_LSR_DR))
>> + ch = serial_in(up, UART_RX);
>
> Maybe add a comment before this condition stating why this character
> read is necessary ?
>
> --
> balbi
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] serial: omap: fix the overrun case
2012-09-21 11:39 ` Poddar, Sourav
@ 2012-09-21 12:07 ` Felipe Balbi
0 siblings, 0 replies; 10+ messages in thread
From: Felipe Balbi @ 2012-09-21 12:07 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 21, 2012 at 05:09:56PM +0530, Poddar, Sourav wrote:
> Hi Felipe,
>
> On Fri, Sep 21, 2012 at 4:30 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Fri, Sep 21, 2012 at 03:52:56PM +0530, Shubhrajyoti D wrote:
> >> Overrun also causes an internal flag to be set, which disables further
> >> reception. Before the next frame can
> >> be received, the MPU must:
> >> ? Reset the RX FIFO.
> >> ? clear the internal flag.
> >>
> >> In the uart mode a dummy read is needed. Add the same.
> >
> > Very nice patch but I think commit log can be a bit more verbose.
> >
> > Please make the problem a little clearer. Why do we even get that
> > interrupt fired if BRK_ERROR_BITS aren't set ?
> >
> According to LSR registers, there are few other bits like RX_FIFO_E( atleast 1
> character in RX_FIFO or TX FIFO Empty), which might be the cause of an
> interrupt. ?
right. In that case, is it really correct to just return if
BRK_ERROR_BITS aren't set ? IRQ line will not toggle and we just might
end up with an IRQ storm, right ?
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120921/902c7fce/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] serial: omap: fix the overrun case
2012-09-21 10:22 [PATCH] serial: omap: fix the overrun case Shubhrajyoti D
2012-09-21 11:00 ` Felipe Balbi
@ 2012-09-21 11:18 ` Poddar, Sourav
2012-09-21 14:18 ` Kevin Hilman
2 siblings, 0 replies; 10+ messages in thread
From: Poddar, Sourav @ 2012-09-21 11:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Fri, Sep 21, 2012 at 3:52 PM, Shubhrajyoti D <shubhrajyoti@ti.com> wrote:
> Overrun also causes an internal flag to be set, which disables further
> reception. Before the next frame can
> be received, the MPU must:
> ? Reset the RX FIFO.
> ? clear the internal flag.
>
> In the uart mode a dummy read is needed. Add the same.
>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> - functional testing on omap4sdp
> - Verified idle and suspend path hits off on beagle.
>
> drivers/tty/serial/omap-serial.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index a0d4460..bc22a2b 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
> static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
> {
> unsigned int flag;
> + unsigned char ch = 0;
> +
> + if (!(lsr & UART_LSR_BRK_ERROR_BITS))
By using this flag, you are trying to take into account not just the
overrun case but also
frame, parity and break condition case as the flag is the OR of all these.
I suppose the commit log should reflect this.
> + return;
> +
> + if (likely(lsr & UART_LSR_DR))
> + ch = serial_in(up, UART_RX);
>
> up->port.icount.rx++;
> flag = TTY_NORMAL;
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] serial: omap: fix the overrun case
2012-09-21 10:22 [PATCH] serial: omap: fix the overrun case Shubhrajyoti D
2012-09-21 11:00 ` Felipe Balbi
2012-09-21 11:18 ` Poddar, Sourav
@ 2012-09-21 14:18 ` Kevin Hilman
2012-09-21 15:08 ` Shubhrajyoti Datta
2 siblings, 1 reply; 10+ messages in thread
From: Kevin Hilman @ 2012-09-21 14:18 UTC (permalink / raw)
To: linux-arm-kernel
Shubhrajyoti D <shubhrajyoti@ti.com> writes:
> Overrun also causes an internal flag to be set, which disables further
> reception. Before the next frame can
> be received, the MPU must:
> ? Reset the RX FIFO.
> ? clear the internal flag.
>
> In the uart mode a dummy read is needed. Add the same.
>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> - functional testing on omap4sdp
> - Verified idle and suspend path hits off on beagle.
Tested-by: Kevin Hilman <khilman@ti.com>
This fixes the console hang I was seeing on 3530/Overo.
Thanks,
Kevin
> drivers/tty/serial/omap-serial.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index a0d4460..bc22a2b 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
> static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
> {
> unsigned int flag;
> + unsigned char ch = 0;
> +
> + if (!(lsr & UART_LSR_BRK_ERROR_BITS))
> + return;
> +
> + if (likely(lsr & UART_LSR_DR))
> + ch = serial_in(up, UART_RX);
>
> up->port.icount.rx++;
> flag = TTY_NORMAL;
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] serial: omap: fix the overrun case
2012-09-21 14:18 ` Kevin Hilman
@ 2012-09-21 15:08 ` Shubhrajyoti Datta
0 siblings, 0 replies; 10+ messages in thread
From: Shubhrajyoti Datta @ 2012-09-21 15:08 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 21, 2012 at 7:48 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Shubhrajyoti D <shubhrajyoti@ti.com> writes:
>
[...]
>>
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>> ---
>> - functional testing on omap4sdp
>> - Verified idle and suspend path hits off on beagle.
>
> Tested-by: Kevin Hilman <khilman@ti.com>
>
> This fixes the console hang I was seeing on 3530/Overo.
Thanks for the test.
Could you test the v2
http://www.spinics.net/lists/arm-kernel/msg197050.html
I have removed the redundant check.
Thanks,
>
> Thanks,
>
> Kevin
>
>> drivers/tty/serial/omap-serial.c | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index a0d4460..bc22a2b 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
>> static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
>> {
>> unsigned int flag;
>> + unsigned char ch = 0;
>> +
>> + if (!(lsr & UART_LSR_BRK_ERROR_BITS))
>> + return;
>> +
>> + if (likely(lsr & UART_LSR_DR))
>> + ch = serial_in(up, UART_RX);
>>
>> up->port.icount.rx++;
>> flag = TTY_NORMAL;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-09-21 15:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-21 10:22 [PATCH] serial: omap: fix the overrun case Shubhrajyoti D
2012-09-21 11:00 ` Felipe Balbi
2012-09-21 11:16 ` Shubhrajyoti
2012-09-21 11:35 ` Felipe Balbi
2012-09-21 11:48 ` Shubhrajyoti Datta
2012-09-21 11:39 ` Poddar, Sourav
2012-09-21 12:07 ` Felipe Balbi
2012-09-21 11:18 ` Poddar, Sourav
2012-09-21 14:18 ` Kevin Hilman
2012-09-21 15:08 ` Shubhrajyoti Datta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox