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

Message ID 20161202141146.31281-2-lee.jones@linaro.org
State New
Headers show

Commit Message

Lee Jones Dec. 2, 2016, 2:11 p.m.
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(+)

-- 
2.10.2

Comments

Patrice CHOTARD Dec. 5, 2016, 2:11 p.m. | #1
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;

>
Lee Jones Dec. 6, 2016, 12:45 p.m. | #2
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
Lee Jones Dec. 6, 2016, 12:48 p.m. | #3
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

Patch hide | download patch | download mbox

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;