* [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
@ 2017-10-07 1:46 ` Troy Kisky
0 siblings, 0 replies; 10+ messages in thread
From: Troy Kisky @ 2017-10-07 1:46 UTC (permalink / raw)
To: gregkh
Cc: Troy Kisky, linux-serial, u.kleine-koenig, fabio.estevam,
linux-arm-kernel, l.stach
Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
the interrupt routine no longer will start rx dma.
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
drivers/tty/serial/imx.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dfeff3951f93..15b0ecb4cf60 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port *sport)
}
static void clear_rx_errors(struct imx_port *sport);
-static int start_rx_dma(struct imx_port *sport);
-/*
- * If the RXFIFO is filled with some data, and then we
- * arise a DMA operation to receive them.
- */
-static void imx_dma_rxint(struct imx_port *sport)
-{
- unsigned long temp;
- unsigned long flags;
-
- spin_lock_irqsave(&sport->port.lock, flags);
-
- temp = readl(sport->port.membase + USR2);
- if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
-
- imx_disable_rx_int(sport);
-
- /* tell the DMA to receive the data. */
- start_rx_dma(sport);
- }
-
- spin_unlock_irqrestore(&sport->port.lock, flags);
-}
/*
* We have a modem side uart, so the meanings of RTS and CTS are inverted.
@@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
sts = readl(sport->port.membase + USR1);
sts2 = readl(sport->port.membase + USR2);
- if (sts & (USR1_RRDY | USR1_AGTIM)) {
- if (sport->dma_is_enabled)
- imx_dma_rxint(sport);
- else
- imx_rxint(irq, dev_id);
+ if (!sport->dma_is_enabled && (sts & (USR1_RRDY | USR1_AGTIM))) {
+ imx_rxint(irq, dev_id);
ret = IRQ_HANDLED;
}
@@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port *sport)
temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
writel(temp, sport->port.membase + UCR1);
- temp = readl(sport->port.membase + UCR2);
- temp |= UCR2_ATEN;
- writel(temp, sport->port.membase + UCR2);
-
imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
sport->dma_is_enabled = 1;
--
2.11.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
@ 2017-10-07 1:46 ` Troy Kisky
0 siblings, 0 replies; 10+ messages in thread
From: Troy Kisky @ 2017-10-07 1:46 UTC (permalink / raw)
To: linux-arm-kernel
Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
the interrupt routine no longer will start rx dma.
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
drivers/tty/serial/imx.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dfeff3951f93..15b0ecb4cf60 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port *sport)
}
static void clear_rx_errors(struct imx_port *sport);
-static int start_rx_dma(struct imx_port *sport);
-/*
- * If the RXFIFO is filled with some data, and then we
- * arise a DMA operation to receive them.
- */
-static void imx_dma_rxint(struct imx_port *sport)
-{
- unsigned long temp;
- unsigned long flags;
-
- spin_lock_irqsave(&sport->port.lock, flags);
-
- temp = readl(sport->port.membase + USR2);
- if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
-
- imx_disable_rx_int(sport);
-
- /* tell the DMA to receive the data. */
- start_rx_dma(sport);
- }
-
- spin_unlock_irqrestore(&sport->port.lock, flags);
-}
/*
* We have a modem side uart, so the meanings of RTS and CTS are inverted.
@@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
sts = readl(sport->port.membase + USR1);
sts2 = readl(sport->port.membase + USR2);
- if (sts & (USR1_RRDY | USR1_AGTIM)) {
- if (sport->dma_is_enabled)
- imx_dma_rxint(sport);
- else
- imx_rxint(irq, dev_id);
+ if (!sport->dma_is_enabled && (sts & (USR1_RRDY | USR1_AGTIM))) {
+ imx_rxint(irq, dev_id);
ret = IRQ_HANDLED;
}
@@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port *sport)
temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
writel(temp, sport->port.membase + UCR1);
- temp = readl(sport->port.membase + UCR2);
- temp |= UCR2_ATEN;
- writel(temp, sport->port.membase + UCR2);
-
imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
sport->dma_is_enabled = 1;
--
2.11.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
2017-10-07 1:46 ` Troy Kisky
@ 2017-10-11 6:22 ` Han, Nandor (GE Healthcare)
-1 siblings, 0 replies; 10+ messages in thread
From: Han, Nandor (GE Healthcare) @ 2017-10-11 6:22 UTC (permalink / raw)
To: Troy Kisky, gregkh
Cc: fabio.estevam, u.kleine-koenig, linux-arm-kernel, linux-serial, l.stach
> -----Original Message-----
> From: linux-serial-owner@vger.kernel.org [mailto:linux-serial-
> owner@vger.kernel.org] On Behalf Of Troy Kisky
> Sent: 07 October 2017 04:47
> To: gregkh@linuxfoundation.org
> Cc: fabio.estevam@nxp.com; l.stach@pengutronix.de; u.kleine-
> koenig@pengutronix.de; linux-serial@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Troy Kisky <troy.kisky@boundarydevices.com>
> Subject: EXT: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if
> dma in use
>
> Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
>
> the interrupt routine no longer will start rx dma.
I think you have 2 commits here:
- one that disables ageing timer when DMA is in use
- one that cleans the leftovers after 4dec2f119e86.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
> drivers/tty/serial/imx.c | 34 ++--------------------------------
> 1 file changed, 2 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index
> dfeff3951f93..15b0ecb4cf60 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port
> *sport) }
>
> static void clear_rx_errors(struct imx_port *sport); -static int
> start_rx_dma(struct imx_port *sport);
> -/*
> - * If the RXFIFO is filled with some data, and then we
> - * arise a DMA operation to receive them.
> - */
> -static void imx_dma_rxint(struct imx_port *sport) -{
> - unsigned long temp;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&sport->port.lock, flags);
> -
> - temp = readl(sport->port.membase + USR2);
> - if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
> -
> - imx_disable_rx_int(sport);
> -
> - /* tell the DMA to receive the data. */
> - start_rx_dma(sport);
> - }
> -
> - spin_unlock_irqrestore(&sport->port.lock, flags);
> -}
>
> /*
> * We have a modem side uart, so the meanings of RTS and CTS are inverted.
> @@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
> sts = readl(sport->port.membase + USR1);
> sts2 = readl(sport->port.membase + USR2);
>
> - if (sts & (USR1_RRDY | USR1_AGTIM)) {
> - if (sport->dma_is_enabled)
> - imx_dma_rxint(sport);
> - else
> - imx_rxint(irq, dev_id);
> + if (!sport->dma_is_enabled && (sts & (USR1_RRDY |
> USR1_AGTIM))) {
> + imx_rxint(irq, dev_id);
I don't think we need the check `!sport->dma_is_enabled ` since RRDY and AGTIM interrupts should be disable when DMA is enabled.
> ret = IRQ_HANDLED;
> }
>
> @@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port
> *sport)
> temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
> writel(temp, sport->port.membase + UCR1);
>
> - temp = readl(sport->port.membase + UCR2);
> - temp |= UCR2_ATEN;
> - writel(temp, sport->port.membase + UCR2);
> -
I think this should be moved to imx_startup. At least RRDY is configured there.
<snip>
Regards,
Nandor
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
@ 2017-10-11 6:22 ` Han, Nandor (GE Healthcare)
0 siblings, 0 replies; 10+ messages in thread
From: Han, Nandor (GE Healthcare) @ 2017-10-11 6:22 UTC (permalink / raw)
To: linux-arm-kernel
> -----Original Message-----
> From: linux-serial-owner at vger.kernel.org [mailto:linux-serial-
> owner at vger.kernel.org] On Behalf Of Troy Kisky
> Sent: 07 October 2017 04:47
> To: gregkh at linuxfoundation.org
> Cc: fabio.estevam at nxp.com; l.stach at pengutronix.de; u.kleine-
> koenig at pengutronix.de; linux-serial at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; Troy Kisky <troy.kisky@boundarydevices.com>
> Subject: EXT: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if
> dma in use
>
> Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
>
> the interrupt routine no longer will start rx dma.
I think you have 2 commits here:
- one that disables ageing timer when DMA is in use
- one that cleans the leftovers after 4dec2f119e86.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
> drivers/tty/serial/imx.c | 34 ++--------------------------------
> 1 file changed, 2 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index
> dfeff3951f93..15b0ecb4cf60 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port
> *sport) }
>
> static void clear_rx_errors(struct imx_port *sport); -static int
> start_rx_dma(struct imx_port *sport);
> -/*
> - * If the RXFIFO is filled with some data, and then we
> - * arise a DMA operation to receive them.
> - */
> -static void imx_dma_rxint(struct imx_port *sport) -{
> - unsigned long temp;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&sport->port.lock, flags);
> -
> - temp = readl(sport->port.membase + USR2);
> - if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
> -
> - imx_disable_rx_int(sport);
> -
> - /* tell the DMA to receive the data. */
> - start_rx_dma(sport);
> - }
> -
> - spin_unlock_irqrestore(&sport->port.lock, flags);
> -}
>
> /*
> * We have a modem side uart, so the meanings of RTS and CTS are inverted.
> @@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
> sts = readl(sport->port.membase + USR1);
> sts2 = readl(sport->port.membase + USR2);
>
> - if (sts & (USR1_RRDY | USR1_AGTIM)) {
> - if (sport->dma_is_enabled)
> - imx_dma_rxint(sport);
> - else
> - imx_rxint(irq, dev_id);
> + if (!sport->dma_is_enabled && (sts & (USR1_RRDY |
> USR1_AGTIM))) {
> + imx_rxint(irq, dev_id);
I don't think we need the check `!sport->dma_is_enabled ` since RRDY and AGTIM interrupts should be disable when DMA is enabled.
> ret = IRQ_HANDLED;
> }
>
> @@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port
> *sport)
> temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
> writel(temp, sport->port.membase + UCR1);
>
> - temp = readl(sport->port.membase + UCR2);
> - temp |= UCR2_ATEN;
> - writel(temp, sport->port.membase + UCR2);
> -
I think this should be moved to imx_startup. At least RRDY is configured there.
<snip>
Regards,
Nandor
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
2017-10-11 6:22 ` Han, Nandor (GE Healthcare)
@ 2017-10-11 16:09 ` Troy Kisky
-1 siblings, 0 replies; 10+ messages in thread
From: Troy Kisky @ 2017-10-11 16:09 UTC (permalink / raw)
To: Han, Nandor (GE Healthcare), gregkh
Cc: fabio.estevam, u.kleine-koenig, linux-arm-kernel, linux-serial, l.stach
On 10/10/2017 11:22 PM, Han, Nandor (GE Healthcare) wrote:
>
>
>> -----Original Message-----
>> From: linux-serial-owner@vger.kernel.org [mailto:linux-serial-
>> owner@vger.kernel.org] On Behalf Of Troy Kisky
>> Sent: 07 October 2017 04:47
>> To: gregkh@linuxfoundation.org
>> Cc: fabio.estevam@nxp.com; l.stach@pengutronix.de; u.kleine-
>> koenig@pengutronix.de; linux-serial@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; Troy Kisky <troy.kisky@boundarydevices.com>
>> Subject: EXT: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if
>> dma in use
>>
>> Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
>>
>> the interrupt routine no longer will start rx dma.
>
> I think you have 2 commits here:
> - one that disables ageing timer when DMA is in use
> - one that cleans the leftovers after 4dec2f119e86.
>
I could kill 3 lines, in the 1st patch, and the other 27 in the next.
But the 2 patches would not be independent. You should not apply the
second without the 1st. I think the change is simple enough not to
warrant a series.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
>> drivers/tty/serial/imx.c | 34 ++--------------------------------
>> 1 file changed, 2 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index
>> dfeff3951f93..15b0ecb4cf60 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port
>> *sport) }
>>
>> static void clear_rx_errors(struct imx_port *sport); -static int
>> start_rx_dma(struct imx_port *sport);
>> -/*
>> - * If the RXFIFO is filled with some data, and then we
>> - * arise a DMA operation to receive them.
>> - */
>> -static void imx_dma_rxint(struct imx_port *sport) -{
>> - unsigned long temp;
>> - unsigned long flags;
>> -
>> - spin_lock_irqsave(&sport->port.lock, flags);
>> -
>> - temp = readl(sport->port.membase + USR2);
>> - if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
>> -
>> - imx_disable_rx_int(sport);
>> -
>> - /* tell the DMA to receive the data. */
>> - start_rx_dma(sport);
>> - }
>> -
>> - spin_unlock_irqrestore(&sport->port.lock, flags);
>> -}
>>
>> /*
>> * We have a modem side uart, so the meanings of RTS and CTS are inverted.
>> @@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
>> sts = readl(sport->port.membase + USR1);
>> sts2 = readl(sport->port.membase + USR2);
>>
>> - if (sts & (USR1_RRDY | USR1_AGTIM)) {
>> - if (sport->dma_is_enabled)
>> - imx_dma_rxint(sport);
>> - else
>> - imx_rxint(irq, dev_id);
>> + if (!sport->dma_is_enabled && (sts & (USR1_RRDY |
>> USR1_AGTIM))) {
>> + imx_rxint(irq, dev_id);
>
> I don't think we need the check `!sport->dma_is_enabled ` since RRDY and AGTIM interrupts should be disable when DMA is enabled.
If I AND the status with its corresponding interrupt enable. But that is just as complicated, and
more different from the original.
>
>> ret = IRQ_HANDLED;
>> }
>>
>> @@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port
>> *sport)
>> temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
>> writel(temp, sport->port.membase + UCR1);
>>
>> - temp = readl(sport->port.membase + UCR2);
>> - temp |= UCR2_ATEN;
>> - writel(temp, sport->port.membase + UCR2);
>> -
>
> I think this should be moved to imx_startup. At least RRDY is configured there.
>
Currently, non-dma mode does not use the ageing timer. You get an interrupt immediately when
1 character is in the fifo. So, enabling the ageing timer and changing fifo level, should really be
a separate patch. But it would be a nice patch.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
@ 2017-10-11 16:09 ` Troy Kisky
0 siblings, 0 replies; 10+ messages in thread
From: Troy Kisky @ 2017-10-11 16:09 UTC (permalink / raw)
To: linux-arm-kernel
On 10/10/2017 11:22 PM, Han, Nandor (GE Healthcare) wrote:
>
>
>> -----Original Message-----
>> From: linux-serial-owner at vger.kernel.org [mailto:linux-serial-
>> owner at vger.kernel.org] On Behalf Of Troy Kisky
>> Sent: 07 October 2017 04:47
>> To: gregkh at linuxfoundation.org
>> Cc: fabio.estevam at nxp.com; l.stach at pengutronix.de; u.kleine-
>> koenig at pengutronix.de; linux-serial at vger.kernel.org; linux-arm-
>> kernel at lists.infradead.org; Troy Kisky <troy.kisky@boundarydevices.com>
>> Subject: EXT: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if
>> dma in use
>>
>> Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
>>
>> the interrupt routine no longer will start rx dma.
>
> I think you have 2 commits here:
> - one that disables ageing timer when DMA is in use
> - one that cleans the leftovers after 4dec2f119e86.
>
I could kill 3 lines, in the 1st patch, and the other 27 in the next.
But the 2 patches would not be independent. You should not apply the
second without the 1st. I think the change is simple enough not to
warrant a series.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
>> drivers/tty/serial/imx.c | 34 ++--------------------------------
>> 1 file changed, 2 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index
>> dfeff3951f93..15b0ecb4cf60 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -732,29 +732,6 @@ static void imx_disable_rx_int(struct imx_port
>> *sport) }
>>
>> static void clear_rx_errors(struct imx_port *sport); -static int
>> start_rx_dma(struct imx_port *sport);
>> -/*
>> - * If the RXFIFO is filled with some data, and then we
>> - * arise a DMA operation to receive them.
>> - */
>> -static void imx_dma_rxint(struct imx_port *sport) -{
>> - unsigned long temp;
>> - unsigned long flags;
>> -
>> - spin_lock_irqsave(&sport->port.lock, flags);
>> -
>> - temp = readl(sport->port.membase + USR2);
>> - if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
>> -
>> - imx_disable_rx_int(sport);
>> -
>> - /* tell the DMA to receive the data. */
>> - start_rx_dma(sport);
>> - }
>> -
>> - spin_unlock_irqrestore(&sport->port.lock, flags);
>> -}
>>
>> /*
>> * We have a modem side uart, so the meanings of RTS and CTS are inverted.
>> @@ -816,11 +793,8 @@ static irqreturn_t imx_int(int irq, void *dev_id)
>> sts = readl(sport->port.membase + USR1);
>> sts2 = readl(sport->port.membase + USR2);
>>
>> - if (sts & (USR1_RRDY | USR1_AGTIM)) {
>> - if (sport->dma_is_enabled)
>> - imx_dma_rxint(sport);
>> - else
>> - imx_rxint(irq, dev_id);
>> + if (!sport->dma_is_enabled && (sts & (USR1_RRDY |
>> USR1_AGTIM))) {
>> + imx_rxint(irq, dev_id);
>
> I don't think we need the check `!sport->dma_is_enabled ` since RRDY and AGTIM interrupts should be disable when DMA is enabled.
If I AND the status with its corresponding interrupt enable. But that is just as complicated, and
more different from the original.
>
>> ret = IRQ_HANDLED;
>> }
>>
>> @@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port
>> *sport)
>> temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
>> writel(temp, sport->port.membase + UCR1);
>>
>> - temp = readl(sport->port.membase + UCR2);
>> - temp |= UCR2_ATEN;
>> - writel(temp, sport->port.membase + UCR2);
>> -
>
> I think this should be moved to imx_startup. At least RRDY is configured there.
>
Currently, non-dma mode does not use the ageing timer. You get an interrupt immediately when
1 character is in the fifo. So, enabling the ageing timer and changing fifo level, should really be
a separate patch. But it would be a nice patch.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
2017-10-11 16:09 ` Troy Kisky
@ 2017-10-13 14:47 ` Han, Nandor (GE Healthcare)
-1 siblings, 0 replies; 10+ messages in thread
From: Han, Nandor (GE Healthcare) @ 2017-10-13 14:47 UTC (permalink / raw)
To: Troy Kisky, gregkh
Cc: fabio.estevam, u.kleine-koenig, linux-arm-kernel, linux-serial, l.stach
> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> Sent: 11 October 2017 19:10
> To: Han, Nandor (GE Healthcare) <nandor.han@ge.com>;
> gregkh@linuxfoundation.org
> Cc: fabio.estevam@nxp.com; l.stach@pengutronix.de; u.kleine-
> koenig@pengutronix.de; linux-serial@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: EXT: Re: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if
> dma in use
>
> On 10/10/2017 11:22 PM, Han, Nandor (GE Healthcare) wrote:
<snip>
> >> - imx_rxint(irq, dev_id);
> >> + if (!sport->dma_is_enabled && (sts & (USR1_RRDY |
> >> USR1_AGTIM))) {
> >> + imx_rxint(irq, dev_id);
> >
> > I don't think we need the check `!sport->dma_is_enabled ` since RRDY and
> AGTIM interrupts should be disable when DMA is enabled.
>
> If I AND the status with its corresponding interrupt enable. But that is just as
> complicated, and more different from the original.
>
>
Yes, I agree, but it would be the right thing to do. :)
> >
> >> ret = IRQ_HANDLED;
> >> }
> >>
> >> @@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port
> >> *sport)
> >> temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
> >> writel(temp, sport->port.membase + UCR1);
> >>
> >> - temp = readl(sport->port.membase + UCR2);
> >> - temp |= UCR2_ATEN;
> >> - writel(temp, sport->port.membase + UCR2);
> >> -
> >
> > I think this should be moved to imx_startup. At least RRDY is configured
> there.
> >
>
>
> Currently, non-dma mode does not use the ageing timer. You get an interrupt
> immediately when
> 1 character is in the fifo. So, enabling the ageing timer and changing fifo level,
> should really be a separate patch. But it would be a nice patch.
>
>
Ok but in this case, we are left with aging timer and rrdy check in the interrupt and imx-startup enables only rrdy. (which is not much different than what it was before, since aging timer didn't get a chance to be triggered)
I agree that changing the fifo level to give a chance also to the aging timer will be the right thing to do here.
Overall, I think is ok, my main concern with not enabling the aging timer in startup is that people can be confused about this.
Reviewed-by: Nandor Han <Nandor.han@ge.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
@ 2017-10-13 14:47 ` Han, Nandor (GE Healthcare)
0 siblings, 0 replies; 10+ messages in thread
From: Han, Nandor (GE Healthcare) @ 2017-10-13 14:47 UTC (permalink / raw)
To: linux-arm-kernel
> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky at boundarydevices.com]
> Sent: 11 October 2017 19:10
> To: Han, Nandor (GE Healthcare) <nandor.han@ge.com>;
> gregkh at linuxfoundation.org
> Cc: fabio.estevam at nxp.com; l.stach at pengutronix.de; u.kleine-
> koenig at pengutronix.de; linux-serial at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org
> Subject: EXT: Re: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if
> dma in use
>
> On 10/10/2017 11:22 PM, Han, Nandor (GE Healthcare) wrote:
<snip>
> >> - imx_rxint(irq, dev_id);
> >> + if (!sport->dma_is_enabled && (sts & (USR1_RRDY |
> >> USR1_AGTIM))) {
> >> + imx_rxint(irq, dev_id);
> >
> > I don't think we need the check `!sport->dma_is_enabled ` since RRDY and
> AGTIM interrupts should be disable when DMA is enabled.
>
> If I AND the status with its corresponding interrupt enable. But that is just as
> complicated, and more different from the original.
>
>
Yes, I agree, but it would be the right thing to do. :)
> >
> >> ret = IRQ_HANDLED;
> >> }
> >>
> >> @@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port
> >> *sport)
> >> temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
> >> writel(temp, sport->port.membase + UCR1);
> >>
> >> - temp = readl(sport->port.membase + UCR2);
> >> - temp |= UCR2_ATEN;
> >> - writel(temp, sport->port.membase + UCR2);
> >> -
> >
> > I think this should be moved to imx_startup. At least RRDY is configured
> there.
> >
>
>
> Currently, non-dma mode does not use the ageing timer. You get an interrupt
> immediately when
> 1 character is in the fifo. So, enabling the ageing timer and changing fifo level,
> should really be a separate patch. But it would be a nice patch.
>
>
Ok but in this case, we are left with aging timer and rrdy check in the interrupt and imx-startup enables only rrdy. (which is not much different than what it was before, since aging timer didn't get a chance to be triggered)
I agree that changing the fifo level to give a chance also to the aging timer will be the right thing to do here.
Overall, I think is ok, my main concern with not enabling the aging timer in startup is that people can be confused about this.
Reviewed-by: Nandor Han <Nandor.han@ge.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
2017-10-11 6:22 ` Han, Nandor (GE Healthcare)
@ 2017-10-20 11:56 ` gregkh at linuxfoundation.org
-1 siblings, 0 replies; 10+ messages in thread
From: gregkh @ 2017-10-20 11:56 UTC (permalink / raw)
To: Han, Nandor (GE Healthcare)
Cc: Troy Kisky, linux-serial, u.kleine-koenig, fabio.estevam,
linux-arm-kernel, l.stach
On Wed, Oct 11, 2017 at 06:22:53AM +0000, Han, Nandor (GE Healthcare) wrote:
>
>
> > -----Original Message-----
> > From: linux-serial-owner@vger.kernel.org [mailto:linux-serial-
> > owner@vger.kernel.org] On Behalf Of Troy Kisky
> > Sent: 07 October 2017 04:47
> > To: gregkh@linuxfoundation.org
> > Cc: fabio.estevam@nxp.com; l.stach@pengutronix.de; u.kleine-
> > koenig@pengutronix.de; linux-serial@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; Troy Kisky <troy.kisky@boundarydevices.com>
> > Subject: EXT: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if
> > dma in use
> >
> > Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
> >
> > the interrupt routine no longer will start rx dma.
>
> I think you have 2 commits here:
> - one that disables ageing timer when DMA is in use
> - one that cleans the leftovers after 4dec2f119e86.
I agree, Troy, please make a patch series here.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use
@ 2017-10-20 11:56 ` gregkh at linuxfoundation.org
0 siblings, 0 replies; 10+ messages in thread
From: gregkh at linuxfoundation.org @ 2017-10-20 11:56 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 11, 2017 at 06:22:53AM +0000, Han, Nandor (GE Healthcare) wrote:
>
>
> > -----Original Message-----
> > From: linux-serial-owner at vger.kernel.org [mailto:linux-serial-
> > owner at vger.kernel.org] On Behalf Of Troy Kisky
> > Sent: 07 October 2017 04:47
> > To: gregkh at linuxfoundation.org
> > Cc: fabio.estevam at nxp.com; l.stach at pengutronix.de; u.kleine-
> > koenig at pengutronix.de; linux-serial at vger.kernel.org; linux-arm-
> > kernel at lists.infradead.org; Troy Kisky <troy.kisky@boundarydevices.com>
> > Subject: EXT: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if
> > dma in use
> >
> > Since commit 4dec2f119e86 ("imx-serial: RX DMA startup latency")
> >
> > the interrupt routine no longer will start rx dma.
>
> I think you have 2 commits here:
> - one that disables ageing timer when DMA is in use
> - one that cleans the leftovers after 4dec2f119e86.
I agree, Troy, please make a patch series here.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-20 11:56 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-07 1:46 [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use Troy Kisky
2017-10-07 1:46 ` Troy Kisky
2017-10-11 6:22 ` Han, Nandor (GE Healthcare)
2017-10-11 6:22 ` Han, Nandor (GE Healthcare)
2017-10-11 16:09 ` Troy Kisky
2017-10-11 16:09 ` Troy Kisky
2017-10-13 14:47 ` Han, Nandor (GE Healthcare)
2017-10-13 14:47 ` Han, Nandor (GE Healthcare)
2017-10-20 11:56 ` gregkh
2017-10-20 11:56 ` gregkh at linuxfoundation.org
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.