2023-11-16 04:03:47

by Crescent CY Hsieh

[permalink] [raw]
Subject: Re: [PATCH v4] tty: serial: Add RS422 flag to struct serial_rs485

On Tue, Nov 14, 2023 at 08:50:42PM -0600, Brenda Streiff wrote:
> If I compare this to your original patch set [1] for your hardware, then
> your proposed flag would be used in the following ways, correct?
>
> RS-232: rs485->flags = 0
> RS-422: rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_RS422
> RS-485 (2-wire half-duplex): rs485->flags = SER_RS485_ENABLED
> RS-485 (4-wire full-duplex): rs485->flags = SER_RS485_ENABLED|SER_RS485_RX_DURING_TX
>
> In iot2040_rs485_config in 8250_exar.c [2] we already seem to have:
> RS-232: rs485->flags = 0
> RS-422: rs485->flags = SER_RS485_ENABLED|SER_RS485_RX_DURING_TX
> RS-485 (2-wire half-duplex?): rs485->flags = SER_RS485_ENABLED
>
> This would seem to create an inconsistency in this API.

I've checked the patch series of iot2040_rs485_config() about RS422 [1],
it seems to be reasonable back then so no one dicussed about that.

> I've also been trying to get a driver for NI's serial hardware upstream [3]; we
> have "RS-485" products that can do both RS-422/RS-485, and also have use of
> functionality to toggle between the two modes, so-- whichever way this flag
> goes-- I'd like to be consistent with how other drivers do it.
>
> [1] https://lore.kernel.org/linux-serial/[email protected]/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/8250/8250_exar.c?h=v6.6#n459
> [3] https://lore.kernel.org/linux-serial/[email protected]/

Originally, I was trying to add a new flag "SER_RS422_ENABLED" to
represent RS422, but Jiri replied "Hopefully not" [2].

So I used an unused flag to represent RS422 as a workaround solution,
then Jiri realized what was I trying to do and recommeded to add RS422
flag [3].

Then, when I was adding a new flag for RS422, Lino suggested to see
RS422 as a mode of RS485 [4].

Anyway, IMO, it's more reasonable to add a flag for representing RS422,
as for the naming, structure or future extention, it would require more
discussion.

[1] https://lore.kernel.org/all/?q=IOT2040_UART_MODE_RS422
[2] https://lore.kernel.org/linux-serial/[email protected]/
[3] https://lore.kernel.org/linux-serial/[email protected]/
[4] https://lore.kernel.org/all/[email protected]/

---
Sincerely,
Crescent CY Hsieh