On 01/12/2016 16:10, Greg KH wrote:
> On Thu, Dec 01, 2016 at 03:20:35PM +0100, Gil Weber wrote:
>> Hello,
Hi,
>> I am unable to communicate in RS485 since I upgrade to the latest kernel
>> version (v4.9-rc7). Previously I was in 3.12.
>>
>> It seems to be caused by commit "0058f0871efe7b01c6f2b3046c68196ab73e96da
>> tty/serial: atmel: fix RS485 half duplex with DMA".
>> RX is disabled during TX, but in my case it is not re-enabled after.
>> If I revert this patch, everything seems to work again.
>
> Any reason you didn't cc: the author of that patch? :)
>
>> Currently enable RX is done in atmel_tx_dma but it is not called if there are
>> still data to transfer... problem in that it is never done later.
>> So I think we should also enable RX in atmel_complete_tx_dma.
>>
>> I have made this fix, and it seems to work now:
>>
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c
>> b/drivers/tty/serial/atmel_serial.c
>> index 74e97dc..4190012 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -806,6 +806,10 @@ static void atmel_complete_tx_dma(void *arg)
>> */
>> if (!uart_circ_empty(xmit))
>> atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
>> + else if (port->rs485.flags & SER_RS485_ENABLED) {
Don't you need to test the SER_RS485_RX_DURING_TX flag as well ?
Hum. I see. It is not present in atmel_tx_dma().
Maybe we can move the test from atmel_tx_dma() to here.
>> + /* DMA done, start RX for RS485 */
>> + atmel_start_rx(port);
>> + }
>>
>> spin_unlock_irqrestore(&port->lock, flags);
>>
>>
>>
>>
>> Is it correct? or maybe there is a better way to fix it?
>> Thanks,
>> Gil
>
> Alexandre, any ideas?
>
Gil, could you try something like:
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 168b10cad47b..11c0117af80b 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -798,6 +798,11 @@ static void atmel_complete_tx_dma(void *arg)
*/
if (!uart_circ_empty(xmit))
atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
+ else if ((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+ /* DMA done, stop TX, start RX for RS485 */
+ atmel_start_rx(port);
+ }
spin_unlock_irqrestore(&port->lock, flags);
}
@@ -900,12 +905,6 @@ static void atmel_tx_dma(struct uart_port *port)
desc->callback = atmel_complete_tx_dma;
desc->callback_param = atmel_port;
atmel_port->cookie_tx = dmaengine_submit(desc);
-
- } else {
- if (port->rs485.flags & SER_RS485_ENABLED) {
- /* DMA done, stop TX, start RX for RS485 */
- atmel_start_rx(port);
- }
}
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
Looking at atmel_{start,stop}_{rx,tx} functions, I wonder if it makes sense to have:
if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
if ((port->rs485.flags & SER_RS485_ENABLED) &&
!(port->rs485.flags & SER_RS485_RX_DURING_TX))
atmel_stop_rx(port);
in atmel_start_tx()
and just:
if ((port->rs485.flags & SER_RS485_ENABLED) &&
!(port->rs485.flags & SER_RS485_RX_DURING_TX))
atmel_start_rx(port);
in atmel_stop_tx() ?
The condition on (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port)) doesn't seems to be necessary.
If there's no DMA nor PDC, atmel_stop_rx(port) should still be called, shouldn't it ?
Richard.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
