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

From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Lukas Wunner <lukas@wunner.de>,
	"open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linux Kernel Mailing List <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>
Subject: Re: [PATCH 1/7] serial: 8250_dwlib: RS485 HW half duplex support
Date: Mon, 7 Mar 2022 11:19:59 +0200 (EET)	[thread overview]
Message-ID: <ab82f6a-8d1b-8e89-4ea-77d1a55667d2@linux.intel.com> (raw)
In-Reply-To: <CAHp75Vdxa_p866t5B7zJ8nHS-v+tu3vLiW0=vaBznnyCGyve_g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1805 bytes --]

On Mon, 7 Mar 2022, Andy Shevchenko wrote:

> On Mon, Mar 7, 2022 at 12:00 AM Lukas Wunner <lukas@wunner.de> wrote:
> > On Wed, Mar 02, 2022 at 11:56:00AM +0200, Ilpo Järvinen wrote:
> 
> ...
> 
> > Does the DesignWare UART use dedicated DE and RE pins instead of
> > the RTS pin?  That would be quite unusual.
> 
> They are muxed with other UART pins on SoC level, but I don't remember
> by heart which ones. According to the Synopsys datasheet they are
> separate signals. It might be that I'm missing something, since the
> last time I looked was last year.

Unusual or not, there is a pin for both DE and RE. DE is muxed with RTS.

> > > +     d->hw_rs485_support = device_property_read_bool(p->dev, "snps,rs485-interface-en");
> > > +     if (d->hw_rs485_support)
> > > +             p->rs485_config = dw8250_rs485_config;
> > > +
> >
> > You wrote in the commit message that rs485 support is present from
> > version 4.0 onward.  Can't we just check the IP version and enable
> > rs485 support for >= 4.0?  That would seem more appropriate instead
> > of introducing yet another new property.
> 
> AFAIU this is dependent on the IP syntheses. I.o.w. version 4.0+ is a
> prerequisite, but doesn't automatically mean that there is a support.
> Unfortunately there is no way to tell this clearly in the IP
> configuration register.

And the IP synthesis only part of the picture, in general case, it'd
also matter that there's something connected to that RE (i.e.,
an RS485 transceiver).

On the board I'm testing with, I can also turn RS485 on/off from BIOS
which makes the pins (mainly RE) behave differently.

I initially had additional version check here while developing this
patch series but it seemed to not provide any added value due those
other factors that need to be considered.

-- 
 i.