On Wed, Jan 26, 2022 at 4:00 PM Tim Harvey <tharvey@xxxxxxxxxxxxx> wrote: > > On Mon, Jan 24, 2022 at 1:52 AM Richard Genoud <richard.genoud@xxxxxxxxx> 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@xxxxxxxxxxxxxxx/ > > > [2] https://lore.kernel.org/linux-serial/20220104103203.2033673-1-tomasz.mon@xxxxxxxxxxxxxxx/ > > > > > > 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
