linux-kernel - Re: [PATCH v4] tty: serial: Add RS422 flag to struct serial

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 16 Nov 2023 12:03:20 +0800
From:   Crescent CY Hsieh <crescentcy.hsieh@...a.com>
To:     Brenda Streiff <brenda.streiff@...com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
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/20231018091739.10125-7-crescentcy.hsieh@moxa.com/
> [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@ni.com/

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@kernel.org/
[3] https://lore.kernel.org/linux-serial/3332a86c-1a1d-4b78-bbfa-8ac3e2e642a1@kernel.org/
[4] https://lore.kernel.org/all/3fc18b13-fef0-439e-abf0-1fe4e46b224a@gmx.de/

---
Sincerely,
Crescent CY Hsieh