2016-12-02 14:11:37

by Lee Jones

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

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

Signed-off-by: Lee Jones <[email protected]>
---
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


2016-12-02 14:11:39

by Lee Jones

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

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 <[email protected]>
---
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

2016-12-02 14:12:03

by Lee Jones

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

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 <[email protected]>
---
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

2016-12-05 14:13:46

by Patrice CHOTARD

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

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 <[email protected]>
> ---
> 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;
>

2016-12-06 08:40:53

by Patrice CHOTARD

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

Hi Lee

Except the typo in patch 2

Acked-by: Patrice Chotard <[email protected]>

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 <[email protected]>
> ---
> 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 */
>

2016-12-06 12:31:38

by Peter Griffin

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

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 <[email protected]>
> > ---
> > 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.

2016-12-06 12:45:01

by Lee Jones

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

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 <[email protected]>
> > > > ---
> > > > 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

2016-12-06 12:51:36

by Lee Jones

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

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 <[email protected]>
> > > ---
> > > 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

2016-12-06 13:04:29

by Peter Griffin

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

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 <[email protected]>
> > > > > ---
> > > > > 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.