serial: mxs: remove the unused macro
PATCH 1/3 serial: mxs: remove the unused macro
The MXS_AUART_DMA_CONFIG is originally used to check if the DT node is configured with the DMA property.
But now, the MXS_AUART_DMA_CONFIG is not used any more. Just remove it.
--- drivers/tty/serial/mxs-auart.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
drivers/tty/serial/mxs-auart.c +1 -4
@@ -134,7 +134,6 @@ enum mxs_auart_type {struct mxs_auart_port {struct uart_port port; -#define MXS_AUART_DMA_CONFIG 0x1#define MXS_AUART_DMA_ENABLED 0x2#define MXS_AUART_DMA_TX_SYNC 2/* bit 2 */ #define MXS_AUART_DMA_RX_READY 3/* bit 3 */
@@ -640,7 +639,7 @@ static void mxs_auart_settermios(struct uart_port *u,* we can only implement the DMA support for auart * in mx28. */ -if (is_imx28_auart (s) && (s->flags &MXS_AUART_DMA_CONFIG )) { +if (is_imx28_auart (s)) {if (!mxs_auart_dma_init (s))/* enable DMA tranfer */ ctrl2 |=AUART_CTRL2_TXDMAE |AUART_CTRL2_RXDMAE
@@ -1002,8 +1001,6 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s, } s->port.line = ret; - s->flags |=MXS_AUART_DMA_CONFIG ; -return 0 ; }
Signature · 1 line
1.7.1
PATCH 2/3 serial: mxs: enable the DMA only when the rts/cts is enabled
The originall DMA support works only when the rts/cts is enabled. But after several patchs, the DMA support has lost this limit.
This patch adds an optional property for the uart DT node which indicates the uart has rts and cts.
This patch also adds the check for rts/cts before we enable the DMA for the uart.
--- .../bindings/tty/serial/fsl-mxs-auart.txt | 3 +++ drivers/tty/serial/mxs-auart.c | 5 ++++- 2 files changed, 7 insertions(+), 1 deletions(-)
Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt +3 -0
@@ -10,6 +10,9 @@ Required properties: Refer to dma.txt and fsl-mxs-dma.txt for details. - dma-names: "rx" for RX channel, "tx" for TX channel. +Optional properties: +- fsl,uart-has-rtscts : Indicate the uart has rts and cts + Example: auart0: serial at 8006a000 { compatible = "fsl,imx28-auart", "fsl,imx23-auart";
drivers/tty/serial/mxs-auart.c +4 -1
@@ -134,6 +134,7 @@ enum mxs_auart_type {struct mxs_auart_port {struct uart_port port; +#define MXS_AUART_RTSCTS 1#define MXS_AUART_DMA_ENABLED 0x2#define MXS_AUART_DMA_TX_SYNC 2/* bit 2 */ #define MXS_AUART_DMA_RX_READY 3/* bit 3 */
@@ -639,7 +640,7 @@ static void mxs_auart_settermios(struct uart_port *u,* we can only implement the DMA support for auart * in mx28. */ -if (is_imx28_auart (s)) { +if (is_imx28_auart (s) && (s->flags &MXS_AUART_RTSCTS )) {if (!mxs_auart_dma_init (s))/* enable DMA tranfer */ ctrl2 |=AUART_CTRL2_TXDMAE |AUART_CTRL2_RXDMAE
@@ -1001,6 +1002,8 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s, } s->port.line = ret; +if (of_get_property (np,"fsl,uart-has-rtscts" ,NULL )) + s->flags |=MXS_AUART_RTSCTS ;return 0 ; }
Signature · 1 line
1.7.1
PATCH 2/3 serial: mxs: enable the DMA only when the rts/cts is enabled
Quoted 1 line · Huang Shijie
The originall DMA support works only when the rts/cts is enabled.
s/originall/original/. I'd drop "the" and capitalize RTS/CTS.
Quoted 1 line
But after several patchs, the DMA support has lost this limit.
s/patchs/patches/, s/the //.
do you want to say that the driver fails to only enable DMA when RTS/CTS are available; or that today the driver can handle DMA just fine even without RTS/CTS? I interpret your commit log as the latter, your patch implements the former however.
Quoted 2 lines
This patch adds an optional property for the uart DT node which indicates the uart has rts and cts.
which indicates that the UART has RTS and CTS lines.
Quoted 63 lines
This patch also adds the check for rts/cts before we enable the DMA for the uart.
Signed-off-by Huang Shijie <[email protected]>--- .../bindings/tty/serial/fsl-mxs-auart.txt | 3 +++ drivers/tty/serial/mxs-auart.c | 5 ++++- 2 files changed, 7 insertions(+), 1 deletions(-)Documentation/devicetree/bindings/tty/serial/fsl-mxs-auart.txt +3
-0@@ -10,6 +10,9 @@ Required properties: Refer to dma.txt and fsl-mxs-dma.txt for details. - dma-names: "rx" for RX channel, "tx" for TX channel. +Optional properties: +- fsl,uart-has-rtscts : Indicate the uart has rts and cts + Example: auart0: serial at 8006a000 { compatible = "fsl,imx28-auart", "fsl,imx23-auart";drivers/tty/serial/mxs-auart.c +4
-1@@ -134,6 +134,7 @@ enum mxs_auart_type {struct mxs_auart_port {struct uart_port port; +#define MXS_AUART_RTSCTS 1#define MXS_AUART_DMA_ENABLED 0x2#define MXS_AUART_DMA_TX_SYNC 2/* bit 2 */ #define MXS_AUART_DMA_RX_READY 3/* bit 3 */ @@ -639,7 +640,7 @@ static void mxs_auart_settermios(struct uart_port *u,* we can only implement the DMA support for auart * in mx28. */ -if (is_imx28_auart (s)) { +if (is_imx28_auart (s) && (s->flags &MXS_AUART_RTSCTS )) {if (!mxs_auart_dma_init (s))/* enable DMA tranfer */ ctrl2 |=AUART_CTRL2_TXDMAE |AUART_CTRL2_RXDMAE @@ -1001,6 +1002,8 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s, } s->port.line = ret; +if (of_get_property (np,"fsl,uart-has-rtscts" ,NULL )) + s->flags |=MXS_AUART_RTSCTS ;return 0 ; }Signature · 8 lines
1.7.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 2/3 serial: mxs: enable the DMA only when the rts/cts is enabled
? 2013?07?15? 16:27, Uwe Kleine-K?nig ??:
Quoted 4 lines
do you want to say that the driver fails to only enable DMA when RTS/CTS are available; or that today the driver can handle DMA just fine even without RTS/CTS? I interpret your commit log as the latter, your patch implements the former however.
in the mxs-auart, if the RTS/CTS is not invalid, the DMA should be not enabled.
But current code lost the limit, a uart without the RTS/CTS may also enables the DMA in which case the uart
may does not run or run in a abnormal way.
thanks
Huang Shijie
PATCH 2/3 serial: mxs: enable the DMA only when the rts/cts is enabled
Hello,
Quoted 11 lines · Huang Shijie
? 2013?07?15? 16:27, Uwe Kleine-K?nig ??:
Quoted 4 lines
do you want to say that the driver fails to only enable DMA when RTS/CTS are available; or that today the driver can handle DMA just fine even without RTS/CTS? I interpret your commit log as the latter, your patch implements the former however.
in the mxs-auart, if the RTS/CTS is not invalid, the DMA should be not enabled.
But current code lost the limit, a uart without the RTS/CTS may also enables the DMA in which case the uart
may does not run or run in a abnormal way.
So this sounds like a fix that should go into stable and so preferably should be the first patch in your series.
Something like:
serial: mxs-auart: DMA unreliable without RTS/CTS
According to [add some document name here] DMA doesn't work reliable without hardware handshaking. So make DMA dependant on a newly introdused property "fsl,uart-has-rtscts".
The flag is only used to decide if dma should be enabled. So I think an in-code comment would be nice, too. Is it still correct to set AUART_CTRL2_RTSEN and AUART_CTRL2_RTS and read AUART_STAT_CTS when fsl,uart-has-rtscts is not provided?
(BTW, mxs_auart_set_mctrl has:
u32 ctrl = readl(u->membase + AUART_CTRL2);
ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
if (mctrl & TIOCM_RTS) {
if (tty_port_cts_enabled(&u->state->port))
ctrl |= AUART_CTRL2_RTSEN;
else
ctrl |= AUART_CTRL2_RTS;
}
s->ctrl = mctrl;A comment for the diligent reader about the difference between RTSEN and RTS would be nice. Also I wonder if shadowing mctrl is sensible and used correctly here.)
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 2/3 serial: mxs: enable the DMA only when the rts/cts is enabled
? 2013?07?15? 17:07, Uwe Kleine-K?nig ??:
Quoted 8 lines
Hello,
Quoted 5 lines · Huang Shijie
? 2013?07?15? 16:27, Uwe Kleine-K?nig ??:
Quoted 4 lines
do you want to say that the driver fails to only enable DMA when RTS/CTS are available; or that today the driver can handle DMA just fine even without RTS/CTS? I interpret your commit log as the latter, your patch implements the former however.
Quoted 8 lines
Quoted 6 lines
in the mxs-auart, if the RTS/CTS is not invalid, the DMA should be not enabled.
But current code lost the limit, a uart without the RTS/CTS may also enables the DMA in which case the uart
may does not run or run in a abnormal way.So this sounds like a fix that should go into stable and so preferably should be the first patch in your series.
This patch depends on the first patch. :)
Quoted 4 lines
Something like:
serial: mxs-auart: DMA unreliable without RTS/CTS
Quoted 8 lines
According to [add some document name here] DMA doesn't work reliable without hardware handshaking. So make DMA dependant on a newly introdused property "fsl,uart-has-rtscts".
Cc stable at kernel.org # [first affected version]The flag is only used to decide if dma should be enabled. So I think an in-code comment would be nice, too. Is it still correct to set
ok.
Quoted 2 lines
AUART_CTRL2_RTSENandAUART_CTRL2_RTSand readAUART_STAT_CTSwhen fsl,uart-has-rtscts is not provided?
i think it's correct. (if i have a imx28-evk board, i can test it.)
If you enable the RTS/CTS from the application, the tty_port_cts_enabled() will be true.
we will set the AUART_CTRL2_RTSEN in the end when the "fsl,uart-has-rtscts" is enabled.
Quoted 14 lines
(BTW,
mxs_auart_set_mctrlhas:u32 ctrl = readl(u->membase + AUART_CTRL2); ctrl&= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS); if (mctrl& TIOCM_RTS) { if (tty_port_cts_enabled(&u->state->port)) ctrl |= AUART_CTRL2_RTSEN; else ctrl |= AUART_CTRL2_RTS; } s->ctrl = mctrl;
Quoted 2 lines
A comment for the diligent reader about the difference between RTSEN and RTS would be nice. Also I wonder if shadowing mctrl is sensible and used
Please see the spec about the RTSEN and RTS.
RTSEN enable the flow control by the hardware; RTS enable the flow control by the software.
thanks
Huang Shijie
Quoted 5 lines
correctly here.)
Best regards
Uwe
PATCH 2/3 serial: mxs: enable the DMA only when the rts/cts is enabled
Quoted 12 lines · Huang Shijie
? 2013?07?15? 17:07, Uwe Kleine-K?nig ??:
Quoted 11 lines
Hello,
Quoted 8 lines · Huang Shijie
? 2013?07?15? 16:27, Uwe Kleine-K?nig ??:
Quoted 4 lines
do you want to say that the driver fails to only enable DMA when RTS/CTS are available; or that today the driver can handle DMA just fine even without RTS/CTS? I interpret your commit log as the latter, your patch implements the former however.
in the mxs-auart, if the RTS/CTS is not invalid, the DMA should be not enabled.
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
But current code lost the limit, a uart without the RTS/CTS may also enables the DMA in which case the uart
may does not run or run in a abnormal way.So this sounds like a fix that should go into stable and so preferably should be the first patch in your series.
This patch depends on the first patch. :)
But it's not a hard dependency.
Quoted 35 lines
Quoted 4 lines
Something like:
serial: mxs-auart: DMA unreliable without RTS/CTS
Quoted 8 lines
According to [add some document name here] DMA doesn't work reliable without hardware handshaking. So make DMA dependant on a newly introdused property "fsl,uart-has-rtscts".
Cc stable at kernel.org # [first affected version]The flag is only used to decide if dma should be enabled. So I think an in-code comment would be nice, too. Is it still correct to set
ok.
Quoted 2 lines
AUART_CTRL2_RTSENandAUART_CTRL2_RTSand readAUART_STAT_CTSwhen fsl,uart-has-rtscts is not provided?i think it's correct. (if i have a imx28-evk board, i can test it.)
If you enable the RTS/CTS from the application, the
tty_port_cts_enabled()will be true.
we will set theAUART_CTRL2_RTSENin the end when the "fsl,uart-has-rtscts" is enabled.Quoted 12 lines
(BTW,
mxs_auart_set_mctrlhas:u32 ctrl = readl(u->membase + AUART_CTRL2); ctrl&= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS); if (mctrl& TIOCM_RTS) { if (tty_port_cts_enabled(&u->state->port)) ctrl |= AUART_CTRL2_RTSEN; else ctrl |= AUART_CTRL2_RTS; }
Quoted 5 lines
Quoted 4 lines
s->ctrl = mctrl;A comment for the diligent reader about the difference between RTSEN and RTS would be nice. Also I wonder if shadowing mctrl is sensible and used
Please see the spec about the RTSEN and RTS.
Right, it's not hard for me. Other might not have the manual handy, so a comment telling:
Quoted 2 lines
RTSEN enable the flow control by the hardware; RTS enable the flow control by the software.
would be nice.
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 2/3 serial: mxs: enable the DMA only when the rts/cts is enabled
Quoted 3 lines · Uwe Kleine-König
Quoted 1 line
This patch depends on the first patch. :)
But it's not a hard dependency.
do you mean i should merge the patch 1 and patch 2 into a single patch?
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Something like:
serial: mxs-auart: DMA unreliable without RTS/CTS
According to [add some document name here] DMA doesn't work reliable without hardware handshaking. So make DMA dependant on
Quoted 7 lines
Quoted 1 line
Please see the spec about the RTSEN and RTS.
Right, it's not hard for me. Other might not have the manual handy, so a comment telling:
Quoted 2 lines
RTSEN enable the flow control by the hardware;
RTS enable the flow control by the software.would be nice.
i think we need another patch to add this comment.
thanks
Huang Shijie
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 3/3 ARM: dts: imx28-evk: add the rts/cts property for auart0
In reply to Huang ShijieAdd the rts/cts property for auart0 which means we enable the DMA support for it.
--- arch/arm/boot/dts/imx28-evk.dts | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/boot/dts/imx28-evk.dts +1 -0
@@ -220,6 +220,7 @@ auart0: serial at 8006a000 { pinctrl-names = "default"; pinctrl-0 = <&auart0_pins_a>; + fsl,uart-has-rtscts; status = "okay"; };
Signature · 1 line
1.7.1
PATCH 1/3 serial: mxs: remove the unused macro
In reply to Huang ShijieHello,
Quoted 33 lines · Huang Shijie
The
MXS_AUART_DMA_CONFIGis originally used to check if the DT node is configured with the DMA property.But now, the
MXS_AUART_DMA_CONFIGis not used any more. Just remove it.Signed-off-by Huang Shijie <[email protected]>--- drivers/tty/serial/mxs-auart.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)drivers/tty/serial/mxs-auart.c +1
-2@@ -134,7 +134,6 @@ enum mxs_auart_type {struct mxs_auart_port {struct uart_port port; -#define MXS_AUART_DMA_CONFIG 0x1#define MXS_AUART_DMA_ENABLED 0x2#define MXS_AUART_DMA_TX_SYNC 2/* bit 2 */ #define MXS_AUART_DMA_RX_READY 3/* bit 3 */ @@ -640,7 +639,7 @@ static void mxs_auart_settermios(struct uart_port *u,* we can only implement the DMA support for auart * in mx28. */ -if (is_imx28_auart (s) && (s->flags &MXS_AUART_DMA_CONFIG )) { +if (is_imx28_auart (s)) {if (!mxs_auart_dma_init (s))/* enable DMA tranfer */ ctrl2 |=AUART_CTRL2_TXDMAE |AUART_CTRL2_RXDMAE
drivers/tty/serial/mxs-auart.c static int serial_mxs_probe_dt(struct mxs_auart_port *s, · line 1006
Quoted 5 lines
drivers/tty/serial/mxs-auart.c +0
-2@@ -1002,8 +1001,6 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s, } s->port.line = ret; - s->flags |=MXS_AUART_DMA_CONFIG ; -
You're suggesting that MXS_AUART_DMA_CONFIG was set here unconditionally before and so the check in the hunk above is not necessary anymore. (<pedantic>So your "not used any more" is technically wrong.</pedantic>)
However serial_mxs_probe_dt returns earlier if the device was not probed by dt (which I think we can assume to not apply, still it should be mentioned in the commit log).
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/3 serial: mxs: remove the unused macro
Quoted 2 lines · Uwe Kleine-König
You're suggesting that
MXS_AUART_DMA_CONFIGwas set here unconditionally before and so the check in the hunk above is not necessary anymore.
yes, that's what i want to say.
Quoted 1 line
(<pedantic>So your "not used any more" is technically wrong.</pedantic>)
ok, I will change this comment.
thanks
Huang Shijie
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html