linux-kernel - Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

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: <9c2d96c0-44cf-c84c-5ff7-34c74716a23b@linux.intel.com>
Date:   Wed, 9 Mar 2022 14:19:39 +0200 (EET)
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     Lukas Wunner <lukas@...ner.de>
cc:     linux-serial <linux-serial@...r.kernel.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Johan Hovold <johan@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Raymond Tan <raymond.tan@...el.com>,
        Heiko Stuebner <heiko@...ech.de>, Rob Herring <robh@...nel.org>
Subject: Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support

On Mon, 7 Mar 2022, Ilpo Järvinen wrote:

> On Sun, 6 Mar 2022, Lukas Wunner wrote:
> 
> > On Wed, Mar 02, 2022 at 11:56:00AM +0200, Ilpo Järvinen wrote:
> 
> > > +	/*
> > > +	 * XXX: Though we could interpret the "RTS" timings as Driver Enable
> > > +	 * (DE) assertion/de-assertion timings, initially not supporting that.
> > > +	 * Ideally we should have timing values for the Driver instead of the
> > > +	 * RTS signal.
> > > +	 */
> > > +	rs485->delay_rts_before_send = 0;
> > > +	rs485->delay_rts_after_send = 0;
> > 
> > I don't quite understand this code comment.
> 
> It seemed to be missing one "Enable" word.
> 
> Here's my interpretation of it (this comment was written by somebody 
> else, perhaps either Heikki or Andy):
> 
> Since this HW has dedicated DE/RE in contrast to RTS, it is not specified 
> anywhere that delay_rts_* should apply to them as well and that the 
> writer of that comment was hoping to have something dedicated to them
> rather than repurposing RTS-related fields.
> 
> I could of course change this if everything called RTS should be applied 
> to DE as well?

Now that it has been pretty much established that anything called "rts" 
should be applied to DE as well, I took another look on implementing these 
delays.

It turns out to be impractical to do/ineffective because "serial clock 
periods" are used as the unit by the HW ("serial clock periods" is not as 
clearly defined by the datasheet as I'd like but it is most likely based 
on the high-rated uartclk cycles). With the uartclk I've on test HW, the 
combined delay with max turnaround time and DE assert/de-assert timings 
cannot do even the smallest possible non-zero value (1 msec). That's 
because the TAT and DET registers allow only 16-bit and 8-bit values for 
delays.

I also attempted to test it by writing the maximum values into them and 
got no visible difference. There a note about +1 for delay in TAT so to 
play safe I put 0xfff0 instead 0xffff (if the HW couldn't handle that 
16-bit overflow properly).

Perhaps the SW half-duplex with DE/RE will have to be used to cover this 
case?

-- 
 i.