[PATCH 1/3] ARM: dts: STiH410-b2260: Identify the UART RTS line

[PATCH 1/3] ARM: dts: STiH410-b2260: Identify the UART RTS line

[Lee Jones]

Configure the UART RTS line as a GPIO for manipulation within the UART driver.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih410-b2260.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/stih410-b2260.dts b/arch/arm/boot/dts/stih410-b2260.dts
index 7fb507f..f46603f 100644
--- a/arch/arm/boot/dts/stih410-b2260.dts
+++ b/arch/arm/boot/dts/stih410-b2260.dts
@@ -63,6 +63,7 @@
 		uart0: serial@9830000 {
 			label = "LS-UART0";
 			status = "okay";
+			rts-gpios = <&pio17 3 GPIO_ACTIVE_LOW>;
 		};
 
 		/* Low speed expansion connector */
-- 
2.10.2

[PATCH 2/3] serial: st-asc: Provide RTS functionality

[Lee Jones]

Until this point, it has not been possible for serial applications
to toggle the UART RTS line.  This can be useful with certain
configurations. For example, when using a Mezzanine on a Linaro
96board, RTS line is used to take the the on-board microcontroller
in and out of reset.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/tty/serial/st-asc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index 379e5bd..ce46898 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -30,6 +30,7 @@
 #include <linux/of_platform.h>
 #include <linux/serial_core.h>
 #include <linux/clk.h>
+#include <linux/gpio/consumer.h>
 
 #define DRIVER_NAME "st-asc"
 #define ASC_SERIAL_NAME "ttyAS"
@@ -38,6 +39,7 @@
 
 struct asc_port {
 	struct uart_port port;
+	struct gpio_desc *rts;
 	struct clk *clk;
 	unsigned int hw_flow_control:1;
 	unsigned int force_m1:1;
@@ -381,12 +383,16 @@ static unsigned int asc_tx_empty(struct uart_port *port)
 
 static void asc_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
+	struct asc_port *ascport = to_asc_port(port);
+
 	/*
 	 * This routine is used for seting signals of: DTR, DCD, CTS/RTS
 	 * We use ASC's hardware for CTS/RTS, so don't need any for that.
 	 * Some boards have DTR and DCD implemented using PIO pins,
 	 * code to do this should be hooked in here.
 	 */
+
+	gpiod_set_value(ascport->rts, mctrl & TIOCM_RTS);
 }
 
 static unsigned int asc_get_mctrl(struct uart_port *port)
@@ -731,12 +737,20 @@ MODULE_DEVICE_TABLE(of, asc_match);
 static int asc_serial_probe(struct platform_device *pdev)
 {
 	int ret;
+	struct device_node *np = pdev->dev.of_node;
 	struct asc_port *ascport;
+	struct gpio_desc *gpiod;
 
 	ascport = asc_of_get_asc_port(pdev);
 	if (!ascport)
 		return -ENODEV;
 
+	gpiod = devm_get_gpiod_from_child(&pdev->dev, "rts", &np->fwnode);
+	if (!IS_ERR(gpiod)) {
+		gpiod_direction_output(gpiod, 0);
+		ascport->rts = gpiod;
+	}
+
 	ret = asc_init_port(ascport, pdev);
 	if (ret)
 		return ret;
-- 
2.10.2

[PATCH 3/3] serial: st-asc: Ignore the parity error bit if 8-bit mode is enabled

[Lee Jones]

The datasheet states:

"If the MODE field selects an 8-bit frame then this [parity error] bit
 is undefined. Software should ignore this bit when reading 8-bit frames."

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/tty/serial/st-asc.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index ce46898..7008eb7 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -289,9 +289,19 @@ static void asc_transmit_chars(struct uart_port *port)
 static void asc_receive_chars(struct uart_port *port)
 {
 	struct tty_port *tport = &port->state->port;
-	unsigned long status;
+	unsigned long status, mode;
 	unsigned long c = 0;
 	char flag;
+	bool ignore_pe = false;
+
+	/*
+	 * Datasheet states: If the MODE field selects an 8-bit frame then
+	 * this [parity error] bit is undefined. Software should ignore this
+	 * bit when reading 8-bit frames.
+	 */
+	mode = asc_in(port, ASC_CTL) & ASC_CTL_MODE_MSK;
+	if (mode == ASC_CTL_MODE_8BIT || mode == ASC_CTL_MODE_8BIT_PAR)
+		ignore_pe = true;
 
 	if (port->irq_wake)
 		pm_wakeup_event(tport->tty->dev, 0);
@@ -301,8 +311,8 @@ static void asc_receive_chars(struct uart_port *port)
 		flag = TTY_NORMAL;
 		port->icount.rx++;
 
-		if ((c & (ASC_RXBUF_FE | ASC_RXBUF_PE)) ||
-			status & ASC_STA_OE) {
+		if (status & ASC_STA_OE || c & ASC_RXBUF_FE ||
+		    (c & ASC_RXBUF_PE && !ignore_pe)) {
 
 			if (c & ASC_RXBUF_FE) {
 				if (c == (ASC_RXBUF_FE | ASC_RXBUF_DUMMY_RX)) {
-- 
2.10.2

Re: [PATCH 2/3] serial: st-asc: Provide RTS functionality

[Patrice Chotard]

Hi Lee

On 12/02/2016 03:11 PM, Lee Jones wrote:
> Until this point, it has not been possible for serial applications
> to toggle the UART RTS line.  This can be useful with certain
> configurations. For example, when using a Mezzanine on a Linaro
> 96board, RTS line is used to take the the on-board microcontroller

typo "the the on"

> in and out of reset.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/tty/serial/st-asc.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
> index 379e5bd..ce46898 100644
> --- a/drivers/tty/serial/st-asc.c
> +++ b/drivers/tty/serial/st-asc.c
> @@ -30,6 +30,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/serial_core.h>
>  #include <linux/clk.h>
> +#include <linux/gpio/consumer.h>
>  
>  #define DRIVER_NAME "st-asc"
>  #define ASC_SERIAL_NAME "ttyAS"
> @@ -38,6 +39,7 @@
>  
>  struct asc_port {
>  	struct uart_port port;
> +	struct gpio_desc *rts;
>  	struct clk *clk;
>  	unsigned int hw_flow_control:1;
>  	unsigned int force_m1:1;
> @@ -381,12 +383,16 @@ static unsigned int asc_tx_empty(struct uart_port *port)
>  
>  static void asc_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  {
> +	struct asc_port *ascport = to_asc_port(port);
> +
>  	/*
>  	 * This routine is used for seting signals of: DTR, DCD, CTS/RTS
>  	 * We use ASC's hardware for CTS/RTS, so don't need any for that.
>  	 * Some boards have DTR and DCD implemented using PIO pins,
>  	 * code to do this should be hooked in here.
>  	 */
> +
> +	gpiod_set_value(ascport->rts, mctrl & TIOCM_RTS);
>  }
>  
>  static unsigned int asc_get_mctrl(struct uart_port *port)
> @@ -731,12 +737,20 @@ MODULE_DEVICE_TABLE(of, asc_match);
>  static int asc_serial_probe(struct platform_device *pdev)
>  {
>  	int ret;
> +	struct device_node *np = pdev->dev.of_node;
>  	struct asc_port *ascport;
> +	struct gpio_desc *gpiod;
>  
>  	ascport = asc_of_get_asc_port(pdev);
>  	if (!ascport)
>  		return -ENODEV;
>  
> +	gpiod = devm_get_gpiod_from_child(&pdev->dev, "rts", &np->fwnode);
> +	if (!IS_ERR(gpiod)) {
> +		gpiod_direction_output(gpiod, 0);
> +		ascport->rts = gpiod;
> +	}
> +
>  	ret = asc_init_port(ascport, pdev);
>  	if (ret)
>  		return ret;
> 

Re: [PATCH 1/3] ARM: dts: STiH410-b2260: Identify the UART RTS line

[Patrice Chotard]

Hi Lee

Except the typo in patch 2

Acked-by: Patrice Chotard <patrice.chotard@st.com>

Thanks

On 12/02/2016 03:11 PM, Lee Jones wrote:
> Configure the UART RTS line as a GPIO for manipulation within the UART driver.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/boot/dts/stih410-b2260.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/stih410-b2260.dts b/arch/arm/boot/dts/stih410-b2260.dts
> index 7fb507f..f46603f 100644
> --- a/arch/arm/boot/dts/stih410-b2260.dts
> +++ b/arch/arm/boot/dts/stih410-b2260.dts
> @@ -63,6 +63,7 @@
>  		uart0: serial@9830000 {
>  			label = "LS-UART0";
>  			status = "okay";
> +			rts-gpios = <&pio17 3 GPIO_ACTIVE_LOW>;
>  		};
>  
>  		/* Low speed expansion connector */
> 

Re: [STLinux Kernel] [PATCH 2/3] serial: st-asc: Provide RTS functionality

[Peter Griffin]

Hi Lee,

On Mon, 05 Dec 2016, Patrice Chotard wrote:

> Hi Lee
> 
> On 12/02/2016 03:11 PM, Lee Jones wrote:
> > Until this point, it has not been possible for serial applications
> > to toggle the UART RTS line.  This can be useful with certain
> > configurations. For example, when using a Mezzanine on a Linaro
> > 96board, RTS line is used to take the the on-board microcontroller
> 
> typo "the the on"
> 
> > in and out of reset.
> > 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/tty/serial/st-asc.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
> > index 379e5bd..ce46898 100644
> > --- a/drivers/tty/serial/st-asc.c
> > +++ b/drivers/tty/serial/st-asc.c
> > @@ -30,6 +30,7 @@
> >  #include <linux/of_platform.h>
> >  #include <linux/serial_core.h>
> >  #include <linux/clk.h>
> > +#include <linux/gpio/consumer.h>
> >  
> >  #define DRIVER_NAME "st-asc"
> >  #define ASC_SERIAL_NAME "ttyAS"
> > @@ -38,6 +39,7 @@
> >  
> >  struct asc_port {
> >  	struct uart_port port;
> > +	struct gpio_desc *rts;
> >  	struct clk *clk;
> >  	unsigned int hw_flow_control:1;
> >  	unsigned int force_m1:1;
> > @@ -381,12 +383,16 @@ static unsigned int asc_tx_empty(struct uart_port *port)
> >  
> >  static void asc_set_mctrl(struct uart_port *port, unsigned int mctrl)
> >  {
> > +	struct asc_port *ascport = to_asc_port(port);
> > +
> >  	/*
> >  	 * This routine is used for seting signals of: DTR, DCD, CTS/RTS
> >  	 * We use ASC's hardware for CTS/RTS, so don't need any for that.
> >  	 * Some boards have DTR and DCD implemented using PIO pins,
> >  	 * code to do this should be hooked in here.
> >  	 */
> > +
> > +	gpiod_set_value(ascport->rts, mctrl & TIOCM_RTS);

This now puts the code and the comment above out of sync with each
other.

However just checking the stih410 datasheet and I don't think this patch
to control RTS via a GPIO is required anyway.

The correct way of handling this is to add UART10_RTS and UART10_CTS pins
to the pinctrl_serial0 group so they are properly configured for their cts/rts
alternate function. Once the pins are correctly configured, the IP block should
control the signals correctly like the comment says.

regards,

Peter.

Re: [STLinux Kernel] [PATCH 2/3] serial: st-asc: Provide RTS functionality

[Lee Jones]

On Tue, 06 Dec 2016, Peter Griffin wrote:
> On Mon, 05 Dec 2016, Patrice Chotard wrote:
> > On 12/02/2016 03:11 PM, Lee Jones wrote:
> > > Until this point, it has not been possible for serial applications
> > > to toggle the UART RTS line.  This can be useful with certain
> > > configurations. For example, when using a Mezzanine on a Linaro
> > > 96board, RTS line is used to take the the on-board microcontroller
> > 
> > typo "the the on"
> > 
> > > in and out of reset.
> > > 
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >  drivers/tty/serial/st-asc.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
> > > index 379e5bd..ce46898 100644
> > > --- a/drivers/tty/serial/st-asc.c
> > > +++ b/drivers/tty/serial/st-asc.c
> > > @@ -30,6 +30,7 @@
> > >  #include <linux/of_platform.h>
> > >  #include <linux/serial_core.h>
> > >  #include <linux/clk.h>
> > > +#include <linux/gpio/consumer.h>
> > >  
> > >  #define DRIVER_NAME "st-asc"
> > >  #define ASC_SERIAL_NAME "ttyAS"
> > > @@ -38,6 +39,7 @@
> > >  
> > >  struct asc_port {
> > >  	struct uart_port port;
> > > +	struct gpio_desc *rts;
> > >  	struct clk *clk;
> > >  	unsigned int hw_flow_control:1;
> > >  	unsigned int force_m1:1;
> > > @@ -381,12 +383,16 @@ static unsigned int asc_tx_empty(struct uart_port *port)
> > >  
> > >  static void asc_set_mctrl(struct uart_port *port, unsigned int mctrl)
> > >  {
> > > +	struct asc_port *ascport = to_asc_port(port);
> > > +
> > >  	/*
> > >  	 * This routine is used for seting signals of: DTR, DCD, CTS/RTS
> > >  	 * We use ASC's hardware for CTS/RTS, so don't need any for that.
> > >  	 * Some boards have DTR and DCD implemented using PIO pins,
> > >  	 * code to do this should be hooked in here.
> > >  	 */
> > > +
> > > +	gpiod_set_value(ascport->rts, mctrl & TIOCM_RTS);
> 
> This now puts the code and the comment above out of sync with each
> other.
> 
> However just checking the stih410 datasheet and I don't think this patch
> to control RTS via a GPIO is required anyway.
> 
> The correct way of handling this is to add UART10_RTS and UART10_CTS pins
> to the pinctrl_serial0 group so they are properly configured for their cts/rts
> alternate function. Once the pins are correctly configured, the IP block should
> control the signals correctly like the comment says.

That was the first thing I tried.  It didn't work.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [STLinux Kernel] [PATCH 2/3] serial: st-asc: Provide RTS functionality

[Lee Jones]

On Tue, 06 Dec 2016, Lee Jones wrote:

> On Tue, 06 Dec 2016, Peter Griffin wrote:
> > On Mon, 05 Dec 2016, Patrice Chotard wrote:
> > > On 12/02/2016 03:11 PM, Lee Jones wrote:
> > > > Until this point, it has not been possible for serial applications
> > > > to toggle the UART RTS line.  This can be useful with certain
> > > > configurations. For example, when using a Mezzanine on a Linaro
> > > > 96board, RTS line is used to take the the on-board microcontroller
> > > 
> > > typo "the the on"
> > > 
> > > > in and out of reset.
> > > > 
> > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > > ---
> > > >  drivers/tty/serial/st-asc.c | 14 ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > > 
> > > > diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
> > > > index 379e5bd..ce46898 100644
> > > > --- a/drivers/tty/serial/st-asc.c
> > > > +++ b/drivers/tty/serial/st-asc.c
> > > > @@ -30,6 +30,7 @@
> > > >  #include <linux/of_platform.h>
> > > >  #include <linux/serial_core.h>
> > > >  #include <linux/clk.h>
> > > > +#include <linux/gpio/consumer.h>
> > > >  
> > > >  #define DRIVER_NAME "st-asc"
> > > >  #define ASC_SERIAL_NAME "ttyAS"
> > > > @@ -38,6 +39,7 @@
> > > >  
> > > >  struct asc_port {
> > > >  	struct uart_port port;
> > > > +	struct gpio_desc *rts;
> > > >  	struct clk *clk;
> > > >  	unsigned int hw_flow_control:1;
> > > >  	unsigned int force_m1:1;
> > > > @@ -381,12 +383,16 @@ static unsigned int asc_tx_empty(struct uart_port *port)
> > > >  
> > > >  static void asc_set_mctrl(struct uart_port *port, unsigned int mctrl)
> > > >  {
> > > > +	struct asc_port *ascport = to_asc_port(port);
> > > > +
> > > >  	/*
> > > >  	 * This routine is used for seting signals of: DTR, DCD, CTS/RTS
> > > >  	 * We use ASC's hardware for CTS/RTS, so don't need any for that.
> > > >  	 * Some boards have DTR and DCD implemented using PIO pins,
> > > >  	 * code to do this should be hooked in here.
> > > >  	 */
> > > > +
> > > > +	gpiod_set_value(ascport->rts, mctrl & TIOCM_RTS);
> > 
> > This now puts the code and the comment above out of sync with each
> > other.
> > 
> > However just checking the stih410 datasheet and I don't think this patch
> > to control RTS via a GPIO is required anyway.
> > 
> > The correct way of handling this is to add UART10_RTS and UART10_CTS pins
> > to the pinctrl_serial0 group so they are properly configured for their cts/rts
> > alternate function. Once the pins are correctly configured, the IP block should
> > control the signals correctly like the comment says.
> 
> That was the first thing I tried.  It didn't work.

       cts = <&pio17 2 ALT1 IN>;
       rts = <&pio17 3 ALT1 OUT>;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [STLinux Kernel] [PATCH 2/3] serial: st-asc: Provide RTS functionality

[Peter Griffin]

Hi Lee,

On Tue, 06 Dec 2016, Lee Jones wrote:

> On Tue, 06 Dec 2016, Lee Jones wrote:
> 
> > On Tue, 06 Dec 2016, Peter Griffin wrote:
> > > On Mon, 05 Dec 2016, Patrice Chotard wrote:
> > > > On 12/02/2016 03:11 PM, Lee Jones wrote:
> > > > > Until this point, it has not been possible for serial applications
> > > > > to toggle the UART RTS line.  This can be useful with certain
> > > > > configurations. For example, when using a Mezzanine on a Linaro
> > > > > 96board, RTS line is used to take the the on-board microcontroller
> > > > 
> > > > typo "the the on"
> > > > 
> > > > > in and out of reset.
> > > > > 
> > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > > > ---
> > > > >  drivers/tty/serial/st-asc.c | 14 ++++++++++++++
> > > > >  1 file changed, 14 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
> > > > > index 379e5bd..ce46898 100644
> > > > > --- a/drivers/tty/serial/st-asc.c
> > > > > +++ b/drivers/tty/serial/st-asc.c
> > > > > @@ -30,6 +30,7 @@
> > > > >  #include <linux/of_platform.h>
> > > > >  #include <linux/serial_core.h>
> > > > >  #include <linux/clk.h>
> > > > > +#include <linux/gpio/consumer.h>
> > > > >  
> > > > >  #define DRIVER_NAME "st-asc"
> > > > >  #define ASC_SERIAL_NAME "ttyAS"
> > > > > @@ -38,6 +39,7 @@
> > > > >  
> > > > >  struct asc_port {
> > > > >  	struct uart_port port;
> > > > > +	struct gpio_desc *rts;
> > > > >  	struct clk *clk;
> > > > >  	unsigned int hw_flow_control:1;
> > > > >  	unsigned int force_m1:1;
> > > > > @@ -381,12 +383,16 @@ static unsigned int asc_tx_empty(struct uart_port *port)
> > > > >  
> > > > >  static void asc_set_mctrl(struct uart_port *port, unsigned int mctrl)
> > > > >  {
> > > > > +	struct asc_port *ascport = to_asc_port(port);
> > > > > +
> > > > >  	/*
> > > > >  	 * This routine is used for seting signals of: DTR, DCD, CTS/RTS
> > > > >  	 * We use ASC's hardware for CTS/RTS, so don't need any for that.
> > > > >  	 * Some boards have DTR and DCD implemented using PIO pins,
> > > > >  	 * code to do this should be hooked in here.
> > > > >  	 */
> > > > > +
> > > > > +	gpiod_set_value(ascport->rts, mctrl & TIOCM_RTS);
> > > 
> > > This now puts the code and the comment above out of sync with each
> > > other.
> > > 
> > > However just checking the stih410 datasheet and I don't think this patch
> > > to control RTS via a GPIO is required anyway.
> > > 
> > > The correct way of handling this is to add UART10_RTS and UART10_CTS pins
> > > to the pinctrl_serial0 group so they are properly configured for their cts/rts
> > > alternate function. Once the pins are correctly configured, the IP block should
> > > control the signals correctly like the comment says.
> > 
> > That was the first thing I tried.  It didn't work.
> 
>        cts = <&pio17 2 ALT1 IN>;
>        rts = <&pio17 3 ALT1 OUT>;
>

That implies that either some additional configuration of the IP is required in
st-asc to get the IP to control those signals or that there is a bug in the
ST pinctrl driver and the correct settings aren't being applied.

regards,

Peter.