Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Mar 2022 14:19:39 +0200 (EET)
From:   =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
To:     Lukas Wunner <lukas@wunner.de>
cc:     linux-serial <linux-serial@vger.kernel.org>,
        Jiri Slaby <jirislaby@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Johan Hovold <johan@kernel.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Raymond Tan <raymond.tan@intel.com>,
        Heiko Stuebner <heiko@sntech.de>, Rob Herring <robh@kernel.org>
Subject: Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support
In-Reply-To: <c2607267-798b-d7a0-86f6-4a729c22911f@linux.intel.com>
Message-ID: <9c2d96c0-44cf-c84c-5ff7-34c74716a23b@linux.intel.com>
References: <20220302095606.14818-1-ilpo.jarvinen@linux.intel.com> <20220302095606.14818-2-ilpo.jarvinen@linux.intel.com> <20220306184857.GA19394@wunner.de> <c2607267-798b-d7a0-86f6-4a729c22911f@linux.intel.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-1618434843-1646828384=:1769"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-1618434843-1646828384=:1769
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 8BIT

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.

--8323329-1618434843-1646828384=:1769--