Re: [PATCH] arm: omap2: vc: fix 'or' always true warning
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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]
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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