From: Tony Lindgren <tony@atomide.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>
Cc: "Andy Shevchenko" <andriy.shevchenko@intel.com>,
"Dhruva Gole" <d-gole@ti.com>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Johan Hovold" <johan@kernel.org>,
"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
"Vignesh Raghavendra" <vigneshr@ti.com>,
linux-omap@vger.kernel.org, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH 3/4] serial: 8250: omap: Fix life cycle issues for interrupt handlers
Date: Mon, 8 May 2023 11:20:13 +0300 [thread overview]
Message-ID: <20230508082014.23083-4-tony@atomide.com> (raw)
In-Reply-To: <20230508082014.23083-1-tony@atomide.com>
We have the struct uart_8250_port instance recycled on device rebind while
the struct omap8250_priv instance is not. For the console uart,
__tty_hangup() does not call tty->ops->hangup() as cons_filp stays open,
and uart shutdown won't get called. This means we have a stale
priv->wakeirq handler around after unbind, and port->irq is not freed on
unbind.
There's no need to claim the interrupts on startup. We can fix this and
simplify the driver a bit by claiming the interrupts in probe, and clearing
them on remove. For the device interrupt, we can use devm_request_irq().
To do this, we change omap8250_irq() to use struct omap8250_priv data
directly so we don't have to wait for the assigned port from
serial8250_register_8250_port().
We must also drop IRQF_SHARED to set IRQ_NOAUTOEN to avoid spurious
interrupts until the port has been registered. There's no need for
IRQF_SHARED for 8250_omap, the serial port interrupt lines are dedicated
lines.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/tty/serial/8250/8250_omap.c | 33 ++++++++++++++---------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -623,9 +623,9 @@ static int omap_8250_dma_handle_irq(struct uart_port *port);
static irqreturn_t omap8250_irq(int irq, void *dev_id)
{
- struct uart_port *port = dev_id;
- struct omap8250_priv *priv = port->private_data;
- struct uart_8250_port *up = up_to_u8250p(port);
+ struct omap8250_priv *priv = dev_id;
+ struct uart_8250_port *up = serial8250_get_port(priv->line);
+ struct uart_port *port = &up->port;
unsigned int iir, lsr;
int ret;
@@ -709,11 +709,6 @@ static int omap_8250_startup(struct uart_port *port)
}
}
- ret = request_irq(port->irq, omap8250_irq, IRQF_SHARED,
- dev_name(port->dev), port);
- if (ret < 0)
- goto err;
-
up->ier = UART_IER_RLSI | UART_IER_RDI;
serial_out(up, UART_IER, up->ier);
@@ -730,14 +725,11 @@ static int omap_8250_startup(struct uart_port *port)
if (up->dma && !(priv->habit & UART_HAS_EFR2))
up->dma->rx_dma(up);
+ enable_irq(up->port.irq);
+
pm_runtime_mark_last_busy(port->dev);
pm_runtime_put_autosuspend(port->dev);
return 0;
-err:
- pm_runtime_mark_last_busy(port->dev);
- pm_runtime_put_autosuspend(port->dev);
- dev_pm_clear_wake_irq(port->dev);
- return ret;
}
static void omap_8250_shutdown(struct uart_port *port)
@@ -757,6 +749,8 @@ static void omap_8250_shutdown(struct uart_port *port)
up->ier = 0;
serial_out(up, UART_IER, 0);
+ disable_irq_nosync(up->port.irq);
+ dev_pm_clear_wake_irq(port->dev);
if (up->dma)
serial8250_release_dma(up);
@@ -770,8 +764,6 @@ static void omap_8250_shutdown(struct uart_port *port)
pm_runtime_mark_last_busy(port->dev);
pm_runtime_put_autosuspend(port->dev);
- free_irq(port->irq, port);
- dev_pm_clear_wake_irq(port->dev);
}
static void omap_8250_throttle(struct uart_port *port)
@@ -1451,8 +1443,6 @@ static int omap8250_probe(struct platform_device *pdev)
&up.overrun_backoff_time_ms) != 0)
up.overrun_backoff_time_ms = 0;
- priv->wakeirq = irq_of_parse_and_map(np, 1);
-
pdata = of_device_get_match_data(&pdev->dev);
if (pdata)
priv->habit |= pdata->habit;
@@ -1530,6 +1520,15 @@ static int omap8250_probe(struct platform_device *pdev)
}
}
#endif
+
+ irq_set_status_flags(irq, IRQ_NOAUTOEN);
+ ret = devm_request_irq(&pdev->dev, irq, omap8250_irq, 0,
+ dev_name(&pdev->dev), priv);
+ if (ret < 0)
+ return ret;
+
+ priv->wakeirq = irq_of_parse_and_map(np, 1);
+
ret = serial8250_register_8250_port(&up);
if (ret < 0) {
dev_err(&pdev->dev, "unable to register 8250 port\n");
--
2.40.1
next prev parent reply other threads:[~2023-05-08 8:20 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-08 8:20 [PATCH 0/4] More console rebind changes for 8250_omap serial driver Tony Lindgren
2023-05-08 8:20 ` [PATCH 1/4] serial: 8250: omap: Fix freeing of resources on failed register Tony Lindgren
2023-05-08 9:55 ` Ilpo Järvinen
2023-05-08 11:08 ` Tony Lindgren
2023-05-08 8:20 ` [PATCH 2/4] serial: 8250: omap: Fix imprecise external abort for omap_8250_pm() Tony Lindgren
2023-05-08 8:20 ` Tony Lindgren [this message]
2023-05-08 8:20 ` [PATCH 4/4] serial: 8250: omap: Shut down on remove for console uart Tony Lindgren
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=20230508082014.23083-4-tony@atomide.com \
--to=tony@atomide.com \
--cc=andriy.shevchenko@intel.com \
--cc=bigeasy@linutronix.de \
--cc=d-gole@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jirislaby@kernel.org \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=vigneshr@ti.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.