Skip to content

Commit

Permalink
serial: 8250: omap: Fix imprecise external abort for omap_8250_pm()
Browse files Browse the repository at this point in the history 
We must idle the uart only after serial8250_unregister_port(). Otherwise
unbinding the uart via sysfs while doing cat on the port produces an
imprecise external abort:

mem_serial_in from omap_8250_pm+0x44/0xf4
omap_8250_pm from uart_hangup+0xe0/0x194
uart_hangup from __tty_hangup.part.0+0x37c/0x3a8
__tty_hangup.part.0 from uart_remove_one_port+0x9c/0x22c
uart_remove_one_port from serial8250_unregister_port+0x60/0xe8
serial8250_unregister_port from omap8250_remove+0x6c/0xd0
omap8250_remove from platform_remove+0x28/0x54

Turns out the driver needs to have runtime PM functional before the
driver probe calls serial8250_register_8250_port(). And it needs
runtime PM after driver remove calls serial8250_unregister_port().

On probe, we need to read registers before registering the port in
omap_serial_fill_features_erratas(). We do that with custom uart_read()
already.

On remove, after serial8250_unregister_port(), we need to write to the
uart registers to idle the device. Let's add a custom uart_write() for
that.

Currently the uart register access depends on port->membase to be
initialized, which won't work after serial8250_unregister_port().
Let's use priv->membase instead, and use it for runtime PM related
functions to remove the dependency to port->membase for early and
late register access.

Note that during use, we need to check for a valid port in the runtime PM
related functions. This is needed for the optional wakeup configuration.
We now need to set the drvdata a bit earlier so it's available for the
runtime PM functions.

With the port checks in runtime PM functions, the old checks for priv in
omap8250_runtime_suspend() and omap8250_runtime_resume() functions are no
longer needed and are removed.

Signed-off-by: Tony Lindgren <tony@atomide.com>
Link: https://lore.kernel.org/r/20230508082014.23083-3-tony@atomide.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Tony Lindgren authored and Greg Kroah-Hartman committed May 13, 2023
1 parent b9ab22c commit 398cecc
Showing 1 changed file with 38 additions and 32 deletions.
70 changes: 38 additions & 32 deletions drivers/tty/serial/8250/8250_omap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "8250.h"

#define DEFAULT_CLK_SPEED 48000000
#define OMAP_UART_REGSHIFT 2

#define UART_ERRATA_i202_MDR1_ACCESS (1 << 0)
#define OMAP_UART_WER_HAS_TX_WAKEUP (1 << 1)
Expand Down Expand Up @@ -115,6 +116,7 @@
#define UART_OMAP_RX_LVL 0x19

struct omap8250_priv {
void __iomem *membase;
int line;
u8 habit;
u8 mdr1;
Expand Down Expand Up @@ -159,9 +161,14 @@ static void omap_8250_rx_dma_flush(struct uart_8250_port *p);
static inline void omap_8250_rx_dma_flush(struct uart_8250_port *p) { }
#endif

static u32 uart_read(struct uart_8250_port *up, u32 reg)
static u32 uart_read(struct omap8250_priv *priv, u32 reg)
{
return readl(up->port.membase + (reg << up->port.regshift));
return readl(priv->membase + (reg << OMAP_UART_REGSHIFT));
}

static void uart_write(struct omap8250_priv *priv, u32 reg, u32 val)
{
writel(val, priv->membase + (reg << OMAP_UART_REGSHIFT));
}

/*
Expand Down Expand Up @@ -548,7 +555,7 @@ static void omap_serial_fill_features_erratas(struct uart_8250_port *up,
u32 mvr, scheme;
u16 revision, major, minor;

mvr = uart_read(up, UART_OMAP_MVER);
mvr = uart_read(priv, UART_OMAP_MVER);

/* Check revision register scheme */
scheme = mvr >> OMAP_UART_MVR_SCHEME_SHIFT;
Expand Down Expand Up @@ -1394,7 +1401,7 @@ static int omap8250_probe(struct platform_device *pdev)
UPF_HARD_FLOW;
up.port.private_data = priv;

up.port.regshift = 2;
up.port.regshift = OMAP_UART_REGSHIFT;
up.port.fifosize = 64;
up.tx_loadsz = 64;
up.capabilities = UART_CAP_FIFO;
Expand Down Expand Up @@ -1457,13 +1464,17 @@ static int omap8250_probe(struct platform_device *pdev)
DEFAULT_CLK_SPEED);
}

priv->membase = membase;
priv->line = -ENODEV;
priv->latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
priv->calc_latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
cpu_latency_qos_add_request(&priv->pm_qos_request, priv->latency);
INIT_WORK(&priv->qos_work, omap8250_uart_qos_work);

spin_lock_init(&priv->rx_dma_lock);

platform_set_drvdata(pdev, priv);

device_init_wakeup(&pdev->dev, true);
pm_runtime_enable(&pdev->dev);
pm_runtime_use_autosuspend(&pdev->dev);
Expand Down Expand Up @@ -1525,7 +1536,6 @@ static int omap8250_probe(struct platform_device *pdev)
goto err;
}
priv->line = ret;
platform_set_drvdata(pdev, priv);
pm_runtime_mark_last_busy(&pdev->dev);
pm_runtime_put_autosuspend(&pdev->dev);
return 0;
Expand All @@ -1547,11 +1557,12 @@ static int omap8250_remove(struct platform_device *pdev)
if (err)
return err;

serial8250_unregister_port(priv->line);
priv->line = -ENODEV;
pm_runtime_dont_use_autosuspend(&pdev->dev);
pm_runtime_put_sync(&pdev->dev);
flush_work(&priv->qos_work);
pm_runtime_disable(&pdev->dev);
serial8250_unregister_port(priv->line);
cpu_latency_qos_remove_request(&priv->pm_qos_request);
device_init_wakeup(&pdev->dev, false);
return 0;
Expand Down Expand Up @@ -1627,7 +1638,6 @@ static int omap8250_lost_context(struct uart_8250_port *up)
static int omap8250_soft_reset(struct device *dev)
{
struct omap8250_priv *priv = dev_get_drvdata(dev);
struct uart_8250_port *up = serial8250_get_port(priv->line);
int timeout = 100;
int sysc;
int syss;
Expand All @@ -1641,20 +1651,20 @@ static int omap8250_soft_reset(struct device *dev)
* needing omap8250_soft_reset() quirk. Do it in two writes as
* recommended in the comment for omap8250_update_scr().
*/
serial_out(up, UART_OMAP_SCR, OMAP_UART_SCR_DMAMODE_1);
serial_out(up, UART_OMAP_SCR,
uart_write(priv, UART_OMAP_SCR, OMAP_UART_SCR_DMAMODE_1);
uart_write(priv, UART_OMAP_SCR,
OMAP_UART_SCR_DMAMODE_1 | OMAP_UART_SCR_DMAMODE_CTL);

sysc = serial_in(up, UART_OMAP_SYSC);
sysc = uart_read(priv, UART_OMAP_SYSC);

/* softreset the UART */
sysc |= OMAP_UART_SYSC_SOFTRESET;
serial_out(up, UART_OMAP_SYSC, sysc);
uart_write(priv, UART_OMAP_SYSC, sysc);

/* By experiments, 1us enough for reset complete on AM335x */
do {
udelay(1);
syss = serial_in(up, UART_OMAP_SYSS);
syss = uart_read(priv, UART_OMAP_SYSS);
} while (--timeout && !(syss & OMAP_UART_SYSS_RESETDONE));

if (!timeout) {
Expand All @@ -1668,21 +1678,18 @@ static int omap8250_soft_reset(struct device *dev)
static int omap8250_runtime_suspend(struct device *dev)
{
struct omap8250_priv *priv = dev_get_drvdata(dev);
struct uart_8250_port *up;

/* In case runtime-pm tries this before we are setup */
if (!priv)
return 0;
struct uart_8250_port *up = NULL;

up = serial8250_get_port(priv->line);
if (priv->line >= 0)
up = serial8250_get_port(priv->line);
/*
* When using 'no_console_suspend', the console UART must not be
* suspended. Since driver suspend is managed by runtime suspend,
* preventing runtime suspend (by returning error) will keep device
* active during suspend.
*/
if (priv->is_suspending && !console_suspend_enabled) {
if (uart_console(&up->port))
if (up && uart_console(&up->port))
return -EBUSY;
}

Expand All @@ -1693,13 +1700,15 @@ static int omap8250_runtime_suspend(struct device *dev)
if (ret)
return ret;

/* Restore to UART mode after reset (for wakeup) */
omap8250_update_mdr1(up, priv);
/* Restore wakeup enable register */
serial_out(up, UART_OMAP_WER, priv->wer);
if (up) {
/* Restore to UART mode after reset (for wakeup) */
omap8250_update_mdr1(up, priv);
/* Restore wakeup enable register */
serial_out(up, UART_OMAP_WER, priv->wer);
}
}

if (up->dma && up->dma->rxchan)
if (up && up->dma && up->dma->rxchan)
omap_8250_rx_dma_flush(up);

priv->latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
Expand All @@ -1711,18 +1720,15 @@ static int omap8250_runtime_suspend(struct device *dev)
static int omap8250_runtime_resume(struct device *dev)
{
struct omap8250_priv *priv = dev_get_drvdata(dev);
struct uart_8250_port *up;

/* In case runtime-pm tries this before we are setup */
if (!priv)
return 0;
struct uart_8250_port *up = NULL;

up = serial8250_get_port(priv->line);
if (priv->line >= 0)
up = serial8250_get_port(priv->line);

if (omap8250_lost_context(up))
if (up && omap8250_lost_context(up))
omap8250_restore_regs(up);

if (up->dma && up->dma->rxchan && !(priv->habit & UART_HAS_EFR2))
if (up && up->dma && up->dma->rxchan && !(priv->habit & UART_HAS_EFR2))
omap_8250_rx_dma(up);

priv->latency = priv->calc_latency;
Expand Down

0 comments on commit 398cecc

Please sign in to comment.