Message-ID: <b35730df8288469fbaf67b5ceae4eece@dh-electronics.com>
Date:   Thu, 14 Dec 2023 13:41:47 +0000
From:   Christoph Niedermaier <cniedermaier@...electronics.com>
To:     Crescent CY Hsieh <crescentcy.hsieh@...a.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Lino Sanfilippo <LinoSanfilippo@....de>
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: 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.

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.

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.


Regards
Christoph

Powered by blists - more mailing lists