Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1264799 > unrolled thread

[PATCH v2 3/3] tty: Add software emulated RS485 support for 8250

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-07 11:20 +0100
    Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 Peter Hurley <peter@hurleysoftware.com> - 2015-11-07 17:10 +0100
      Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-08 12:00 +0100
        Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 Peter Hurley <peter@hurleysoftware.com> - 2015-11-09 15:50 +0100
          Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-09 16:50 +0100
            Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 Peter Hurley <peter@hurleysoftware.com> - 2015-11-09 22:40 +0100
              Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-09 22:50 +0100
                Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 Peter Hurley <peter@hurleysoftware.com> - 2015-11-09 23:10 +0100
                  Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-10 12:40 +0100
    Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 Peter Hurley <peter@hurleysoftware.com> - 2015-11-10 17:20 +0100
      Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-10 17:30 +0100
        Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 Peter Hurley <peter@hurleysoftware.com> - 2015-11-10 18:00 +0100
      Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-12 13:40 +0100
        Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 Peter Hurley <peter@hurleysoftware.com> - 2015-11-12 14:40 +0100
          Re: [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250 "Matwey V. Kornilov" <matwey@sai.msu.ru> - 2015-11-13 09:40 +0100

#1264799 — [PATCH v2 3/3] tty: Add software emulated RS485 support for 8250

Implementation of software emulation of RS485 direction handling is based
on omap-serial driver.  It is acts as the following. At transmission start,
RTS is set (if required) and receiver is off (if required). At transmission
stop, RTS is set (if required) and fifo is flushed.

Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
 drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 52d82d2..a9291f7 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
 	pm_runtime_mark_last_busy(p->port.dev);
 	pm_runtime_put_autosuspend(p->port.dev);
 }
+static void serial8250_stop_rx(struct uart_port *port);
+static void serial8250_rs485_start_tx(struct uart_8250_port *p)
+{
+	if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
+		return;
+
+	if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
+		serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
+		if (p->port.rs485.delay_rts_before_send > 0)
+			mdelay(p->port.rs485.delay_rts_before_send);
+	}
+	if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
+		serial8250_stop_rx(&p->port);
+}
+static void serial8250_rs485_stop_tx(struct uart_8250_port *p)
+{
+	if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
+		return;
 
+	if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
+		if (p->port.rs485.delay_rts_after_send > 0)
+			mdelay(p->port.rs485.delay_rts_after_send);
+		serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
+	}
+	/*
+	* Empty the RX FIFO, we are not interested in anything
+	* received during the half-duplex transmission.
+	*/
+	if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
+		serial8250_clear_fifos(p);
+}
 /*
  * IER sleep support.  UARTs which have EFRs need the "extended
  * capability" bit enabled.  Note that on XR16C850s, we need to
@@ -1309,6 +1339,7 @@ static void serial8250_stop_tx(struct uart_port *port)
 		up->acr |= UART_ACR_TXDIS;
 		serial_icr_write(up, UART_ACR, up->acr);
 	}
+	serial8250_rs485_stop_tx(up);
 	serial8250_rpm_put(up);
 }
 
@@ -1317,6 +1348,7 @@ static void serial8250_start_tx(struct uart_port *port)
 	struct uart_8250_port *up = up_to_u8250p(port);
 
 	serial8250_rpm_get_tx(up);
+	serial8250_rs485_start_tx(up);
 
 	if (up->dma && !up->dma->tx_dma(up))
 		return;
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1264870

Hi Matwey,

On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
> Implementation of software emulation of RS485 direction handling is based
> on omap-serial driver.  It is acts as the following. At transmission start,
> RTS is set (if required) and receiver is off (if required). At transmission
> stop, RTS is set (if required) and fifo is flushed.

Comments below.

> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> ---
>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 52d82d2..a9291f7 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>  	pm_runtime_mark_last_busy(p->port.dev);
>  	pm_runtime_put_autosuspend(p->port.dev);
>  }

Newline req'd here.

> +static void serial8250_stop_rx(struct uart_port *port);

You can eliminate this forward decl by relocating serial8250_stop_rx() to here
(in a separate patch).

> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
> +{
> +	if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
> +		return;
> +
> +	if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
> +		serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
> +		if (p->port.rs485.delay_rts_before_send > 0)
> +			mdelay(p->port.rs485.delay_rts_before_send);

So irqs are off for x msecs, and this cpu can't be used for anything else now?
I think this needs to be solved differently; maybe with a timer?

> +	}
> +	if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
> +		serial8250_stop_rx(&p->port);
> +}

Newline req'd here.

> +static void serial8250_rs485_stop_tx(struct uart_8250_port *p)
> +{
> +	if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
> +		return;
>  
> +	if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
> +		if (p->port.rs485.delay_rts_after_send > 0)
> +			mdelay(p->port.rs485.delay_rts_after_send);

Same issue with irqs off.

> +		serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
> +	}
> +	/*
> +	* Empty the RX FIFO, we are not interested in anything
> +	* received during the half-duplex transmission.
> +	*/
> +	if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
> +		serial8250_clear_fifos(p);
> +}
>  /*
>   * IER sleep support.  UARTs which have EFRs need the "extended
>   * capability" bit enabled.  Note that on XR16C850s, we need to
> @@ -1309,6 +1339,7 @@ static void serial8250_stop_tx(struct uart_port *port)
>  		up->acr |= UART_ACR_TXDIS;
>  		serial_icr_write(up, UART_ACR, up->acr);
>  	}
> +	serial8250_rs485_stop_tx(up);
>  	serial8250_rpm_put(up);
>  }
>  
> @@ -1317,6 +1348,7 @@ static void serial8250_start_tx(struct uart_port *port)
>  	struct uart_8250_port *up = up_to_u8250p(port);
>  
>  	serial8250_rpm_get_tx(up);
> +	serial8250_rs485_start_tx(up);
>  
>  	if (up->dma && !up->dma->tx_dma(up))
>  		return;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1265074

2015-11-07 19:03 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> Hi Matwey,
>
> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>> Implementation of software emulation of RS485 direction handling is based
>> on omap-serial driver.  It is acts as the following. At transmission start,
>> RTS is set (if required) and receiver is off (if required). At transmission
>> stop, RTS is set (if required) and fifo is flushed.
>
> Comments below.
>
>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>> ---
>>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>> index 52d82d2..a9291f7 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>>       pm_runtime_mark_last_busy(p->port.dev);
>>       pm_runtime_put_autosuspend(p->port.dev);
>>  }
>
> Newline req'd here.
>
>> +static void serial8250_stop_rx(struct uart_port *port);
>
> You can eliminate this forward decl by relocating serial8250_stop_rx() to here
> (in a separate patch).
>
>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>> +{
>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>> +             return;
>> +
>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>> +             if (p->port.rs485.delay_rts_before_send > 0)
>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>
> So irqs are off for x msecs, and this cpu can't be used for anything else now?
> I think this needs to be solved differently; maybe with a timer?

Call of serial8250_start_tx is wrapped with spin_lock_irq in serial_core.c:2154
I've tried to use msleep instead of mdelay but got "BUG: scheduling
while atomic".

>
>> +     }
>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>> +             serial8250_stop_rx(&p->port);
>> +}
>
> Newline req'd here.
>
>> +static void serial8250_rs485_stop_tx(struct uart_8250_port *p)
>> +{
>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>> +             return;
>>
>> +     if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
>> +             if (p->port.rs485.delay_rts_after_send > 0)
>> +                     mdelay(p->port.rs485.delay_rts_after_send);
>
> Same issue with irqs off.
>
>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>> +     }
>> +     /*
>> +     * Empty the RX FIFO, we are not interested in anything
>> +     * received during the half-duplex transmission.
>> +     */
>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>> +             serial8250_clear_fifos(p);
>> +}
>>  /*
>>   * IER sleep support.  UARTs which have EFRs need the "extended
>>   * capability" bit enabled.  Note that on XR16C850s, we need to
>> @@ -1309,6 +1339,7 @@ static void serial8250_stop_tx(struct uart_port *port)
>>               up->acr |= UART_ACR_TXDIS;
>>               serial_icr_write(up, UART_ACR, up->acr);
>>       }
>> +     serial8250_rs485_stop_tx(up);
>>       serial8250_rpm_put(up);
>>  }
>>
>> @@ -1317,6 +1348,7 @@ static void serial8250_start_tx(struct uart_port *port)
>>       struct uart_8250_port *up = up_to_u8250p(port);
>>
>>       serial8250_rpm_get_tx(up);
>> +     serial8250_rs485_start_tx(up);
>>
>>       if (up->dma && !up->dma->tx_dma(up))
>>               return;
>>
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1265735

On 11/08/2015 05:52 AM, Matwey V. Kornilov wrote:
> 2015-11-07 19:03 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>> Hi Matwey,
>>
>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>> Implementation of software emulation of RS485 direction handling is based
>>> on omap-serial driver.  It is acts as the following. At transmission start,
>>> RTS is set (if required) and receiver is off (if required). At transmission
>>> stop, RTS is set (if required) and fifo is flushed.
>>
>> Comments below.
>>
>>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>>> ---
>>>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>>>  1 file changed, 32 insertions(+)
>>>
>>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>>> index 52d82d2..a9291f7 100644
>>> --- a/drivers/tty/serial/8250/8250_port.c
>>> +++ b/drivers/tty/serial/8250/8250_port.c
>>> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>>>       pm_runtime_mark_last_busy(p->port.dev);
>>>       pm_runtime_put_autosuspend(p->port.dev);
>>>  }
>>
>> Newline req'd here.
>>
>>> +static void serial8250_stop_rx(struct uart_port *port);
>>
>> You can eliminate this forward decl by relocating serial8250_stop_rx() to here
>> (in a separate patch).
>>
>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>> +{
>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>> +             return;
>>> +
>>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>> +             if (p->port.rs485.delay_rts_before_send > 0)
>>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>>
>> So irqs are off for x msecs, and this cpu can't be used for anything else now?
>> I think this needs to be solved differently; maybe with a timer?
> 
> Call of serial8250_start_tx is wrapped with spin_lock_irq in serial_core.c:2154

Yep, which is why I pointed out "irqs are off for x msecs".

> I've tried to use msleep instead of mdelay but got "BUG: scheduling
> while atomic".

Right, can't sleep while irqs are off, which is why I suggested something
like a timer.

Regards,
Peter Hurley

>>> +     }
>>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>>> +             serial8250_stop_rx(&p->port);
>>> +}
>>
>> Newline req'd here.
>>
>>> +static void serial8250_rs485_stop_tx(struct uart_8250_port *p)
>>> +{
>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>> +             return;
>>>
>>> +     if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
>>> +             if (p->port.rs485.delay_rts_after_send > 0)
>>> +                     mdelay(p->port.rs485.delay_rts_after_send);
>>
>> Same issue with irqs off.
>>
>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>> +     }
>>> +     /*
>>> +     * Empty the RX FIFO, we are not interested in anything
>>> +     * received during the half-duplex transmission.
>>> +     */
>>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>>> +             serial8250_clear_fifos(p);
>>> +}
>>>  /*
>>>   * IER sleep support.  UARTs which have EFRs need the "extended
>>>   * capability" bit enabled.  Note that on XR16C850s, we need to
>>> @@ -1309,6 +1339,7 @@ static void serial8250_stop_tx(struct uart_port *port)
>>>               up->acr |= UART_ACR_TXDIS;
>>>               serial_icr_write(up, UART_ACR, up->acr);
>>>       }
>>> +     serial8250_rs485_stop_tx(up);
>>>       serial8250_rpm_put(up);
>>>  }
>>>
>>> @@ -1317,6 +1348,7 @@ static void serial8250_start_tx(struct uart_port *port)
>>>       struct uart_8250_port *up = up_to_u8250p(port);
>>>
>>>       serial8250_rpm_get_tx(up);
>>> +     serial8250_rs485_start_tx(up);
>>>
>>>       if (up->dma && !up->dma->tx_dma(up))
>>>               return;
>>>
>>
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1265772

2015-11-09 17:40 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> On 11/08/2015 05:52 AM, Matwey V. Kornilov wrote:
>> 2015-11-07 19:03 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>> Hi Matwey,
>>>
>>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>>> Implementation of software emulation of RS485 direction handling is based
>>>> on omap-serial driver.  It is acts as the following. At transmission start,
>>>> RTS is set (if required) and receiver is off (if required). At transmission
>>>> stop, RTS is set (if required) and fifo is flushed.
>>>
>>> Comments below.
>>>
>>>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>>>> ---
>>>>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>>>>  1 file changed, 32 insertions(+)
>>>>
>>>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>>>> index 52d82d2..a9291f7 100644
>>>> --- a/drivers/tty/serial/8250/8250_port.c
>>>> +++ b/drivers/tty/serial/8250/8250_port.c
>>>> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>>>>       pm_runtime_mark_last_busy(p->port.dev);
>>>>       pm_runtime_put_autosuspend(p->port.dev);
>>>>  }
>>>
>>> Newline req'd here.
>>>
>>>> +static void serial8250_stop_rx(struct uart_port *port);
>>>
>>> You can eliminate this forward decl by relocating serial8250_stop_rx() to here
>>> (in a separate patch).
>>>
>>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>>> +{
>>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>>> +             return;
>>>> +
>>>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>>> +             if (p->port.rs485.delay_rts_before_send > 0)
>>>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>>>
>>> So irqs are off for x msecs, and this cpu can't be used for anything else now?
>>> I think this needs to be solved differently; maybe with a timer?
>>
>> Call of serial8250_start_tx is wrapped with spin_lock_irq in serial_core.c:2154
>
> Yep, which is why I pointed out "irqs are off for x msecs".
>
>> I've tried to use msleep instead of mdelay but got "BUG: scheduling
>> while atomic".
>
> Right, can't sleep while irqs are off, which is why I suggested something
> like a timer.

I am not sure that understand you correctly. Do you think that the
following would be ok?

wait_queue_head_t wait;
init_waitqueue_head(&wait);
wait_event_timeout(wait, 0, p->port.rs485.delay_rts_before_send * HZ / 1000);

>
> Regards,
> Peter Hurley
>
>>>> +     }
>>>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>>>> +             serial8250_stop_rx(&p->port);
>>>> +}
>>>
>>> Newline req'd here.
>>>
>>>> +static void serial8250_rs485_stop_tx(struct uart_8250_port *p)
>>>> +{
>>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>>> +             return;
>>>>
>>>> +     if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
>>>> +             if (p->port.rs485.delay_rts_after_send > 0)
>>>> +                     mdelay(p->port.rs485.delay_rts_after_send);
>>>
>>> Same issue with irqs off.
>>>
>>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>>> +     }
>>>> +     /*
>>>> +     * Empty the RX FIFO, we are not interested in anything
>>>> +     * received during the half-duplex transmission.
>>>> +     */
>>>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>>>> +             serial8250_clear_fifos(p);
>>>> +}
>>>>  /*
>>>>   * IER sleep support.  UARTs which have EFRs need the "extended
>>>>   * capability" bit enabled.  Note that on XR16C850s, we need to
>>>> @@ -1309,6 +1339,7 @@ static void serial8250_stop_tx(struct uart_port *port)
>>>>               up->acr |= UART_ACR_TXDIS;
>>>>               serial_icr_write(up, UART_ACR, up->acr);
>>>>       }
>>>> +     serial8250_rs485_stop_tx(up);
>>>>       serial8250_rpm_put(up);
>>>>  }
>>>>
>>>> @@ -1317,6 +1348,7 @@ static void serial8250_start_tx(struct uart_port *port)
>>>>       struct uart_8250_port *up = up_to_u8250p(port);
>>>>
>>>>       serial8250_rpm_get_tx(up);
>>>> +     serial8250_rs485_start_tx(up);
>>>>
>>>>       if (up->dma && !up->dma->tx_dma(up))
>>>>               return;
>>>>
>>>
>>
>>
>>
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266039

On 11/09/2015 10:45 AM, Matwey V. Kornilov wrote:
> 2015-11-09 17:40 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>> On 11/08/2015 05:52 AM, Matwey V. Kornilov wrote:
>>> 2015-11-07 19:03 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:

[...]

>>>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>>>> +{
>>>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>>>> +             return;
>>>>> +
>>>>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>>>> +             if (p->port.rs485.delay_rts_before_send > 0)
>>>>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>>>>
>>>> So irqs are off for x msecs, and this cpu can't be used for anything else now?
>>>> I think this needs to be solved differently; maybe with a timer?
>>>
>>> Call of serial8250_start_tx is wrapped with spin_lock_irq in serial_core.c:2154
>>
>> Yep, which is why I pointed out "irqs are off for x msecs".
>>
>>> I've tried to use msleep instead of mdelay but got "BUG: scheduling
>>> while atomic".
>>
>> Right, can't sleep while irqs are off, which is why I suggested something
>> like a timer.
> 
> I am not sure that understand you correctly. Do you think that the
> following would be ok?
> 
> wait_queue_head_t wait;
> init_waitqueue_head(&wait);
> wait_event_timeout(wait, 0, p->port.rs485.delay_rts_before_send * HZ / 1000);

Except for spinning, there is no way to wait-in-place with irqs off.

You'll need to do something more complex, like
1. raise RTS
2. start a timer _and return early without starting tx_
3. timer goes off, handler actually starts tx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266048

2015-11-10 0:30 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> On 11/09/2015 10:45 AM, Matwey V. Kornilov wrote:
>> 2015-11-09 17:40 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>> On 11/08/2015 05:52 AM, Matwey V. Kornilov wrote:
>>>> 2015-11-07 19:03 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>
> [...]
>
>>>>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>>>>> +{
>>>>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>>>>> +             return;
>>>>>> +
>>>>>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>>>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>>>>> +             if (p->port.rs485.delay_rts_before_send > 0)
>>>>>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>>>>>
>>>>> So irqs are off for x msecs, and this cpu can't be used for anything else now?
>>>>> I think this needs to be solved differently; maybe with a timer?
>>>>
>>>> Call of serial8250_start_tx is wrapped with spin_lock_irq in serial_core.c:2154
>>>
>>> Yep, which is why I pointed out "irqs are off for x msecs".
>>>
>>>> I've tried to use msleep instead of mdelay but got "BUG: scheduling
>>>> while atomic".
>>>
>>> Right, can't sleep while irqs are off, which is why I suggested something
>>> like a timer.
>>
>> I am not sure that understand you correctly. Do you think that the
>> following would be ok?
>>
>> wait_queue_head_t wait;
>> init_waitqueue_head(&wait);
>> wait_event_timeout(wait, 0, p->port.rs485.delay_rts_before_send * HZ / 1000);
>
> Except for spinning, there is no way to wait-in-place with irqs off.
>
> You'll need to do something more complex, like
> 1. raise RTS
> 2. start a timer _and return early without starting tx_
> 3. timer goes off, handler actually starts tx
>

I think this could lead to race conditions.
AFAIU when the kernel calls ops->start_tx(uport) and the function
returns, then it is supposed that the tx has been started. And that
could be not true, if the timer is used.


-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266052

On 11/09/2015 04:43 PM, Matwey V. Kornilov wrote:
> 2015-11-10 0:30 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>> On 11/09/2015 10:45 AM, Matwey V. Kornilov wrote:
>>> 2015-11-09 17:40 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>> On 11/08/2015 05:52 AM, Matwey V. Kornilov wrote:
>>>>> 2015-11-07 19:03 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>>>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>
>> [...]
>>
>>>>>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>>>>>> +{
>>>>>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>>>>>> +             return;
>>>>>>> +
>>>>>>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>>>>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>>>>>> +             if (p->port.rs485.delay_rts_before_send > 0)
>>>>>>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>>>>>>
>>>>>> So irqs are off for x msecs, and this cpu can't be used for anything else now?
>>>>>> I think this needs to be solved differently; maybe with a timer?
>>>>>
>>>>> Call of serial8250_start_tx is wrapped with spin_lock_irq in serial_core.c:2154
>>>>
>>>> Yep, which is why I pointed out "irqs are off for x msecs".
>>>>
>>>>> I've tried to use msleep instead of mdelay but got "BUG: scheduling
>>>>> while atomic".
>>>>
>>>> Right, can't sleep while irqs are off, which is why I suggested something
>>>> like a timer.
>>>
>>> I am not sure that understand you correctly. Do you think that the
>>> following would be ok?
>>>
>>> wait_queue_head_t wait;
>>> init_waitqueue_head(&wait);
>>> wait_event_timeout(wait, 0, p->port.rs485.delay_rts_before_send * HZ / 1000);
>>
>> Except for spinning, there is no way to wait-in-place with irqs off.
>>
>> You'll need to do something more complex, like
>> 1. raise RTS
>> 2. start a timer _and return early without starting tx_
>> 3. timer goes off, handler actually starts tx
>>
> 
> I think this could lead to race conditions.
> AFAIU when the kernel calls ops->start_tx(uport) and the function
> returns, then it is supposed that the tx has been started.

No; start_tx() must cause tx to become started, but tx does not
have to have _already_ started when start_tx() returns.

It would be very inefficient for start_tx() to _guarantee_ tx has
already started _before_ returning. Note the 8250 driver merely
writes to IER (which could be buffered and bridged).

> And that could be not true, if the timer is used.

It's true that using a timer will be more complex with more state
to manage, but being unable to service interrupts with this cpu for
milliseconds is unacceptable.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266422

2015-11-10 1:05 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> On 11/09/2015 04:43 PM, Matwey V. Kornilov wrote:
>> 2015-11-10 0:30 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>> On 11/09/2015 10:45 AM, Matwey V. Kornilov wrote:
>>>> 2015-11-09 17:40 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>>> On 11/08/2015 05:52 AM, Matwey V. Kornilov wrote:
>>>>>> 2015-11-07 19:03 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>>>>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>>
>>> [...]
>>>
>>>>>>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>>>>>>> +{
>>>>>>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>>>>>>> +             return;
>>>>>>>> +
>>>>>>>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>>>>>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>>>>>>> +             if (p->port.rs485.delay_rts_before_send > 0)
>>>>>>>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>>>>>>>
>>>>>>> So irqs are off for x msecs, and this cpu can't be used for anything else now?
>>>>>>> I think this needs to be solved differently; maybe with a timer?
>>>>>>
>>>>>> Call of serial8250_start_tx is wrapped with spin_lock_irq in serial_core.c:2154
>>>>>
>>>>> Yep, which is why I pointed out "irqs are off for x msecs".
>>>>>
>>>>>> I've tried to use msleep instead of mdelay but got "BUG: scheduling
>>>>>> while atomic".
>>>>>
>>>>> Right, can't sleep while irqs are off, which is why I suggested something
>>>>> like a timer.
>>>>
>>>> I am not sure that understand you correctly. Do you think that the
>>>> following would be ok?
>>>>
>>>> wait_queue_head_t wait;
>>>> init_waitqueue_head(&wait);
>>>> wait_event_timeout(wait, 0, p->port.rs485.delay_rts_before_send * HZ / 1000);
>>>
>>> Except for spinning, there is no way to wait-in-place with irqs off.
>>>
>>> You'll need to do something more complex, like
>>> 1. raise RTS
>>> 2. start a timer _and return early without starting tx_
>>> 3. timer goes off, handler actually starts tx
>>>
>>
>> I think this could lead to race conditions.
>> AFAIU when the kernel calls ops->start_tx(uport) and the function
>> returns, then it is supposed that the tx has been started.
>
> No; start_tx() must cause tx to become started, but tx does not
> have to have _already_ started when start_tx() returns.
>
> It would be very inefficient for start_tx() to _guarantee_ tx has
> already started _before_ returning. Note the 8250 driver merely
> writes to IER (which could be buffered and bridged).
>

Thank you, now this is becoming clear to me.

>> And that could be not true, if the timer is used.
>
> It's true that using a timer will be more complex with more state
> to manage, but being unable to service interrupts with this cpu for
> milliseconds is unacceptable.
>
> Regards,
> Peter Hurley
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266623

Hi Matwey,

I noticed 3 other issues here; see below.

On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
> Implementation of software emulation of RS485 direction handling is based
> on omap-serial driver.  It is acts as the following. At transmission start,
> RTS is set (if required) and receiver is off (if required). At transmission
> stop, RTS is set (if required) and fifo is flushed.
> 
> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> ---
>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 52d82d2..a9291f7 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>  	pm_runtime_mark_last_busy(p->port.dev);
>  	pm_runtime_put_autosuspend(p->port.dev);
>  }
> +static void serial8250_stop_rx(struct uart_port *port);
> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
> +{
> +	if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
> +		return;
> +
> +	if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
> +		serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);

The SER_RS485_RTS_ON_SEND bit is supposed to be the logic level of RTS,
so RTS should be driven to either 0 or 1 here (not just to 1).

> +		if (p->port.rs485.delay_rts_before_send > 0)
> +			mdelay(p->port.rs485.delay_rts_before_send);
> +	}
> +	if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
> +		serial8250_stop_rx(&p->port);
> +}
> +static void serial8250_rs485_stop_tx(struct uart_8250_port *p)
> +{
> +	if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
> +		return;

Unlike omap-serial, the 8250 stop_tx() will trigger _before_ the transmitter
is drained. Some mechanism is required to defer until the transmitter
is empty.

> +	if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
> +		if (p->port.rs485.delay_rts_after_send > 0)
> +			mdelay(p->port.rs485.delay_rts_after_send);
> +		serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);

As with the SER_RS485_RTS_ON_SEND, RTS should be driven to either 0 or 1 here
as well (not just to 1).

Regards,
Peter Hurley

> +	}
> +	/*
> +	* Empty the RX FIFO, we are not interested in anything
> +	* received during the half-duplex transmission.
> +	*/
> +	if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
> +		serial8250_clear_fifos(p);
> +}
>  /*
>   * IER sleep support.  UARTs which have EFRs need the "extended
>   * capability" bit enabled.  Note that on XR16C850s, we need to
> @@ -1309,6 +1339,7 @@ static void serial8250_stop_tx(struct uart_port *port)
>  		up->acr |= UART_ACR_TXDIS;
>  		serial_icr_write(up, UART_ACR, up->acr);
>  	}
> +	serial8250_rs485_stop_tx(up);
>  	serial8250_rpm_put(up);
>  }
>  
> @@ -1317,6 +1348,7 @@ static void serial8250_start_tx(struct uart_port *port)
>  	struct uart_8250_port *up = up_to_u8250p(port);
>  
>  	serial8250_rpm_get_tx(up);
> +	serial8250_rs485_start_tx(up);
>  
>  	if (up->dma && !up->dma->tx_dma(up))
>  		return;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266627

2015-11-10 19:12 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> Hi Matwey,
>
> I noticed 3 other issues here; see below.
>
> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>> Implementation of software emulation of RS485 direction handling is based
>> on omap-serial driver.  It is acts as the following. At transmission start,
>> RTS is set (if required) and receiver is off (if required). At transmission
>> stop, RTS is set (if required) and fifo is flushed.
>>
>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>> ---
>>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>> index 52d82d2..a9291f7 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>>       pm_runtime_mark_last_busy(p->port.dev);
>>       pm_runtime_put_autosuspend(p->port.dev);
>>  }
>> +static void serial8250_stop_rx(struct uart_port *port);
>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>> +{
>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>> +             return;
>> +
>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>
> The SER_RS485_RTS_ON_SEND bit is supposed to be the logic level of RTS,
> so RTS should be driven to either 0 or 1 here (not just to 1).
>
>> +             if (p->port.rs485.delay_rts_before_send > 0)
>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>> +     }
>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>> +             serial8250_stop_rx(&p->port);
>> +}
>> +static void serial8250_rs485_stop_tx(struct uart_8250_port *p)
>> +{
>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>> +             return;
>
> Unlike omap-serial, the 8250 stop_tx() will trigger _before_ the transmitter
> is drained. Some mechanism is required to defer until the transmitter
> is empty.
>

In serial8250_stop_tx() I see the following:

        if (port->type == PORT_16C950) {
                up->acr |= UART_ACR_TXDIS;
                serial_icr_write(up, UART_ACR, up->acr);
        }

AFAIU this will disable transmission unconditionally, or not? What
will happen here with data in FIFO?

I think that parts of my rs485 stuff have to be moved to
serial8250_tx_chars where It will be simpler to handle things without
involving mdelays.

>> +     if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
>> +             if (p->port.rs485.delay_rts_after_send > 0)
>> +                     mdelay(p->port.rs485.delay_rts_after_send);
>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>
> As with the SER_RS485_RTS_ON_SEND, RTS should be driven to either 0 or 1 here
> as well (not just to 1).
>
> Regards,
> Peter Hurley
>
>> +     }
>> +     /*
>> +     * Empty the RX FIFO, we are not interested in anything
>> +     * received during the half-duplex transmission.
>> +     */
>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>> +             serial8250_clear_fifos(p);
>> +}
>>  /*
>>   * IER sleep support.  UARTs which have EFRs need the "extended
>>   * capability" bit enabled.  Note that on XR16C850s, we need to
>> @@ -1309,6 +1339,7 @@ static void serial8250_stop_tx(struct uart_port *port)
>>               up->acr |= UART_ACR_TXDIS;
>>               serial_icr_write(up, UART_ACR, up->acr);
>>       }
>> +     serial8250_rs485_stop_tx(up);
>>       serial8250_rpm_put(up);
>>  }
>>
>> @@ -1317,6 +1348,7 @@ static void serial8250_start_tx(struct uart_port *port)
>>       struct uart_8250_port *up = up_to_u8250p(port);
>>
>>       serial8250_rpm_get_tx(up);
>> +     serial8250_rs485_start_tx(up);
>>
>>       if (up->dma && !up->dma->tx_dma(up))
>>               return;
>>
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266644

On 11/10/2015 11:25 AM, Matwey V. Kornilov wrote:
> 2015-11-10 19:12 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>> Hi Matwey,
>>
>> I noticed 3 other issues here; see below.
>>
>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>> Implementation of software emulation of RS485 direction handling is based
>>> on omap-serial driver.  It is acts as the following. At transmission start,
>>> RTS is set (if required) and receiver is off (if required). At transmission
>>> stop, RTS is set (if required) and fifo is flushed.
>>>
>>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>>> ---
>>>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>>>  1 file changed, 32 insertions(+)
>>>
>>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>>> index 52d82d2..a9291f7 100644
>>> --- a/drivers/tty/serial/8250/8250_port.c
>>> +++ b/drivers/tty/serial/8250/8250_port.c
>>> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>>>       pm_runtime_mark_last_busy(p->port.dev);
>>>       pm_runtime_put_autosuspend(p->port.dev);
>>>  }
>>> +static void serial8250_stop_rx(struct uart_port *port);
>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>> +{
>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>> +             return;
>>> +
>>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>
>> The SER_RS485_RTS_ON_SEND bit is supposed to be the logic level of RTS,
>> so RTS should be driven to either 0 or 1 here (not just to 1).
>>
>>> +             if (p->port.rs485.delay_rts_before_send > 0)
>>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>>> +     }
>>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>>> +             serial8250_stop_rx(&p->port);
>>> +}
>>> +static void serial8250_rs485_stop_tx(struct uart_8250_port *p)
>>> +{
>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>> +             return;
>>
>> Unlike omap-serial, the 8250 stop_tx() will trigger _before_ the transmitter
>> is drained. Some mechanism is required to defer until the transmitter
>> is empty.
>>
> 
> In serial8250_stop_tx() I see the following:
> 
>         if (port->type == PORT_16C950) {
>                 up->acr |= UART_ACR_TXDIS;
>                 serial_icr_write(up, UART_ACR, up->acr);
>         }
> 
> AFAIU this will disable transmission unconditionally, or not?

Yes, the transmitter will be stopped.

> What will happen here with data in FIFO?

It is preserved in the transmitter and restarted if/when start_tx() is called.
The 950 fifo is 128 bytes, so waiting for it to drain when the serial core has
commanded stop is not ok. Some of the other chips have even larger fifos but
I don't have that hardware so can't really test disabling their transmitters
also.

Note this is why there is a distinction between
a. __stop_tx(), which is an internal helper for disabling the THRE interrupt
   when all the tx data has been written to the fifo, and
b. serial8250_stop_tx(), which is the stop() method for the 8250 driver.
   The serial core calls the stop_tx() method when,
   1. tty core stops the tty (software flow control, ioctl(TCXONC, TCOOFF), etc)
   2. system pm suspend
   3. sw-assisted CTS flow control (ie., CRTSCTS)

When the serial core calls the stop_tx(), the driver is expected to stop
promptly, not drain i/o.

Regards,
Peter Hurley

> I think that parts of my rs485 stuff have to be moved to
> serial8250_tx_chars where It will be simpler to handle things without
> involving mdelays.
> 
>>> +     if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
>>> +             if (p->port.rs485.delay_rts_after_send > 0)
>>> +                     mdelay(p->port.rs485.delay_rts_after_send);
>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>
>> As with the SER_RS485_RTS_ON_SEND, RTS should be driven to either 0 or 1 here
>> as well (not just to 1).
>>
>> Regards,
>> Peter Hurley
>>
>>> +     }
>>> +     /*
>>> +     * Empty the RX FIFO, we are not interested in anything
>>> +     * received during the half-duplex transmission.
>>> +     */
>>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>>> +             serial8250_clear_fifos(p);
>>> +}
>>>  /*
>>>   * IER sleep support.  UARTs which have EFRs need the "extended
>>>   * capability" bit enabled.  Note that on XR16C850s, we need to
>>> @@ -1309,6 +1339,7 @@ static void serial8250_stop_tx(struct uart_port *port)
>>>               up->acr |= UART_ACR_TXDIS;
>>>               serial_icr_write(up, UART_ACR, up->acr);
>>>       }
>>> +     serial8250_rs485_stop_tx(up);
>>>       serial8250_rpm_put(up);
>>>  }
>>>
>>> @@ -1317,6 +1348,7 @@ static void serial8250_start_tx(struct uart_port *port)
>>>       struct uart_8250_port *up = up_to_u8250p(port);
>>>
>>>       serial8250_rpm_get_tx(up);
>>> +     serial8250_rs485_start_tx(up);
>>>
>>>       if (up->dma && !up->dma->tx_dma(up))
>>>               return;
>>>
>>
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267886

2015-11-10 19:12 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> Hi Matwey,
>
> I noticed 3 other issues here; see below.
>
> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>> Implementation of software emulation of RS485 direction handling is based
>> on omap-serial driver.  It is acts as the following. At transmission start,
>> RTS is set (if required) and receiver is off (if required). At transmission
>> stop, RTS is set (if required) and fifo is flushed.
>>
>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>> ---
>>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>> index 52d82d2..a9291f7 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>>       pm_runtime_mark_last_busy(p->port.dev);
>>       pm_runtime_put_autosuspend(p->port.dev);
>>  }
>> +static void serial8250_stop_rx(struct uart_port *port);
>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>> +{
>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>> +             return;
>> +
>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>
> The SER_RS485_RTS_ON_SEND bit is supposed to be the logic level of RTS,
> so RTS should be driven to either 0 or 1 here (not just to 1).

By the way, I've found that p->mcr caches MCR inconsistently. Is it
supposed to be so? Or p->mcr is not for caching?

>
>> +             if (p->port.rs485.delay_rts_before_send > 0)
>> +                     mdelay(p->port.rs485.delay_rts_before_send);
>> +     }
>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>> +             serial8250_stop_rx(&p->port);
>> +}
>> +static void serial8250_rs485_stop_tx(struct uart_8250_port *p)
>> +{
>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>> +             return;
>
> Unlike omap-serial, the 8250 stop_tx() will trigger _before_ the transmitter
> is drained. Some mechanism is required to defer until the transmitter
> is empty.
>
>> +     if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) {
>> +             if (p->port.rs485.delay_rts_after_send > 0)
>> +                     mdelay(p->port.rs485.delay_rts_after_send);
>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>
> As with the SER_RS485_RTS_ON_SEND, RTS should be driven to either 0 or 1 here
> as well (not just to 1).
>
> Regards,
> Peter Hurley
>
>> +     }
>> +     /*
>> +     * Empty the RX FIFO, we are not interested in anything
>> +     * received during the half-duplex transmission.
>> +     */
>> +     if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX))
>> +             serial8250_clear_fifos(p);
>> +}
>>  /*
>>   * IER sleep support.  UARTs which have EFRs need the "extended
>>   * capability" bit enabled.  Note that on XR16C850s, we need to
>> @@ -1309,6 +1339,7 @@ static void serial8250_stop_tx(struct uart_port *port)
>>               up->acr |= UART_ACR_TXDIS;
>>               serial_icr_write(up, UART_ACR, up->acr);
>>       }
>> +     serial8250_rs485_stop_tx(up);
>>       serial8250_rpm_put(up);
>>  }
>>
>> @@ -1317,6 +1348,7 @@ static void serial8250_start_tx(struct uart_port *port)
>>       struct uart_8250_port *up = up_to_u8250p(port);
>>
>>       serial8250_rpm_get_tx(up);
>> +     serial8250_rs485_start_tx(up);
>>
>>       if (up->dma && !up->dma->tx_dma(up))
>>               return;
>>
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267926

On 11/12/2015 07:34 AM, Matwey V. Kornilov wrote:
> 2015-11-10 19:12 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>> Implementation of software emulation of RS485 direction handling is based
>>> on omap-serial driver.  It is acts as the following. At transmission start,
>>> RTS is set (if required) and receiver is off (if required). At transmission
>>> stop, RTS is set (if required) and fifo is flushed.
>>>
>>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>>> ---
>>>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>>>  1 file changed, 32 insertions(+)
>>>
>>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>>> index 52d82d2..a9291f7 100644
>>> --- a/drivers/tty/serial/8250/8250_port.c
>>> +++ b/drivers/tty/serial/8250/8250_port.c
>>> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>>>       pm_runtime_mark_last_busy(p->port.dev);
>>>       pm_runtime_put_autosuspend(p->port.dev);
>>>  }
>>> +static void serial8250_stop_rx(struct uart_port *port);
>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>> +{
>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>> +             return;
>>> +
>>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>
>> The SER_RS485_RTS_ON_SEND bit is supposed to be the logic level of RTS,
>> so RTS should be driven to either 0 or 1 here (not just to 1).
> 
> By the way, I've found that p->mcr caches MCR inconsistently. Is it
> supposed to be so? Or p->mcr is not for caching?

Not for caching; it's for modal settings (eg., AFE) that the 8250 port
driver needs to merge in when changing mctrl bits (DTR/RTS/etc).

The serial core caches the mctrl bits in uart_port->mctrl, but IMO
it would be simpler to always set/clear RTS here (rather than like
the omap-serial driver where it checks the gpio value first).

Is the assumption that userspace will not perform conflicting
operations, such as ioctl(TIOCMSET) or ioctl(TCSETSx), with RS485 enabled?

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1268688

2015-11-12 16:35 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> On 11/12/2015 07:34 AM, Matwey V. Kornilov wrote:
>> 2015-11-10 19:12 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>>> Implementation of software emulation of RS485 direction handling is based
>>>> on omap-serial driver.  It is acts as the following. At transmission start,
>>>> RTS is set (if required) and receiver is off (if required). At transmission
>>>> stop, RTS is set (if required) and fifo is flushed.
>>>>
>>>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>>>> ---
>>>>  drivers/tty/serial/8250/8250_port.c | 32 ++++++++++++++++++++++++++++++++
>>>>  1 file changed, 32 insertions(+)
>>>>
>>>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>>>> index 52d82d2..a9291f7 100644
>>>> --- a/drivers/tty/serial/8250/8250_port.c
>>>> +++ b/drivers/tty/serial/8250/8250_port.c
>>>> @@ -559,7 +559,37 @@ static void serial8250_rpm_put_tx(struct uart_8250_port *p)
>>>>       pm_runtime_mark_last_busy(p->port.dev);
>>>>       pm_runtime_put_autosuspend(p->port.dev);
>>>>  }
>>>> +static void serial8250_stop_rx(struct uart_port *port);
>>>> +static void serial8250_rs485_start_tx(struct uart_8250_port *p)
>>>> +{
>>>> +     if (p->capabilities & UART_CAP_HW485 || !(p->port.rs485.flags & SER_RS485_ENABLED))
>>>> +             return;
>>>> +
>>>> +     if (p->port.rs485.flags & SER_RS485_RTS_ON_SEND) {
>>>> +             serial_port_out(&p->port, UART_MCR, UART_MCR_RTS);
>>>
>>> The SER_RS485_RTS_ON_SEND bit is supposed to be the logic level of RTS,
>>> so RTS should be driven to either 0 or 1 here (not just to 1).
>>
>> By the way, I've found that p->mcr caches MCR inconsistently. Is it
>> supposed to be so? Or p->mcr is not for caching?
>
> Not for caching; it's for modal settings (eg., AFE) that the 8250 port
> driver needs to merge in when changing mctrl bits (DTR/RTS/etc).
>
> The serial core caches the mctrl bits in uart_port->mctrl, but IMO
> it would be simpler to always set/clear RTS here (rather than like
> the omap-serial driver where it checks the gpio value first).
>
> Is the assumption that userspace will not perform conflicting
> operations, such as ioctl(TIOCMSET) or ioctl(TCSETSx), with RS485 enabled?

I've sent v3 series and put all RTS related stuff into separate
functions. Could you please continue this discussion in "v3 5/5"?
TIOCMGET should work correctly now, because it reads register. I am
not sure that TIOCMSET should be limited somehow, user knows better
what he wants.

> Regards,
> Peter Hurley
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web