linux-kernel - RE: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios binding

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]
Message-ID: <cc59c5bb16574073ba8b2bf9bc59bc7c@dh-electronics.com>
Date:   Thu, 14 Dec 2023 14:50:36 +0000
From:   Christoph Niedermaier <cniedermaier@...electronics.com>
To:     Lino Sanfilippo <LinoSanfilippo@....de>,
        Crescent CY Hsieh <crescentcy.hsieh@...a.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC:     Lukas Wunner <lukas@...ner.de>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        "brenda.streiff@...com" <brenda.streiff@...com>,
        Tomas Paukrt <tomaspaukrt@...il.cz>
Subject: RE: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios
 binding

From: Lino Sanfilippo <LinoSanfilippo@....de>
Sent: Thursday, December 14, 2023 3:04 PM
> 
> Hi,
> 
> On 14.12.23 14:41, Christoph Niedermaier wrote:
>> From: Crescent CY Hsieh <crescentcy.hsieh@...a.com>
>> Sent: Thursday, December 14, 2023 11:25 AM
>>> On Mon, Dec 11, 2023 at 03:07:59PM +0200, Andy Shevchenko wrote:
>>>> On Sat, Dec 09, 2023 at 12:47:47PM +0100, Lino Sanfilippo wrote:
>>>>> On 06.12.23 16:42, Lino Sanfilippo wrote:
>>>>
>>>>>>>>> Crescent CY Hsieh (+cc) is in parallel trying to add an RS-422 mode bit
>>>>>>>>> to struct serial_rs485:
>>>>>>>>>
>>>>>>>>> https://lore.kernel.org/all/20231121095122.15948-1-crescentcy.hsieh@moxa.com/
>>>>>>>>>
>>>>>>>>
>>>>>>>> That new flag was suggested by me instead of using SER_RS422_ENABLED, which
>>>>>>>> would mostly be redundant to SER_RS485_ENABLED.
>>>>>
>>>>> A cleaner solution would probably be to not handle RS422 with the RS485 settings at
>>>>> all, but to introduce another set of ioctls to set and read it.
>>>>>
>>>>> An own RS422 structure like
>>>>>
>>>>> struct serial_rs422 {
>>>>>     __u32   flags;
>>>>> #define SER_RS422_ENABLED           (1 << 0)
>>>>> #define SER_RS422_TERMINATE_BUS             (1 << 1)
>>>>> };
>>>>>
>>>>>
>>>>> could be used as the parameter for these new ioctls.
>>>>>
>>>>> Any comments on this?
>>>>
>>>> I have (maybe not so constructive) a comment. Please, at all means try to not
>>>> extend the existing serial data structures, we have too many ones with too many
>>>> fields already. For user space, though, one may use unions and flags, but for
>>>> internal ones it might be better ways, I think.
>>>
>>> How about revising the name of 'TIOCSRS485' and 'serial_rs485' to a
>>> general one, and put RS422 and RS485 configuration flags into that
>>> structure?
>>>
>>> So that in userspace it could set RS422 or RS485 configurations using a
>>> single ioctl command and one structure.
>>>
>>> In this way, it won't be confused in userspace and won't add new data
>>> structure internally as well.
>>>
>>
>> I will summarize the current situation from my point of view, maybe it helps:
>>
>> RS-232:
>>   - Full Duplex Point-to-Point connection
>>   - No transceiver control with RTS
>>   - No termination
>>   - No extra struct in use
>>
>> RS-422:
>>   - Full Duplex Point-to-Point connection
>>   - No transceiver control with RTS needed
>>   - Termination possible
>>   - Extra struct serial_rs485 needed if termination is used
>>  => RS-422 can be used in RS-232 operation, but if a termination should be
>>     switchable the RS485 flag has to be enabled. But then also transceiver
>>     control will be enabled. Not a very satisfying situation.
>>
> 
> Thats why I vote for a  RS422 mode.
> 
>> RS-485 (2-wire) very common:
>>   - Half Duplex RS-485 bus
>>   - Transceiver control with RTS is needed
>>   - Termination possible
>>   - Extra struct serial_rs485 is needed
>>  => RS-485 has to be enabled and configured:
>>     - Set SER_RS485_ENABLED
>>     - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
>>     - Set/clear SER_RS485_RX_DURING_TX depending on whether
>>       the receiver path should be on or off during sending.
>>       If it's set it allows to monitor the sending on the bus
>>       and detect whether another bus device is transmitting
>>       at the same time.
>>     - Set/clear SER_RS485_TERMINATE_BUS for bus termination.
>>
>> RS-485 (4-wire) little used:
>>   - Full Duplex RS-485 bus
>>   - Transceiver control with RTS is needed
>>   - Termination possible
>>   - Extra struct serial_rs485 is needed
>>  => RS-485 has to be enabled and configured:
>>     - Set SER_RS485_ENABLED
>>     - Set SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND
>>     - Set SER_RS485_RX_DURING_TX, as the receiver should always
>>       be enabled independently of TX, because TX and RX are
>>       separated from each other by their own wires.
>>     - Set/clear SER_RS485_TERMINATE_BUS for bus termination.
> 
> How can the driver distinguish between RS485 full duplex and half duplex then?
> In full duplex RTS control is not needed AFAIU.

I think we don't need to distinguish, because for a full duplex RS-485
transceiver also needs RTS control. For example look at the full duplex
RS-485 transceiver ADM3491E [1]. It's a full duplex transceiver (A/B and Z/Y)
that has DE (Driver enable) and DI (Driver Input) pins for controlling TX. I
think the RS-485 master doesn't need it. The DE pin could also be set
permanently high. But if we have more than one RS-485 slaves it's needed to
avoid blocking of each other on the receiving wires of the RS-485 master.

[1] https://www.analog.com/en/products/adm3491e.html

>> I think the GPIOs reflect the flag states and are meaningful:
>> - SER_RS485_TERMINATE_BUS: Switch bus termination on/off by GPIO
>> - SER_RS485_RX_DURING_TX:  Used to enable/disable RX during TX
>>                            in hardware by GPIO (for 2-wire)
>> - SER_RS485_ENABLED:       Muxing between RS-232 and RS-485 by GPIO
>>
>> Switching RS-485 on during boot could also be handled by a devicetree
>> overlay. Evaluate the GPIO and load a DTO accordingly before booting.
>>
>> Please correct me if I have misrepresented something...
>>
>> If I looked at it in this new way, I would discard my idea with the
>> FULL_DUPLEX and HALF_DUPLEX. For a better use of RS-422 it would be
>> good to disable transceiver control via RTS. It can be done by clearing
>> the existing flags SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND
>> at the same time, but I think it is confusing. Better would be a flag
>> for RS-422:
>>
>> RS-422:                     Set SER_RS422_MODE for disabling
>>                             transceiver control via RTS.
>> RS-485 (2-wire and 4-wire): Clear SER_RS422_MODE for enabling
>>                             transceiver control via RTS.
>>
>> Finally, at present it is also not possible to distinguish between RS485
>> 2-wire and 4-wire operation. I think it isn't necessary, as different
>> hardware has to be used anyway. The hardware then determines the
>> configuration, see above.
> 
> But the driver should somehow be informed that there exists a full
> duplex hardware setup, so that it does not need to control the RTS line.
> Maybe by means of a device tree property?

See above.

Regards
Christoph