Messages in this thread | | | Date | Fri, 23 Jun 2023 13:18:49 -0500 | Subject | Re: [PATCH v4 tty-next 2/2] serial: 8250: add driver for NI UARTs | From | Brenda Streiff <> |
| |
Apologies for the delay in response. I have your other requested changes in a pending v5 patchset, but answering this question took some digging (and I got bounced between projects in the intervening time)
On 5/8/23 03:01, Ilpo Järvinen wrote: > On Fri, 5 May 2023, Brenda Streiff wrote:
>> +static const struct serial_rs485 ni16550_rs485_supported = { >> + .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND, >> + /* >> + * delay_rts_* and RX_DURING_TX are not supported. >> + * >> + * RTS_{ON,AFTER}_SEND are supported, but ignored; the transceiver >> + * is connected in only one way and we don't need userspace to tell >> + * us, but want to retain compatibility with applications that do. > > This is a bit odd statement. If application wants to specify those flags, > they will be ignored (cleared) by serial core without triggering an error > even when driver does not claim to support them. > > Or are you saying some application sets these flags and fails if it > doesn't get the same flags back?
Yes, this is the case.
There's an unfortunate amount of historical baggage on our side here.
The hardware supports four modes: Wire Mode | Tx enabled? | Rx enabled? -----------------------|--------------------|------------------------ NI16650_PCR_RS422 | Always | Always NI16650_PCR_ECHO_RS485 | If DTR asserted | Always NI16650_PCR_DTR_RS485 | If DTR asserted | Disabled when TX enabled NI16650_PCR_AUTO_RS485 | If data in TX FIFO | Disabled when TX enabled
These four hardware modes map directly to "wire mode" settings in higher level NI software APIs, such as NI-VISA's VI_ATTR_ASRL_WIRE_MODE property [1] on other operating systems, and our drivers for those OSes just present those settings directly in their ABI. Linux already had support for RS485, but when the driver was first authored for Linux [2] (and then kept out-of-tree for a decade), it was done so with an (erroneous) understanding of the RS485 flags:
Wire Mode struct serial_rs485::flags ----------------------- -------------------- NI16650_PCR_RS422: 0 NI16650_PCR_ECHO_RS485: SER_RS485_ENABLED|SER_RS485_RX_DURING_TX NI16650_PCR_DTR_RS485: SER_RS485_ENABLED NI16650_PCR_AUTO_RS485: SER_RS485_ENABLED|SER_RS485_RTS_ON_SEND
The "RS485 serial_rs485 sanitization" patchset [3] ended up breaking us and forced us to reevaluate it, because now some of those states were invalid (a driver cannot now simultaneously support not having RTS_ON_SEND at the same time as supporting having RTS_ON_SEND).
Ultimately we made the decision that we only care about the RS422 and AUTO_RS485 modes, but now we have a slightly different problem, in that there's a bunch of userspace middleware that is still coded to expect all four modes; so if we remove support for the SER_RS485_RTS_ON_SEND flag, even though we don't need it, we end up with: - middleware sets SER_RS485_ENABLED|SER_RS485_RTS_ON_SEND - kernel coerces this into SER_RS485_ENABLED - middleware reads back the value, sees the flags are different, and assumes that the mode isn't properly supported and errors.
So we need to "support" SER_RS485_RTS_ON_SEND, even though we don't require the presence or absence of the flag to direct any hardware configuration change, in order to avoid this coercion happening.
Ideally we would have a userspace more tolerant of the kernel now coercing flags differently than it used to, but unfortunately we're stuck dealing with it for a while.
If there's a better way you'd prefer this be handled, let me know. We're trying to walk the line between "the driver needs to present the ABI correctly to userspace" and "our userspace relied on it having been incorrectly implemented in the past".
[1] https://www.ni.com/docs/en-US/bundle/ni-visa/page/ni-visa/vi_attr_asrl_wire_mode.html [2] https://github.com/ni/linux/blob/nilrt_pub/14.0/3.10/drivers/tty/serial/8250/8250_ni16550.c#L96 [3] https://lore.kernel.org/all/20220606100433.13793-1-ilpo.jarvinen@linux.intel.com/
| |