linux-kernel - Re: [PATCH] serial: Revert RS485 polarity change on UART open

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: <20220329100328.GA2090@wunner.de>
Date:   Tue, 29 Mar 2022 12:03:28 +0200
From:   Lukas Wunner <lukas@...ner.de>
To:     Matthias Schiffer <matthias.schiffer@...tq-group.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
        Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        Lino Sanfilippo <LinoSanfilippo@....de>
Subject: Re: [PATCH] serial: Revert RS485 polarity change on UART open

[cc += Ilpo, Lino]

On Tue, Mar 29, 2022 at 10:50:50AM +0200, Matthias Schiffer wrote:
> While the change of the RS485 polarity in
> commit d3b3404df318 ("serial: Fix incorrect rs485 polarity on uart open")
> might have made sense based on the original intention of the
> rs485-rts-active-low flag (*), this is not how it is implemented in
> various drivers:
[...]
> [(*) My understanding of the mentioned commit's description is that
> rs485-rts-active-low should have referred to the electical signal level
> of the RTS pin, rather than the logical RTS state as understood by the
> UART controller.]

Since RTS is often just a GPIO on a pin controller that's configured
to function as RTS, my expectation would be that the same rules apply
to RTS polarity as those that apply to *any* GPIO.

According to Documentation/devicetree/bindings/gpio/gpio.txt:

"A gpio-specifier should contain a flag indicating the GPIO polarity; active-
 high or active-low. If it does, the following best practices should be
 followed:
 The gpio-specifier's polarity flag should represent the physical level at the
                                                         ^^^^^^^^^^^^^^
 GPIO controller that achieves (or represents, for inputs) a logically asserted
 value at the device."

> At least the 8250 and the i.MX UART drivers interpret rs485-rts-active-low

Which 8250 driver are you referring to specifically?  When developing
d3b3404df318, I tested with 8250_bcm2835aux.c and amba-pl011.c.  Both
worked exactly the way they should.

If imx.c and others have historically interpreted rs485-rts-active-low
to mean that the physical level is "high" when active, then we could just
amend imx_uart_probe() such that after calling uart_get_rs485_mode(),
the SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND bits are
flipped.  Would that work for you?

I'll go through the drivers to check which ones are affected.  I'm sorry
that you're seeing breakage, it's surprising to me that these different
interpretations of rs485-rts-active-low exist.

Thanks,

Lukas