[PATCH v2 1/3] tty: Introduce UART_CAP_HW485

[PATCH v2 1/3] tty: Introduce UART_CAP_HW485

[Matwey V. Kornilov]

Introduce new capability UART_CAP_HW485 to mark 8250 UARTs which have
hardware support of line direction control.

Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
Changes since v1:
 - Commit message has been wrapped

 drivers/tty/serial/8250/8250.h         | 1 +
 drivers/tty/serial/8250/8250_fintek.c  | 1 +
 drivers/tty/serial/8250/8250_lpc18xx.c | 1 +
 drivers/tty/serial/8250/8250_pci.c     | 1 +
 4 files changed, 4 insertions(+)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index d54dcd8..92a4f47 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -69,6 +69,7 @@ struct serial8250_config {
 	unsigned int	flags;
 };
 
+#define UART_CAP_HW485	(1 << 7)	/* UART has hardware direction control for RS485 */
 #define UART_CAP_FIFO	(1 << 8)	/* UART has FIFO */
 #define UART_CAP_EFR	(1 << 9)	/* UART has EFR */
 #define UART_CAP_SLEEP	(1 << 10)	/* UART has IER sleep */
diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index 8947439..f2831a8 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -208,6 +208,7 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
 	uart.port.iobase = pnp_port_start(dev, 0);
 	uart.port.iotype = UPIO_PORT;
 	uart.port.rs485_config = fintek_8250_rs485_config;
+	uart.capabilities = UART_CAP_HW485;
 
 	uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
 	if (pnp_irq_flags(dev, 0) & IORESOURCE_IRQ_SHAREABLE)
diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index 99cd478..9d30276 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -175,6 +175,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
 	uart.port.private_data = data;
 	uart.port.rs485_config = lpc18xx_rs485_config;
 	uart.port.serial_out = lpc18xx_uart_serial_out;
+	uart.capabilities = UART_CAP_HW485;
 
 	uart.dma = &data->dma;
 	uart.dma->rxconf.src_maxburst = 1;
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 4097f3f..c7c0ae9 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1599,6 +1599,7 @@ static int pci_fintek_setup(struct serial_private *priv,
 	port->port.iotype = UPIO_PORT;
 	port->port.iobase = iobase;
 	port->port.rs485_config = pci_fintek_rs485_config;
+	port->capabilities = UART_CAP_HW485;
 
 	data = devm_kzalloc(&pdev->dev, sizeof(u8), GFP_KERNEL);
 	if (!data)
-- 
2.6.2

[PATCH v2 2/3] tty: Implement default fallback serial8250_rs485_config

[Matwey V. Kornilov]

When 8250 driver doesn't have its own hardware RS485 support and doesn't
want to override rs485_config callback, then default
serial8250_rs485_config is used. It just stores supplied by user-space
config.

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

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 3912646..8f292da 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -395,6 +395,12 @@ static void serial8250_release_rsa_resource(struct uart_8250_port *up)
 	}
 }
 #endif
+static int serial8250_rs485_config(struct uart_port *port,
+				struct serial_rs485 *rs485)
+{
+	port->rs485 = *rs485;
+	return 0;
+}
 
 static const struct uart_ops *base_ops;
 static struct uart_ops univ8250_port_ops;
@@ -990,6 +996,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		uart->port.rs485	= up->port.rs485;
 		uart->dma		= up->dma;
 
+		/* Use software RS485 support when hardware one is not available */
+		if (!(uart->capabilities & UART_CAP_HW485) && !uart->port.rs485_config)
+			uart->port.rs485_config = serial8250_rs485_config;
+
 		/* Take tx_loadsz from fifosize if it wasn't set separately */
 		if (uart->port.fifosize && !uart->tx_loadsz)
 			uart->tx_loadsz = uart->port.fifosize;
-- 
2.6.2

Re: [PATCH v2 2/3] tty: Implement default fallback serial8250_rs485_config

[Peter Hurley]

Hi Matwey,

On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
> When 8250 driver doesn't have its own hardware RS485 support and doesn't
> want to override rs485_config callback, then default
> serial8250_rs485_config is used. It just stores supplied by user-space
> config.
> 
> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> ---
>  drivers/tty/serial/8250/8250_core.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 3912646..8f292da 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -395,6 +395,12 @@ static void serial8250_release_rsa_resource(struct uart_8250_port *up)
>  	}
>  }
>  #endif
> +static int serial8250_rs485_config(struct uart_port *port,
> +				struct serial_rs485 *rs485)
> +{
> +	port->rs485 = *rs485;
> +	return 0;
> +}
>  
>  static const struct uart_ops *base_ops;
>  static struct uart_ops univ8250_port_ops;
> @@ -990,6 +996,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
>  		uart->port.rs485	= up->port.rs485;
>  		uart->dma		= up->dma;
>  
> +		/* Use software RS485 support when hardware one is not available */
> +		if (!(uart->capabilities & UART_CAP_HW485) && !uart->port.rs485_config)
> +			uart->port.rs485_config = serial8250_rs485_config;

To setup a default 8250 port function,
1. define the default 8250 port function in 8250/8250_port.c
2. add the port function to the dispatch table (serial8250_pops)
3. add a test for 8250 sub-driver override in serial8250_register_8250_port()
   Eg.,

		if (up->port.rs485_config)
			uart->port.rs485_config = up->port.rs485_config;

Regards,
Peter Hurley

>  		/* Take tx_loadsz from fifosize if it wasn't set separately */
>  		if (uart->port.fifosize && !uart->tx_loadsz)
>  			uart->tx_loadsz = uart->port.fifosize;
> 

Re: [PATCH v2 2/3] tty: Implement default fallback serial8250_rs485_config

[Matwey V. Kornilov]

2015-11-07 15:29 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> Hi Matwey,
>
> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>> When 8250 driver doesn't have its own hardware RS485 support and doesn't
>> want to override rs485_config callback, then default
>> serial8250_rs485_config is used. It just stores supplied by user-space
>> config.
>>
>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>> ---
>>  drivers/tty/serial/8250/8250_core.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>> index 3912646..8f292da 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -395,6 +395,12 @@ static void serial8250_release_rsa_resource(struct uart_8250_port *up)
>>       }
>>  }
>>  #endif
>> +static int serial8250_rs485_config(struct uart_port *port,
>> +                             struct serial_rs485 *rs485)
>> +{
>> +     port->rs485 = *rs485;
>> +     return 0;
>> +}
>>
>>  static const struct uart_ops *base_ops;
>>  static struct uart_ops univ8250_port_ops;
>> @@ -990,6 +996,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
>>               uart->port.rs485        = up->port.rs485;
>>               uart->dma               = up->dma;
>>
>> +             /* Use software RS485 support when hardware one is not available */
>> +             if (!(uart->capabilities & UART_CAP_HW485) && !uart->port.rs485_config)
>> +                     uart->port.rs485_config = serial8250_rs485_config;
>
> To setup a default 8250 port function,
> 1. define the default 8250 port function in 8250/8250_port.c
> 2. add the port function to the dispatch table (serial8250_pops)

rs485_config is not a member of struct uart_ops, should I add it there?

> 3. add a test for 8250 sub-driver override in serial8250_register_8250_port()
>    Eg.,
>
>                 if (up->port.rs485_config)
>                         uart->port.rs485_config = up->port.rs485_config;
>
> Regards,
> Peter Hurley
>
>>               /* Take tx_loadsz from fifosize if it wasn't set separately */
>>               if (uart->port.fifosize && !uart->tx_loadsz)
>>                       uart->tx_loadsz = uart->port.fifosize;
>>
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382

Re: [PATCH v2 2/3] tty: Implement default fallback serial8250_rs485_config

[Peter Hurley]

On 11/07/2015 08:51 AM, Matwey V. Kornilov wrote:
> 2015-11-07 15:29 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>> Hi Matwey,
>>
>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>> When 8250 driver doesn't have its own hardware RS485 support and doesn't
>>> want to override rs485_config callback, then default
>>> serial8250_rs485_config is used. It just stores supplied by user-space
>>> config.
>>>
>>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>>> ---
>>>  drivers/tty/serial/8250/8250_core.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>>> index 3912646..8f292da 100644
>>> --- a/drivers/tty/serial/8250/8250_core.c
>>> +++ b/drivers/tty/serial/8250/8250_core.c
>>> @@ -395,6 +395,12 @@ static void serial8250_release_rsa_resource(struct uart_8250_port *up)
>>>       }
>>>  }
>>>  #endif
>>> +static int serial8250_rs485_config(struct uart_port *port,
>>> +                             struct serial_rs485 *rs485)
>>> +{
>>> +     port->rs485 = *rs485;
>>> +     return 0;
>>> +}
>>>
>>>  static const struct uart_ops *base_ops;
>>>  static struct uart_ops univ8250_port_ops;
>>> @@ -990,6 +996,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
>>>               uart->port.rs485        = up->port.rs485;
>>>               uart->dma               = up->dma;
>>>
>>> +             /* Use software RS485 support when hardware one is not available */
>>> +             if (!(uart->capabilities & UART_CAP_HW485) && !uart->port.rs485_config)
>>> +                     uart->port.rs485_config = serial8250_rs485_config;
>>
>> To setup a default 8250 port function,
>> 1. define the default 8250 port function in 8250/8250_port.c
>> 2. add the port function to the dispatch table (serial8250_pops)
> 
> rs485_config is not a member of struct uart_ops, should I add it there?

Whoops, sorry! :/
For now, assign the default 8250 port function in serial8250_set_defaults().

Regards,
Peter Hurley

>> 3. add a test for 8250 sub-driver override in serial8250_register_8250_port()
>>    Eg.,
>>
>>                 if (up->port.rs485_config)
>>                         uart->port.rs485_config = up->port.rs485_config;
>>
>> Regards,
>> Peter Hurley
>>
>>>               /* Take tx_loadsz from fifosize if it wasn't set separately */
>>>               if (uart->port.fifosize && !uart->tx_loadsz)
>>>                       uart->tx_loadsz = uart->port.fifosize;
>>>
>>
> 
> 
> 

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

[Matwey V. Kornilov]

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

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

[Peter Hurley]

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;
> 

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

[Matwey V. Kornilov]

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

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

[Peter Hurley]

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;
>>>
>>
> 
> 
> 

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

[Matwey V. Kornilov]

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

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

[Peter Hurley]

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

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

[Matwey V. Kornilov]

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

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

[Peter Hurley]

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

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

[Matwey V. Kornilov]

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

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

[Peter Hurley]

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;
> 

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

[Matwey V. Kornilov]

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

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

[Peter Hurley]

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;
>>>
>>
> 
> 
> 

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

[Matwey V. Kornilov]

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

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

[Peter Hurley]

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

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

[Matwey V. Kornilov]

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

Re: [PATCH v2 1/3] tty: Introduce UART_CAP_HW485

[Peter Hurley]

On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
> Introduce new capability UART_CAP_HW485 to mark 8250 UARTs which have
> hardware support of line direction control.

Since capabilities are not exported to user-space, how will user-space
know if the port only provides emulated 485 (eg., where only HW485 is
acceptable)?

Regards,
Peter Hurley

> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
> ---
> Changes since v1:
>  - Commit message has been wrapped
> 
>  drivers/tty/serial/8250/8250.h         | 1 +
>  drivers/tty/serial/8250/8250_fintek.c  | 1 +
>  drivers/tty/serial/8250/8250_lpc18xx.c | 1 +
>  drivers/tty/serial/8250/8250_pci.c     | 1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index d54dcd8..92a4f47 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -69,6 +69,7 @@ struct serial8250_config {
>  	unsigned int	flags;
>  };
>  
> +#define UART_CAP_HW485	(1 << 7)	/* UART has hardware direction control for RS485 */
>  #define UART_CAP_FIFO	(1 << 8)	/* UART has FIFO */
>  #define UART_CAP_EFR	(1 << 9)	/* UART has EFR */
>  #define UART_CAP_SLEEP	(1 << 10)	/* UART has IER sleep */
> diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
> index 8947439..f2831a8 100644
> --- a/drivers/tty/serial/8250/8250_fintek.c
> +++ b/drivers/tty/serial/8250/8250_fintek.c
> @@ -208,6 +208,7 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
>  	uart.port.iobase = pnp_port_start(dev, 0);
>  	uart.port.iotype = UPIO_PORT;
>  	uart.port.rs485_config = fintek_8250_rs485_config;
> +	uart.capabilities = UART_CAP_HW485;
>  
>  	uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
>  	if (pnp_irq_flags(dev, 0) & IORESOURCE_IRQ_SHAREABLE)
> diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
> index 99cd478..9d30276 100644
> --- a/drivers/tty/serial/8250/8250_lpc18xx.c
> +++ b/drivers/tty/serial/8250/8250_lpc18xx.c
> @@ -175,6 +175,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
>  	uart.port.private_data = data;
>  	uart.port.rs485_config = lpc18xx_rs485_config;
>  	uart.port.serial_out = lpc18xx_uart_serial_out;
> +	uart.capabilities = UART_CAP_HW485;
>  
>  	uart.dma = &data->dma;
>  	uart.dma->rxconf.src_maxburst = 1;
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 4097f3f..c7c0ae9 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1599,6 +1599,7 @@ static int pci_fintek_setup(struct serial_private *priv,
>  	port->port.iotype = UPIO_PORT;
>  	port->port.iobase = iobase;
>  	port->port.rs485_config = pci_fintek_rs485_config;
> +	port->capabilities = UART_CAP_HW485;
>  
>  	data = devm_kzalloc(&pdev->dev, sizeof(u8), GFP_KERNEL);
>  	if (!data)
> 

Re: [PATCH v2 1/3] tty: Introduce UART_CAP_HW485

[Matwey V. Kornilov]

2015-11-07 15:22 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>> Introduce new capability UART_CAP_HW485 to mark 8250 UARTs which have
>> hardware support of line direction control.
>
> Since capabilities are not exported to user-space, how will user-space
> know if the port only provides emulated 485 (eg., where only HW485 is
> acceptable)?

I cannot imagine the case when user really has to care about
implementation details.
In both cases the user will be provided with the consistent API
(TIOCGRS485/TIOCSRS485 ioctls) resulting into the same behavior.
Could you please expand your question?

>
> Regards,
> Peter Hurley
>
>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>> ---
>> Changes since v1:
>>  - Commit message has been wrapped
>>
>>  drivers/tty/serial/8250/8250.h         | 1 +
>>  drivers/tty/serial/8250/8250_fintek.c  | 1 +
>>  drivers/tty/serial/8250/8250_lpc18xx.c | 1 +
>>  drivers/tty/serial/8250/8250_pci.c     | 1 +
>>  4 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
>> index d54dcd8..92a4f47 100644
>> --- a/drivers/tty/serial/8250/8250.h
>> +++ b/drivers/tty/serial/8250/8250.h
>> @@ -69,6 +69,7 @@ struct serial8250_config {
>>       unsigned int    flags;
>>  };
>>
>> +#define UART_CAP_HW485       (1 << 7)        /* UART has hardware direction control for RS485 */
>>  #define UART_CAP_FIFO        (1 << 8)        /* UART has FIFO */
>>  #define UART_CAP_EFR (1 << 9)        /* UART has EFR */
>>  #define UART_CAP_SLEEP       (1 << 10)       /* UART has IER sleep */
>> diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
>> index 8947439..f2831a8 100644
>> --- a/drivers/tty/serial/8250/8250_fintek.c
>> +++ b/drivers/tty/serial/8250/8250_fintek.c
>> @@ -208,6 +208,7 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
>>       uart.port.iobase = pnp_port_start(dev, 0);
>>       uart.port.iotype = UPIO_PORT;
>>       uart.port.rs485_config = fintek_8250_rs485_config;
>> +     uart.capabilities = UART_CAP_HW485;
>>
>>       uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
>>       if (pnp_irq_flags(dev, 0) & IORESOURCE_IRQ_SHAREABLE)
>> diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
>> index 99cd478..9d30276 100644
>> --- a/drivers/tty/serial/8250/8250_lpc18xx.c
>> +++ b/drivers/tty/serial/8250/8250_lpc18xx.c
>> @@ -175,6 +175,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
>>       uart.port.private_data = data;
>>       uart.port.rs485_config = lpc18xx_rs485_config;
>>       uart.port.serial_out = lpc18xx_uart_serial_out;
>> +     uart.capabilities = UART_CAP_HW485;
>>
>>       uart.dma = &data->dma;
>>       uart.dma->rxconf.src_maxburst = 1;
>> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
>> index 4097f3f..c7c0ae9 100644
>> --- a/drivers/tty/serial/8250/8250_pci.c
>> +++ b/drivers/tty/serial/8250/8250_pci.c
>> @@ -1599,6 +1599,7 @@ static int pci_fintek_setup(struct serial_private *priv,
>>       port->port.iotype = UPIO_PORT;
>>       port->port.iobase = iobase;
>>       port->port.rs485_config = pci_fintek_rs485_config;
>> +     port->capabilities = UART_CAP_HW485;
>>
>>       data = devm_kzalloc(&pdev->dev, sizeof(u8), GFP_KERNEL);
>>       if (!data)
>>
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382

Re: [PATCH v2 1/3] tty: Introduce UART_CAP_HW485

[Peter Hurley]

On 11/07/2015 07:39 AM, Matwey V. Kornilov wrote:
> 2015-11-07 15:22 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>> Introduce new capability UART_CAP_HW485 to mark 8250 UARTs which have
>>> hardware support of line direction control.
>>
>> Since capabilities are not exported to user-space, how will user-space
>> know if the port only provides emulated 485 (eg., where only HW485 is
>> acceptable)?
> 
> I cannot imagine the case when user really has to care about
> implementation details.
> In both cases the user will be provided with the consistent API
> (TIOCGRS485/TIOCSRS485 ioctls) resulting into the same behavior.
> Could you please expand your question?

Well, the entire serial core is a 'consistent' API but yet we
still report to user-space what the port type is. Same with lspci
and lsusb.

Regards,
Peter Hurley


>>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>>> ---
>>> Changes since v1:
>>>  - Commit message has been wrapped
>>>
>>>  drivers/tty/serial/8250/8250.h         | 1 +
>>>  drivers/tty/serial/8250/8250_fintek.c  | 1 +
>>>  drivers/tty/serial/8250/8250_lpc18xx.c | 1 +
>>>  drivers/tty/serial/8250/8250_pci.c     | 1 +
>>>  4 files changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
>>> index d54dcd8..92a4f47 100644
>>> --- a/drivers/tty/serial/8250/8250.h
>>> +++ b/drivers/tty/serial/8250/8250.h
>>> @@ -69,6 +69,7 @@ struct serial8250_config {
>>>       unsigned int    flags;
>>>  };
>>>
>>> +#define UART_CAP_HW485       (1 << 7)        /* UART has hardware direction control for RS485 */
>>>  #define UART_CAP_FIFO        (1 << 8)        /* UART has FIFO */
>>>  #define UART_CAP_EFR (1 << 9)        /* UART has EFR */
>>>  #define UART_CAP_SLEEP       (1 << 10)       /* UART has IER sleep */
>>> diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
>>> index 8947439..f2831a8 100644
>>> --- a/drivers/tty/serial/8250/8250_fintek.c
>>> +++ b/drivers/tty/serial/8250/8250_fintek.c
>>> @@ -208,6 +208,7 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
>>>       uart.port.iobase = pnp_port_start(dev, 0);
>>>       uart.port.iotype = UPIO_PORT;
>>>       uart.port.rs485_config = fintek_8250_rs485_config;
>>> +     uart.capabilities = UART_CAP_HW485;
>>>
>>>       uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
>>>       if (pnp_irq_flags(dev, 0) & IORESOURCE_IRQ_SHAREABLE)
>>> diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
>>> index 99cd478..9d30276 100644
>>> --- a/drivers/tty/serial/8250/8250_lpc18xx.c
>>> +++ b/drivers/tty/serial/8250/8250_lpc18xx.c
>>> @@ -175,6 +175,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
>>>       uart.port.private_data = data;
>>>       uart.port.rs485_config = lpc18xx_rs485_config;
>>>       uart.port.serial_out = lpc18xx_uart_serial_out;
>>> +     uart.capabilities = UART_CAP_HW485;
>>>
>>>       uart.dma = &data->dma;
>>>       uart.dma->rxconf.src_maxburst = 1;
>>> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
>>> index 4097f3f..c7c0ae9 100644
>>> --- a/drivers/tty/serial/8250/8250_pci.c
>>> +++ b/drivers/tty/serial/8250/8250_pci.c
>>> @@ -1599,6 +1599,7 @@ static int pci_fintek_setup(struct serial_private *priv,
>>>       port->port.iotype = UPIO_PORT;
>>>       port->port.iobase = iobase;
>>>       port->port.rs485_config = pci_fintek_rs485_config;
>>> +     port->capabilities = UART_CAP_HW485;
>>>
>>>       data = devm_kzalloc(&pdev->dev, sizeof(u8), GFP_KERNEL);
>>>       if (!data)
>>>
>>
> 
> 
> 

Re: [PATCH v2 1/3] tty: Introduce UART_CAP_HW485

[Matwey V. Kornilov]

2015-11-07 18:32 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
> On 11/07/2015 07:39 AM, Matwey V. Kornilov wrote:
>> 2015-11-07 15:22 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>>> Introduce new capability UART_CAP_HW485 to mark 8250 UARTs which have
>>>> hardware support of line direction control.
>>>
>>> Since capabilities are not exported to user-space, how will user-space
>>> know if the port only provides emulated 485 (eg., where only HW485 is
>>> acceptable)?
>>
>> I cannot imagine the case when user really has to care about
>> implementation details.
>> In both cases the user will be provided with the consistent API
>> (TIOCGRS485/TIOCSRS485 ioctls) resulting into the same behavior.
>> Could you please expand your question?
>
> Well, the entire serial core is a 'consistent' API but yet we
> still report to user-space what the port type is. Same with lspci
> and lsusb.

Yes, that is correct.
I mean if you want the knowledge about HW485 be provided to the
user-space via some ioctl, then I cannot understand what application
is going to do with it.
It has single one option: use provided RS485 if it needs RS485, it
cannot purchase and install other hardware in runtime.
But I think we could add new read-only flag to struct serial_rs485,
say SER_RS485_SOFTWARE then user will know that software emulation is
being used.
Would it be ok?

>
> Regards,
> Peter Hurley
>
>
>>>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>>>> ---
>>>> Changes since v1:
>>>>  - Commit message has been wrapped
>>>>
>>>>  drivers/tty/serial/8250/8250.h         | 1 +
>>>>  drivers/tty/serial/8250/8250_fintek.c  | 1 +
>>>>  drivers/tty/serial/8250/8250_lpc18xx.c | 1 +
>>>>  drivers/tty/serial/8250/8250_pci.c     | 1 +
>>>>  4 files changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
>>>> index d54dcd8..92a4f47 100644
>>>> --- a/drivers/tty/serial/8250/8250.h
>>>> +++ b/drivers/tty/serial/8250/8250.h
>>>> @@ -69,6 +69,7 @@ struct serial8250_config {
>>>>       unsigned int    flags;
>>>>  };
>>>>
>>>> +#define UART_CAP_HW485       (1 << 7)        /* UART has hardware direction control for RS485 */
>>>>  #define UART_CAP_FIFO        (1 << 8)        /* UART has FIFO */
>>>>  #define UART_CAP_EFR (1 << 9)        /* UART has EFR */
>>>>  #define UART_CAP_SLEEP       (1 << 10)       /* UART has IER sleep */
>>>> diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
>>>> index 8947439..f2831a8 100644
>>>> --- a/drivers/tty/serial/8250/8250_fintek.c
>>>> +++ b/drivers/tty/serial/8250/8250_fintek.c
>>>> @@ -208,6 +208,7 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
>>>>       uart.port.iobase = pnp_port_start(dev, 0);
>>>>       uart.port.iotype = UPIO_PORT;
>>>>       uart.port.rs485_config = fintek_8250_rs485_config;
>>>> +     uart.capabilities = UART_CAP_HW485;
>>>>
>>>>       uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
>>>>       if (pnp_irq_flags(dev, 0) & IORESOURCE_IRQ_SHAREABLE)
>>>> diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
>>>> index 99cd478..9d30276 100644
>>>> --- a/drivers/tty/serial/8250/8250_lpc18xx.c
>>>> +++ b/drivers/tty/serial/8250/8250_lpc18xx.c
>>>> @@ -175,6 +175,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
>>>>       uart.port.private_data = data;
>>>>       uart.port.rs485_config = lpc18xx_rs485_config;
>>>>       uart.port.serial_out = lpc18xx_uart_serial_out;
>>>> +     uart.capabilities = UART_CAP_HW485;
>>>>
>>>>       uart.dma = &data->dma;
>>>>       uart.dma->rxconf.src_maxburst = 1;
>>>> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
>>>> index 4097f3f..c7c0ae9 100644
>>>> --- a/drivers/tty/serial/8250/8250_pci.c
>>>> +++ b/drivers/tty/serial/8250/8250_pci.c
>>>> @@ -1599,6 +1599,7 @@ static int pci_fintek_setup(struct serial_private *priv,
>>>>       port->port.iotype = UPIO_PORT;
>>>>       port->port.iobase = iobase;
>>>>       port->port.rs485_config = pci_fintek_rs485_config;
>>>> +     port->capabilities = UART_CAP_HW485;
>>>>
>>>>       data = devm_kzalloc(&pdev->dev, sizeof(u8), GFP_KERNEL);
>>>>       if (!data)
>>>>
>>>
>>
>>
>>
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382

Re: [PATCH v2 1/3] tty: Introduce UART_CAP_HW485

[Peter Hurley]

On 11/07/2015 04:10 PM, Matwey V. Kornilov wrote:
> 2015-11-07 18:32 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>> On 11/07/2015 07:39 AM, Matwey V. Kornilov wrote:
>>> 2015-11-07 15:22 GMT+03:00 Peter Hurley <peter@hurleysoftware.com>:
>>>> On 11/07/2015 05:09 AM, Matwey V. Kornilov wrote:
>>>>> Introduce new capability UART_CAP_HW485 to mark 8250 UARTs which have
>>>>> hardware support of line direction control.
>>>>
>>>> Since capabilities are not exported to user-space, how will user-space
>>>> know if the port only provides emulated 485 (eg., where only HW485 is
>>>> acceptable)?
>>>
>>> I cannot imagine the case when user really has to care about
>>> implementation details.
>>> In both cases the user will be provided with the consistent API
>>> (TIOCGRS485/TIOCSRS485 ioctls) resulting into the same behavior.
>>> Could you please expand your question?
>>
>> Well, the entire serial core is a 'consistent' API but yet we
>> still report to user-space what the port type is. Same with lspci
>> and lsusb.
> 
> Yes, that is correct.
> I mean if you want the knowledge about HW485 be provided to the
> user-space via some ioctl, then I cannot understand what application
> is going to do with it.
> It has single one option: use provided RS485 if it needs RS485, it
> cannot purchase and install other hardware in runtime.
> But I think we could add new read-only flag to struct serial_rs485,
> say SER_RS485_SOFTWARE then user will know that software emulation is
> being used.
> Would it be ok?

Yes, something like that would be fine.

Regards,
Peter Hurley


>>>>> Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
>>>>> ---
>>>>> Changes since v1:
>>>>>  - Commit message has been wrapped
>>>>>
>>>>>  drivers/tty/serial/8250/8250.h         | 1 +
>>>>>  drivers/tty/serial/8250/8250_fintek.c  | 1 +
>>>>>  drivers/tty/serial/8250/8250_lpc18xx.c | 1 +
>>>>>  drivers/tty/serial/8250/8250_pci.c     | 1 +
>>>>>  4 files changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
>>>>> index d54dcd8..92a4f47 100644
>>>>> --- a/drivers/tty/serial/8250/8250.h
>>>>> +++ b/drivers/tty/serial/8250/8250.h
>>>>> @@ -69,6 +69,7 @@ struct serial8250_config {
>>>>>       unsigned int    flags;
>>>>>  };
>>>>>
>>>>> +#define UART_CAP_HW485       (1 << 7)        /* UART has hardware direction control for RS485 */
>>>>>  #define UART_CAP_FIFO        (1 << 8)        /* UART has FIFO */
>>>>>  #define UART_CAP_EFR (1 << 9)        /* UART has EFR */
>>>>>  #define UART_CAP_SLEEP       (1 << 10)       /* UART has IER sleep */
>>>>> diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
>>>>> index 8947439..f2831a8 100644
>>>>> --- a/drivers/tty/serial/8250/8250_fintek.c
>>>>> +++ b/drivers/tty/serial/8250/8250_fintek.c
>>>>> @@ -208,6 +208,7 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
>>>>>       uart.port.iobase = pnp_port_start(dev, 0);
>>>>>       uart.port.iotype = UPIO_PORT;
>>>>>       uart.port.rs485_config = fintek_8250_rs485_config;
>>>>> +     uart.capabilities = UART_CAP_HW485;
>>>>>
>>>>>       uart.port.flags |= UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
>>>>>       if (pnp_irq_flags(dev, 0) & IORESOURCE_IRQ_SHAREABLE)
>>>>> diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
>>>>> index 99cd478..9d30276 100644
>>>>> --- a/drivers/tty/serial/8250/8250_lpc18xx.c
>>>>> +++ b/drivers/tty/serial/8250/8250_lpc18xx.c
>>>>> @@ -175,6 +175,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
>>>>>       uart.port.private_data = data;
>>>>>       uart.port.rs485_config = lpc18xx_rs485_config;
>>>>>       uart.port.serial_out = lpc18xx_uart_serial_out;
>>>>> +     uart.capabilities = UART_CAP_HW485;
>>>>>
>>>>>       uart.dma = &data->dma;
>>>>>       uart.dma->rxconf.src_maxburst = 1;
>>>>> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
>>>>> index 4097f3f..c7c0ae9 100644
>>>>> --- a/drivers/tty/serial/8250/8250_pci.c
>>>>> +++ b/drivers/tty/serial/8250/8250_pci.c
>>>>> @@ -1599,6 +1599,7 @@ static int pci_fintek_setup(struct serial_private *priv,
>>>>>       port->port.iotype = UPIO_PORT;
>>>>>       port->port.iobase = iobase;
>>>>>       port->port.rs485_config = pci_fintek_rs485_config;
>>>>> +     port->capabilities = UART_CAP_HW485;
>>>>>
>>>>>       data = devm_kzalloc(&pdev->dev, sizeof(u8), GFP_KERNEL);
>>>>>       if (!data)