Linux-Serial Archive on lore.kernel.org
 help / color / Atom feed
From: "Heiko Stübner" <heiko@sntech.de>
To: Giulio Benetti <giulio.benetti@micronovasrl.com>
Cc: gregkh@linuxfoundation.org, jslaby@suse.com,
	andriy.shevchenko@linux.intel.com, matwey.kornilov@gmail.com,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	lukas@wunner.de, christoph.muellner@theobroma-systems.com
Subject: Re: [PATCH v2 4/7] serial: 8250: Handle implementations not having TEMT interrupt using em485
Date: Thu, 26 Mar 2020 01:05:17 +0100
Message-ID: <12195570.sTQbgxCmNy@diego> (raw)
In-Reply-To: <3a5df648-b054-3338-f7a4-4c01783eabf6@micronovasrl.com>

Hi Giulio,

Am Donnerstag, 26. März 2020, 00:47:38 CET schrieb Giulio Benetti:
> very cleaner way to handle TEMT as a capability!
> And I've found one thing...
> 
> Il 26/03/2020 00:14, Heiko Stuebner ha scritto:
> > From: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > 
> > Some 8250 ports have a TEMT interrupt but it's not a part of the 8250
> > standard, instead only available on some implementations.
> > 
> > The current em485 implementation does not work on ports without it.
> > The only chance to make it work is to loop-read on LSR register.
> > 
> > So add UART_CAP_TEMT to mark 8250 uarts having this interrupt,
> > update all current em485 users with that capability and make
> > the stop_tx function loop-read on uarts not having it.
> > 
> > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > [moved to use added UART_CAP_TEMT, use readx_poll_timeout]
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > ---
> >   drivers/tty/serial/8250/8250.h            |  1 +
> >   drivers/tty/serial/8250/8250_bcm2835aux.c |  2 +-
> >   drivers/tty/serial/8250/8250_of.c         |  2 ++
> >   drivers/tty/serial/8250/8250_omap.c       |  2 +-
> >   drivers/tty/serial/8250/8250_port.c       | 25 +++++++++++++++++++----
> >   5 files changed, 26 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> > index 52bb21205bb6..770eb00db497 100644
> > --- a/drivers/tty/serial/8250/8250.h
> > +++ b/drivers/tty/serial/8250/8250.h
> > @@ -82,6 +82,7 @@ struct serial8250_config {
> >   #define UART_CAP_MINI	(1 << 17)	/* Mini UART on BCM283X family lacks:
> >   					 * STOP PARITY EPAR SPAR WLEN5 WLEN6
> >   					 */
> > +#define UART_CAP_TEMT	(1 << 18)	/* UART has TEMT interrupt */
> >   
> >   #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
> >   #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
> > diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
> > index 12d03e678295..3881242424ca 100644
> > --- a/drivers/tty/serial/8250/8250_bcm2835aux.c
> > +++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
> > @@ -91,7 +91,7 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
> >   		return -ENOMEM;
> >   
> >   	/* initialize data */
> > -	up.capabilities = UART_CAP_FIFO | UART_CAP_MINI;
> > +	up.capabilities = UART_CAP_FIFO | UART_CAP_MINI | UART_CAP_TEMT;
> >   	up.port.dev = &pdev->dev;
> >   	up.port.regshift = 2;
> >   	up.port.type = PORT_16550;
> > diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
> > index 65e9045dafe6..841f6fcb2878 100644
> > --- a/drivers/tty/serial/8250/8250_of.c
> > +++ b/drivers/tty/serial/8250/8250_of.c
> > @@ -225,6 +225,8 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
> >   			&port8250.overrun_backoff_time_ms) != 0)
> >   		port8250.overrun_backoff_time_ms = 0;
> >   
> > +	port8250.capabilities |= UART_CAP_TEMT;
> > +
> 
> Shouldn't this be NOT UART_CAP_TEMT set by default? On all other
> vendor specific files you enable it, I think here you shouldn't enable
> it too by default. Right?

8250_of does use the em485 emulation - see of_platform_serial_setup()
So I did go by the lazy assumption that any 8250 driver using rs485
before my series always used the interrupt driver code path, so
implicitly required to have the TEMT interrupt.

Of course, you're right that with the 8250_of maybe not all variants
actually do have this interrupt, so falling back to the polling here might
be safer.


Heiko



  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25 23:14 [PATCH RFC v2 0/7] serial: 8250: Add rs485 emulation to 8250_dw Heiko Stuebner
2020-03-25 23:14 ` [PATCH DON'T APPLY v2 1/7] serial: Allow uart_get_rs485_mode() to return errno Heiko Stuebner
2020-03-25 23:14 ` [PATCH DON'T APPLY v2 2/7] dt-bindings: serial: Add binding for rs485 bus termination GPIO Heiko Stuebner
2020-03-25 23:14 ` [PATCH DON'T APPLY v2 3/7] serial: 8250: Support " Heiko Stuebner
2020-03-25 23:14 ` [PATCH v2 4/7] serial: 8250: Handle implementations not having TEMT interrupt using em485 Heiko Stuebner
2020-03-25 23:47   ` Giulio Benetti
2020-03-26  0:05     ` Heiko Stübner [this message]
2020-03-26  2:02       ` Giulio Benetti
2020-03-25 23:14 ` [PATCH v2 5/7] dt-bindings: serial: Add binding for rs485 receiver enable GPIO Heiko Stuebner
2020-03-25 23:14 ` [PATCH v2 6/7] serial: 8250: Support separate rs485 rx-enable GPIO Heiko Stuebner
2020-03-25 23:14 ` [PATCH v2 7/7] serial: 8250_dw: add em485 support Heiko Stuebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=12195570.sTQbgxCmNy@diego \
    --to=heiko@sntech.de \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=christoph.muellner@theobroma-systems.com \
    --cc=giulio.benetti@micronovasrl.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=matwey.kornilov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Serial Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-serial/0 linux-serial/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-serial linux-serial/ https://lore.kernel.org/linux-serial \
		linux-serial@vger.kernel.org
	public-inbox-index linux-serial

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-serial


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git