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

[Crescent CY Hsieh]

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/20231018091739.10125-7-crescentcy.hsieh@xxxxxxxx/
> [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/20231023210458.447779-3-brenda.streiff@xxxxxx/

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/92aed0d9-791f-4708-8a73-4c78457a710e@xxxxxxxxxx/
[3] https://lore.kernel.org/linux-serial/3332a86c-1a1d-4b78-bbfa-8ac3e2e642a1@xxxxxxxxxx/
[4] https://lore.kernel.org/all/3fc18b13-fef0-439e-abf0-1fe4e46b224a@xxxxxx/

---
Sincerely,
Crescent CY Hsieh