All of lore.kernel.org
 help / color / mirror / Atom feed
* Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX
@ 2022-01-06 23:55 Tim Harvey
  2022-01-14  3:08 ` Tim Harvey
  0 siblings, 1 reply; 9+ messages in thread
From: Tim Harvey @ 2022-01-06 23:55 UTC (permalink / raw)
  To: linux-serial

Greetings,

I have long used modem control lines via GPIO with the IMX driver for
RS485 via rts-gpios in the scenario where an RS485 transceiver
transmit enable is connected to a GPIO representing RTS.

Therefore I 'thought' that one could use rts-gpios and cts-gpios for
hardware flow control on an IMX UART but it appears I may be wrong as
when I define those along with 'uart-has-rtscts' UART communication
ceases between the IMX and a device using hardware flow control.

As an example take the following which describes a UART with RTSCTS
flow control connected to a broadcom Bluetooth HCI radio:

The following works:
&uart4 {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_uart4>,<&pinctrl_bten>;
        uart-has-rtscts;
        status = "okay";

        bluetooth {
                compatible = "brcm,bcm4330-bt";
                shutdown-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
        };
};

&iomuxc {
        pinctrl_bten: btengrp {
                fsl,pins = <
                        MX6QDL_PAD_GPIO_2__GPIO1_IO02           0x1b0b1
                >;
        };

        pinctrl_uart4: uart4grp {
                fsl,pins = <
                        MX6QDL_PAD_CSI0_DAT12__UART4_TX_DATA    0x1b0b1
                        MX6QDL_PAD_CSI0_DAT13__UART4_RX_DATA    0x1b0b1
                        MX6QDL_PAD_CSI0_DAT16__UART4_RTS_B      0x1b0b1
                        MX6QDL_PAD_CSI0_DAT17__UART4_CTS_B      0x1b0b1
                >;
        };
};

The following where I've replaced the RTS/CTS with gpio does not work:
&uart4 {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_uart4>,<&pinctrl_bten>;
        rts-gpios = <&gpio6 2 GPIO_ACTIVE_LOW>;
        cts-gpios = <&gpio6 3 GPIO_ACTIVE_LOW>;
        uart-has-rtscts;
        status = "okay";

        bluetooth {
                compatible = "brcm,bcm4330-bt";
                shutdown-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
        };
};

&iomuxc {
        pinctrl_bten: btengrp {
                fsl,pins = <
                        MX6QDL_PAD_GPIO_2__GPIO1_IO02           0x1b0b1
                >;
        };

        pinctrl_uart4: uart4grp {
                fsl,pins = <
                        MX6QDL_PAD_CSI0_DAT12__UART4_TX_DATA    0x1b0b1
                        MX6QDL_PAD_CSI0_DAT13__UART4_RX_DATA    0x1b0b1
                        MX6QDL_PAD_CSI0_DAT16__GPIO6_IO02       0x1b0b1
                        MX6QDL_PAD_CSI0_DAT17__GPIO6_IO03       0x1b0b1
                >;
        };
};

In the above example of course I could simply use the version that
works but the reason I want to use rts-gpios and cts-gpios is that I
have cases where I don't have access to the RTS/CTS pinmux options and
thus can only use gpio's.

I'm hoping perhaps I just have something misconfigured or that perhaps
something simple is missing from drivers/tty/serial/imx.c in order to
use gpio's as hardware flow control. I've tried flipping the rts/cts
gpios and polarity in case I had something simply backwards but that
still did not work.

Any ideas or suggestions?

Best regards,

Tim

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX
  2022-01-06 23:55 Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX Tim Harvey
@ 2022-01-14  3:08 ` Tim Harvey
  2022-01-14  6:19   ` Tomasz Moń
  0 siblings, 1 reply; 9+ messages in thread
From: Tim Harvey @ 2022-01-14  3:08 UTC (permalink / raw)
  To: linux-serial
  Cc: Uwe Kleine-König, Sascha Hauer, Shawn Guo, Huang Shijie,
	Dirk Behme, Jiri Slaby, Richard Genoud

On Thu, Jan 6, 2022 at 3:55 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> Greetings,
>
> I have long used modem control lines via GPIO with the IMX driver for
> RS485 via rts-gpios in the scenario where an RS485 transceiver
> transmit enable is connected to a GPIO representing RTS.
>
> Therefore I 'thought' that one could use rts-gpios and cts-gpios for
> hardware flow control on an IMX UART but it appears I may be wrong as
> when I define those along with 'uart-has-rtscts' UART communication
> ceases between the IMX and a device using hardware flow control.
>
> As an example take the following which describes a UART with RTSCTS
> flow control connected to a broadcom Bluetooth HCI radio:
>
> The following works:
> &uart4 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_uart4>,<&pinctrl_bten>;
>         uart-has-rtscts;
>         status = "okay";
>
>         bluetooth {
>                 compatible = "brcm,bcm4330-bt";
>                 shutdown-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
>         };
> };
>
> &iomuxc {
>         pinctrl_bten: btengrp {
>                 fsl,pins = <
>                         MX6QDL_PAD_GPIO_2__GPIO1_IO02           0x1b0b1
>                 >;
>         };
>
>         pinctrl_uart4: uart4grp {
>                 fsl,pins = <
>                         MX6QDL_PAD_CSI0_DAT12__UART4_TX_DATA    0x1b0b1
>                         MX6QDL_PAD_CSI0_DAT13__UART4_RX_DATA    0x1b0b1
>                         MX6QDL_PAD_CSI0_DAT16__UART4_RTS_B      0x1b0b1
>                         MX6QDL_PAD_CSI0_DAT17__UART4_CTS_B      0x1b0b1
>                 >;
>         };
> };
>
> The following where I've replaced the RTS/CTS with gpio does not work:
> &uart4 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_uart4>,<&pinctrl_bten>;
>         rts-gpios = <&gpio6 2 GPIO_ACTIVE_LOW>;
>         cts-gpios = <&gpio6 3 GPIO_ACTIVE_LOW>;
>         uart-has-rtscts;
>         status = "okay";
>
>         bluetooth {
>                 compatible = "brcm,bcm4330-bt";
>                 shutdown-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
>         };
> };
>
> &iomuxc {
>         pinctrl_bten: btengrp {
>                 fsl,pins = <
>                         MX6QDL_PAD_GPIO_2__GPIO1_IO02           0x1b0b1
>                 >;
>         };
>
>         pinctrl_uart4: uart4grp {
>                 fsl,pins = <
>                         MX6QDL_PAD_CSI0_DAT12__UART4_TX_DATA    0x1b0b1
>                         MX6QDL_PAD_CSI0_DAT13__UART4_RX_DATA    0x1b0b1
>                         MX6QDL_PAD_CSI0_DAT16__GPIO6_IO02       0x1b0b1
>                         MX6QDL_PAD_CSI0_DAT17__GPIO6_IO03       0x1b0b1
>                 >;
>         };
> };
>
> In the above example of course I could simply use the version that
> works but the reason I want to use rts-gpios and cts-gpios is that I
> have cases where I don't have access to the RTS/CTS pinmux options and
> thus can only use gpio's.
>
> I'm hoping perhaps I just have something misconfigured or that perhaps
> something simple is missing from drivers/tty/serial/imx.c in order to
> use gpio's as hardware flow control. I've tried flipping the rts/cts
> gpios and polarity in case I had something simply backwards but that
> still did not work.
>
> Any ideas or suggestions?
>
> Best regards,
>
> Tim

Adding some previous main committers to imx uart driver and mctrl feature to cc.

From looking over drivers/tty/serial/imx.c, serial_core.c, and
serial_mctrl_gpio.c I see that rts-gpios is the 'output' from the IMX
so I need to flip my gpios as:
         cts-gpios = <&gpio6 2 GPIO_ACTIVE_LOW>; // input to IMX from
BT_UART_RTS_L output on brcm chip
         rts-gpios = <&gpio6 3 GPIO_ACTIVE_LOW>; // output from IMX to
BT_UART_CTS_L input on brcm chip

I see the following happening when the brcm bluetooth uart driver initializes:
mctrl_gpio_init registers irq handler for cts gpio (gpio6 2)
mctrl_gpio_init configures output for rts gpio (gpio6 3)
uart_change_speed is called and sets CRTSCTS enabled and calls stop_tx
(imx_uart_stop_tx)
mctrl_gpio_irq_handle is called and it sees CTS=1 then calls
uart_handle_cts_change which calls start_tx

So I believe the CTS input is handled correctly but I see nothing that
asserts the RTS  output. The imx.c driver has imx_uart_rts_active and
imx_uart_rts_inactive which call mctrl_gpio_set to manipulate the RTS
output yet this is only done when the UART is configured for RS485
mode to handle the case where RTS is used as an output to enable the
RS485 transmit enable.

So I believe in order to support using gpios for rts/cts in the imx
uart driver I must find the right place to call imx_uart_rts_active
and imx_uart_rts_inactive when the FIFO is not full and full
respectively. I'm not that familiar with the Linux uart driver
framework - am I on the right track and if so any ideas where this is
best done?

Best Regards,

Tim

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX
  2022-01-14  3:08 ` Tim Harvey
@ 2022-01-14  6:19   ` Tomasz Moń
  2022-01-24  9:52     ` Richard Genoud
  2022-01-24 23:56     ` Tim Harvey
  0 siblings, 2 replies; 9+ messages in thread
From: Tomasz Moń @ 2022-01-14  6:19 UTC (permalink / raw)
  To: Tim Harvey, linux-serial
  Cc: Uwe Kleine-König, Sascha Hauer, Shawn Guo, Huang Shijie,
	Dirk Behme, Jiri Slaby, Richard Genoud

On 14.01.2022 04:08, Tim Harvey wrote:
> So I believe in order to support using gpios for rts/cts in the imx
> uart driver I must find the right place to call imx_uart_rts_active
> and imx_uart_rts_inactive when the FIFO is not full and full
> respectively. I'm not that familiar with the Linux uart driver
> framework - am I on the right track and if so any ideas where this is
> best done?

It is not really the driver (and thus FIFO level), but rather the amount
of free space in tty buffer (checked by Line Discipline workqueue) that
determines when to throttle (set RTS inactive). This mostly works fine,
but fails [1] when the RX interrupt frequency is too high [2].

The throttle/unthrottle request, when termios CRTSCTS is set, is seen by
the driver as the call to .set_mctrl (imx_uart_set_mctrl) with TIOCM_RTS
bit cleared/set in mctrl parameter. Currently imx_uart_set_mctrl() only
controls the UCR2_CTS and UCR2_CTSC bits based on mctrl.

To support your case you would most likely have to add the gpio handling
in imx_uart_set_mctrl(). However, I am unaware what other issues you
might encounter (i.e. if it is not done there yet simply because nobody
had that use case or if there is some deeper problem).

[1] https://lore.kernel.org/linux-serial/10e723c0-a28b-de0d-0632-0bd250478313@camlingroup.com/
[2] https://lore.kernel.org/linux-serial/20220104103203.2033673-1-tomasz.mon@camlingroup.com/

Best Regards,
Tomasz Mon


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX
  2022-01-14  6:19   ` Tomasz Moń
@ 2022-01-24  9:52     ` Richard Genoud
  2022-01-27  0:00       ` Tim Harvey
  2022-01-24 23:56     ` Tim Harvey
  1 sibling, 1 reply; 9+ messages in thread
From: Richard Genoud @ 2022-01-24  9:52 UTC (permalink / raw)
  To: Tomasz Moń
  Cc: Uwe Kleine-König, Sascha Hauer, Shawn Guo, Huang Shijie,
	Dirk Behme, Jiri Slaby, Tim Harvey, linux-serial


Hi,

Le 14/01/2022 à 07:19, Tomasz Moń a écrit :
> On 14.01.2022 04:08, Tim Harvey wrote:
>> So I believe in order to support using gpios for rts/cts in the imx
>> uart driver I must find the right place to call imx_uart_rts_active
>> and imx_uart_rts_inactive when the FIFO is not full and full
>> respectively. I'm not that familiar with the Linux uart driver
>> framework - am I on the right track and if so any ideas where this is
>> best done?
> 
> It is not really the driver (and thus FIFO level), but rather the amount
> of free space in tty buffer (checked by Line Discipline workqueue) that
> determines when to throttle (set RTS inactive). This mostly works fine,
> but fails [1] when the RX interrupt frequency is too high [2].
> 
> The throttle/unthrottle request, when termios CRTSCTS is set, is seen by
> the driver as the call to .set_mctrl (imx_uart_set_mctrl) with TIOCM_RTS
> bit cleared/set in mctrl parameter. Currently imx_uart_set_mctrl() only
> controls the UCR2_CTS and UCR2_CTSC bits based on mctrl.
> 
> To support your case you would most likely have to add the gpio handling
> in imx_uart_set_mctrl(). However, I am unaware what other issues you
> might encounter (i.e. if it is not done there yet simply because nobody
> had that use case or if there is some deeper problem).
> 
> [1] https://lore.kernel.org/linux-serial/10e723c0-a28b-de0d-0632-0bd250478313@camlingroup.com/
> [2] https://lore.kernel.org/linux-serial/20220104103203.2033673-1-tomasz.mon@camlingroup.com/
> 
> Best Regards,
> Tomasz Mon
> 

I'd suggest to start testing with a serial port connected to nothing, and check the pins values
with a scope or a voltmeter.
Setting pins values from userspace can done quite easily with :
#include <unistd.h>
#include <termios.h>
#include <stdio.h>
#include <sys/ioctl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

void usage(char *prog)
{
	printf("usage: %s serial_port id 0/1 sleep_time_sec\n", prog);
	printf("%s\n", "1:DTR	DTR (data terminal ready)");
	printf("%s\n", "2:RTS	RTS (request to send)");
	printf("%s\n", "3:Both");
}

int main(int argc, char **argv)
{
	int fd;
	unsigned status = 0;
	int enable;
	int err;

	if (argc < 5) {
		usage(argv[0]);
		return -1;
	}

	fd = open(argv[1], O_RDWR | O_NOCTTY);

	enable = atoi(argv[3]);

	if (fd < 0)
		return -1;


	switch(atoi(argv[2])) {
	case 0:
		if (enable)
			status |= TIOCM_LE;
		break;
	case 1:
		if (enable)
			status |= TIOCM_DTR;
		break;
	case 2:
		if (enable)
			status |= TIOCM_RTS;
		break;
	case 3:
		if (enable)
			status |= TIOCM_DTR | TIOCM_RTS;
		break;
	default:
		printf("unknown signal\n");
	}

	err = ioctl(fd, TIOCMSET, &status);
	sleep(atoi(argv[4]));
out:
	if (fd > -1)
		close(fd);

	return err;
}

regards,
Richard

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX
  2022-01-14  6:19   ` Tomasz Moń
  2022-01-24  9:52     ` Richard Genoud
@ 2022-01-24 23:56     ` Tim Harvey
  2022-01-25 10:17       ` Richard Genoud
  1 sibling, 1 reply; 9+ messages in thread
From: Tim Harvey @ 2022-01-24 23:56 UTC (permalink / raw)
  To: Tomasz Moń
  Cc: linux-serial, Uwe Kleine-König, Sascha Hauer, Shawn Guo,
	Huang Shijie, Dirk Behme, Jiri Slaby, Richard Genoud

On Thu, Jan 13, 2022 at 10:19 PM Tomasz Moń <tomasz.mon@camlingroup.com> wrote:
>
> On 14.01.2022 04:08, Tim Harvey wrote:
> > So I believe in order to support using gpios for rts/cts in the imx
> > uart driver I must find the right place to call imx_uart_rts_active
> > and imx_uart_rts_inactive when the FIFO is not full and full
> > respectively. I'm not that familiar with the Linux uart driver
> > framework - am I on the right track and if so any ideas where this is
> > best done?
>
> It is not really the driver (and thus FIFO level), but rather the amount
> of free space in tty buffer (checked by Line Discipline workqueue) that
> determines when to throttle (set RTS inactive). This mostly works fine,
> but fails [1] when the RX interrupt frequency is too high [2].
>
> The throttle/unthrottle request, when termios CRTSCTS is set, is seen by
> the driver as the call to .set_mctrl (imx_uart_set_mctrl) with TIOCM_RTS
> bit cleared/set in mctrl parameter. Currently imx_uart_set_mctrl() only
> controls the UCR2_CTS and UCR2_CTSC bits based on mctrl.
>
> To support your case you would most likely have to add the gpio handling
> in imx_uart_set_mctrl(). However, I am unaware what other issues you
> might encounter (i.e. if it is not done there yet simply because nobody
> had that use case or if there is some deeper problem).
>
> [1] https://lore.kernel.org/linux-serial/10e723c0-a28b-de0d-0632-0bd250478313@camlingroup.com/
> [2] https://lore.kernel.org/linux-serial/20220104103203.2033673-1-tomasz.mon@camlingroup.com/
>
> Best Regards,
> Tomasz Mon
>

Tomasz,

Thanks for the info. Currently imx_uart_set_mctrl calls mctrl_gpio_set
which does toggle the rts-gpio per mctrl & TIOCM_RTS.

Also, there's something in
Documentation/devicetree/bindings/serial/serial.yaml that puzzles me:

if:
  required:
    - uart-has-rtscts
then:
  properties:
    cts-gpios: false
    rts-gpios: false

That would seem to indicate to me that if you define 'uart-has-rtscts'
you should not be defining 'cts-gpios' or 'rts-gpios' but I found that
when I omitted 'uart-has-rtscts' I could no longer enable hardware
flow control. Is my understanding of the yaml wrong or is this just
not accurate?

Best regards,

Tim

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX
  2022-01-24 23:56     ` Tim Harvey
@ 2022-01-25 10:17       ` Richard Genoud
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Genoud @ 2022-01-25 10:17 UTC (permalink / raw)
  To: Tim Harvey, Tomasz Moń
  Cc: linux-serial, Uwe Kleine-König, Sascha Hauer, Shawn Guo,
	Huang Shijie, Dirk Behme, Jiri Slaby




Le 25/01/2022 à 00:56, Tim Harvey a écrit :
> On Thu, Jan 13, 2022 at 10:19 PM Tomasz Moń <tomasz.mon@camlingroup.com> wrote:
>>
>> On 14.01.2022 04:08, Tim Harvey wrote:
>>> So I believe in order to support using gpios for rts/cts in the imx
>>> uart driver I must find the right place to call imx_uart_rts_active
>>> and imx_uart_rts_inactive when the FIFO is not full and full
>>> respectively. I'm not that familiar with the Linux uart driver
>>> framework - am I on the right track and if so any ideas where this is
>>> best done?
>>
>> It is not really the driver (and thus FIFO level), but rather the amount
>> of free space in tty buffer (checked by Line Discipline workqueue) that
>> determines when to throttle (set RTS inactive). This mostly works fine,
>> but fails [1] when the RX interrupt frequency is too high [2].
>>
>> The throttle/unthrottle request, when termios CRTSCTS is set, is seen by
>> the driver as the call to .set_mctrl (imx_uart_set_mctrl) with TIOCM_RTS
>> bit cleared/set in mctrl parameter. Currently imx_uart_set_mctrl() only
>> controls the UCR2_CTS and UCR2_CTSC bits based on mctrl.
>>
>> To support your case you would most likely have to add the gpio handling
>> in imx_uart_set_mctrl(). However, I am unaware what other issues you
>> might encounter (i.e. if it is not done there yet simply because nobody
>> had that use case or if there is some deeper problem).
>>
>> [1] https://lore.kernel.org/linux-serial/10e723c0-a28b-de0d-0632-0bd250478313@camlingroup.com/
>> [2] https://lore.kernel.org/linux-serial/20220104103203.2033673-1-tomasz.mon@camlingroup.com/
>>
>> Best Regards,
>> Tomasz Mon
>>
> 
> Tomasz,
> 
> Thanks for the info. Currently imx_uart_set_mctrl calls mctrl_gpio_set
> which does toggle the rts-gpio per mctrl & TIOCM_RTS.
> 
> Also, there's something in
> Documentation/devicetree/bindings/serial/serial.yaml that puzzles me:
> 
> if:
>    required:
>      - uart-has-rtscts
> then:
>    properties:
>      cts-gpios: false
>      rts-gpios: false
> 
> That would seem to indicate to me that if you define 'uart-has-rtscts'
> you should not be defining 'cts-gpios' or 'rts-gpios' but I found that
> when I omitted 'uart-has-rtscts' I could no longer enable hardware
> flow control. Is my understanding of the yaml wrong or is this just
> not accurate?
Yes, it seems that you're right
cf older textual description :
https://elixir.bootlin.com/linux/v4.20.17/source/Documentation/devicetree/bindings/serial/serial.txt

Regards,
Richard

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX
  2022-01-24  9:52     ` Richard Genoud
@ 2022-01-27  0:00       ` Tim Harvey
  2022-02-10 21:30         ` Tim Harvey
  0 siblings, 1 reply; 9+ messages in thread
From: Tim Harvey @ 2022-01-27  0:00 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Tomasz Moń,
	Uwe Kleine-König, Sascha Hauer, Shawn Guo, Huang Shijie,
	Dirk Behme, Jiri Slaby, linux-serial

On Mon, Jan 24, 2022 at 1:52 AM Richard Genoud <richard.genoud@gmail.com> wrote:
>
>
> Hi,
>
> Le 14/01/2022 à 07:19, Tomasz Moń a écrit :
> > On 14.01.2022 04:08, Tim Harvey wrote:
> >> So I believe in order to support using gpios for rts/cts in the imx
> >> uart driver I must find the right place to call imx_uart_rts_active
> >> and imx_uart_rts_inactive when the FIFO is not full and full
> >> respectively. I'm not that familiar with the Linux uart driver
> >> framework - am I on the right track and if so any ideas where this is
> >> best done?
> >
> > It is not really the driver (and thus FIFO level), but rather the amount
> > of free space in tty buffer (checked by Line Discipline workqueue) that
> > determines when to throttle (set RTS inactive). This mostly works fine,
> > but fails [1] when the RX interrupt frequency is too high [2].
> >
> > The throttle/unthrottle request, when termios CRTSCTS is set, is seen by
> > the driver as the call to .set_mctrl (imx_uart_set_mctrl) with TIOCM_RTS
> > bit cleared/set in mctrl parameter. Currently imx_uart_set_mctrl() only
> > controls the UCR2_CTS and UCR2_CTSC bits based on mctrl.
> >
> > To support your case you would most likely have to add the gpio handling
> > in imx_uart_set_mctrl(). However, I am unaware what other issues you
> > might encounter (i.e. if it is not done there yet simply because nobody
> > had that use case or if there is some deeper problem).
> >
> > [1] https://lore.kernel.org/linux-serial/10e723c0-a28b-de0d-0632-0bd250478313@camlingroup.com/
> > [2] https://lore.kernel.org/linux-serial/20220104103203.2033673-1-tomasz.mon@camlingroup.com/
> >
> > Best Regards,
> > Tomasz Mon
> >
>
> I'd suggest to start testing with a serial port connected to nothing, and check the pins values
> with a scope or a voltmeter.
> Setting pins values from userspace can done quite easily with :
> #include <unistd.h>
> #include <termios.h>
> #include <stdio.h>
> #include <sys/ioctl.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
>
> void usage(char *prog)
> {
>         printf("usage: %s serial_port id 0/1 sleep_time_sec\n", prog);
>         printf("%s\n", "1:DTR   DTR (data terminal ready)");
>         printf("%s\n", "2:RTS   RTS (request to send)");
>         printf("%s\n", "3:Both");
> }
>
> int main(int argc, char **argv)
> {
>         int fd;
>         unsigned status = 0;
>         int enable;
>         int err;
>
>         if (argc < 5) {
>                 usage(argv[0]);
>                 return -1;
>         }
>
>         fd = open(argv[1], O_RDWR | O_NOCTTY);
>
>         enable = atoi(argv[3]);
>
>         if (fd < 0)
>                 return -1;
>
>
>         switch(atoi(argv[2])) {
>         case 0:
>                 if (enable)
>                         status |= TIOCM_LE;
>                 break;
>         case 1:
>                 if (enable)
>                         status |= TIOCM_DTR;
>                 break;
>         case 2:
>                 if (enable)
>                         status |= TIOCM_RTS;
>                 break;
>         case 3:
>                 if (enable)
>                         status |= TIOCM_DTR | TIOCM_RTS;
>                 break;
>         default:
>                 printf("unknown signal\n");
>         }
>
>         err = ioctl(fd, TIOCMSET, &status);
>         sleep(atoi(argv[4]));
> out:
>         if (fd > -1)
>                 close(fd);
>
>         return err;
> }
>

Richard,

Thanks, I've been able to use this as well as terminal apps like
picocom and a scope to ensure that the RTS gpio is getting asserted
properly and that the CTS gpio is getting handled accordingly. Both
these signals are configured with internal pull-ups.

With for example:
        cts-gpios = <&gpio6 2 GPIO_ACTIVE_LOW>; // input to IMX
        rts-gpios = <&gpio6 3 GPIO_ACTIVE_LOW>; // output from IMX

I see that when rts is set to 1 (ie 'uart_test /dev/ttymxc3 2 1 3' or
'picocom /dev/ttymxc3 --flow h') the normally pulled-up logic high RTS
gpio goes to 0V and when rts is set to 0 it goes back to 3.3V via the
pull-up.

I've also been able to add debugging to ensure that when the CTS
signal is manually grounded that mctrl_gpio_irq_handle is called with
CTS=1 and imx_uart_start_tx is called to enable transmission and when
CTS signal is released from GND going back to 3.3V via the pull-up
mctrl_gpio_irq_handle is called with CTS=0 and imx_uart_stop_tx is
called to halt transmission.

I've also verified that changing the gpio descriptor in the dts above
to GPIO_ACTIVE_HIGH negates the above logic which is clearly wrong as
these are active-low signals.

In this specific case the device I am connecting the IMX
UART3_TX/UART3_RX and the GPIO's for RTS/CTS to is a Laird
Sterling-LWB wifi/BT chip. The datasheet [1] shows:
pin 31: BT_UART_RTS_L output UART Request-to-send
pin 32: BT_UART_CTS_L input UART Clear-to-send
pin 33: BT_UART_TXD output UART transmit
pin 34: BT_UART_RXD input UART input

They are connected to the IMX as:
BT_UART_RXD(pin34) <- IMX CSI0_DAT12_UART4_TXD
BT_UART_TXD(pin33) -> IMX CSI0_DAT13_UART4_RXD
BT_UART_CTS_L(pin32) <- IMX CSI0_DAT17_UART4_RTS_B (GPIO6_IO3)
BT_UART_RTS_L(pin31) -> IMX CSI0_DAT16_UART4_CTS_B (GPIO6_IO2)

And again if pinmuxed as RTS/CTS communication with the BT HCI is fine
but if pinmuxed as GPIO and configured as the following BT HCI
communication fails:
        cts-gpios = <&gpio6 2 GPIO_ACTIVE_LOW>; /* in to IMX from HCI
BT_UART_RTS_L output */
        rts-gpios = <&gpio6 3 GPIO_ACTIVE_LOW>; /* out from IMX to HCI
BT_UART_CTS_L input */

I'm not sure what else to look at here.

Best regards,

Tim
[1] - https://www.lairdconnect.com/documentation/datasheet-sterling-lwb

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX
  2022-01-27  0:00       ` Tim Harvey
@ 2022-02-10 21:30         ` Tim Harvey
  2022-02-12 11:50           ` Sergey Organov
  0 siblings, 1 reply; 9+ messages in thread
From: Tim Harvey @ 2022-02-10 21:30 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Tomasz Moń,
	Uwe Kleine-König, Sascha Hauer, Shawn Guo, Huang Shijie,
	Dirk Behme, Jiri Slaby, linux-serial

On Wed, Jan 26, 2022 at 4:00 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Mon, Jan 24, 2022 at 1:52 AM Richard Genoud <richard.genoud@gmail.com> wrote:
> >
> >
> > Hi,
> >
> > Le 14/01/2022 à 07:19, Tomasz Moń a écrit :
> > > On 14.01.2022 04:08, Tim Harvey wrote:
> > >> So I believe in order to support using gpios for rts/cts in the imx
> > >> uart driver I must find the right place to call imx_uart_rts_active
> > >> and imx_uart_rts_inactive when the FIFO is not full and full
> > >> respectively. I'm not that familiar with the Linux uart driver
> > >> framework - am I on the right track and if so any ideas where this is
> > >> best done?
> > >
> > > It is not really the driver (and thus FIFO level), but rather the amount
> > > of free space in tty buffer (checked by Line Discipline workqueue) that
> > > determines when to throttle (set RTS inactive). This mostly works fine,
> > > but fails [1] when the RX interrupt frequency is too high [2].
> > >
> > > The throttle/unthrottle request, when termios CRTSCTS is set, is seen by
> > > the driver as the call to .set_mctrl (imx_uart_set_mctrl) with TIOCM_RTS
> > > bit cleared/set in mctrl parameter. Currently imx_uart_set_mctrl() only
> > > controls the UCR2_CTS and UCR2_CTSC bits based on mctrl.
> > >
> > > To support your case you would most likely have to add the gpio handling
> > > in imx_uart_set_mctrl(). However, I am unaware what other issues you
> > > might encounter (i.e. if it is not done there yet simply because nobody
> > > had that use case or if there is some deeper problem).
> > >
> > > [1] https://lore.kernel.org/linux-serial/10e723c0-a28b-de0d-0632-0bd250478313@camlingroup.com/
> > > [2] https://lore.kernel.org/linux-serial/20220104103203.2033673-1-tomasz.mon@camlingroup.com/
> > >
> > > Best Regards,
> > > Tomasz Mon
> > >
> >
> > I'd suggest to start testing with a serial port connected to nothing, and check the pins values
> > with a scope or a voltmeter.
> > Setting pins values from userspace can done quite easily with :
> > #include <unistd.h>
> > #include <termios.h>
> > #include <stdio.h>
> > #include <sys/ioctl.h>
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > #include <fcntl.h>
> >
> > void usage(char *prog)
> > {
> >         printf("usage: %s serial_port id 0/1 sleep_time_sec\n", prog);
> >         printf("%s\n", "1:DTR   DTR (data terminal ready)");
> >         printf("%s\n", "2:RTS   RTS (request to send)");
> >         printf("%s\n", "3:Both");
> > }
> >
> > int main(int argc, char **argv)
> > {
> >         int fd;
> >         unsigned status = 0;
> >         int enable;
> >         int err;
> >
> >         if (argc < 5) {
> >                 usage(argv[0]);
> >                 return -1;
> >         }
> >
> >         fd = open(argv[1], O_RDWR | O_NOCTTY);
> >
> >         enable = atoi(argv[3]);
> >
> >         if (fd < 0)
> >                 return -1;
> >
> >
> >         switch(atoi(argv[2])) {
> >         case 0:
> >                 if (enable)
> >                         status |= TIOCM_LE;
> >                 break;
> >         case 1:
> >                 if (enable)
> >                         status |= TIOCM_DTR;
> >                 break;
> >         case 2:
> >                 if (enable)
> >                         status |= TIOCM_RTS;
> >                 break;
> >         case 3:
> >                 if (enable)
> >                         status |= TIOCM_DTR | TIOCM_RTS;
> >                 break;
> >         default:
> >                 printf("unknown signal\n");
> >         }
> >
> >         err = ioctl(fd, TIOCMSET, &status);
> >         sleep(atoi(argv[4]));
> > out:
> >         if (fd > -1)
> >                 close(fd);
> >
> >         return err;
> > }
> >
>
> Richard,
>
> Thanks, I've been able to use this as well as terminal apps like
> picocom and a scope to ensure that the RTS gpio is getting asserted
> properly and that the CTS gpio is getting handled accordingly. Both
> these signals are configured with internal pull-ups.
>
> With for example:
>         cts-gpios = <&gpio6 2 GPIO_ACTIVE_LOW>; // input to IMX
>         rts-gpios = <&gpio6 3 GPIO_ACTIVE_LOW>; // output from IMX
>
> I see that when rts is set to 1 (ie 'uart_test /dev/ttymxc3 2 1 3' or
> 'picocom /dev/ttymxc3 --flow h') the normally pulled-up logic high RTS
> gpio goes to 0V and when rts is set to 0 it goes back to 3.3V via the
> pull-up.
>
> I've also been able to add debugging to ensure that when the CTS
> signal is manually grounded that mctrl_gpio_irq_handle is called with
> CTS=1 and imx_uart_start_tx is called to enable transmission and when
> CTS signal is released from GND going back to 3.3V via the pull-up
> mctrl_gpio_irq_handle is called with CTS=0 and imx_uart_stop_tx is
> called to halt transmission.
>
> I've also verified that changing the gpio descriptor in the dts above
> to GPIO_ACTIVE_HIGH negates the above logic which is clearly wrong as
> these are active-low signals.
>
> In this specific case the device I am connecting the IMX
> UART3_TX/UART3_RX and the GPIO's for RTS/CTS to is a Laird
> Sterling-LWB wifi/BT chip. The datasheet [1] shows:
> pin 31: BT_UART_RTS_L output UART Request-to-send
> pin 32: BT_UART_CTS_L input UART Clear-to-send
> pin 33: BT_UART_TXD output UART transmit
> pin 34: BT_UART_RXD input UART input
>
> They are connected to the IMX as:
> BT_UART_RXD(pin34) <- IMX CSI0_DAT12_UART4_TXD
> BT_UART_TXD(pin33) -> IMX CSI0_DAT13_UART4_RXD
> BT_UART_CTS_L(pin32) <- IMX CSI0_DAT17_UART4_RTS_B (GPIO6_IO3)
> BT_UART_RTS_L(pin31) -> IMX CSI0_DAT16_UART4_CTS_B (GPIO6_IO2)
>
> And again if pinmuxed as RTS/CTS communication with the BT HCI is fine
> but if pinmuxed as GPIO and configured as the following BT HCI
> communication fails:
>         cts-gpios = <&gpio6 2 GPIO_ACTIVE_LOW>; /* in to IMX from HCI
> BT_UART_RTS_L output */
>         rts-gpios = <&gpio6 3 GPIO_ACTIVE_LOW>; /* out from IMX to HCI
> BT_UART_CTS_L input */
>
> I'm not sure what else to look at here.
>
> Best regards,
>
> Tim
> [1] - https://www.lairdconnect.com/documentation/datasheet-sterling-lwb

I have found the issue here which causes hardware flow control when
using GPIO's with the imx UART driver to fail. The
imx_uart_set_termios() function clears the UCR2_IRTS whenver hardware
flow control is enabled which configures the transmitter to only send
with the RTS pin is asserted. In the case of a GPIO being used instead
of the dedicated internal RTS pin for the uart, this will keep the
transmitter from ever transmitting. In the hardware flow control case
where a GPIO is used UCR2_IRTS must be set to ignore the RTS pin. We
can use the have_rtsgpio flag which is set when 'rts-gpios' property
is used as a qualifier for this.

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index df8a0c8b8b29..d506cbd679dd 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1674,8 +1674,7 @@ imx_uart_set_termios(struct uart_port *port,
struct ktermios *termios,
                if (ucr2 & UCR2_CTS)
                        ucr2 |= UCR2_CTSC;
        }
-
-       if (termios->c_cflag & CRTSCTS)
+       if (!sport->have_rtsgpio && termios->c_cflag & CRTSCTS)
                ucr2 &= ~UCR2_IRTS;
        if (termios->c_cflag & CSTOPB)
                ucr2 |= UCR2_STPB;

If this makes sense, I'll send a patch.

Best regards,

Tim

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX
  2022-02-10 21:30         ` Tim Harvey
@ 2022-02-12 11:50           ` Sergey Organov
  0 siblings, 0 replies; 9+ messages in thread
From: Sergey Organov @ 2022-02-12 11:50 UTC (permalink / raw)
  To: Tim Harvey
  Cc: Richard Genoud, Tomasz Moń,
	Uwe Kleine-König, Sascha Hauer, Shawn Guo, Huang Shijie,
	Dirk Behme, Jiri Slaby, linux-serial

Tim Harvey <tharvey@gateworks.com> writes:


[...]

> I have found the issue here which causes hardware flow control when
> using GPIO's with the imx UART driver to fail. The
> imx_uart_set_termios() function clears the UCR2_IRTS whenver hardware
> flow control is enabled which configures the transmitter to only send
> with the RTS pin is asserted. In the case of a GPIO being used instead
> of the dedicated internal RTS pin for the uart, this will keep the
> transmitter from ever transmitting. In the hardware flow control case
> where a GPIO is used UCR2_IRTS must be set to ignore the RTS pin. We
> can use the have_rtsgpio flag which is set when 'rts-gpios' property
> is used as a qualifier for this.
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index df8a0c8b8b29..d506cbd679dd 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1674,8 +1674,7 @@ imx_uart_set_termios(struct uart_port *port,
> struct ktermios *termios,
>                 if (ucr2 & UCR2_CTS)
>                         ucr2 |= UCR2_CTSC;
>         }
> -
> -       if (termios->c_cflag & CRTSCTS)
> +       if (!sport->have_rtsgpio && termios->c_cflag & CRTSCTS)
>                 ucr2 &= ~UCR2_IRTS;
>         if (termios->c_cflag & CSTOPB)
>                 ucr2 |= UCR2_STPB;
>
> If this makes sense, I'll send a patch.

To me it does make sense. Driver should stay away from UCR2_IRTS indeed
if software (GPIO) RTS/CTS management is in use.

-- Sergey Organov

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-02-12 11:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-06 23:55 Modem control lines for RTSCTS hardware flow control via rts-gpio and cts-gpio with IMX Tim Harvey
2022-01-14  3:08 ` Tim Harvey
2022-01-14  6:19   ` Tomasz Moń
2022-01-24  9:52     ` Richard Genoud
2022-01-27  0:00       ` Tim Harvey
2022-02-10 21:30         ` Tim Harvey
2022-02-12 11:50           ` Sergey Organov
2022-01-24 23:56     ` Tim Harvey
2022-01-25 10:17       ` Richard Genoud

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.