On 9/2/20 10:08 AM, Fabrice Gasnier wrote:
> On 8/31/20 7:10 PM, Marek Vasut wrote:
>> While the STM32 does support RS485 drive-enable control within the
>> UART IP itself, some systems have the drive-enable line connected
>> to a pin which cannot be pinmuxed as RTS. Add support for toggling
>> the RTS GPIO line using the modem control GPIOs to provide at least
>> some sort of emulation.
>>
>> Fixes: 7df5081cbf5e ("serial: stm32: Add RS485 RTS GPIO control")
>> Signed-off-by: Marek Vasut <marex@xxxxxxx>
>> Cc: Alexandre Torgue <alexandre.torgue@xxxxxx>
>> Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
>> Cc: Manivannan Sadhasivam <mani@xxxxxxxxxx>
>> Cc: Fabrice Gasnier <fabrice.gasnier@xxxxxx>
>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> Cc: linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
>> ---
>> V2: Use mctrl_gpio_set() instead of stm32_set_mctrl()
>> V3: - Actually toggle the RTS line before and after TX
>> - Undo 7df5081cbf5e ("serial: stm32: Add RS485 RTS GPIO control")
>> which was previous version of this patch ; I messed up.
>> ---
>> drivers/tty/serial/stm32-usart.c | 33 ++++++++++++++++++++++++--------
>> 1 file changed, 25 insertions(+), 8 deletions(-)
>
>
> Hi Marek,
>
> This seems sensible. I've few comments on the commit tittle and commit
> message:
> - tittle: this could be named as a "fix" rather than "add... again" ?
> - I may have missed it... Is it a V3 ?
There was a V2 which got applied before I had the chance to send a V3.
> Could explain what is being fixed? Why moving the mctrl_gpio_* calls
> away from the stm32_config_rs485()/set_termios() routines to the
> start_tx/stop_tx ops improves/fixes the RS485 RTS GPIO control (what was
> wrong) ?
Because set_termios is not called every time there is a transfer, but
the DE GPIOs must be toggled every time there is a transfer (to enable
the DE on TX and disable it right after TX).
> Thanks,
> Best regards,
> Fabrice
>
>>
>> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
>> index 143300a80090..23f7453441ae 100644
>> --- a/drivers/tty/serial/stm32-usart.c
>> +++ b/drivers/tty/serial/stm32-usart.c
>> @@ -129,13 +129,9 @@ static int stm32_config_rs485(struct uart_port *port,
>> if (rs485conf->flags & SER_RS485_RTS_ON_SEND) {
>> cr3 &= ~USART_CR3_DEP;
>> rs485conf->flags &= ~SER_RS485_RTS_AFTER_SEND;
>> - mctrl_gpio_set(stm32_port->gpios,
>> - stm32_port->port.mctrl & ~TIOCM_RTS);
>> } else {
>> cr3 |= USART_CR3_DEP;
>> rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
>> - mctrl_gpio_set(stm32_port->gpios,
>> - stm32_port->port.mctrl | TIOCM_RTS);
>> }
>>
>> writel_relaxed(cr3, port->membase + ofs->cr3);
>> @@ -541,17 +537,42 @@ static void stm32_disable_ms(struct uart_port *port)
>> /* Transmit stop */
>> static void stm32_stop_tx(struct uart_port *port)
>> {
>> + struct stm32_port *stm32_port = to_stm32_port(port);
>> + struct serial_rs485 *rs485conf = &port->rs485;
>> +
>> stm32_tx_interrupt_disable(port);
>> +
>> + if (rs485conf->flags & SER_RS485_ENABLED) {
>> + if (rs485conf->flags & SER_RS485_RTS_ON_SEND) {
>> + mctrl_gpio_set(stm32_port->gpios,
>> + stm32_port->port.mctrl & ~TIOCM_RTS);
>> + } else {
>> + mctrl_gpio_set(stm32_port->gpios,
>> + stm32_port->port.mctrl | TIOCM_RTS);
>> + }
>> + }
>> }
>>
>> /* There are probably characters waiting to be transmitted. */
>> static void stm32_start_tx(struct uart_port *port)
>> {
>> + struct stm32_port *stm32_port = to_stm32_port(port);
>> + struct serial_rs485 *rs485conf = &port->rs485;
>> struct circ_buf *xmit = &port->state->xmit;
>>
>> if (uart_circ_empty(xmit))
>> return;
>>
>> + if (rs485conf->flags & SER_RS485_ENABLED) {
>> + if (rs485conf->flags & SER_RS485_RTS_ON_SEND) {
>> + mctrl_gpio_set(stm32_port->gpios,
>> + stm32_port->port.mctrl | TIOCM_RTS);
>> + } else {
>> + mctrl_gpio_set(stm32_port->gpios,
>> + stm32_port->port.mctrl & ~TIOCM_RTS);
>> + }
>> + }
>> +
>> stm32_transmit_chars(port);
>> }
>>
>> @@ -851,13 +872,9 @@ static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>> if (rs485conf->flags & SER_RS485_RTS_ON_SEND) {
>> cr3 &= ~USART_CR3_DEP;
>> rs485conf->flags &= ~SER_RS485_RTS_AFTER_SEND;
>> - mctrl_gpio_set(stm32_port->gpios,
>> - stm32_port->port.mctrl & ~TIOCM_RTS);
>> } else {
>> cr3 |= USART_CR3_DEP;
>> rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
>> - mctrl_gpio_set(stm32_port->gpios,
>> - stm32_port->port.mctrl | TIOCM_RTS);
>> }
>>
>> } else {
>>
