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: <5c140498-69e3-4187-8703-db0c41e7ca89@gmx.de>
Date:   Wed, 6 Dec 2023 16:42:53 +0100
From:   Lino Sanfilippo <LinoSanfilippo@....de>
To:     Christoph Niedermaier <cniedermaier@...electronics.com>,
        Lukas Wunner <lukas@...ner.de>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     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>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "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>,
        Crescent CY Hsieh <crescentcy.hsieh@...a.com>,
        Tomas Paukrt <tomaspaukrt@...il.cz>
Subject: Re: [PATCH 1/2] dt-bindings: serial: rs485: add rs485-mux-gpios
 binding


Hi,

On 27.11.23 13:14, Christoph Niedermaier wrote:
> From: Lino Sanfilippo [mailto:LinoSanfilippo@....de]
> Sent: Sunday, November 26, 2023 12:40 AM
>
> Hi,
>
>> On 22.11.23 at 15:53, Lukas Wunner wrote:
>>> On Mon, Nov 20, 2023 at 04:10:54PM +0100, Rasmus Villemoes wrote:
>>>> Some boards are capable of both rs232 and rs485, and control which
>>>> external terminals are active via a gpio-controlled mux. Allow
>>>> describing that gpio in DT so that the kernel can transparently handle
>>>> the proper setting when the uart is switched between rs232 and rs485
>>>> modes.
>>>
>>> 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.
>> I dont know if it is a good choice in the long term to handle both modes within
>> the RS485 configuration. It would be cleaner to have an own RS422 structure with
>> its own flags and properties. And until now the only flag that seems to make sense
>> for both RS422 and RS485 is AFAICS SER_RS485_TERMINATE_BUS.
>>
>> On the other hand the bus termination is at least a property that both modes have
>> in common. And handling RS422 in its own structure would require another ioctl
>> to set and get the the RS422 settings.
>>
>> But maybe there are more or better possibilities to handle RS4822 support. I would like to
>> hear other ideas.
>>
>>
>>
>>> I don't know whether that makes sense at all (I had thought RS-422 is
>>> the same as RS-485 with full-duplex, i.e. SER_RS485_ENABLED plus
>>> SER_RS485_RX_DURING_TX)
>
> With RS-485 full duplex, SER_RS485_RX_DURING_TX makes no sense to me.
> See below.
>
>>>
>>> But if that patch gets accepted, we'd have *three* different modes:
>>> RS-232, RS-485, RS-422.
>>
>> Actually we would have four (as Brenda already wrote,
>> see https://lore.kernel.org/all/c6ea912f-d5ab-4761-813d-3b6b6be141cb@ni.com/),
>> and with the propose SER_RS485_MODE_RS422 flag these modes would be used like
>>
>> 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 my point of view there are also two different modes for the RS-485 2-wire
> half-duplex bus depending on the flag SER_RS485_RX_DURING_TX.
> - SER_RS485_RX_DURING_TX is not set: The device doesn't see the bus during sending
>                                      (RX is off during sending).
> - SER_RS485_RX_DURING_TX is set:     The device see want is on bus during sending
>                                      (RX is also on during sending), so you can
>                                      see your transmission and also if another bus
>                                      device is transmitting at the same time.
>
> On RS-485 4-wire TX and RX are separated by wires. So the definition of
> SER_RS485_RX_DURING_TX above makes no sense, because you can receive all the time
> without worrying about TX. On the software side RS-485 4-wire full duplex it behaves
> like RS-232. So we don't need transceiver controlling by the RTS pin.
>> Basically for me the SER_RS485_ENABLED flag is to enable the RTS control for the
> transceiver. Maybe on software side we can distinguish between half and full duplex
> mode and whether RX is enabled during sending by the flag SER_RS485_RX_DURING_TX:
> RS-232:                          rs485->flags = 0
> RS-422 / RS-485 (4-wire):        rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_FULL_DUPLEX

How can we switch between RS485 (4-wire) and RS422 then? AFAIU they are not the same. And even
if a driver behaves the same in both modes it needs to know when to switch from one mode to the
other.

> RS-485 (2-wire NO RX_DURING_TX): rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX
> RS-485 (2-wire RX_DURING_TX):    rs485->flags = SER_RS485_ENABLED|SER_RS485_MODE_HALF_DUPLEX|SER_RS485_RX_DURING_TX

I think we can omit the SER_RS485_MODE_HALF_DUPLEX flag if we assume that a missing
SER_RS485_MODE_FULL_DUPLEX means half duplex (i.e. controlling the RTS line).

> SER_RS485_MODE_FULL_DUPLEX and SER_RS485_MODE_HALF_DUPLEX can be defined at the
> same bit. If SER_RS485_MODE_HALF_DUPLEX will be defined as 0 it breaks nothing.
> With SER_RS485_MODE_FULL_DUPLEX, the RTS pin does not need to be controlled
>
>>
>>>  A single GPIO seems insufficient to handle that.
>>
>> GPIOs for RS485 is another thing.
>>
>> I mean, currently we have a GPIO for RS485 termination (I introduced it with commit
>> 44b27aec9d9680875).
>> Christoph introduced support for a rx-during-tx GPIO (see commit 163f080eb717). Tomas
>> intends
>> to add a GPIO which enables RS485 if asserted
>> (see https://lore.kernel.org/all/3Za.ZZs%7D.ndXI8CMee4.1bN6eQ@seznam.cz/) and with Rasmus
>> patches
>> we are about to add a MUX-GPIO which is to be asserted if RS485 is enabled.
>>
>> I wonder where this will end and if we really have to support every possible GPIO
>> in the serial core.
>
> 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 stop 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.
>

Regards,
Lino