Re: [PATCH] arm: omap2: vc: fix 'or' always true warning

2015-09-01 Thread Frans Klaver
On Tue, Sep 1, 2015 at 5:46 PM, Tony Lindgren <t...@atomide.com> wrote:
> * Frans Klaver <frans.kla...@xsens.com> [150828 03:14]:
>> From: Frans Klaver <franskla...@gmail.com>
>>
>> Fix the warning:
>> arch/arm/mach-omap2/vc.c:302:47: warning: logical ‘or’ of collectively 
>> exhaustive tests is always true [-Wlogical-op]
>>
>> As we're toggling both CLKREQ and OFFMODE, we should also be checking
>> OFFMODE.
>>
>> Signed-off-by: Frans Klaver <franskla...@gmail.com>
>
> Thanks apppying into omap-for-v4.3/fixes.

Thanks!

Frans
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] arm: omap2: vc: fix 'or' always true warning

2015-08-28 Thread Frans Klaver
From: Frans Klaver franskla...@gmail.com

Fix the warning:
arch/arm/mach-omap2/vc.c:302:47: warning: logical ‘or’ of collectively 
exhaustive tests is always true [-Wlogical-op]

As we're toggling both CLKREQ and OFFMODE, we should also be checking
OFFMODE.

Signed-off-by: Frans Klaver franskla...@gmail.com
---
 arch/arm/mach-omap2/vc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index 076fd20d7e5a..807bc79e3e3d 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -300,7 +300,7 @@ static void __init omap3_vc_init_pmic_signaling(struct 
voltagedomain *voltdm)
 
val = voltdm-read(OMAP3_PRM_POLCTRL_OFFSET);
if (!(val  OMAP3430_PRM_POLCTRL_CLKREQ_POL) ||
-   (val  OMAP3430_PRM_POLCTRL_CLKREQ_POL)) {
+   (val  OMAP3430_PRM_POLCTRL_OFFMODE_POL)) {
val |= OMAP3430_PRM_POLCTRL_CLKREQ_POL;
val = ~OMAP3430_PRM_POLCTRL_OFFMODE_POL;
pr_debug(PM: fixing sys_clkreq and sys_off_mode polarity to 
0x%x\n,
-- 
2.3.4

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] mtd: omap: fix mtd devices not showing up

2014-10-30 Thread Frans Klaver
On 30 October 2014 02:47:04 CET, Brian Norris computersforpe...@gmail.com 
wrote:
On Tue, Oct 28, 2014 at 11:46:57AM +0200, Roger Quadros wrote:
 On 10/27/2014 04:34 PM, Frans Klaver wrote:
  Since commit 6d178ef2fd5e (mtd: nand: Move ELM driver and rename
as
  omap_elm), I don't have any mtd devices present on my am335x. This
  changes the link order of the omap_elm and omap2 objects, causing
them
  to probe in the wrong order.
  
  To fix this, make elm_config defer probing until the omap_elm
driver is
  actually loaded.
  
  Signed-off-by: Frans Klaver frans.kla...@xsens.com
 
 Acked-by: Roger Quadros rog...@ti.com

Pushed to l2-mtd.git/for-3.18. Thanks!

Thanks 

While this might be considered a link-order bug from long ago, it's not
actually important until 3.18-rc1 where we changed the link order,
right? So it doesn't need to go to -stable?

Correct. 

Thanks, 
Frans 


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] mtd: omap: fix mtd devices not showing up

2014-10-29 Thread Frans Klaver
On Tue, Oct 28, 2014 at 11:46:57AM +0200, Roger Quadros wrote:
 On 10/27/2014 04:34 PM, Frans Klaver wrote:
  Since commit 6d178ef2fd5e (mtd: nand: Move ELM driver and rename as
  omap_elm), I don't have any mtd devices present on my am335x. This
  changes the link order of the omap_elm and omap2 objects, causing them
  to probe in the wrong order.
  
  To fix this, make elm_config defer probing until the omap_elm driver is
  actually loaded.
  
  Signed-off-by: Frans Klaver frans.kla...@xsens.com
 
 Acked-by: Roger Quadros rog...@ti.com

Thanks. How about Ezequiel's remark about doing both changes?

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH typo-resend] mtd: omap: fix mtd devices not showing up

2014-10-28 Thread Frans Klaver
On Mon, Oct 27, 2014 at 04:27:45PM -0300, Ezequiel Garcia wrote:
 
 On 10/27/2014 10:56 AM, Roger Quadros wrote:
 
  Alternatively we could fix either elm_config() or omap_nand_probe() to
  return -EPROBE_DEFER in case the device is present but driver not yet 
  probed.
  
 
 Yes, that's a good idea. Can't we do both? Getting a systematic deferred
 probe sounds like a bit silly to me.

It may be. If we do both and something moves again, or something is
added that depends on omap_elm but is probed earlier, you won't be
noticing the new systematic probe deferral until you really start
looking at it.

There isn't a real dependency system in the drivers, is there?

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] mtd: omap: fix mtd devices not showing up

2014-10-27 Thread Frans Klaver
Since commit 6d178ef2fd5e (mtd: nand: Move ELM driver and rename as
omap_elm), I don't have any mtd devices present on my am335x. This
appears to be related to the link order of the omap_elm and omap2
objects. Fix it by swapping the two in the Makefile.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
I hoped this wouldn't make a difference, but it very much did. I don't
have a proper explanation for why this actually fixes it for me though.

 drivers/mtd/nand/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 9c847e4..24b81ec 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -26,8 +26,8 @@ obj-$(CONFIG_MTD_NAND_CS553X) += cs553x_nand.o
 obj-$(CONFIG_MTD_NAND_NDFC)+= ndfc.o
 obj-$(CONFIG_MTD_NAND_ATMEL)   += atmel_nand.o
 obj-$(CONFIG_MTD_NAND_GPIO)+= gpio.o
-obj-$(CONFIG_MTD_NAND_OMAP2)   += omap2.o
 obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)  += omap_elm.o
+obj-$(CONFIG_MTD_NAND_OMAP2)   += omap2.o
 obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o
 obj-$(CONFIG_MTD_NAND_PXA3xx)  += pxa3xx_nand.o
 obj-$(CONFIG_MTD_NAND_TMIO)+= tmio_nand.o
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH typo-resend] mtd: omap: fix mtd devices not showing up

2014-10-27 Thread Frans Klaver
Since commit 6d178ef2fd5e (mtd: nand: Move ELM driver and rename as
omap_elm), I don't have any mtd devices present on my am335x. This
appears to be related to the link order of the omap_elm and omap2
objects. Fix it by swapping the two in the Makefile.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
I hoped this wouldn't make a difference, but it very much did. I don't
have a proper explanation for why this actually fixes it for me though.

Resend due to misspelling in ...infradead.org

 drivers/mtd/nand/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 9c847e4..24b81ec 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -26,8 +26,8 @@ obj-$(CONFIG_MTD_NAND_CS553X) += cs553x_nand.o
 obj-$(CONFIG_MTD_NAND_NDFC)+= ndfc.o
 obj-$(CONFIG_MTD_NAND_ATMEL)   += atmel_nand.o
 obj-$(CONFIG_MTD_NAND_GPIO)+= gpio.o
-obj-$(CONFIG_MTD_NAND_OMAP2)   += omap2.o
 obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)  += omap_elm.o
+obj-$(CONFIG_MTD_NAND_OMAP2)   += omap2.o
 obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270_nand.o
 obj-$(CONFIG_MTD_NAND_PXA3xx)  += pxa3xx_nand.o
 obj-$(CONFIG_MTD_NAND_TMIO)+= tmio_nand.o
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH typo-resend] mtd: omap: fix mtd devices not showing up

2014-10-27 Thread Frans Klaver
On Mon, Oct 27, 2014 at 03:56:41PM +0200, Roger Quadros wrote:
 Hi Frans,

Hi,

 On 10/27/2014 03:32 PM, Frans Klaver wrote:
  Since commit 6d178ef2fd5e (mtd: nand: Move ELM driver and rename as
  omap_elm), I don't have any mtd devices present on my am335x. This
  appears to be related to the link order of the omap_elm and omap2
  objects. Fix it by swapping the two in the Makefile.
 
 Alternatively we could fix either elm_config() or omap_nand_probe() to
 return -EPROBE_DEFER in case the device is present but driver not yet probed.

I'd say that that would actually be nicer than having to depend on the
link order. I'll have a go at that.

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] mtd: omap: fix mtd devices not showing up

2014-10-27 Thread Frans Klaver
Since commit 6d178ef2fd5e (mtd: nand: Move ELM driver and rename as
omap_elm), I don't have any mtd devices present on my am335x. This
changes the link order of the omap_elm and omap2 objects, causing them
to probe in the wrong order.

To fix this, make elm_config defer probing until the omap_elm driver is
actually loaded.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 drivers/mtd/nand/omap_elm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/omap_elm.c b/drivers/mtd/nand/omap_elm.c
index b4f61c7..0585310 100644
--- a/drivers/mtd/nand/omap_elm.c
+++ b/drivers/mtd/nand/omap_elm.c
@@ -115,7 +115,7 @@ int elm_config(struct device *dev, enum bch_ecc bch_type,
 
if (!info) {
dev_err(dev, Unable to configure elm - device not probed?\n);
-   return -ENODEV;
+   return -EPROBE_DEFER;
}
/* ELM cannot detect ECC errors for chunks  1KB */
if (ecc_step_size  ((ELM_ECC_SIZE + 1) / 2)) {
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-10-13 Thread Frans Klaver
On Thu, Oct 02, 2014 at 12:27:23PM +0200, Sebastian Andrzej Siewior wrote:
 * Frans Klaver | 2014-09-30 10:44:16 [+0200]:
 
uart_test-483   [000] dnh.17.861018: serial8250_interrupt: irq 89
uart_test-483   [000] dnh.17.861026: serial8250_interrupt: 89 e
 e? Did was the routine invoked 0xe times or this a register?

No, this marks the end of serial8250_interrupt().


uart_test-479   [000] d.H.17.861124: serial8250_interrupt: irq 89
uart_test-479   [000] d.H.17.861133: serial8250_handle_irq: l1 
  IIR cc LSR 61
uart_test-479   [000] d.H.17.861135: serial8250_handle_irq: 
  rx_dma(up, iir)
uart_test-479   [000] d.H.17.861139: serial8250_rx_dma: l1, 
  UART_IIR_RX_TIMEOUT
uart_test-479   [000] d.H.17.861147: __dma_rx_do_complete: error: 
  1, uart_bug_dma: 32
uart_test-479   [000] d.H.17.861150: serial8250_handle_irq: 
  rx_chars(up, status)
uart_test-479   [000] d.H.17.861190: serial8250_handle_irq: 
  rx_dma(up, 0)
 timeout, manual purge. Do you have an idea how many bytes were manually
 received?

Here's an excerpt from a new trial using v10
(previous was v10_pre3). We manually read 10 bytes before we start
trying to get all bytes without dma. 

   uart_test-483   [000] dnH.18.182647: serial8250_interrupt: irq 89
   uart_test-483   [000] dnH.18.182653: omap_8250_dma_handle_irq: 
rx_dma(up, iir)
   uart_test-483   [000] dnH.18.182655: omap_8250_rx_dma: 
UART_IIR_RX_TIMEOUT
   uart_test-483   [000] dnH.18.182657: omap_8250_rx_dma: rx was running
   uart_test-483   [000] dnH.18.182662: __dma_rx_do_complete: count = 0
   uart_test-483   [000] dnH.18.182666: omap_8250_dma_handle_irq: 
rx_chars(up, status)
   uart_test-483   [000] dnH.18.182683: serial8250_rx_chars: bytes read 
= 10
   uart_test-483   [000] dnH.18.182685: omap_8250_dma_handle_irq: 
rx_dma(up, 0)
   uart_test-483   [000] dnH.18.182698: omap_8250_dma_handle_irq: 
rpm_put(up)
   uart_test-483   [000] dnH.18.182701: serial8250_interrupt: 89 end
   uart_test-488   [000] ..s.18.185353: __dma_rx_do_complete: count = 48
   uart_test-488   [000] ..s.18.185366: __dma_rx_do_complete: rx_dma(p, 
0)
 kworker/0:1-10[000] d.H.18.185486: serial8250_interrupt: irq 89
 kworker/0:1-10[000] d.H.18.185496: omap_8250_dma_handle_irq: 
rpm_put(up)
 kworker/0:1-10[000] d.H.18.185501: serial8250_interrupt: 89 end
 kworker/0:1-10[000] ..s.18.185520: __dma_rx_do_complete: count = 48
 kworker/0:1-10[000] ..s.18.185526: __dma_rx_do_complete: rx_dma(p, 
0)

Below things go downhill again.

 kworker/0:1-10[000] ..s.18.227273: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-483   [000] ..s.18.227396: __dma_rx_do_complete: count = 48
   uart_test-483   [000] ..s.18.227404: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-485   [000] .ns.18.227540: __dma_rx_do_complete: count = 48
   uart_test-485   [000] .ns.18.227547: __dma_rx_do_complete: rx_dma(p, 
0)
 kworker/0:2-65[000] ..s.18.227660: __dma_rx_do_complete: count = 48
 kworker/0:2-65[000] ..s.18.227667: __dma_rx_do_complete: rx_dma(p, 
0)
 kworker/0:1-10[000] .ns.18.227786: __dma_rx_do_complete: count = 48
 kworker/0:1-10[000] .ns.18.227793: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-477   [000] ..s.18.227921: __dma_rx_do_complete: count = 48
   uart_test-477   [000] ..s.18.227928: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-481   [000] ..s.18.228051: __dma_rx_do_complete: count = 48
   uart_test-481   [000] ..s.18.228057: __dma_rx_do_complete: rx_dma(p, 
0)
 kworker/0:1-10[000] ..s.18.228185: __dma_rx_do_complete: count = 48
 kworker/0:1-10[000] ..s.18.228191: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-485   [000] ..s.18.228318: __dma_rx_do_complete: count = 48
   uart_test-485   [000] ..s.18.228324: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-488   [000] d.h.18.228445: serial8250_interrupt: irq 89
   uart_test-488   [000] d.h.18.228454: omap_8250_dma_handle_irq: 
rpm_put(up)
   uart_test-488   [000] d.h.18.228459: serial8250_interrupt: 89 end
   uart_test-488   [000] d.H.18.228472: serial8250_interrupt: irq 89
   uart_test-488   [000] d.H.18.228477: omap_8250_dma_handle_irq: 
rx_dma(up, iir)
   uart_test-488   [000] d.H.18.228479: omap_8250_rx_dma: 
UART_IIR_RX_TIMEOUT
   uart_test-488   [000] d.H.18.228480: omap_8250_rx_dma: rx was running
   uart_test-488   [000] d.H.18.228487: __dma_rx_do_complete: count = 48
   uart_test-488   [000] d.H.18.228500: omap_8250_dma_handle_irq: 
rx_chars(up, status)
   uart_test-488   [000] d.H.18.228517: serial8250_rx_chars: bytes read 
= 10
   uart_test-488   [000

Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-30 Thread Frans Klaver
On Mon, Sep 29, 2014 at 12:30:43PM +0200, Frans Klaver wrote:
 On Mon, Sep 29, 2014 at 11:54:40AM +0200, Sebastian Andrzej Siewior wrote:
  For your too much work for irq problem: Could you add trace_printk()
  in tx/rx dma start/complete, and irq routine? The interresting part is
  what is the irq routine doing once entered. It might be a condition that
  is ignored at first and acked later while serving another event. Or it
  is really doing something and this is more or less legal.
 

Here's some trace output I get. I hope branches become clear by the
calls they do.

   uart_test-482   [000] .ns.17.860139: __dma_rx_do_complete: error: 0, 
uart_bug_dma: 32
   uart_test-482   [000] .ns.17.860141: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-480   [000] ..s.17.860260: __dma_rx_do_complete: error: 0, 
uart_bug_dma: 32
   uart_test-480   [000] ..s.17.860263: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-478   [000] ..s.17.860369: __dma_rx_do_complete: error: 0, 
uart_bug_dma: 32
   uart_test-478   [000] ..s.17.860372: __dma_rx_do_complete: rx_dma(p, 
0)
 kworker/0:1-10[000] ..s.17.860508: __dma_rx_do_complete: error: 0, 
uart_bug_dma: 32
 kworker/0:1-10[000] ..s.17.860512: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-477   [000] ..s.17.860634: __dma_rx_do_complete: error: 0, 
uart_bug_dma: 32
   uart_test-477   [000] ..s.17.860642: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-477   [000] ..s.17.860768: __dma_rx_do_complete: error: 0, 
uart_bug_dma: 32
   uart_test-477   [000] ..s.17.860772: __dma_rx_do_complete: rx_dma(p, 
0)
 kworker/0:1-10[000] ..s.17.860900: __dma_rx_do_complete: error: 0, 
uart_bug_dma: 32
 kworker/0:1-10[000] ..s.17.860904: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-483   [000] dnh.17.861018: serial8250_interrupt: irq 89
   uart_test-483   [000] dnh.17.861026: serial8250_interrupt: 89 e
   uart_test-483   [000] .ns.17.861045: __dma_rx_do_complete: error: 0, 
uart_bug_dma: 32
   uart_test-483   [000] .ns.17.861047: __dma_rx_do_complete: rx_dma(p, 
0)
   uart_test-479   [000] d.H.17.861124: serial8250_interrupt: irq 89
   uart_test-479   [000] d.H.17.861133: serial8250_handle_irq: l1 IIR 
cc LSR 61
   uart_test-479   [000] d.H.17.861135: serial8250_handle_irq: 
rx_dma(up, iir)
   uart_test-479   [000] d.H.17.861139: serial8250_rx_dma: l1, 
UART_IIR_RX_TIMEOUT
   uart_test-479   [000] d.H.17.861147: __dma_rx_do_complete: error: 1, 
uart_bug_dma: 32
   uart_test-479   [000] d.H.17.861150: serial8250_handle_irq: 
rx_chars(up, status)
   uart_test-479   [000] d.H.17.861190: serial8250_handle_irq: 
rx_dma(up, 0)
   uart_test-479   [000] d.H.17.861205: serial8250_interrupt: 89 e
 kworker/0:1-10[000] dnH.17.864949: serial8250_interrupt: irq 89

So far so good. We're just entered interrupt where stuff goes awry. The
following pattern repeats over 600 times:

 kworker/0:1-10[000] dnH.17.865198: serial8250_handle_irq: l1 IIR 
cc LSR 61
 kworker/0:1-10[000] dnH.17.865254: serial8250_handle_irq: 
rx_dma(up, iir)
 kworker/0:1-10[000] dnH.17.865333: serial8250_rx_dma: l1, 
UART_IIR_RX_TIMEOUT
 kworker/0:1-10[000] dnH.17.865626: __dma_rx_do_complete: error: 1, 
uart_bug_dma: 32
 kworker/0:1-10[000] dnH.17.865747: serial8250_handle_irq: 
rx_chars(up, status)
 kworker/0:1-10[000] dnH.17.868797: serial8250_handle_irq: 
rx_dma(up, 0)

ending with:

 kworker/0:1-10[000] dnH.20.027093: serial8250_interrupt: 
serial8250: too much work for irq89
 kworker/0:1-10[000] dnH.20.027181: serial8250_interrupt: 89 e

This clogs the entire system until I disconnect the communication lines.

So we get an RX timeout. The receiver fifo trigger level was not reached
and 8250_core is left to copy the remaining data. I would expect that if
the trigger level wasn't reached, we wouldn't need to be doing this that
often. On the other hand, could we be trapped reading data from rx
without dma helping us? And how could we resolve this?

Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-29 Thread Frans Klaver
On Thu, Sep 25, 2014 at 05:14:03PM +0200, Sebastian Andrzej Siewior wrote:
 * Frans Klaver | 2014-09-22 11:28:54 [+0200]:
 
 I guess then we'd still have to answer the question why the yocto build
 calls set_termios() so often, but that's not on you then. Did you notice
 it even changing settings? We might want to fix this even if the kernel
 should be able to cope.
 
 could you please test uart_v10_pre3? I dropped a few register sets and
 delayed the register update until TX-DMA is complete. I am traveling
 currently and have only my boneblack and it passes my limited testing.
 If it solves your problems, too then I would address Heikki's latest
 comments and prepare the final v10 (and I hope I didn't break beagle
 board in between).

This version fixes the console things for us. It also increases the
amount of data we can push over the serial port. If I push the data
towards our requirements, we're not there yet. I get the too much work
for irq notice still. However, I don't think you'd need to be fixing
that in this series (or at all). We had similar issues there with
omap-serial as well.

As far as I'm concerned, this is

Tested-by: Frans Klaver frans.kla...@xsens.com

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 12/16] tty: serial: 8250_dma: handle the UART RDI event while DMA remains idle

2014-09-29 Thread Frans Klaver
On Wed, Sep 10, 2014 at 09:30:07PM +0200, Sebastian Andrzej Siewior wrote:
 Sometimes the OMAP UART does not signal the DMA engine to unload the FIFO.
 Usually this happens when we have threshold bytes in the FIFO
 and start the DMA transfer. It seems that in those cases the UART won't
 trigger the transfer once the requested threshold is reached. In some
 rare cases the UART does not trigger the DMA transfer even if programmed
 while the FIFO was empty.
 In those cases the UART drops an RDI event and we have to empty the FIFO
 manually. If we ignore it because the DMA transfer is programmed then we
 will enter the function a few times until we receive the RX_TIMEOUT
 event. At that point the FIFO is usually full and we risk to overflow
 the FIFO.
 
 Reviewed-by: Tony Lindgren t...@atomide.com
 Tested-by: Tony Lindgren t...@atomide.com
 Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
 ---
  drivers/tty/serial/8250/8250_dma.c | 18 ++
  1 file changed, 18 insertions(+)
 
 diff --git a/drivers/tty/serial/8250/8250_dma.c 
 b/drivers/tty/serial/8250/8250_dma.c
 index fa1dc966f394..898a6781d0b3 100644
 --- a/drivers/tty/serial/8250/8250_dma.c
 +++ b/drivers/tty/serial/8250/8250_dma.c
 @@ -193,6 +193,24 @@ int serial8250_rx_dma(struct uart_8250_port *p, unsigned 
 int iir)
   __dma_rx_do_complete(p, true);
   }
   return -ETIMEDOUT;
 + case UART_IIR_RDI:
 + if (p-bugs  UART_BUG_DMA_RX)
 + break;
 + /*
 +  * The OMAP UART is a special BEAST. If we receive RDI we _have_
 +  * a DMA transfer programmed but it didn't worked. One reason is

didn't work

 +  * that we were too slow and there were too many bytes in the
 +  * FIFO, the UART counted wrong and never kicked the DMA engine
 +  * to do anything. That means once we receive RDI on OMAP than

then

 +  * the DMA won't do anything soon so we have to cancel the DMA
 +  * transfer and purge the FIFO manually.
 +  */
 + if (dma-rx_running) {
 + dmaengine_pause(dma-rxchan);
 + __dma_rx_do_complete(p, true);
 + }
 + return -ETIMEDOUT;
 +
   default:
   break;
   }
 -- 
 2.1.0
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 13/16] tty: serial: 8250_dma: add pm runtime

2014-09-29 Thread Frans Klaver
On Wed, Sep 10, 2014 at 09:30:08PM +0200, Sebastian Andrzej Siewior wrote:
 There is nothing to do for RPM in the RX path. If the HW goes off then it
 won't assert the DMA line and the transfer won't happen. So we hope that
 the HW does not go off for RX to work (DMA or PIO makes no difference
 here).
 
 For TX the situation is slightly different. RPM is enabled on
 start_tx(). We can't disable RPM on DMA complete callback because there
 is still data in the FIFO which is being sent. We have to wait until
 the FIFO is empty before we disable it.
 For this to happen we fake a TX sent error and enable THRI. Once the
 FIFO is empty we receive an interrupt and since the TTY-buffer is still
 empty we put RPM via __stop_tx(). Should it been filed then in the
 start_tx() path we should program the DMA transfer and remove the error
 flag and the THRI bit.

That last sentence starts out a bit messy.


 
 Reviewed-by: Tony Lindgren t...@atomide.com
 Tested-by: Tony Lindgren t...@atomide.com
 Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
 ---
  drivers/tty/serial/8250/8250_dma.c | 16 +++-
  1 file changed, 11 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/tty/serial/8250/8250_dma.c 
 b/drivers/tty/serial/8250/8250_dma.c
 index 898a6781d0b3..e8850219b150 100644
 --- a/drivers/tty/serial/8250/8250_dma.c
 +++ b/drivers/tty/serial/8250/8250_dma.c
 @@ -21,6 +21,7 @@ static void __dma_tx_complete(void *param)
   struct uart_8250_dma*dma = p-dma;
   struct circ_buf *xmit = p-port.state-xmit;
   unsigned long   flags;
 + bool en_thri = false;
  
   dma_sync_single_for_cpu(dma-txchan-device-dev, dma-tx_addr,
   UART_XMIT_SIZE, DMA_TO_DEVICE);
 @@ -40,11 +41,16 @@ static void __dma_tx_complete(void *param)
   int ret;
  
   ret = serial8250_tx_dma(p);
 - if (ret) {
 - dma-tx_err = 1;
 - p-ier |= UART_IER_THRI;
 - serial_port_out(p-port, UART_IER, p-ier);
 - }
 + if (ret)
 + en_thri = true;
 +
 + } else if (p-capabilities  UART_CAP_RPM)
 + en_thri = true;
 +
 + if (en_thri) {
 + dma-tx_err = 1;
 + p-ier |= UART_IER_THRI;
 + serial_port_out(p-port, UART_IER, p-ier);
   }
  
   spin_unlock_irqrestore(p-port.lock, flags);
 -- 
 2.1.0
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/16] tty: serial: Add 8250-core based omap driver

2014-09-29 Thread Frans Klaver
On Wed, Sep 10, 2014 at 09:30:01PM +0200, Sebastian Andrzej Siewior wrote:
 + /*
 +  * We enable TRIG_GRANU for RX and TX and additionaly we set
 +  * SCR_TX_EMPTY bit. The result is the following:
 +  * - RX_TRIGGER amount of bytes in the FIFO will cause an interrupt.
 +  * - less than RX_TRIGGER number of bytes will also cause an interrupt
 +  *   once the UART decides that there no new bytes arriving.
 +  * - Once THRE is enabled, the interrupt will be fired once the FIFO is
 +  *   empty - the trigger level is ignored here.
 +  *
 +  * Once DMA is enabled:
 +  * - UART will assert the TX DMA line once there is room for TX_TRIGGER
 +  *   bytes in the TX FIFO. On each assert the DMA engine will move
 +  *   TX_TRIGGER bytes into the FIFO.
 +  * - UART will assert the RX DMA line once there are RX_TRIGGER bytes in
 +  *   the FIFO and move RX_TRIGGER bytes.
 +  * This is because treshold and trigger values are the same.

threshold

 + /*
 +  * It claims to be 16C750 compatible however it is a little different.
 +  * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
 +  * have) is enabled via EFR instead of MCR. The type is set here 8250
 +  * just to get things going. UNKNOWN does not work for a few reasons and
 +  * we don't need our own type since we don't use 8250's set_termios()
 +  * and our bugs are handeld via the bug member.

handled

 +  */
 + up.port.type = PORT_8250;
 + up.port.iotype = UPIO_MEM;
 + up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW |
 + UPF_HARD_FLOW;
 + up.port.private_data = priv;
 +
 + up.port.regshift = 2;
 + up.port.fifosize = 64;
 + up.tx_loadsz = 64;
 + up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
 +#ifdef CONFIG_PM_RUNTIME
 + /*
 +  * PM_RUNTIME is mostly transparent. However to do it right we need to a

need to _do_ a ...?

 +  * TX empty interrupt before we can put the device to auto idle. So if
 +  * PM_RUNTIME is not enabled we don't add that flag and can spare that
 +  * one extra interrupt in the TX path.
 +  */

snip

 +config SERIAL_8250_OMAP
 + tristate Support for OMAP internal UART (8250 based driver)
 + depends on SERIAL_8250  ARCH_OMAP2PLUS
 + help
 +   If you have a machine based on an Texas Instruments OMAP CPU you
 +   can enable its onboard serial ports by enabling this option.
 +
 +   This driver is in early stage and uses ttyS instead of ttyO.
 +

I just wondered if this driver should be marked experimental?


Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 02/16] tty: serial: 8250_core: add run time pm

2014-09-29 Thread Frans Klaver
On Wed, Sep 10, 2014 at 09:29:57PM +0200, Sebastian Andrzej Siewior wrote:
 +/*
 + * This two wrapper ensure, that enable_runtime_pm_tx() can be called more 
 than

These two wrappers ensure that enable_runtime_pm_tx() ...

 + * once and disable_runtime_pm_tx() will still disable RPM because the fifo 
 is
 + * empty and the HW can idle again.
 + */
 +static void serial8250_rpm_get_tx(struct uart_8250_port *p)
 +{
 + unsigned char rpm_active;
 +
 + if (!(p-capabilities  UART_CAP_RPM))
 + return;
 +
 + rpm_active = xchg(p-rpm_tx_active, 1);
 + if (rpm_active)
 + return;
 + pm_runtime_get_sync(p-port.dev);
 +}
 +
 +static void serial8250_rpm_put_tx(struct uart_8250_port *p)
 +{
 + unsigned char rpm_active;
 +
 + if (!(p-capabilities  UART_CAP_RPM))
 + return;
 +
 + rpm_active = xchg(p-rpm_tx_active, 0);
 + if (!rpm_active)
 + return;
 + pm_runtime_mark_last_busy(p-port.dev);
 + pm_runtime_put_autosuspend(p-port.dev);
 +}
 +


 @@ -1469,7 +1531,12 @@ void serial8250_tx_chars(struct uart_8250_port *up)
  
   DEBUG_INTR(THRE...);
  
 - if (uart_circ_empty(xmit))
 + /*
 +  * With RPM enabled, we have to wait once the FIFO is empty before the

s,once,until,? Or do I not understand the sentence correctly?

 +  * HW can go idle. So we get here once again with empty FIFO and disable
 +  * the interrupt and RPM in __stop_tx()
 +  */
 + if (uart_circ_empty(xmit)  !(up-capabilities  UART_CAP_RPM))
   __stop_tx(up);
  }
  EXPORT_SYMBOL_GPL(serial8250_tx_chars);
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-29 Thread Frans Klaver
On Mon, Sep 29, 2014 at 11:54:40AM +0200, Sebastian Andrzej Siewior wrote:
 There is a patch named ARM: edma: unconditionally ack the error
 interrupt. I have the feeling that this is not really required once we
 delay set_termios. I couldn't reproduce the bug with beagleblack with my
 usual test case.

I think so too. I didn't need it either.


 For your too much work for irq problem: Could you add trace_printk()
 in tx/rx dma start/complete, and irq routine? The interresting part is
 what is the irq routine doing once entered. It might be a condition that
 is ignored at first and acked later while serving another event. Or it
 is really doing something and this is more or less legal.

I'll have a look at that.

 
 Thanks,
 Frans
 
 Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/16] tty: serial: Add 8250-core based omap driver

2014-09-29 Thread Frans Klaver
On Mon, Sep 29, 2014 at 3:27 PM, Sebastian Andrzej Siewior
bige...@linutronix.de wrote:
 * Frans Klaver | 2014-09-29 11:38:23 [+0200]:

threshold
 fixed

 +/*
 + * It claims to be 16C750 compatible however it is a little different.
 + * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
 + * have) is enabled via EFR instead of MCR. The type is set here 8250
 + * just to get things going. UNKNOWN does not work for a few reasons 
 and
 + * we don't need our own type since we don't use 8250's set_termios()
 + * and our bugs are handeld via the bug member.

handled
 replaced that last line with
 or pm callback.

 since there no bugs member anymore.


 + */
 +up.port.type = PORT_8250;
 +up.port.iotype = UPIO_MEM;
 +up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW |
 +UPF_HARD_FLOW;
 +up.port.private_data = priv;
 +
 +up.port.regshift = 2;
 +up.port.fifosize = 64;
 +up.tx_loadsz = 64;
 +up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
 +#ifdef CONFIG_PM_RUNTIME
 +/*
 + * PM_RUNTIME is mostly transparent. However to do it right we need to 
 a

need to _do_ a ...?
 I think dropping that 'to' should fix it.

Yup.


 + * TX empty interrupt before we can put the device to auto idle. So if
 + * PM_RUNTIME is not enabled we don't add that flag and can spare that
 + * one extra interrupt in the TX path.
 + */

snip

 +config SERIAL_8250_OMAP
 +tristate Support for OMAP internal UART (8250 based driver)
 +depends on SERIAL_8250  ARCH_OMAP2PLUS
 +help
 +  If you have a machine based on an Texas Instruments OMAP CPU you
 +  can enable its onboard serial ports by enabling this option.
 +
 +  This driver is in early stage and uses ttyS instead of ttyO.
 +

I just wondered if this driver should be marked experimental?
 What did you have in mind? CONFIG_EXPERIMENTAL is gone. After all that
 debuging that I had in the meantime I was thinking about dropping that
 early stage.

That was the other option. I'm good with that. Also, I never noticed
CONFIG_EXPERIMENTAL being gone, so that's down the drain ;).


Thanks,
Frans

 Sebastian

 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH resend 1/2] tty: omap-serial: fix division by zero

2014-09-25 Thread Frans Klaver
If the chosen baud rate is large enough (e.g. 3.5 megabaud), the
calculated n values in serial_omap_is_baud_mode16() may become 0. This
causes a division by zero when calculating the difference between
calculated and desired baud rates. To prevent this, cap the n13 and n16
values on 1.

Division by zero in kernel.
[c00132e0] (unwind_backtrace) from [c00112ec] (show_stack+0x10/0x14)
[c00112ec] (show_stack) from [c01ed7bc] (Ldiv0+0x8/0x10)
[c01ed7bc] (Ldiv0) from [c023805c] (serial_omap_baud_is_mode16+0x4c/0x68)
[c023805c] (serial_omap_baud_is_mode16) from [c02396b4] 
(serial_omap_set_termios+0x90/0x8d8)
[c02396b4] (serial_omap_set_termios) from [c0230a0c] 
(uart_change_speed+0xa4/0xa8)
[c0230a0c] (uart_change_speed) from [c0231798] (uart_set_termios+0xa0/0x1fc)
[c0231798] (uart_set_termios) from [c022bb44] (tty_set_termios+0x248/0x2c0)
[c022bb44] (tty_set_termios) from [c022c17c] (set_termios+0x248/0x29c)
[c022c17c] (set_termios) from [c022c3e4] (tty_mode_ioctl+0x1c8/0x4e8)
[c022c3e4] (tty_mode_ioctl) from [c0227e70] (tty_ioctl+0xa94/0xb18)
[c0227e70] (tty_ioctl) from [c00cf45c] (do_vfs_ioctl+0x4a0/0x560)
[c00cf45c] (do_vfs_ioctl) from [c00cf568] (SyS_ioctl+0x4c/0x74)
[c00cf568] (SyS_ioctl) from [c000e480] (ret_fast_syscall+0x0/0x30)

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
Resend to add sta...@vger.kernel.org to CC. As far as I know, this applies to
v3.9 and up.

Thanks,
Frans

 drivers/tty/serial/omap-serial.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d017cec..e454b7c 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -254,8 +254,16 @@ serial_omap_baud_is_mode16(struct uart_port *port, 
unsigned int baud)
 {
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
-   int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
-   int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+   int baudAbsDiff13;
+   int baudAbsDiff16;
+
+   if (n13 == 0)
+   n13 = 1;
+   if (n16 == 0)
+   n16 = 1;
+
+   baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
+   baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
if (baudAbsDiff13  0)
baudAbsDiff13 = -baudAbsDiff13;
if (baudAbsDiff16  0)
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH resend 1/2] tty: omap-serial: fix division by zero

2014-09-25 Thread Frans Klaver
On Thu, Sep 25, 2014 at 12:16:28PM +0200, Greg Kroah-Hartman wrote:
 On Thu, Sep 25, 2014 at 11:19:51AM +0200, Frans Klaver wrote:
  If the chosen baud rate is large enough (e.g. 3.5 megabaud), the
  calculated n values in serial_omap_is_baud_mode16() may become 0. This
  causes a division by zero when calculating the difference between
  calculated and desired baud rates. To prevent this, cap the n13 and n16
  values on 1.
  
  Division by zero in kernel.
  [c00132e0] (unwind_backtrace) from [c00112ec] (show_stack+0x10/0x14)
  [c00112ec] (show_stack) from [c01ed7bc] (Ldiv0+0x8/0x10)
  [c01ed7bc] (Ldiv0) from [c023805c] 
  (serial_omap_baud_is_mode16+0x4c/0x68)
  [c023805c] (serial_omap_baud_is_mode16) from [c02396b4] 
  (serial_omap_set_termios+0x90/0x8d8)
  [c02396b4] (serial_omap_set_termios) from [c0230a0c] 
  (uart_change_speed+0xa4/0xa8)
  [c0230a0c] (uart_change_speed) from [c0231798] 
  (uart_set_termios+0xa0/0x1fc)
  [c0231798] (uart_set_termios) from [c022bb44] 
  (tty_set_termios+0x248/0x2c0)
  [c022bb44] (tty_set_termios) from [c022c17c] (set_termios+0x248/0x29c)
  [c022c17c] (set_termios) from [c022c3e4] (tty_mode_ioctl+0x1c8/0x4e8)
  [c022c3e4] (tty_mode_ioctl) from [c0227e70] (tty_ioctl+0xa94/0xb18)
  [c0227e70] (tty_ioctl) from [c00cf45c] (do_vfs_ioctl+0x4a0/0x560)
  [c00cf45c] (do_vfs_ioctl) from [c00cf568] (SyS_ioctl+0x4c/0x74)
  [c00cf568] (SyS_ioctl) from [c000e480] (ret_fast_syscall+0x0/0x30)
  
  Signed-off-by: Frans Klaver frans.kla...@xsens.com
  ---
  Resend to add sta...@vger.kernel.org to CC. As far as I know, this applies 
  to
  v3.9 and up.
 
 formletter
 
 This is not the correct way to submit patches for inclusion in the
 stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
 for how to do this properly.
 
 /formletter
 
 I'll take care of this, but go read that file for your next time...

Ah, right. I'll heed that next time. Thanks for taking care of it.

Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-25 Thread Frans Klaver
On 25 September 2014 17:14:03 CEST, Sebastian Andrzej Siewior 
bige...@linutronix.de wrote:
* Frans Klaver | 2014-09-22 11:28:54 [+0200]:

I guess then we'd still have to answer the question why the yocto
build
calls set_termios() so often, but that's not on you then. Did you
notice
it even changing settings? We might want to fix this even if the
kernel
should be able to cope.

could you please test uart_v10_pre3? I dropped a few register sets and
delayed the register update until TX-DMA is complete. I am traveling
currently and have only my boneblack and it passes my limited testing.
If it solves your problems, too then I would address Heikki's latest
comments and prepare the final v10 (and I hope I didn't break beagle
board in between).

That'll be Monday earliest for me. 


Thanks again,
Frans

Sebastian

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] tty: omap-serial: pull out calculation from baud_is_mode16

2014-09-24 Thread Frans Klaver
To determine the correct divisor, we need to know the difference between
the desired baud rate and the actual baud rate. The calculation for this
difference is implemented twice within omap_serial_baud_is_mode16().
Pull out the calculation for easier maintenance.

While at it, remove the CamelCasing from the variable names.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 42 +++-
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index e454b7c..18c30ca 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -239,6 +239,26 @@ static void serial_omap_enable_wakeup(struct 
uart_omap_port *up, bool enable)
 }
 
 /*
+ * Calculate the absolute difference between the desired and actual baud
+ * rate for the given mode.
+ */
+static inline int calculate_baud_abs_diff(struct uart_port *port,
+   unsigned int baud, unsigned int mode)
+{
+   unsigned int n = port-uartclk / (mode * baud);
+   int abs_diff;
+
+   if (n == 0)
+   n = 1;
+
+   abs_diff = baud - (port-uartclk / (mode * n));
+   if (abs_diff  0)
+   abs_diff = -abs_diff;
+
+   return abs_diff;
+}
+
+/*
  * serial_omap_baud_is_mode16 - check if baud rate is MODE16X
  * @port: uart port info
  * @baud: baudrate for which mode needs to be determined
@@ -252,24 +272,10 @@ static void serial_omap_enable_wakeup(struct 
uart_omap_port *up, bool enable)
 static bool
 serial_omap_baud_is_mode16(struct uart_port *port, unsigned int baud)
 {
-   unsigned int n13 = port-uartclk / (13 * baud);
-   unsigned int n16 = port-uartclk / (16 * baud);
-   int baudAbsDiff13;
-   int baudAbsDiff16;
-
-   if (n13 == 0)
-   n13 = 1;
-   if (n16 == 0)
-   n16 = 1;
-
-   baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
-   baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
-   if (baudAbsDiff13  0)
-   baudAbsDiff13 = -baudAbsDiff13;
-   if (baudAbsDiff16  0)
-   baudAbsDiff16 = -baudAbsDiff16;
-
-   return (baudAbsDiff13 = baudAbsDiff16);
+   int abs_diff_13 = calculate_baud_abs_diff(port, baud, 13);
+   int abs_diff_16 = calculate_baud_abs_diff(port, baud, 16);
+
+   return (abs_diff_13 = abs_diff_16);
 }
 
 /*
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/2]

2014-09-24 Thread Frans Klaver
On Tue, Sep 23, 2014 at 11:58 PM, Greg Kroah-Hartman wrote:
 So both would be needed to be backported to stable kernels?  Why not
 just do the fix first, then the cleanup afterward, to make backporting
 easier?

Sure thing. I read something about cleaning up first, then actually changing
stuff, but it doesn't really make sense to move bugs around before fixing them,
unless fixing them requires moving them around.

Anyway, here's the respin.

v1..v2:
  - swapped fix and cleanup to ease backporting

Frans Klaver (2):
  tty: omap-serial: fix division by zero
  tty: omap-serial: pull out calculation from baud_is_mode16

 drivers/tty/serial/omap-serial.c | 34 --
 1 file changed, 24 insertions(+), 10 deletions(-)

-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] tty: omap-serial: fix division by zero

2014-09-24 Thread Frans Klaver
If the chosen baud rate is large enough (e.g. 3.5 megabaud), the
calculated n values in serial_omap_is_baud_mode16() may become 0. This
causes a division by zero when calculating the difference between
calculated and desired baud rates. To prevent this, cap the n13 and n16
values on 1.

Division by zero in kernel.
[c00132e0] (unwind_backtrace) from [c00112ec] (show_stack+0x10/0x14)
[c00112ec] (show_stack) from [c01ed7bc] (Ldiv0+0x8/0x10)
[c01ed7bc] (Ldiv0) from [c023805c] (serial_omap_baud_is_mode16+0x4c/0x68)
[c023805c] (serial_omap_baud_is_mode16) from [c02396b4] 
(serial_omap_set_termios+0x90/0x8d8)
[c02396b4] (serial_omap_set_termios) from [c0230a0c] 
(uart_change_speed+0xa4/0xa8)
[c0230a0c] (uart_change_speed) from [c0231798] (uart_set_termios+0xa0/0x1fc)
[c0231798] (uart_set_termios) from [c022bb44] (tty_set_termios+0x248/0x2c0)
[c022bb44] (tty_set_termios) from [c022c17c] (set_termios+0x248/0x29c)
[c022c17c] (set_termios) from [c022c3e4] (tty_mode_ioctl+0x1c8/0x4e8)
[c022c3e4] (tty_mode_ioctl) from [c0227e70] (tty_ioctl+0xa94/0xb18)
[c0227e70] (tty_ioctl) from [c00cf45c] (do_vfs_ioctl+0x4a0/0x560)
[c00cf45c] (do_vfs_ioctl) from [c00cf568] (SyS_ioctl+0x4c/0x74)
[c00cf568] (SyS_ioctl) from [c000e480] (ret_fast_syscall+0x0/0x30)

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d017cec..e454b7c 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -254,8 +254,16 @@ serial_omap_baud_is_mode16(struct uart_port *port, 
unsigned int baud)
 {
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
-   int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
-   int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+   int baudAbsDiff13;
+   int baudAbsDiff16;
+
+   if (n13 == 0)
+   n13 = 1;
+   if (n16 == 0)
+   n16 = 1;
+
+   baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
+   baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
if (baudAbsDiff13  0)
baudAbsDiff13 = -baudAbsDiff13;
if (baudAbsDiff16  0)
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] tty: omap-serial: fix division by zero

2014-09-24 Thread Frans Klaver
On Wed, Sep 24, 2014 at 01:08:52AM -0700, Greg Kroah-Hartman wrote:
 On Wed, Sep 24, 2014 at 09:55:21AM +0200, Frans Klaver wrote:
  If the chosen baud rate is large enough (e.g. 3.5 megabaud), the
  calculated n values in serial_omap_is_baud_mode16() may become 0. This
  causes a division by zero when calculating the difference between
  calculated and desired baud rates. To prevent this, cap the n13 and n16
  values on 1.
  
  Division by zero in kernel.
  [c00132e0] (unwind_backtrace) from [c00112ec] (show_stack+0x10/0x14)
  [c00112ec] (show_stack) from [c01ed7bc] (Ldiv0+0x8/0x10)
  [c01ed7bc] (Ldiv0) from [c023805c] 
  (serial_omap_baud_is_mode16+0x4c/0x68)
  [c023805c] (serial_omap_baud_is_mode16) from [c02396b4] 
  (serial_omap_set_termios+0x90/0x8d8)
  [c02396b4] (serial_omap_set_termios) from [c0230a0c] 
  (uart_change_speed+0xa4/0xa8)
  [c0230a0c] (uart_change_speed) from [c0231798] 
  (uart_set_termios+0xa0/0x1fc)
  [c0231798] (uart_set_termios) from [c022bb44] 
  (tty_set_termios+0x248/0x2c0)
  [c022bb44] (tty_set_termios) from [c022c17c] (set_termios+0x248/0x29c)
  [c022c17c] (set_termios) from [c022c3e4] (tty_mode_ioctl+0x1c8/0x4e8)
  [c022c3e4] (tty_mode_ioctl) from [c0227e70] (tty_ioctl+0xa94/0xb18)
  [c0227e70] (tty_ioctl) from [c00cf45c] (do_vfs_ioctl+0x4a0/0x560)
  [c00cf45c] (do_vfs_ioctl) from [c00cf568] (SyS_ioctl+0x4c/0x74)
  [c00cf568] (SyS_ioctl) from [c000e480] (ret_fast_syscall+0x0/0x30)
  
  Signed-off-by: Frans Klaver frans.kla...@xsens.com
  ---
   drivers/tty/serial/omap-serial.c | 12 ++--
   1 file changed, 10 insertions(+), 2 deletions(-)
 
 Should this go to the stable kernel trees as well?

It's been there for a while. Introduced in 5fe2123647c1508 OMAP/serial:
Support 1Mbaud and similar baudrates that require Mode16 instead of
Mode13 (v3.9). It's probably a good idea to do so. stable@... needs a
cc?

Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] tty: omap-serial: fix division by zero

2014-09-24 Thread Frans Klaver
On Wed, Sep 24, 2014 at 10:41:11AM +0200, Frans Klaver wrote:
 On Wed, Sep 24, 2014 at 01:08:52AM -0700, Greg Kroah-Hartman wrote:
  On Wed, Sep 24, 2014 at 09:55:21AM +0200, Frans Klaver wrote:
   If the chosen baud rate is large enough (e.g. 3.5 megabaud), the
   calculated n values in serial_omap_is_baud_mode16() may become 0. This
   causes a division by zero when calculating the difference between
   calculated and desired baud rates. To prevent this, cap the n13 and n16
   values on 1.
   
   Division by zero in kernel.
   [c00132e0] (unwind_backtrace) from [c00112ec] (show_stack+0x10/0x14)
   [c00112ec] (show_stack) from [c01ed7bc] (Ldiv0+0x8/0x10)
   [c01ed7bc] (Ldiv0) from [c023805c] 
   (serial_omap_baud_is_mode16+0x4c/0x68)
   [c023805c] (serial_omap_baud_is_mode16) from [c02396b4] 
   (serial_omap_set_termios+0x90/0x8d8)
   [c02396b4] (serial_omap_set_termios) from [c0230a0c] 
   (uart_change_speed+0xa4/0xa8)
   [c0230a0c] (uart_change_speed) from [c0231798] 
   (uart_set_termios+0xa0/0x1fc)
   [c0231798] (uart_set_termios) from [c022bb44] 
   (tty_set_termios+0x248/0x2c0)
   [c022bb44] (tty_set_termios) from [c022c17c] (set_termios+0x248/0x29c)
   [c022c17c] (set_termios) from [c022c3e4] (tty_mode_ioctl+0x1c8/0x4e8)
   [c022c3e4] (tty_mode_ioctl) from [c0227e70] (tty_ioctl+0xa94/0xb18)
   [c0227e70] (tty_ioctl) from [c00cf45c] (do_vfs_ioctl+0x4a0/0x560)
   [c00cf45c] (do_vfs_ioctl) from [c00cf568] (SyS_ioctl+0x4c/0x74)
   [c00cf568] (SyS_ioctl) from [c000e480] (ret_fast_syscall+0x0/0x30)
   
   Signed-off-by: Frans Klaver frans.kla...@xsens.com
   ---
drivers/tty/serial/omap-serial.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
  
  Should this go to the stable kernel trees as well?
 
 It's been there for a while. Introduced in 5fe2123647c1508 OMAP/serial:
 Support 1Mbaud and similar baudrates that require Mode16 instead of
 Mode13 (v3.9). It's probably a good idea to do so. stable@... needs a
 cc?

And would it be enough to just resend this particular patch in that
case?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-09-23 Thread Frans Klaver
On Wed, Sep 17, 2014 at 02:13:03PM +0200, Frans Klaver wrote:
 On Wed, Sep 17, 2014 at 08:01:08AM -0400, Peter Hurley wrote:
  On 09/16/2014 04:50 AM, Frans Klaver wrote:
   On Mon, Sep 15, 2014 at 01:31:56PM -0400, Peter Hurley wrote:
   On 09/15/2014 11:39 AM, Peter Hurley wrote:
   On 09/15/2014 10:00 AM, 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.
  
   Why is the threadirqs kernel boot option not sufficient?
   Or conversely, shouldn't this be selectable?
  
   
   I wasn't aware of the threadirqs boot option. I also wouldn't know if
   this should be selectable. What would be a reason to favor the
   non-threaded irq over the threaded irq?
  
  Not everyone cares enough about serial to dedicate kthreads to it :)
 
 Fair enough. In that light, we might not care enough about other
 subsystems to dedicate kthreads to it :). Selectable seems reasonable in
 that case.
 
 
   Also, do you see the same performance differential when you implement 
   this
   in the 8250 driver (that is, on top of Sebastian's omap-8250 
   conversion)?
  
   
   I haven't gotten Sebastian's driver to work properly yet on the console.
   There was no reason for me yet to throw my omap changes on top of
   Sebastian's queue.

Doing the threaded interrupt change on the 8250 driver doesn't seem as
trivial. Unless I'm mistaken, that version of this patch would mess with
all other 8250 based serial drivers, if it's done properly. Incidentally
I did try using threadirqs, but that didn't give my any significant
results. I mostly noticed a difference in the console.


   
   PS - To overflow the 64 byte RX FIFO at those data rates means interrupt
   latency in excess of 250us?
   
   At 3686400 baud it should take about 174 us to fill a 64 byte buffer. I
   haven't done any measurements on the interrupt latency though. If you
   consider that we're sending about 1kB of data, 240 times a second, we're
   spending a lot of time reading data from the uart. I can imagine the
   system has other work to do as well.
  
  System work should not keep irqs from being serviced. Even 174us is a long
  time not to service an interrupt. Maybe console writes are keeping the isr
  from running?
 
 That's quite possible. I'll have to redo the test setup I had for this to
 give you a decent answer. I'll have to do that anyway as Sebastian's
 8250 conversion improves.

I haven't had time yet to look into this any further. I'll accept that
this patch may fix a case most people aren't the least interested in.
I'll also happily accept that I probably need a better argumentation
than this works better for us.Would it make sense to drop this patch
and resubmit the other three? As I mentioned in the previous run, I
think these are useful in any case.

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-09-23 Thread Frans Klaver
On 23 September 2014 19:17:20 CEST, Peter Hurley pe...@hurleysoftware.com 
wrote:
On 09/23/2014 04:24 AM, Frans Klaver wrote:
 On Wed, Sep 17, 2014 at 02:13:03PM +0200, Frans Klaver wrote:
 On Wed, Sep 17, 2014 at 08:01:08AM -0400, Peter Hurley wrote:
 On 09/16/2014 04:50 AM, Frans Klaver wrote:
 On Mon, Sep 15, 2014 at 01:31:56PM -0400, Peter Hurley wrote:
 On 09/15/2014 11:39 AM, Peter Hurley wrote:
 On 09/15/2014 10:00 AM, 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.

 Why is the threadirqs kernel boot option not sufficient?
 Or conversely, shouldn't this be selectable?


 I wasn't aware of the threadirqs boot option. I also wouldn't know
if
 this should be selectable. What would be a reason to favor the
 non-threaded irq over the threaded irq?

 Not everyone cares enough about serial to dedicate kthreads to it
:)

 Fair enough. In that light, we might not care enough about other
 subsystems to dedicate kthreads to it :). Selectable seems
reasonable in
 that case.


 Also, do you see the same performance differential when you
implement this
 in the 8250 driver (that is, on top of Sebastian's omap-8250
conversion)?


 I haven't gotten Sebastian's driver to work properly yet on the
console.
 There was no reason for me yet to throw my omap changes on top of
 Sebastian's queue.
 
 Doing the threaded interrupt change on the 8250 driver doesn't seem
as
 trivial. Unless I'm mistaken, that version of this patch would mess
with
 all other 8250 based serial drivers, if it's done properly.
Incidentally
 I did try using threadirqs, but that didn't give my any significant
 results. I mostly noticed a difference in the console.
 
 

 PS - To overflow the 64 byte RX FIFO at those data rates means
interrupt
 latency in excess of 250us?

 At 3686400 baud it should take about 174 us to fill a 64 byte
buffer. I
 haven't done any measurements on the interrupt latency though. If
you
 consider that we're sending about 1kB of data, 240 times a second,
we're
 spending a lot of time reading data from the uart. I can imagine
the
 system has other work to do as well.

 System work should not keep irqs from being serviced. Even 174us is
a long
 time not to service an interrupt. Maybe console writes are keeping
the isr
 from running?

 That's quite possible. I'll have to redo the test setup I had for
this to
 give you a decent answer. I'll have to do that anyway as Sebastian's
 8250 conversion improves.
 
 I haven't had time yet to look into this any further. I'll accept
that
 this patch may fix a case most people aren't the least interested in.
 I'll also happily accept that I probably need a better argumentation
 than this works better for us.Would it make sense to drop this
patch
 and resubmit the other three? As I mentioned in the previous run, I
 think these are useful in any case.

I would've thought the first 2 patches had already been picked up
because
they fix div-by-zero faults.

I've had no confirmation of that happening so far. I also don't know if I 
should expect one. Who'd take these patches? Tony? 


I don't really have a problem with the patch (except for it should be
selectable, even if that's just a CONFIG_ setting). At the same time,
the performance results don't really make sense; so if there's actually
an underlying problem, I'd rather that get addressed (and the long
interrupt latency may be the underlying problem).

Your questions got me thinking a bit more. I concluded that it's hard to define 
why this difference in performance is so big. I only got to it by just trying 
and seeing what would happen. 


As far as the 8250 driver and threaded irqs go, I just was hoping for
another data point with a simple hard-coded test jig, not a full-blown
patch series for all of them. Sorry for the misunderstanding.

Ah yes. I'll see what I can do about it. It does make sense to get that data 
point somehow. 

Thanks, 
Frans 


Regards,
Peter Hurley

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] Fix a division by zero

2014-09-23 Thread Frans Klaver
Hi Greg,

Here's a couple of patches that fix a divison by zero in omap-serial.c. One's a
cleanup, the other the actual fix.

Thanks,
Frans

Frans Klaver (2):
  tty: omap-serial: pull out calculation from baud_is_mode16
  tty: omap-serial: fix a division by zero

 drivers/tty/serial/omap-serial.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] tty: omap-serial: fix a division by zero

2014-09-23 Thread Frans Klaver
If the chosen baud rate is large enough (e.g. 3.5 megabaud), the
calculated n values in calculate_baud_abs_diff may become 0. This causes
a division by zero when calculating the difference between calculated
and desired baud rates. To prevent this, cap n on 1.

Division by zero in kernel.
[c00132e0] (unwind_backtrace) from [c00112ec] (show_stack+0x10/0x14)
[c00112ec] (show_stack) from [c01ed7bc] (Ldiv0+0x8/0x10)
[c01ed7bc] (Ldiv0) from [c023805c] (serial_omap_baud_is_mode16+0x4c/0x68)
[c023805c] (serial_omap_baud_is_mode16) from [c02396b4] 
(serial_omap_set_termios+0x90/0x8d8)
[c02396b4] (serial_omap_set_termios) from [c0230a0c] 
(uart_change_speed+0xa4/0xa8)
[c0230a0c] (uart_change_speed) from [c0231798] (uart_set_termios+0xa0/0x1fc)
[c0231798] (uart_set_termios) from [c022bb44] (tty_set_termios+0x248/0x2c0)
[c022bb44] (tty_set_termios) from [c022c17c] (set_termios+0x248/0x29c)
[c022c17c] (set_termios) from [c022c3e4] (tty_mode_ioctl+0x1c8/0x4e8)
[c022c3e4] (tty_mode_ioctl) from [c0227e70] (tty_ioctl+0xa94/0xb18)
[c0227e70] (tty_ioctl) from [c00cf45c] (do_vfs_ioctl+0x4a0/0x560)
[c00cf45c] (do_vfs_ioctl) from [c00cf568] (SyS_ioctl+0x4c/0x74)
[c00cf568] (SyS_ioctl) from [c000e480] (ret_fast_syscall+0x0/0x30)

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index ae935ce..7d3f557 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -246,8 +246,12 @@ static inline int calculate_baud_abs_diff(struct uart_port 
*port,
unsigned int baud, unsigned int mode)
 {
unsigned int n = port-uartclk / (mode * baud);
-   int abs_diff = baud - (port-uartclk / (mode * n));
+   int abs_diff;
 
+   if (n == 0)
+   n = 1;
+
+   abs_diff = baud - (port-uartclk / (mode * n));
if (abs_diff  0)
abs_diff = -abs_diff;
 
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] tty: omap-serial: pull out calculation from baud_is_mode16

2014-09-23 Thread Frans Klaver
To determine the correct divisor, we need to know the difference between
the desired baud rate and the actual baud rate. The calculation for this
difference is implemented twice within omap_serial_baud_is_mode16().
Pull out the calculation for easier maintenance.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d017cec..ae935ce 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -239,6 +239,22 @@ static void serial_omap_enable_wakeup(struct 
uart_omap_port *up, bool enable)
 }
 
 /*
+ * Calculate the absolute difference between the desired and actual baud
+ * rate for the given mode.
+ */
+static inline int calculate_baud_abs_diff(struct uart_port *port,
+   unsigned int baud, unsigned int mode)
+{
+   unsigned int n = port-uartclk / (mode * baud);
+   int abs_diff = baud - (port-uartclk / (mode * n));
+
+   if (abs_diff  0)
+   abs_diff = -abs_diff;
+
+   return abs_diff;
+}
+
+/*
  * serial_omap_baud_is_mode16 - check if baud rate is MODE16X
  * @port: uart port info
  * @baud: baudrate for which mode needs to be determined
@@ -252,14 +268,8 @@ static void serial_omap_enable_wakeup(struct 
uart_omap_port *up, bool enable)
 static bool
 serial_omap_baud_is_mode16(struct uart_port *port, unsigned int baud)
 {
-   unsigned int n13 = port-uartclk / (13 * baud);
-   unsigned int n16 = port-uartclk / (16 * baud);
-   int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
-   int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
-   if (baudAbsDiff13  0)
-   baudAbsDiff13 = -baudAbsDiff13;
-   if (baudAbsDiff16  0)
-   baudAbsDiff16 = -baudAbsDiff16;
+   int baudAbsDiff13 = calculate_baud_abs_diff(port, baud, 13);
+   int baudAbsDiff16 = calculate_baud_abs_diff(port, baud, 16);
 
return (baudAbsDiff13 = baudAbsDiff16);
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-09-23 Thread Frans Klaver
On Tue, Sep 23, 2014 at 8:38 PM, Tony Lindgren t...@atomide.com wrote:
 * Frans Klaver franskla...@gmail.com [140923 11:12]:
 On 23 September 2014 19:17:20 CEST, Peter Hurley pe...@hurleysoftware.com 
 wrote:
 
 I would've thought the first 2 patches had already been picked up
 because
 they fix div-by-zero faults.

 I've had no confirmation of that happening so far. I also don't
 know if I should expect one. Who'd take these patches? Tony?

 $ scripts/get_maintainer.pl -f drivers/tty/serial/omap-serial.c

 Shows they should be sent to Greg and linux-serial. Please also Cc
 linux-omap. And if it's a fix, please make sure the subject has the
 magic word fix :)

Done. Thanks.

Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-22 Thread Frans Klaver
On Sun, Sep 21, 2014 at 10:41:00PM +0200, Sebastian Andrzej Siewior wrote:
 * Frans Klaver | 2014-09-17 12:28:12 [+0200]:
 
 - Bone Black: Yocto poky, core-image-minimal
   Login, less file locks up, doesn't show anything. I can exit using
   Ctrl-C.
 
 So I have the same with my and the serial-omap driver. No difference
 here. The trace looks like this:
 |   idle-0 [000] d.h.   444.393585: serial8250_handle_irq: iir 
 cc lsr 61
 |   idle-0 [000] d.h.   444.393605: serial8250_rx_chars: get 0d
 received the enter key
 
 |   idle-0 [000] d.h.   444.393609: serial8250_rx_chars: insert 
 d lsr 61
 |   idle-0 [000] d.h.   444.393614: uart_insert_char: 1
 |   idle-0 [000] d.h.   444.393617: uart_insert_char: 2
 |   idle-0 [000] dnh.   444.393636: serial8250_tx_chars: empty
 |  kworker/0:2-753   [000] d...   444.393686: serial8250_start_tx: empty?1
 |  kworker/0:2-753   [000] d.h.   444.393699: serial8250_handle_irq: iir 
 c2 lsr 60
 |  kworker/0:2-753   [000] d.h.   444.393705: serial8250_tx_chars: empty
 |   sh-1042  [000] d...   444.393822: serial8250_start_tx: empty?1
 |   sh-1042  [000] d.h.   444.393836: serial8250_handle_irq: iir 
 c2 lsr 60
 |   sh-1042  [000] d.h.   444.393842: serial8250_tx_chars: empty
 |   sh-1042  [000] d...   444.393855: serial8250_start_tx: empty?0
 |   sh-1042  [000] d.h.   444.393863: serial8250_handle_irq: iir 
 c2 lsr 60
 |   sh-1042  [000] d.h.   444.393867: serial8250_tx_chars: put 0d
 |   sh-1042  [000] d.h.   444.393871: serial8250_tx_chars: put 0a
 
 shell responded with \r\n which I see and then
 
 |   sh-1042  [000] d.h.   444.394057: serial8250_handle_irq: iir 
 c2 lsr 60
 |   sh-1042  [000] d.h.   444.394065: serial8250_tx_chars: empty
 
 nothing more. less isn't sending data for some reason. Exactly the same
 thing happens in a Debian environment except that it continues:
 …
 | bash-2468  [000] d.h.99.657899: serial8250_tx_chars: put 0a
 | bash-2468  [000] d.h.99.658089: serial8250_handle_irq: iir 
 c2 lsr 60
 | bash-2468  [000] d.h.99.658095: serial8250_tx_chars: empty
 =
 | less-2474  [000] d...99.696038: serial8250_start_tx: empty?0
 | less-2474  [000] d.h.99.696069: serial8250_handle_irq: iir 
 c2 lsr 60
 | less-2474  [000] d.h.99.696078: serial8250_tx_chars: put 1b
 | less-2474  [000] d.h.99.696082: serial8250_tx_chars: put 5b
 | less-2474  [000] d.h.99.696085: serial8250_tx_chars: put 3f
 | less-2474  [000] d.h.99.696087: serial8250_tx_chars: put 31
 
 It has to be something about the environment. Booting Debian and chroot
 into this RFS and less works perfectly. But since it behaves like that
 with both drivers, I guess the problem is somewhere else…
 
   vi runs normally, only occupies part of the total screen estate in
   minicom. After quitting, a weird character shows up (typically I see
   ÿ there), but minicom can use the rest of the screen estate again.
   If we disregard the odd character, this is much like the behavior we
   have on the omap-serial driver.
 - Custom board: Yocto poky, custom image
   Login, less file locks up, showing only ÿ in the top left corner
   of the screen. Can get out of there by having something dumped through
   /dev/kmsg.
 
 I managed to run into something like that with vi on dra7 and with
 little more patience on am335x as well by vi * and then :n.
 
 This gets fixed indeed by writing. Hours of debugging and a lot of hair
 less later: the yocto RFS calls set_termios quite a lot. This includes
 changing the baudrate (not by yocto but the driver sets it to 0 and then
 to the requested one) and this seems to be responsible for the bad
 bytes. I haven't figured out yet I don't see this with omap-serial.
 Even worse: If this (set_termios()) happens while the DMA is still
 active then it might stall it. A write into the FIFO seems to fix it and
 this is where your echo /dev/kmsg fixes things.
 If I delay the restore_registers part of set_termios() until TX-DMA is
 complete then it seems that the TX-DMA stall does not tall anymore.

Wow, thanks for your work here. This does indeed sound hard to trap.

I guess then we'd still have to answer the question why the yocto build
calls set_termios() so often, but that's not on you then. Did you notice
it even changing settings? We might want to fix this even if the kernel
should be able to cope.

Thanks again,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-17 Thread Frans Klaver
Hi,

Yesterday's testing was a bit messy. So here goes again.

On Mon, Sep 15, 2014 at 06:42:04PM +0200, Sebastian Andrzej Siewior wrote:
 On 09/12/2014 12:28 PM, Frans Klaver wrote:
  port config is 115200 8N1. I don't recall doing anything special. I
  boot, login, less file and get a lock.
 
 So I booted my mini Debian 7.6 (basic system + openssh) on my beagle
 bone black which is:
 
 [0.00] AM335X ES2.0 (neon )

Mine's the same.

 configured a console, login, invoked less file. The file was shown, I
 hit on the space key so less shows me more of the file. No lock-up.
 I tried booting via NFS and MMC. I tried various files with less.
 
 My dot config is here
   https://breakpoint.cc/config-am335x-bb.txt.xz
 
 If there is nothing specific to the file you do less on I have no idea
 what else it could if it is not the config.

It could be environmental. I have three test cases right now. Two of
them on the beagle bone black, the third on our custom am335x based
platform.

- All test cases run the same kernel built from uart_v10-pre1.
- For the black, the board, dtb, and u-boot environment are equal for
  the test cases.

- Bone Black: Debian 7.5
  Login, less file doesn't lock up. Scrolling down looks sensible.
  Scrolling up leaves me with a crooked display, provided the minicom
  window is more than 24 lines high. Condensed example:

  Normal less looks like:
  
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do
eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad
minim veniam, quis nostrud exercitation ullamco laboris nisi ut
aliquip ex ea commodo consequat. Duis aute irure dolor in
:

  While after scrolling up it looks like

minim veniam, quis nostrud exercitation ullamco laboris nisi ut
aliquip ex ea commodo consequat. Duis aute irure dolor in
:

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do
eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad

  vi works sensibly, but only occupies part of the total screen estate
  in minicom. After quitting, minicom doesn't use the rest of the screen
  estate anymore.

  After running vi, less doesn't show the weird scrolling behavior
  anymore, since the console has just been limited to 24x80.

- Bone Black: Yocto poky, core-image-minimal
  Login, less file locks up, doesn't show anything. I can exit using
  Ctrl-C.

  vi runs normally, only occupies part of the total screen estate in
  minicom. After quitting, a weird character shows up (typically I see
  ÿ there), but minicom can use the rest of the screen estate again.
  If we disregard the odd character, this is much like the behavior we
  have on the omap-serial driver.

- Custom board: Yocto poky, custom image
  Login, less file locks up, showing only ÿ in the top left corner
  of the screen. Can get out of there by having something dumped through
  /dev/kmsg.

  vi: see Bone Black: Yocto poky, core-image-minimal

Having it summed up like this, I think we're back at ncurses and its
interaction with the serial driver.

Hope this helps. Thanks for your effort so far,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-09-17 Thread Frans Klaver
On Wed, Sep 17, 2014 at 08:01:08AM -0400, Peter Hurley wrote:
 On 09/16/2014 04:50 AM, Frans Klaver wrote:
  On Mon, Sep 15, 2014 at 01:31:56PM -0400, Peter Hurley wrote:
  On 09/15/2014 11:39 AM, Peter Hurley wrote:
  On 09/15/2014 10:00 AM, 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.
 
  Why is the threadirqs kernel boot option not sufficient?
  Or conversely, shouldn't this be selectable?
 
  
  I wasn't aware of the threadirqs boot option. I also wouldn't know if
  this should be selectable. What would be a reason to favor the
  non-threaded irq over the threaded irq?
 
 Not everyone cares enough about serial to dedicate kthreads to it :)

Fair enough. In that light, we might not care enough about other
subsystems to dedicate kthreads to it :). Selectable seems reasonable in
that case.


  Also, do you see the same performance differential when you implement this
  in the 8250 driver (that is, on top of Sebastian's omap-8250 conversion)?
 
  
  I haven't gotten Sebastian's driver to work properly yet on the console.
  There was no reason for me yet to throw my omap changes on top of
  Sebastian's queue.
  
  PS - To overflow the 64 byte RX FIFO at those data rates means interrupt
  latency in excess of 250us?
  
  At 3686400 baud it should take about 174 us to fill a 64 byte buffer. I
  haven't done any measurements on the interrupt latency though. If you
  consider that we're sending about 1kB of data, 240 times a second, we're
  spending a lot of time reading data from the uart. I can imagine the
  system has other work to do as well.
 
 System work should not keep irqs from being serviced. Even 174us is a long
 time not to service an interrupt. Maybe console writes are keeping the isr
 from running?

That's quite possible. I'll have to redo the test setup I had for this to
give you a decent answer. I'll have to do that anyway as Sebastian's
8250 conversion improves.

Thanks for the comments,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-09-16 Thread Frans Klaver
On Mon, Sep 15, 2014 at 01:31:56PM -0400, Peter Hurley wrote:
 On 09/15/2014 11:39 AM, Peter Hurley wrote:
  On 09/15/2014 10:00 AM, 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.
  
  Why is the threadirqs kernel boot option not sufficient?
  Or conversely, shouldn't this be selectable?
 

I wasn't aware of the threadirqs boot option. I also wouldn't know if
this should be selectable. What would be a reason to favor the
non-threaded irq over the threaded irq?

 Also, do you see the same performance differential when you implement this
 in the 8250 driver (that is, on top of Sebastian's omap-8250 conversion)?
 

I haven't gotten Sebastian's driver to work properly yet on the console.
There was no reason for me yet to throw my omap changes on top of
Sebastian's queue.

  PS - To overflow the 64 byte RX FIFO at those data rates means interrupt
  latency in excess of 250us?

At 3686400 baud it should take about 174 us to fill a 64 byte buffer. I
haven't done any measurements on the interrupt latency though. If you
consider that we're sending about 1kB of data, 240 times a second, we're
spending a lot of time reading data from the uart. I can imagine the
system has other work to do as well.

This doesn't mean that we're not interested in Sebastian's driver
anymore though. We really want that dma support.

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-16 Thread Frans Klaver
On Mon, Sep 15, 2014 at 06:42:04PM +0200, Sebastian Andrzej Siewior wrote:
 On 09/12/2014 12:28 PM, Frans Klaver wrote:
  port config is 115200 8N1. I don't recall doing anything special. I
  boot, login, less file and get a lock.
 
 So I booted my mini Debian 7.6 (basic system + openssh) on my beagle
 bone black which is:
 
 [0.00] AM335X ES2.0 (neon )
 
 configured a console, login, invoked less file. The file was shown, I
 hit on the space key so less shows me more of the file. No lock-up.
 I tried booting via NFS and MMC. I tried various files with less.
 
 My dot config is here
   https://breakpoint.cc/config-am335x-bb.txt.xz
 
 If there is nothing specific to the file you do less on I have no idea
 what else it could if it is not the config.

I'll test your config and go through the differences.

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-16 Thread Frans Klaver
On Tue, Sep 16, 2014 at 11:05:40AM +0200, Frans Klaver wrote:
 On Mon, Sep 15, 2014 at 06:42:04PM +0200, Sebastian Andrzej Siewior wrote:
  On 09/12/2014 12:28 PM, Frans Klaver wrote:
   port config is 115200 8N1. I don't recall doing anything special. I
   boot, login, less file and get a lock.
  
  So I booted my mini Debian 7.6 (basic system + openssh) on my beagle
  bone black which is:
  
  [0.00] AM335X ES2.0 (neon )
  
  configured a console, login, invoked less file. The file was shown, I
  hit on the space key so less shows me more of the file. No lock-up.
  I tried booting via NFS and MMC. I tried various files with less.
  
  My dot config is here
  https://breakpoint.cc/config-am335x-bb.txt.xz
  
  If there is nothing specific to the file you do less on I have no idea
  what else it could if it is not the config.
 
 I'll test your config and go through the differences.

So far it looks like it isn't in the config. I've tested both your and
my config on debian wheezy on a boneblack. No lock ups.

Less doesn't fill my entire screen, so it looks a bit funky when
scrolling, but I'm not sure if that's related to the driver.

I'll have to look further for things we did that may have impact here.

 Thanks,
 Frans
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-16 Thread Frans Klaver
On Tue, Sep 16, 2014 at 02:42:00PM +0200, Frans Klaver wrote:
 On Tue, Sep 16, 2014 at 11:05:40AM +0200, Frans Klaver wrote:
  On Mon, Sep 15, 2014 at 06:42:04PM +0200, Sebastian Andrzej Siewior wrote:
   If there is nothing specific to the file you do less on I have no idea
   what else it could if it is not the config.
  
  I'll test your config and go through the differences.
 
 So far it looks like it isn't in the config. I've tested both your and
 my config on debian wheezy on a boneblack. No lock ups.

No, skip that. It is in the config. I'll hunt it down.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/4] omap-serial high-speed fixes/improvements

2014-09-15 Thread Frans Klaver
Hi,

Here's version 3 of my fixes for high speed usage of the omap serial driver.
Sebastian's 8250 based implementation doesn't seem to be quite there yet. As
long as that's the case, I'd like to go ahead with improving this series.

v2..v3
  - fix an undefined variable in prevent division by zero
  - use IRQ_ONESHOT for the threaded irq in use threaded interrupt handler

v1..v2
  - centralize baud_is_mode16's calculation
  - fix/unbreak an uninitialized variable in use threaded interrupt handler
  - read has-hw-flow-control property in of_get_uart_port_info

Frans Klaver (4):
  tty: omap-serial: pull out calculation from baud_is_mode16
  tty: omap-serial: prevent division by zero
  tty: omap-serial: use threaded interrupt handler
  tty: omap-serial: support setting of hardware flow control in dts

 .../devicetree/bindings/serial/omap_serial.txt |  1 +
 drivers/tty/serial/omap-serial.c   | 64 +-
 2 files changed, 50 insertions(+), 15 deletions(-)

-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-09-15 Thread Frans Klaver
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.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 30 +++---
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 7d3f557..398139a 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -575,6 +575,20 @@ 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;
+
+   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 +598,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 +608,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 */
@@ -634,7 +646,7 @@ static irqreturn_t serial_omap_irq(int irq, void *dev_id)
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 +743,19 @@ 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,
+   IRQF_ONESHOT | 
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,
+   IRQF_ONESHOT | 
up-port.irqflags,
+   up-name, up);
if (retval) {
free_irq(up-port.irq, up);
return retval;
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/4] tty: omap-serial: pull out calculation from baud_is_mode16

2014-09-15 Thread Frans Klaver
To determine the correct divisor, we need to know the difference between
the desired baud rate and the actual baud rate. The calculation for this
difference is implemented twice within omap_serial_baud_is_mode16().
Pull out the calculation for easier maintenance.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d017cec..ae935ce 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -239,6 +239,22 @@ static void serial_omap_enable_wakeup(struct 
uart_omap_port *up, bool enable)
 }
 
 /*
+ * Calculate the absolute difference between the desired and actual baud
+ * rate for the given mode.
+ */
+static inline int calculate_baud_abs_diff(struct uart_port *port,
+   unsigned int baud, unsigned int mode)
+{
+   unsigned int n = port-uartclk / (mode * baud);
+   int abs_diff = baud - (port-uartclk / (mode * n));
+
+   if (abs_diff  0)
+   abs_diff = -abs_diff;
+
+   return abs_diff;
+}
+
+/*
  * serial_omap_baud_is_mode16 - check if baud rate is MODE16X
  * @port: uart port info
  * @baud: baudrate for which mode needs to be determined
@@ -252,14 +268,8 @@ static void serial_omap_enable_wakeup(struct 
uart_omap_port *up, bool enable)
 static bool
 serial_omap_baud_is_mode16(struct uart_port *port, unsigned int baud)
 {
-   unsigned int n13 = port-uartclk / (13 * baud);
-   unsigned int n16 = port-uartclk / (16 * baud);
-   int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
-   int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
-   if (baudAbsDiff13  0)
-   baudAbsDiff13 = -baudAbsDiff13;
-   if (baudAbsDiff16  0)
-   baudAbsDiff16 = -baudAbsDiff16;
+   int baudAbsDiff13 = calculate_baud_abs_diff(port, baud, 13);
+   int baudAbsDiff16 = calculate_baud_abs_diff(port, baud, 16);
 
return (baudAbsDiff13 = baudAbsDiff16);
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] tty: omap-serial: prevent division by zero

2014-09-15 Thread Frans Klaver
If the chosen baud rate is large enough (e.g. 3.5 megabaud), the
calculated n values in calculate_baud_abs_diff may become 0. This causes
a division by zero when calculating the difference between calculated
and desired baud rates. To prevent this, cap n on 1.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index ae935ce..7d3f557 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -246,8 +246,12 @@ static inline int calculate_baud_abs_diff(struct uart_port 
*port,
unsigned int baud, unsigned int mode)
 {
unsigned int n = port-uartclk / (mode * baud);
-   int abs_diff = baud - (port-uartclk / (mode * n));
+   int abs_diff;
 
+   if (n == 0)
+   n = 1;
+
+   abs_diff = baud - (port-uartclk / (mode * n));
if (abs_diff  0)
abs_diff = -abs_diff;
 
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4] tty: omap-serial: support setting of hardware flow control in dts

2014-09-15 Thread Frans Klaver
This makes hardware flow control availability configurable from the
device tree.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 Documentation/devicetree/bindings/serial/omap_serial.txt | 1 +
 drivers/tty/serial/omap-serial.c | 4 
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt 
b/Documentation/devicetree/bindings/serial/omap_serial.txt
index 342eedd..1b629e9 100644
--- a/Documentation/devicetree/bindings/serial/omap_serial.txt
+++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
@@ -8,3 +8,4 @@ Required properties:
 
 Optional properties:
 - clock-frequency : frequency of the clock input to the UART
+- has-hw-flow-control : the hardware has flow control capability
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 398139a..bff6874 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1597,6 +1597,10 @@ static struct omap_uart_port_info 
*of_get_uart_port_info(struct device *dev)
 
of_property_read_u32(dev-of_node, clock-frequency,
 omap_up_info-uartclk);
+
+   if (of_property_read_bool(dev-of_node, has-hw-flow-control))
+   omap_up_info-flags |= UPF_HARD_FLOW;
+
return omap_up_info;
 }
 
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-12 Thread Frans Klaver
On Fri, Sep 12, 2014 at 11:51:22AM +0200, Sebastian Andrzej Siewior wrote:
 On 09/12/2014 11:40 AM, Frans Klaver wrote:
 
  I'm not sure. I just reproduced this on a boneblack, using your uart_v9
  branch.
  
  This problem only pops-up if you use DMA. With disabled DMA you don't
  see this, right?
  
  I get the lockup both with and without DMA enabled. Here's the 8250
  config I use. Our full .config is attached in case it may provide
  something relevant.
 
 Hmm. I have a bone black here. Can you tell what you did to get this
 lockup? The port configuration (unless 115200 8N1) and what you did to
 get this lockup.

port config is 115200 8N1. I don't recall doing anything special. I
boot, login, less file and get a lock.


  Something that may also help: when I have a lockup on the boneblack, dma
  is enabled and something is written to console like I described earlier,
  I get the following bad irq:
 
  Full dmesg is also attached.
 
 This one should be stuffed by this:
   [RFC] ARM: edma: unconditionally ack the error interrupt
   https://lkml.org/lkml/2014/9/10/714

OK, that makes the console usable again after I write something to kmsg.

  Hope you find something useful in there.
 
  Thanks,
  Frans
 
 Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-11 Thread Frans Klaver
On Thu, Sep 11, 2014 at 02:50:50PM +0200, Sebastian Andrzej Siewior wrote:
 On 09/11/2014 02:32 PM, Peter Hurley wrote:
  On 09/11/2014 07:42 AM, Sebastian Andrzej Siewior wrote:
  I also need a watchdog timer for TX since it seems that on omap3 the
  DMA engine suddenly forgets to continue with DMA…
  
  One difference I noticed between the omap driver and the 8250 driver is
  the way modem status interrupts are handled.
  
  The omap driver only checks modem status for the UART_IIR_MSI interrupt 
  type.
  The 8250 driver checks modem status at every interrupt (other than NO_INT).
  
  I think the UART_MSR_DCTS bit always reflects that the CTS input has changed
  between reads of the MSR _even if auto CTS is on_. So perhaps the hardware
  is being stopped by uart_handle_cts_change() when auto CTS is on?
 
 I doubt that. What I see from a timer debug is that the TX-FIFO level
 is at 0, the DMA transfer for say 1024 bytes start.
 The FIFO is filled to 64bytes and refilled so it doesn't drop below 50.
 At the time of the stall I see that the DMA engine has outstanding
 bytes which it should transfer and the TX FIFO is empty. If hardware
 flow control stops the transfer, I would expect that the DMA engine
 still fills the TX-FIFO until 64 and then waits. But it doesn't.
 Writing bytes into the FIFO leads to bytes beeing sent (and I see them
 on the other side) but the DMA transfer is still on hold. Canceling the
 DMA transfer and re-programming the remaining bytes transfers the
 remaining bytes.
 
 The odd thing is that I only triggered it with less file. It doesn't
 happen on regular console interaction or cat large-file. And it only
 triggers on beagle board xm (omap34xx) and I wasn't able to reproduce
 it on am335x or dra7. The latter shares the same DMA engine as beagle
 board xm.

I can still reproduce it on am335x. I can get out of it as soon as
something else gets written to the console though.

# echo 3something /dev/kmsg

Frans


 
 I remember also that I disabled the HW/SW float control just to make
 sure it is not it.
 
  
  Regards,
  Peter Hurley
  
  [The UPF_HARD_FLOW thing was pretty much just done for omap even though
  8250 already had auto CTS/auto RTS. Serial core hardware flow control 
  support
  needs a redo as drivers have pretty much tacked stuff on randomly.]
 
 Sebastian
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

2014-09-11 Thread Frans Klaver
On 11 September 2014 18:04:32 CEST, Sebastian Andrzej Siewior 
bige...@linutronix.de wrote:
On 09/11/2014 05:11 PM, Frans Klaver wrote:

 I can still reproduce it on am335x. I can get out of it as soon as
 something else gets written to the console though.
 
 # echo 3something /dev/kmsg

Is this to stall it or to get out of it?

This is to get out of it. I do this from an ssh connection after the console 
got stuck. The 3 kind of ensures the message is actually sent to ttyS0.



 
 Frans

Sebastian

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 00/18] 8250-core based serial driver for OMAP + DMA

2014-09-10 Thread Frans Klaver
On Tue, Sep 09, 2014 at 09:41:20PM +0200, Sebastian Andrzej Siewior wrote:
 On 09/08/2014 08:33 PM, Frans Klaver wrote:
  Thanks. I'll give it a spin on Wednesday.
 
 Could you please pull the upcoming v9 first?
 
  git://git.breakpoint.cc/bigeasy/linux.git uart_v9_pre1
 
 This solves a few of my am335x related issues.

Using v9_pre1, I get a kernel panic in edma_dma_pause() on echan-edesc
being NULL. I do get data before this happens. This is at high speed,
high rate. No mention of the irq having too much to do, though. The more
data I transmit, the more likely this is to occur.

I don't currently have the setup to lower the baudrate. I would probably
need to reproduce this on a beaglebone, instead of our custom board.
I'll see if I can do that.

If you need more info, just let me know.

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 00/18] 8250-core based serial driver for OMAP + DMA

2014-09-08 Thread Frans Klaver
Hi,

On Fri, Sep 05, 2014 at 09:02:35PM +0200, Sebastian Andrzej Siewior wrote:
 This is my complete queue fo the omap serial driver based on the 8250 core
 code. I played with it on beagle bone, am335x-evm and dra7xx including DMA.
 The runtime-pm pieces look now bug-compatible with the omap-serial driver.
 Besides the runtime-om improvement I also fixed a few corner cases for the
 TX-DMA problem. The DMA fixes (in edma and omap-dma) were dropped and the
 problem has been in 8250-dma via patch #13.

Thanks for the respin. I've just tested the series a bit. Here's some
things I ran into.

- Basic console use is better than the previous series. It behaves
  pretty much like the omap-serial implementation.

- ncurses based applications (vi, less) don't play nice for me on the
  console with this series. less doesn't show me anything. vi doesn't
  return to console properly.

- I seem seem to get stuck in a serial8250: too much work for irq%d
  loop somewhat reliably. We have a rather demanding application with
  typically somewhere between 600 and 1000 byte packets being sent at
  240Hz (roughly somewhere between 1.5 and 2 Mb/s). We run at baudrate
  3500k. I get into this too much work thing already when running at
  300 bytes per packet.

I hope this is of some use to you. I'll do more testing later.

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 00/18] 8250-core based serial driver for OMAP + DMA

2014-09-08 Thread Frans Klaver
On Mon, Sep 08, 2014 at 06:33:13PM +0200, Sebastian Andrzej Siewior wrote:
 * Sebastian Andrzej Siewior | 2014-09-08 17:15:01 [+0200]:
 
 * Frans Klaver | 2014-09-08 16:46:18 [+0200]:
 
 - ncurses based applications (vi, less) don't play nice for me on the
   console with this series. less doesn't show me anything. vi doesn't
   return to console properly.
 
 Can you give a test case 
 
 Okay. less. My am335x-evm freezes after a while for no obvious reason.
 The data that hits the RX fifo is still received but the TX won't do
 anything. The DMA request is pending, the FIFO level is @64 bytes and
 the UART doesn't make any progress.
 On beagle-board I see what you described: less on a file and nothing
 happens.

Exactly that, yes.

Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 00/18] 8250-core based serial driver for OMAP + DMA

2014-09-08 Thread Frans Klaver
On Mon, Sep 08, 2014 at 05:15:01PM +0200, Sebastian Andrzej Siewior wrote:
 * Frans Klaver | 2014-09-08 16:46:18 [+0200]:
 
 - I seem seem to get stuck in a serial8250: too much work for irq%d
   loop somewhat reliably. We have a rather demanding application with
   typically somewhere between 600 and 1000 byte packets being sent at
   240Hz (roughly somewhere between 1.5 and 2 Mb/s). We run at baudrate
   3500k. I get into this too much work thing already when running at
   300 bytes per packet.
 
 Do you get this message also at lower baud rates, say 115200?

I don't get this message at lower data rates. Haven't tested lower baud
rates yet.

 What I am trying to understand is why you are spinning in the handler. 
 _With_ DMA you should hardly get into the serial handler under normal 
 conditions. Running at 3.5MB/sec should give one byte every 2.8us and
 48 Bytes every ~137us. This looks like plenty of time to get  out of
 the handler. My *guess* is that serial8250_handle_irq() has IIR often
 set to timeout and you end up fetching byte after byte. 
 
 This patch should protocol when and why you got into the handler.

 diff --git a/drivers/tty/serial/8250/8250_core.c 
 b/drivers/tty/serial/8250/8250_core.c
 index 7111b22de000..59852069e4a0 100644
 --- a/drivers/tty/serial/8250/8250_core.c
 +++ b/drivers/tty/serial/8250/8250_core.c
 @@ -1583,6 +1583,7 @@ int serial8250_handle_irq(struct uart_port *port, 
 unsigned int iir)
   status = serial_port_in(port, UART_LSR);
  
   DEBUG_INTR(status = %x..., status);
 + trace_printk(l%d IIR %x LSR %x\n, port-line, iir, status);
  
   if (status  (UART_LSR_DR | UART_LSR_BI)) {
   if (up-dma)
 @@ -1707,6 +1708,7 @@ static irqreturn_t serial8250_interrupt(int irq, void 
 *dev_id)
  
   spin_unlock(i-lock);
  
 + trace_printk(%d e\n, irq);
   DEBUG_INTR(end.\n);
  
   return IRQ_RETVAL(handled);
 

Thanks. I'll give it a spin on Wednesday.


 I hope this is of some use to you. I'll do more testing later.
 
 Which SoC do you use and do you have DMA enabled?

am335x, DMA is enabled, unless I need to do something extra in the
device tree. We depend on am335x.dtsi, so I would think that would be
automatic if CONFIG_SERIAL_8250_DMA=y.

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-08-22 Thread Frans Klaver
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.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 30 +++---
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 14a0167..1671443a 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -575,6 +575,20 @@ 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;
+
+   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 +598,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 +608,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 */
@@ -634,7 +646,7 @@ static irqreturn_t serial_omap_irq(int irq, void *dev_id)
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 +743,19 @@ 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,
+   IRQF_ONESHOT | 
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,
+   IRQF_ONESHOT | 
up-port.irqflags,
+   up-name, up);
if (retval) {
free_irq(up-port.irq, up);
return retval;
-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-08-21 Thread Frans Klaver
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-08-21 Thread Frans Klaver
On August 21, 2014 11:48:40 PM CEST, Felipe Balbi ba...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-08-20 Thread Frans Klaver
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/15] tty: serial: 8250_core: add run time pm

2014-08-20 Thread Frans Klaver
Hi,

On Fri, Aug 15, 2014 at 07:42:31PM +0200, Sebastian Andrzej Siewior wrote:
 --- a/drivers/tty/serial/8250/8250_core.c
 +++ b/drivers/tty/serial/8250/8250_core.c
 @@ -1899,12 +1984,22 @@ static void wait_for_xmitr(struct uart_8250_port *up, 
 int bits)
  
  static int serial8250_get_poll_char(struct uart_port *port)
  {
 - unsigned char lsr = serial_port_in(port, UART_LSR);
 + unsigned char lsr;
 + int status;
 +
 + serial8250_rpm_get(up);

or up won't be defined below. You probably need 
+   struct uart_8250_port *up = up_to_u8250p(port);
somewhere in there.

 - if (!(lsr  UART_LSR_DR))
 - return NO_POLL_CHAR;
 + lsr = serial_port_in(port, UART_LSR);
  
 - return serial_port_in(port, UART_RX);
 + if (!(lsr  UART_LSR_DR)) {
 + status = NO_POLL_CHAR;
 + goto out;
 + }
 +
 + status = serial_port_in(port, UART_RX);
 +out:
 + serial8250_rpm_put(up);
 + return status;
  }

Cheers,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/15] tty: serial: 8250_core: add run time pm

2014-08-20 Thread Frans Klaver
On Wed, Aug 20, 2014 at 11:23:21AM +0200, Frans Klaver wrote:
 Hi,
 
 On Fri, Aug 15, 2014 at 07:42:31PM +0200, Sebastian Andrzej Siewior wrote:
  --- a/drivers/tty/serial/8250/8250_core.c
  +++ b/drivers/tty/serial/8250/8250_core.c
  @@ -1899,12 +1984,22 @@ static void wait_for_xmitr(struct uart_8250_port 
  *up, int bits)
   
   static int serial8250_get_poll_char(struct uart_port *port)
   {
  -   unsigned char lsr = serial_port_in(port, UART_LSR);
  +   unsigned char lsr;
  +   int status;
  +
  +   serial8250_rpm_get(up);
 
 or up won't be defined below. You probably need 

Obviously I meant to say that 'up' won't be defined here.


 + struct uart_8250_port *up = up_to_u8250p(port);
 somewhere in there.
 
  -   if (!(lsr  UART_LSR_DR))
  -   return NO_POLL_CHAR;
  +   lsr = serial_port_in(port, UART_LSR);
   
  -   return serial_port_in(port, UART_RX);
  +   if (!(lsr  UART_LSR_DR)) {
  +   status = NO_POLL_CHAR;
  +   goto out;
  +   }
  +
  +   status = serial_port_in(port, UART_RX);
  +out:
  +   serial8250_rpm_put(up);
  +   return status;
   }
 
 Cheers,
 Frans
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4] tty: omap-serial: support setting of hardware flow control in dts

2014-08-19 Thread Frans Klaver
This makes hardware flow control availability configurable from the
device tree.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 Documentation/devicetree/bindings/serial/omap_serial.txt | 1 +
 drivers/tty/serial/omap-serial.c | 4 
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt 
b/Documentation/devicetree/bindings/serial/omap_serial.txt
index 342eedd..1b629e9 100644
--- a/Documentation/devicetree/bindings/serial/omap_serial.txt
+++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
@@ -8,3 +8,4 @@ Required properties:
 
 Optional properties:
 - clock-frequency : frequency of the clock input to the UART
+- has-hw-flow-control : the hardware has flow control capability
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 57664b9..7d3b3d2 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1598,6 +1598,10 @@ static struct omap_uart_port_info 
*of_get_uart_port_info(struct device *dev)
 
of_property_read_u32(dev-of_node, clock-frequency,
 omap_up_info-uartclk);
+
+   if (of_property_read_bool(dev-of_node, has-hw-flow-control))
+   omap_up_info-flags |= UPF_HARD_FLOW;
+
return omap_up_info;
 }
 
-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH 3/4] tty: omap-serial: use threaded interrupt handler

2014-08-19 Thread Frans Klaver
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.kla...@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);
+   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;
-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCHv2 0/4] omap-serial high-speed fixes/improvements

2014-08-19 Thread Frans Klaver
Here's version 2 of the patches that should improve the behavior of the omap
serial port at high baud and data rates.

Differences with regard to v1 are:
  - centralize baud_is_mode16's calculation
  - fix/unbreak an uninitialized variable in use threaded interrupt handler
  - read has-hw-flow-control property in of_get_uart_port_info

Frans Klaver (4):
  tty: omap-serial: pull out calculation from baud_is_mode16
  tty: omap-serial: prevent division by zero
  tty: omap-serial: use threaded interrupt handler
  tty: omap-serial: support setting of hardware flow control in dts

 .../devicetree/bindings/serial/omap_serial.txt |  1 +
 drivers/tty/serial/omap-serial.c   | 65 +-
 2 files changed, 51 insertions(+), 15 deletions(-)

-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] tty: omap-serial: prevent division by zero

2014-08-19 Thread Frans Klaver
If the chosen baud rate is large enough (e.g. 3.5 megabaud), the
calculated n values in calculate_baud_abs_diff may become 0. This causes
a division by zero when calculating the difference between calculated
and desired baud rates. To prevent this, cap n on 1.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index ae935ce..14a0167 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -246,8 +246,12 @@ static inline int calculate_baud_abs_diff(struct uart_port 
*port,
unsigned int baud, unsigned int mode)
 {
unsigned int n = port-uartclk / (mode * baud);
-   int abs_diff = baud - (port-uartclk / (mode * n));
+   int abs_diff;
 
+   if (n == 0)
+   n = 1;
+
+   abs_diff = baud - (uartclk / (mode * n));
if (abs_diff  0)
abs_diff = -abs_diff;
 
-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/4] tty: omap-serial: pull out calculation from baud_is_mode16

2014-08-19 Thread Frans Klaver
To determine the correct divisor, we need to know the difference between
the desired baud rate and the actual baud rate. The calculation for this
difference is implemented twice within omap_serial_baud_is_mode16().
Pull out the calculation for easier maintenance.

Signed-off-by: Frans Klaver frans.kla...@xsens.com
---
 drivers/tty/serial/omap-serial.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d017cec..ae935ce 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -239,6 +239,22 @@ static void serial_omap_enable_wakeup(struct 
uart_omap_port *up, bool enable)
 }
 
 /*
+ * Calculate the absolute difference between the desired and actual baud
+ * rate for the given mode.
+ */
+static inline int calculate_baud_abs_diff(struct uart_port *port,
+   unsigned int baud, unsigned int mode)
+{
+   unsigned int n = port-uartclk / (mode * baud);
+   int abs_diff = baud - (port-uartclk / (mode * n));
+
+   if (abs_diff  0)
+   abs_diff = -abs_diff;
+
+   return abs_diff;
+}
+
+/*
  * serial_omap_baud_is_mode16 - check if baud rate is MODE16X
  * @port: uart port info
  * @baud: baudrate for which mode needs to be determined
@@ -252,14 +268,8 @@ static void serial_omap_enable_wakeup(struct 
uart_omap_port *up, bool enable)
 static bool
 serial_omap_baud_is_mode16(struct uart_port *port, unsigned int baud)
 {
-   unsigned int n13 = port-uartclk / (13 * baud);
-   unsigned int n16 = port-uartclk / (16 * baud);
-   int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
-   int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
-   if (baudAbsDiff13  0)
-   baudAbsDiff13 = -baudAbsDiff13;
-   if (baudAbsDiff16  0)
-   baudAbsDiff16 = -baudAbsDiff16;
+   int baudAbsDiff13 = calculate_baud_abs_diff(port, baud, 13);
+   int baudAbsDiff16 = calculate_baud_abs_diff(port, baud, 16);
 
return (baudAbsDiff13 = baudAbsDiff16);
 }
-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html