From cf9aa72d2f91defea23fe29b9e8e941bb2486d5c Mon Sep 17 00:00:00 2001 From: Shenwei Wang Date: Wed, 10 May 2023 15:33:59 -0500 Subject: tty: serial: fsl_lpuart: optimize the timer based EOP logic DMA transfer may end prematurely due to the DMA Rx timeout even during an active transfer because a constant timeout does not accurately simulate an EOP (End Of Package) event. This patch uses a timer to simulate the hardware EOP event. The timer should only complete a DMA transfer once the idle period satisfies a specified interval which is baud rate dependent. The problem has been observed with low baud rates but could occur also with high baud rates. Make the DMA Rx timeout baud rate dependent and check the DMA residue count before copying data to the TTY buffer. If the residue count remains unchanged since the last interrupt, that indicates no new data was received. In this case, the DMA should complete as an EOP event. Otherwise, new data was received during the interval and the EOP condition is not met so restart the DMA Rx timeout Signed-off-by: Shenwei Wang Reviewed-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230510203359.1353469-1-shenwei.wang@nxp.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/fsl_lpuart.c | 50 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index c91916e13648..0e56fa64b4ce 100644 --- a/drivers/tty/serial/fsl_lpuart.c +++ b/drivers/tty/serial/fsl_lpuart.c @@ -238,6 +238,7 @@ /* Rx DMA timeout in ms, which is used to calculate Rx ring buffer size */ #define DMA_RX_TIMEOUT (10) +#define DMA_RX_IDLE_CHARS 8 #define UART_AUTOSUSPEND_TIMEOUT 3000 #define DRIVER_NAME "fsl-lpuart" @@ -282,6 +283,7 @@ struct lpuart_port { struct scatterlist rx_sgl, tx_sgl[2]; struct circ_buf rx_ring; int rx_dma_rng_buf_len; + int last_residue; unsigned int dma_tx_nents; wait_queue_head_t dma_wait; bool is_cs7; /* Set to true when character size is 7 */ @@ -331,7 +333,7 @@ static struct lpuart_soc_data imx8qxp_data = { .devtype = IMX8QXP_LPUART, .iotype = UPIO_MEM32, .reg_off = IMX_REG_OFF, - .rx_watermark = 31, + .rx_watermark = 7, /* A lower watermark is ideal for low baud rates. */ }; static struct lpuart_soc_data imxrt1050_data = { .devtype = IMXRT1050_LPUART, @@ -1255,6 +1257,8 @@ static void lpuart_copy_rx_to_tty(struct lpuart_port *sport) sport->port.icount.rx += copied; } + sport->last_residue = state.residue; + exit: dma_sync_sg_for_device(chan->device->dev, &sport->rx_sgl, 1, DMA_FROM_DEVICE); @@ -1272,11 +1276,37 @@ static void lpuart_dma_rx_complete(void *arg) lpuart_copy_rx_to_tty(sport); } +/* + * Timer function to simulate the hardware EOP (End Of Package) event. + * The timer callback is to check for new RX data and copy to TTY buffer. + * If no new data are received since last interval, the EOP condition is + * met, complete the DMA transfer by copying the data. Otherwise, just + * restart timer. + */ static void lpuart_timer_func(struct timer_list *t) { struct lpuart_port *sport = from_timer(sport, t, lpuart_timer); + struct dma_chan *chan = sport->dma_rx_chan; + struct circ_buf *ring = &sport->rx_ring; + struct dma_tx_state state; + unsigned long flags; + int count; - lpuart_copy_rx_to_tty(sport); + dmaengine_tx_status(chan, sport->dma_rx_cookie, &state); + ring->head = sport->rx_sgl.length - state.residue; + count = CIRC_CNT(ring->head, ring->tail, sport->rx_sgl.length); + + /* Check if new data received before copying */ + if ((count != 0) && (sport->last_residue == state.residue)) + lpuart_copy_rx_to_tty(sport); + else + mod_timer(&sport->lpuart_timer, + jiffies + sport->dma_rx_timeout); + + if (spin_trylock_irqsave(&sport->port.lock, flags)) { + sport->last_residue = state.residue; + spin_unlock_irqrestore(&sport->port.lock, flags); + } } static inline int lpuart_start_rx_dma(struct lpuart_port *sport) @@ -1297,9 +1327,20 @@ static inline int lpuart_start_rx_dma(struct lpuart_port *sport) */ sport->rx_dma_rng_buf_len = (DMA_RX_TIMEOUT * baud / bits / 1000) * 2; sport->rx_dma_rng_buf_len = (1 << fls(sport->rx_dma_rng_buf_len)); + sport->rx_dma_rng_buf_len = max_t(int, + sport->rxfifo_size * 2, + sport->rx_dma_rng_buf_len); + /* + * Keep this condition check in case rxfifo_size is unavailable + * for some SoCs. + */ if (sport->rx_dma_rng_buf_len < 16) sport->rx_dma_rng_buf_len = 16; + sport->last_residue = 0; + sport->dma_rx_timeout = max(nsecs_to_jiffies( + sport->port.frame_time * DMA_RX_IDLE_CHARS), 1UL); + ring->buf = kzalloc(sport->rx_dma_rng_buf_len, GFP_ATOMIC); if (!ring->buf) return -ENOMEM; @@ -1687,12 +1728,13 @@ static void lpuart_rx_dma_startup(struct lpuart_port *sport) if (!sport->dma_rx_chan) goto err; + /* set default Rx DMA timeout */ + sport->dma_rx_timeout = msecs_to_jiffies(DMA_RX_TIMEOUT); + ret = lpuart_start_rx_dma(sport); if (ret) goto err; - /* set Rx DMA timeout */ - sport->dma_rx_timeout = msecs_to_jiffies(DMA_RX_TIMEOUT); if (!sport->dma_rx_timeout) sport->dma_rx_timeout = 1; -- cgit v1.2.3 From 6d8c1fca0a6ee4915781ded60000b3cc2c076e29 Mon Sep 17 00:00:00 2001 From: Hongyu Xie Date: Mon, 1 May 2023 15:09:09 +0800 Subject: serial: pl011: set UART011_CR_RXE in pl011_set_termios after port shutdown UART011_CR_RXE is set in pl011_startup() during normal initialization, and cleared by pl011_disable_uart() during port shutdown. You can use a none-console port in poll mode like kgdboc does with tty_find_polling_driver() after a port shutdown. But pl011_startup() is not called in tty_find_polling_driver(). So you need to set UART011_CR_RXE to reenable receive function. Not sure setting UART011_CR_RXE in pl011_set_termios() is a good idea through. Signed-off-by: Hongyu Xie Link: https://lore.kernel.org/r/20230501070909.1144547-1-xiehongyu1@kylinos.cn Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/amba-pl011.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index d8c2f3455eeb..c5c3f4674459 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -2166,6 +2166,13 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios, * ----------^----------^----------^----------^----- */ pl011_write_lcr_h(uap, lcr_h); + + /* + * Receive was disabled by pl011_disable_uart during shutdown. + * Need to reenable receive if you need to use a tty_driver + * returns from tty_find_polling_driver() after a port shutdown. + */ + old_cr |= UART011_CR_RXE; pl011_write(old_cr, uap, REG_CR); spin_unlock_irqrestore(&port->lock, flags); -- cgit v1.2.3 From b9ab22c2bc8652324a803b3e2be69838920b4025 Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Mon, 8 May 2023 11:20:11 +0300 Subject: serial: 8250: omap: Fix freeing of resources on failed register If serial8250_register_8250_port() fails, the SoC can hang as the deferred PMQoS work will still run as is not flushed and removed. Fixes: 61929cf0169d ("tty: serial: Add 8250-core based omap driver") Signed-off-by: Tony Lindgren Reviewed-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230508082014.23083-2-tony@atomide.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 734f092ef839..05dc568bd389 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -1532,7 +1532,9 @@ static int omap8250_probe(struct platform_device *pdev) err: pm_runtime_dont_use_autosuspend(&pdev->dev); pm_runtime_put_sync(&pdev->dev); + flush_work(&priv->qos_work); pm_runtime_disable(&pdev->dev); + cpu_latency_qos_remove_request(&priv->pm_qos_request); return ret; } -- cgit v1.2.3 From 398cecc24846e867b9f90a0bd22730e3df6b05be Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Mon, 8 May 2023 11:20:12 +0300 Subject: serial: 8250: omap: Fix imprecise external abort for omap_8250_pm() 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 Link: https://lore.kernel.org/r/20230508082014.23083-3-tony@atomide.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 70 ++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 05dc568bd389..57981f48ae69 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -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) @@ -115,6 +116,7 @@ #define UART_OMAP_RX_LVL 0x19 struct omap8250_priv { + void __iomem *membase; int line; u8 habit; u8 mdr1; @@ -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)); } /* @@ -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; @@ -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; @@ -1457,6 +1464,8 @@ 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); @@ -1464,6 +1473,8 @@ static int omap8250_probe(struct platform_device *pdev) 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); @@ -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; @@ -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; @@ -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; @@ -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) { @@ -1668,13 +1678,10 @@ 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, @@ -1682,7 +1689,7 @@ static int omap8250_runtime_suspend(struct device *dev) * active during suspend. */ if (priv->is_suspending && !console_suspend_enabled) { - if (uart_console(&up->port)) + if (up && uart_console(&up->port)) return -EBUSY; } @@ -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; @@ -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; -- cgit v1.2.3 From fef4f600319e35b7121c3ea6005ee38626f4f959 Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Mon, 8 May 2023 11:20:13 +0300 Subject: serial: 8250: omap: Fix life cycle issues for interrupt handlers 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 Link: https://lore.kernel.org/r/20230508082014.23083-4-tony@atomide.com Signed-off-by: Greg Kroah-Hartman --- 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 index 57981f48ae69..bf57e32a1cce 100644 --- 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"); -- cgit v1.2.3 From db86bb6ed4979ed8a987057f9098ea88f85e48be Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Mon, 8 May 2023 11:20:14 +0300 Subject: serial: 8250: omap: Shut down on remove for console uart When unbinding the console uart, we want to power down the uart hardware on remove. For the console uart, the normal shutdown path will never get called as the cons_filp stays open. Let's rearrange the dma related functions a bit so we can call driver shutdown also on console uart rebind. Currently we set up->dma on probe, but that causes issues calling omap_8250_shutdown() on remove. The dma startup will not get called on the next rebind as we still have up->dma set from probe. Note that serial8250_release_dma() already checks for dma so we can remove the check for it in 8205_omap driver. With these changes we also avoid hogging dma virtual channels for the unused uarts that may be limited on some devices. Signed-off-by: Tony Lindgren Link: https://lore.kernel.org/r/20230508082014.23083-5-tony@atomide.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index bf57e32a1cce..5c093dfcee1d 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -679,6 +679,7 @@ static int omap_8250_startup(struct uart_port *port) { struct uart_8250_port *up = up_to_u8250p(port); struct omap8250_priv *priv = port->private_data; + struct uart_8250_dma *dma = &priv->omap8250_dma; int ret; if (priv->wakeirq) { @@ -697,16 +698,16 @@ static int omap_8250_startup(struct uart_port *port) up->msr_saved_flags = 0; /* Disable DMA for console UART */ - if (uart_console(port)) - up->dma = NULL; - - if (up->dma) { + if (dma->fn && !uart_console(port)) { + up->dma = &priv->omap8250_dma; ret = serial8250_request_dma(up); if (ret) { dev_warn_ratelimited(port->dev, "failed to request DMA\n"); up->dma = NULL; } + } else { + up->dma = NULL; } up->ier = UART_IER_RLSI | UART_IER_RDI; @@ -752,8 +753,8 @@ static void omap_8250_shutdown(struct uart_port *port) disable_irq_nosync(up->port.irq); dev_pm_clear_wake_irq(port->dev); - if (up->dma) - serial8250_release_dma(up); + serial8250_release_dma(up); + up->dma = NULL; /* * Disable break condition and FIFOs @@ -1499,24 +1500,24 @@ static int omap8250_probe(struct platform_device *pdev) ret = of_property_count_strings(np, "dma-names"); if (ret == 2) { struct omap8250_dma_params *dma_params = NULL; + struct uart_8250_dma *dma = &priv->omap8250_dma; - up.dma = &priv->omap8250_dma; - up.dma->fn = the_no_dma_filter_fn; - up.dma->tx_dma = omap_8250_tx_dma; - up.dma->rx_dma = omap_8250_rx_dma; + dma->fn = the_no_dma_filter_fn; + dma->tx_dma = omap_8250_tx_dma; + dma->rx_dma = omap_8250_rx_dma; if (pdata) dma_params = pdata->dma_params; if (dma_params) { - up.dma->rx_size = dma_params->rx_size; - up.dma->rxconf.src_maxburst = dma_params->rx_trigger; - up.dma->txconf.dst_maxburst = dma_params->tx_trigger; + dma->rx_size = dma_params->rx_size; + dma->rxconf.src_maxburst = dma_params->rx_trigger; + dma->txconf.dst_maxburst = dma_params->tx_trigger; priv->rx_trigger = dma_params->rx_trigger; priv->tx_trigger = dma_params->tx_trigger; } else { - up.dma->rx_size = RX_TRIGGER; - up.dma->rxconf.src_maxburst = RX_TRIGGER; - up.dma->txconf.dst_maxburst = TX_TRIGGER; + dma->rx_size = RX_TRIGGER; + dma->rxconf.src_maxburst = RX_TRIGGER; + dma->txconf.dst_maxburst = TX_TRIGGER; } } #endif @@ -1550,12 +1551,15 @@ err: static int omap8250_remove(struct platform_device *pdev) { struct omap8250_priv *priv = platform_get_drvdata(pdev); + struct uart_8250_port *up; int err; err = pm_runtime_resume_and_get(&pdev->dev); if (err) return err; + up = serial8250_get_port(priv->line); + omap_8250_shutdown(&up->port); serial8250_unregister_port(priv->line); priv->line = -ENODEV; pm_runtime_dont_use_autosuspend(&pdev->dev); -- cgit v1.2.3 From 9d86719f8769244dc99b8cb6091c41eae3fd684f Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Mon, 8 May 2023 14:19:02 +0300 Subject: serial: 8250: Allow using ports higher than SERIAL_8250_RUNTIME_UARTS We already allocate CONFIG_SERIAL_8250_NR_UARTS, but only allow using CONFIG_SERIAL_8250_RUNTIME_UARTS uarts unless nr_uarts module params is set. This causes issues for using distro kernels on SoCs with a large number of serial ports. Let's allow up to CONFIG_SERIAL_8250_NR_UARTS instead. To do this, we init the ports as needed if the initial uarts was too low. This way there's no need to set the value for CONFIG_SERIAL_8250_RUNTIME_UARTS to some SoC specific higher value. Typically the default value of 4 can be used as set for legacy reasons. Note that limiting the number of intial uarts still works as before unless a serial console on a higher port is specified. In this case we will increase the nr_ports up to the console port specified. Suggested-by: Andrew Davis Signed-off-by: Tony Lindgren Link: https://lore.kernel.org/r/20230508111903.39339-1-tony@atomide.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_core.c | 86 +++++++++++++++++++++++++++---------- 1 file changed, 63 insertions(+), 23 deletions(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 13bf535eedcd..560a3592ec05 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -488,6 +488,34 @@ static inline void serial8250_apply_quirks(struct uart_8250_port *up) up->port.quirks |= skip_txen_test ? UPQ_NO_TXEN_TEST : 0; } +static struct uart_8250_port *serial8250_setup_port(int index) +{ + struct uart_8250_port *up; + + if (index >= UART_NR) + return NULL; + + up = &serial8250_ports[index]; + up->port.line = index; + + serial8250_init_port(up); + if (!base_ops) + base_ops = up->port.ops; + up->port.ops = &univ8250_port_ops; + + timer_setup(&up->timer, serial8250_timeout, 0); + + up->ops = &univ8250_driver_ops; + + if (IS_ENABLED(CONFIG_ALPHA_JENSEN) || + (IS_ENABLED(CONFIG_ALPHA_GENERIC) && alpha_jensen())) + up->port.set_mctrl = alpha_jensen_set_mctrl; + + serial8250_set_defaults(up); + + return up; +} + static void __init serial8250_isa_init_ports(void) { struct uart_8250_port *up; @@ -501,26 +529,13 @@ static void __init serial8250_isa_init_ports(void) if (nr_uarts > UART_NR) nr_uarts = UART_NR; - for (i = 0; i < nr_uarts; i++) { - struct uart_8250_port *up = &serial8250_ports[i]; - struct uart_port *port = &up->port; - - port->line = i; - serial8250_init_port(up); - if (!base_ops) - base_ops = port->ops; - port->ops = &univ8250_port_ops; - - timer_setup(&up->timer, serial8250_timeout, 0); - - up->ops = &univ8250_driver_ops; - - if (IS_ENABLED(CONFIG_ALPHA_JENSEN) || - (IS_ENABLED(CONFIG_ALPHA_GENERIC) && alpha_jensen())) - port->set_mctrl = alpha_jensen_set_mctrl; - - serial8250_set_defaults(up); - } + /* + * Set up initial isa ports based on nr_uart module param, or else + * default to CONFIG_SERIAL_8250_RUNTIME_UARTS. Note that we do not + * need to increase nr_uarts when setting up the initial isa ports. + */ + for (i = 0; i < nr_uarts; i++) + serial8250_setup_port(i); /* chain base port ops to support Remote Supervisor Adapter */ univ8250_port_ops = *base_ops; @@ -586,16 +601,29 @@ static void univ8250_console_write(struct console *co, const char *s, static int univ8250_console_setup(struct console *co, char *options) { + struct uart_8250_port *up; struct uart_port *port; - int retval; + int retval, i; /* * Check whether an invalid uart number has been specified, and * if so, search for the first available port that does have * console support. */ - if (co->index >= nr_uarts) + if (co->index >= UART_NR) co->index = 0; + + /* + * If the console is past the initial isa ports, init more ports up to + * co->index as needed and increment nr_uarts accordingly. + */ + for (i = nr_uarts; i <= co->index; i++) { + up = serial8250_setup_port(i); + if (!up) + return -ENODEV; + nr_uarts++; + } + port = &serial8250_ports[co->index].port; /* link port to console */ port->cons = co; @@ -990,7 +1018,18 @@ int serial8250_register_8250_port(const struct uart_8250_port *up) mutex_lock(&serial_mutex); uart = serial8250_find_match_or_unused(&up->port); - if (uart && uart->port.type != PORT_8250_CIR) { + if (!uart) { + /* + * If the port is past the initial isa ports, initialize a new + * port and increment nr_uarts accordingly. + */ + uart = serial8250_setup_port(nr_uarts); + if (!uart) + goto unlock; + nr_uarts++; + } + + if (uart->port.type != PORT_8250_CIR) { struct mctrl_gpios *gpios; if (uart->port.dev) @@ -1120,6 +1159,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up) } } +unlock: mutex_unlock(&serial_mutex); return ret; -- cgit v1.2.3 From b245aa0cc583c3efa0335c50f309a32e5fec8ab7 Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Thu, 11 May 2023 15:10:24 +0300 Subject: serial: 8250: Change dl_read/write to handle value as u32 Divisor latch read/write functions currently handle the value is int. As the value is related to HW context, u32 makes much more sense than a signed type. While at it, name the parameters in the callback signature. Signed-off-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230511121029.13128-2-ilpo.jarvinen@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250.h | 4 ++-- drivers/tty/serial/8250/8250_em.c | 4 ++-- drivers/tty/serial/8250/8250_port.c | 10 +++++----- drivers/tty/serial/8250/8250_pxa.c | 2 +- drivers/tty/serial/8250/8250_uniphier.c | 4 ++-- include/linux/serial_8250.h | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index 1e8fe44a7099..5418708f4631 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -167,12 +167,12 @@ static unsigned int __maybe_unused serial_icr_read(struct uart_8250_port *up, void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p); -static inline int serial_dl_read(struct uart_8250_port *up) +static inline u32 serial_dl_read(struct uart_8250_port *up) { return up->dl_read(up); } -static inline void serial_dl_write(struct uart_8250_port *up, int value) +static inline void serial_dl_write(struct uart_8250_port *up, u32 value) { up->dl_write(up, value); } diff --git a/drivers/tty/serial/8250/8250_em.c b/drivers/tty/serial/8250/8250_em.c index 25a9ecf26be6..ef5019e944ea 100644 --- a/drivers/tty/serial/8250/8250_em.c +++ b/drivers/tty/serial/8250/8250_em.c @@ -139,12 +139,12 @@ static void serial8250_em_serial_out(struct uart_port *p, int offset, int value) } } -static int serial8250_em_serial_dl_read(struct uart_8250_port *up) +static u32 serial8250_em_serial_dl_read(struct uart_8250_port *up) { return serial_in(up, UART_DLL_EM) | serial_in(up, UART_DLM_EM) << 8; } -static void serial8250_em_serial_dl_write(struct uart_8250_port *up, int value) +static void serial8250_em_serial_dl_write(struct uart_8250_port *up, u32 value) { serial_out(up, UART_DLL_EM, value & 0xff); serial_out(up, UART_DLM_EM, value >> 8 & 0xff); diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index fe8d79c4ae95..344bd447639b 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -325,7 +325,7 @@ static const struct serial8250_config uart_config[] = { }; /* Uart divisor latch read */ -static int default_serial_dl_read(struct uart_8250_port *up) +static u32 default_serial_dl_read(struct uart_8250_port *up) { /* Assign these in pieces to truncate any bits above 7. */ unsigned char dll = serial_in(up, UART_DLL); @@ -335,7 +335,7 @@ static int default_serial_dl_read(struct uart_8250_port *up) } /* Uart divisor latch write */ -static void default_serial_dl_write(struct uart_8250_port *up, int value) +static void default_serial_dl_write(struct uart_8250_port *up, u32 value) { serial_out(up, UART_DLL, value & 0xff); serial_out(up, UART_DLM, value >> 8 & 0xff); @@ -389,12 +389,12 @@ void au_serial_out(struct uart_port *p, int offset, int value) } /* Au1x00 haven't got a standard divisor latch */ -static int au_serial_dl_read(struct uart_8250_port *up) +static u32 au_serial_dl_read(struct uart_8250_port *up) { return __raw_readl(up->port.membase + 0x28); } -static void au_serial_dl_write(struct uart_8250_port *up, int value) +static void au_serial_dl_write(struct uart_8250_port *up, u32 value) { __raw_writel(value, up->port.membase + 0x28); } @@ -847,7 +847,7 @@ static void disable_rsa(struct uart_8250_port *up) static int size_fifo(struct uart_8250_port *up) { unsigned char old_fcr, old_mcr, old_lcr; - unsigned short old_dl; + u32 old_dl; int count; old_lcr = serial_in(up, UART_LCR); diff --git a/drivers/tty/serial/8250/8250_pxa.c b/drivers/tty/serial/8250/8250_pxa.c index 795e55142d4c..28b341f602c6 100644 --- a/drivers/tty/serial/8250/8250_pxa.c +++ b/drivers/tty/serial/8250/8250_pxa.c @@ -60,7 +60,7 @@ static const struct of_device_id serial_pxa_dt_ids[] = { MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids); /* Uart divisor latch write */ -static void serial_pxa_dl_write(struct uart_8250_port *up, int value) +static void serial_pxa_dl_write(struct uart_8250_port *up, u32 value) { unsigned int dll; diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c index a2978abab0db..a405155264b1 100644 --- a/drivers/tty/serial/8250/8250_uniphier.c +++ b/drivers/tty/serial/8250/8250_uniphier.c @@ -145,12 +145,12 @@ static void uniphier_serial_out(struct uart_port *p, int offset, int value) * The divisor latch register exists at different address. * Override dl_read/write callbacks. */ -static int uniphier_serial_dl_read(struct uart_8250_port *up) +static u32 uniphier_serial_dl_read(struct uart_8250_port *up) { return readl(up->port.membase + UNIPHIER_UART_DLR); } -static void uniphier_serial_dl_write(struct uart_8250_port *up, int value) +static void uniphier_serial_dl_write(struct uart_8250_port *up, u32 value) { writel(value, up->port.membase + UNIPHIER_UART_DLR); } diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 6f78f302d272..7b5d558e4e0c 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -129,8 +129,8 @@ struct uart_8250_port { const struct uart_8250_ops *ops; /* 8250 specific callbacks */ - int (*dl_read)(struct uart_8250_port *); - void (*dl_write)(struct uart_8250_port *, int); + u32 (*dl_read)(struct uart_8250_port *up); + void (*dl_write)(struct uart_8250_port *up, u32 value); struct uart_8250_em485 *em485; void (*rs485_start_tx)(struct uart_8250_port *); -- cgit v1.2.3 From 98658ae8f3925d05c2d0c67749ff6cf47b9077a5 Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Thu, 11 May 2023 15:10:25 +0300 Subject: serial: 8250: Document uart_8250_port's ->dl_read/write() Add documentation for the struct uart_8250_port divisor latch function pointers. Documentation is in kernel doc format but don't enable kernel doc yet as many other fields remain undocumented. Signed-off-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230511121029.13128-3-ilpo.jarvinen@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- include/linux/serial_8250.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 7b5d558e4e0c..d64e7bbe1f2f 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -90,8 +90,17 @@ struct uart_8250_em485 { * their own 8250 ports without registering their own * platform device. Using these will make your driver * dependent on the 8250 driver. + * + * @dl_read: ``u32 ()(struct uart_8250_port *port)`` + * + * UART divisor latch read. + * + * @dl_write: ``void ()(struct uart_8250_port *port, u32 value)`` + * + * Write @value into UART divisor latch. + * + * Locking: Caller holds port's lock. */ - struct uart_8250_port { struct uart_port port; struct timer_list timer; /* "no irq" timer */ -- cgit v1.2.3 From 30c61f53fdf23e8a396dca8378615b4900edff6f Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Thu, 11 May 2023 15:10:26 +0300 Subject: serial: 8250: Add dl_read/write, bugs and mapsize into plat_serial8250_port Add mapsize, bugs, and divisor latch read/write functions (->dl_read/write()) into plat_serial8250_port to carry the setup necessary for RT288x/Au1xxx devices over to uart port. Document the added members with kerneldoc style but do not enable kerneldoc yet as there are many fields which remain undocumented. While at it, convert .bugs in struct uart_8250_port to u16 to match it with the type used in struct plat_serial8250_port. Signed-off-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230511121029.13128-4-ilpo.jarvinen@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_core.c | 4 ++++ include/linux/serial_8250.h | 21 ++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 560a3592ec05..4434c3256a92 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -850,12 +850,16 @@ static int serial8250_probe(struct platform_device *dev) uart.port.iotype = p->iotype; uart.port.flags = p->flags; uart.port.mapbase = p->mapbase; + uart.port.mapsize = p->mapsize; uart.port.hub6 = p->hub6; uart.port.has_sysrq = p->has_sysrq; uart.port.private_data = p->private_data; uart.port.type = p->type; + uart.bugs = p->bugs; uart.port.serial_in = p->serial_in; uart.port.serial_out = p->serial_out; + uart.dl_read = p->dl_read; + uart.dl_write = p->dl_write; uart.port.handle_irq = p->handle_irq; uart.port.handle_break = p->handle_break; uart.port.set_termios = p->set_termios; diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index d64e7bbe1f2f..42fc8f64f48e 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -11,13 +11,29 @@ #include #include +struct uart_8250_port; + /* * This is the platform device platform_data structure + * + * @mapsize: Port size for ioremap() + * @bugs: Port bugs + * + * @dl_read: ``u32 ()(struct uart_8250_port *up)`` + * + * UART divisor latch read. + * + * @dl_write: ``void ()(struct uart_8250_port *up, u32 value)`` + * + * Write @value into UART divisor latch. + * + * Locking: Caller holds port's lock. */ struct plat_serial8250_port { unsigned long iobase; /* io base address */ void __iomem *membase; /* ioremap cookie or NULL */ resource_size_t mapbase; /* resource base */ + resource_size_t mapsize; unsigned int uartclk; /* UART clock rate */ unsigned int irq; /* interrupt number */ unsigned long irqflags; /* request_irq flags */ @@ -28,8 +44,11 @@ struct plat_serial8250_port { unsigned char has_sysrq; /* supports magic SysRq */ unsigned int type; /* If UPF_FIXED_TYPE */ upf_t flags; /* UPF_* flags */ + u16 bugs; /* port bugs */ unsigned int (*serial_in)(struct uart_port *, int); void (*serial_out)(struct uart_port *, int, int); + u32 (*dl_read)(struct uart_8250_port *up); + void (*dl_write)(struct uart_8250_port *up, u32 value); void (*set_termios)(struct uart_port *, struct ktermios *new, const struct ktermios *old); @@ -106,7 +125,7 @@ struct uart_8250_port { struct timer_list timer; /* "no irq" timer */ struct list_head list; /* ports on this IRQ */ u32 capabilities; /* port capabilities */ - unsigned short bugs; /* port bugs */ + u16 bugs; /* port bugs */ bool fifo_bug; /* min RX trigger if enabled */ unsigned int tx_loadsz; /* transmit fifo load size */ unsigned char acr; -- cgit v1.2.3 From b334214ea08d941af376fec853621d856b12bc81 Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Thu, 11 May 2023 15:10:27 +0300 Subject: serial: 8250: RT288x/Au1xxx code away from core A non-trivial amount of RT288x/Au1xxx code is encapsulated into ifdeffery in 8250_port / 8250_early and some if UPIO_AU blocks. Create a separate file from them. Also handle errors properly in the cases where RT288x/Au1xxx code is not configured. It seems that 0x1000 mapsize is likely overkill but I've kept it the same as previously (the value was shrunk to that value in commit b2b13cdfd05e ("SERIAL 8250: Fixes for Alchemy UARTs.")). Seemingly, the driver only needs to access register at 0x28 for the divisor latch. The Kconfig side is a bit tricky. As SERIAL_8250_RT288X is bool it can only be =y. It is possible to have SERIAL_8250=m + SERIAL_8250_RT288X=y which required altering when 8250/ is included or the rt288x would not be built. Signed-off-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230511121029.13128-5-ilpo.jarvinen@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- arch/mips/alchemy/common/platform.c | 10 ++- drivers/tty/serial/8250/8250_early.c | 21 ----- drivers/tty/serial/8250/8250_of.c | 4 +- drivers/tty/serial/8250/8250_port.c | 78 ------------------- drivers/tty/serial/8250/8250_rt288x.c | 142 ++++++++++++++++++++++++++++++++++ drivers/tty/serial/8250/Makefile | 1 + drivers/tty/serial/Makefile | 2 +- include/linux/serial_8250.h | 8 +- 8 files changed, 161 insertions(+), 105 deletions(-) create mode 100644 drivers/tty/serial/8250/8250_rt288x.c diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c index b8f3397c59c9..d4ab34b3b404 100644 --- a/arch/mips/alchemy/common/platform.c +++ b/arch/mips/alchemy/common/platform.c @@ -51,9 +51,9 @@ static void alchemy_8250_pm(struct uart_port *port, unsigned int state, #define PORT(_base, _irq) \ { \ .mapbase = _base, \ + .mapsize = 0x1000, \ .irq = _irq, \ .regshift = 2, \ - .iotype = UPIO_AU, \ .flags = UPF_SKIP_TEST | UPF_IOREMAP | \ UPF_FIXED_TYPE, \ .type = PORT_16550A, \ @@ -124,8 +124,14 @@ static void __init alchemy_setup_uarts(int ctype) au1xx0_uart_device.dev.platform_data = ports; /* Fill up uartclk. */ - for (s = 0; s < c; s++) + for (s = 0; s < c; s++) { ports[s].uartclk = uartclk; + if (au_platform_setup(&ports[s]) < 0) { + kfree(ports); + printk(KERN_INFO "Alchemy: missing support for UARTs\n"); + return; + } + } if (platform_device_register(&au1xx0_uart_device)) printk(KERN_INFO "Alchemy: failed to register UARTs\n"); } diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c index 0ebde0ab8167..4299a8bd83d9 100644 --- a/drivers/tty/serial/8250/8250_early.c +++ b/drivers/tty/serial/8250/8250_early.c @@ -36,7 +36,6 @@ static unsigned int serial8250_early_in(struct uart_port *port, int offset) { - int reg_offset = offset; offset <<= port->regshift; switch (port->iotype) { @@ -50,8 +49,6 @@ static unsigned int serial8250_early_in(struct uart_port *port, int offset) return ioread32be(port->membase + offset); case UPIO_PORT: return inb(port->iobase + offset); - case UPIO_AU: - return port->serial_in(port, reg_offset); default: return 0; } @@ -59,7 +56,6 @@ static unsigned int serial8250_early_in(struct uart_port *port, int offset) static void serial8250_early_out(struct uart_port *port, int offset, int value) { - int reg_offset = offset; offset <<= port->regshift; switch (port->iotype) { @@ -78,9 +74,6 @@ static void serial8250_early_out(struct uart_port *port, int offset, int value) case UPIO_PORT: outb(value, port->iobase + offset); break; - case UPIO_AU: - port->serial_out(port, reg_offset, value); - break; } } @@ -199,17 +192,3 @@ OF_EARLYCON_DECLARE(omap8250, "ti,omap3-uart", early_omap8250_setup); OF_EARLYCON_DECLARE(omap8250, "ti,omap4-uart", early_omap8250_setup); #endif - -#ifdef CONFIG_SERIAL_8250_RT288X - -static int __init early_au_setup(struct earlycon_device *dev, const char *opt) -{ - dev->port.serial_in = au_serial_in; - dev->port.serial_out = au_serial_out; - dev->port.iotype = UPIO_AU; - dev->con->write = early_serial8250_write; - return 0; -} -OF_EARLYCON_DECLARE(palmchip, "ralink,rt2880-uart", early_au_setup); - -#endif diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c index 1b461fba15a3..c9f6bd7a7038 100644 --- a/drivers/tty/serial/8250/8250_of.c +++ b/drivers/tty/serial/8250/8250_of.c @@ -171,7 +171,9 @@ static int of_platform_serial_setup(struct platform_device *ofdev, switch (type) { case PORT_RT2880: - port->iotype = UPIO_AU; + ret = rt288x_setup(port); + if (ret) + goto err_unprepare; break; } diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 344bd447639b..0cef9bfd0471 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -341,66 +341,6 @@ static void default_serial_dl_write(struct uart_8250_port *up, u32 value) serial_out(up, UART_DLM, value >> 8 & 0xff); } -#ifdef CONFIG_SERIAL_8250_RT288X - -#define UART_REG_UNMAPPED -1 - -/* Au1x00/RT288x UART hardware has a weird register layout */ -static const s8 au_io_in_map[8] = { - [UART_RX] = 0, - [UART_IER] = 2, - [UART_IIR] = 3, - [UART_LCR] = 5, - [UART_MCR] = 6, - [UART_LSR] = 7, - [UART_MSR] = 8, - [UART_SCR] = UART_REG_UNMAPPED, -}; - -static const s8 au_io_out_map[8] = { - [UART_TX] = 1, - [UART_IER] = 2, - [UART_FCR] = 4, - [UART_LCR] = 5, - [UART_MCR] = 6, - [UART_LSR] = UART_REG_UNMAPPED, - [UART_MSR] = UART_REG_UNMAPPED, - [UART_SCR] = UART_REG_UNMAPPED, -}; - -unsigned int au_serial_in(struct uart_port *p, int offset) -{ - if (offset >= ARRAY_SIZE(au_io_in_map)) - return UINT_MAX; - offset = au_io_in_map[offset]; - if (offset == UART_REG_UNMAPPED) - return UINT_MAX; - return __raw_readl(p->membase + (offset << p->regshift)); -} - -void au_serial_out(struct uart_port *p, int offset, int value) -{ - if (offset >= ARRAY_SIZE(au_io_out_map)) - return; - offset = au_io_out_map[offset]; - if (offset == UART_REG_UNMAPPED) - return; - __raw_writel(value, p->membase + (offset << p->regshift)); -} - -/* Au1x00 haven't got a standard divisor latch */ -static u32 au_serial_dl_read(struct uart_8250_port *up) -{ - return __raw_readl(up->port.membase + 0x28); -} - -static void au_serial_dl_write(struct uart_8250_port *up, u32 value) -{ - __raw_writel(value, up->port.membase + 0x28); -} - -#endif - static unsigned int hub6_serial_in(struct uart_port *p, int offset) { offset = offset << p->regshift; @@ -510,15 +450,6 @@ static void set_io_from_upio(struct uart_port *p) p->serial_out = mem32be_serial_out; break; -#ifdef CONFIG_SERIAL_8250_RT288X - case UPIO_AU: - p->serial_in = au_serial_in; - p->serial_out = au_serial_out; - up->dl_read = au_serial_dl_read; - up->dl_write = au_serial_dl_write; - break; -#endif - default: p->serial_in = io_serial_in; p->serial_out = io_serial_out; @@ -2968,11 +2899,6 @@ static unsigned int serial8250_port_size(struct uart_8250_port *pt) { if (pt->port.mapsize) return pt->port.mapsize; - if (pt->port.iotype == UPIO_AU) { - if (pt->port.type == PORT_RT2880) - return 0x100; - return 0x1000; - } if (is_omap1_8250(pt)) return 0x16 << pt->port.regshift; @@ -3222,10 +3148,6 @@ static void serial8250_config_port(struct uart_port *port, int flags) if (flags & UART_CONFIG_TYPE) autoconfig(up); - /* if access method is AU, it is a 16550 with a quirk */ - if (port->type == PORT_16550A && port->iotype == UPIO_AU) - up->bugs |= UART_BUG_NOMSR; - /* HW bugs may trigger IRQ while IIR == NO_INT */ if (port->type == PORT_TEGRA) up->bugs |= UART_BUG_NOMSR; diff --git a/drivers/tty/serial/8250/8250_rt288x.c b/drivers/tty/serial/8250/8250_rt288x.c new file mode 100644 index 000000000000..51b1cf5476dd --- /dev/null +++ b/drivers/tty/serial/8250/8250_rt288x.c @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * RT288x/Au1xxx driver + */ + +#include +#include +#include +#include +#include +#include + +#include "8250.h" + +#define UART_REG_UNMAPPED -1 + +/* Au1x00/RT288x UART hardware has a weird register layout */ +static const s8 au_io_in_map[8] = { + [UART_RX] = 0, + [UART_IER] = 2, + [UART_IIR] = 3, + [UART_LCR] = 5, + [UART_MCR] = 6, + [UART_LSR] = 7, + [UART_MSR] = 8, + [UART_SCR] = UART_REG_UNMAPPED, +}; + +static const s8 au_io_out_map[8] = { + [UART_TX] = 1, + [UART_IER] = 2, + [UART_FCR] = 4, + [UART_LCR] = 5, + [UART_MCR] = 6, + [UART_LSR] = UART_REG_UNMAPPED, + [UART_MSR] = UART_REG_UNMAPPED, + [UART_SCR] = UART_REG_UNMAPPED, +}; + +static unsigned int au_serial_in(struct uart_port *p, int offset) +{ + if (offset >= ARRAY_SIZE(au_io_in_map)) + return UINT_MAX; + offset = au_io_in_map[offset]; + if (offset == UART_REG_UNMAPPED) + return UINT_MAX; + return __raw_readl(p->membase + (offset << p->regshift)); +} + +static void au_serial_out(struct uart_port *p, int offset, int value) +{ + if (offset >= ARRAY_SIZE(au_io_out_map)) + return; + offset = au_io_out_map[offset]; + if (offset == UART_REG_UNMAPPED) + return; + __raw_writel(value, p->membase + (offset << p->regshift)); +} + +/* Au1x00 haven't got a standard divisor latch */ +static u32 au_serial_dl_read(struct uart_8250_port *up) +{ + return __raw_readl(up->port.membase + 0x28); +} + +static void au_serial_dl_write(struct uart_8250_port *up, u32 value) +{ + __raw_writel(value, up->port.membase + 0x28); +} + +int au_platform_setup(struct plat_serial8250_port *p) +{ + p->iotype = UPIO_AU; + + p->serial_in = au_serial_in; + p->serial_out = au_serial_out; + p->dl_read = au_serial_dl_read; + p->dl_write = au_serial_dl_write; + + p->mapsize = 0x1000; + + p->bugs |= UART_BUG_NOMSR; + + return 0; +} +EXPORT_SYMBOL_GPL(au_platform_setup); + +int rt288x_setup(struct uart_port *p) +{ + struct uart_8250_port *up = up_to_u8250p(p); + + p->iotype = UPIO_AU; + + p->serial_in = au_serial_in; + p->serial_out = au_serial_out; + up->dl_read = au_serial_dl_read; + up->dl_write = au_serial_dl_write; + + p->mapsize = 0x100; + + up->bugs |= UART_BUG_NOMSR; + + return 0; +} +EXPORT_SYMBOL_GPL(rt288x_setup); + +#ifdef CONFIG_SERIAL_8250_CONSOLE +static void au_putc(struct uart_port *port, unsigned char c) +{ + unsigned int status; + + au_serial_out(port, UART_TX, c); + + for (;;) { + status = au_serial_in(port, UART_LSR); + if (uart_lsr_tx_empty(status)) + break; + cpu_relax(); + } +} + +static void au_early_serial8250_write(struct console *console, + const char *s, unsigned int count) +{ + struct earlycon_device *device = console->data; + struct uart_port *port = &device->port; + + uart_console_write(port, s, count, au_putc); +} + +static int __init early_au_setup(struct earlycon_device *dev, const char *opt) +{ + rt288x_setup(&dev->port); + dev->con->write = au_early_serial8250_write; + + return 0; +} +OF_EARLYCON_DECLARE(palmchip, "ralink,rt2880-uart", early_au_setup); +#endif + +MODULE_DESCRIPTION("RT288x/Au1xxx UART driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile index 4fc2fc1f41b6..628b75be312e 100644 --- a/drivers/tty/serial/8250/Makefile +++ b/drivers/tty/serial/8250/Makefile @@ -35,6 +35,7 @@ obj-$(CONFIG_SERIAL_8250_DW) += 8250_dw.o obj-$(CONFIG_SERIAL_8250_EM) += 8250_em.o obj-$(CONFIG_SERIAL_8250_IOC3) += 8250_ioc3.o obj-$(CONFIG_SERIAL_8250_OMAP) += 8250_omap.o +obj-$(CONFIG_SERIAL_8250_RT288X) += 8250_rt288x.o obj-$(CONFIG_SERIAL_8250_LPC18XX) += 8250_lpc18xx.o obj-$(CONFIG_SERIAL_8250_MT6577) += 8250_mtk.o obj-$(CONFIG_SERIAL_8250_UNIPHIER) += 8250_uniphier.o diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile index cd9afd9e3018..531ec3a19dae 100644 --- a/drivers/tty/serial/Makefile +++ b/drivers/tty/serial/Makefile @@ -21,7 +21,7 @@ obj-$(CONFIG_SERIAL_SUNSAB) += sunsab.o obj-$(CONFIG_SERIAL_21285) += 21285.o # Now bring in any enabled 8250/16450/16550 type drivers. -obj-$(CONFIG_SERIAL_8250) += 8250/ +obj-y += 8250/ obj-$(CONFIG_SERIAL_AMBA_PL010) += amba-pl010.o obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index 42fc8f64f48e..eb44420b39ec 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -7,6 +7,7 @@ #ifndef _LINUX_SERIAL_8250_H #define _LINUX_SERIAL_8250_H +#include #include #include #include @@ -211,8 +212,11 @@ void serial8250_set_isa_configurator(void (*v)(int port, struct uart_port *up, u32 *capabilities)); #ifdef CONFIG_SERIAL_8250_RT288X -unsigned int au_serial_in(struct uart_port *p, int offset); -void au_serial_out(struct uart_port *p, int offset, int value); +int rt288x_setup(struct uart_port *p); +int au_platform_setup(struct plat_serial8250_port *p); +#else +static inline int rt288x_setup(struct uart_port *p) { return -ENODEV; } +static inline int au_platform_setup(struct plat_serial8250_port *p) { return -ENODEV; } #endif #endif -- cgit v1.2.3 From 33e3b0eb755074a870fd926ebd84de52e7302090 Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Thu, 11 May 2023 15:10:28 +0300 Subject: serial: 8250_rt288x: Name non-standard divisor latch reg Instead of a literal, add proper name for the non-standard divisor latch register. Signed-off-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230511121029.13128-6-ilpo.jarvinen@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_rt288x.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_rt288x.c b/drivers/tty/serial/8250/8250_rt288x.c index 51b1cf5476dd..b091a1269bfa 100644 --- a/drivers/tty/serial/8250/8250_rt288x.c +++ b/drivers/tty/serial/8250/8250_rt288x.c @@ -12,6 +12,8 @@ #include "8250.h" +#define RT288X_DL 0x28 + #define UART_REG_UNMAPPED -1 /* Au1x00/RT288x UART hardware has a weird register layout */ @@ -60,12 +62,12 @@ static void au_serial_out(struct uart_port *p, int offset, int value) /* Au1x00 haven't got a standard divisor latch */ static u32 au_serial_dl_read(struct uart_8250_port *up) { - return __raw_readl(up->port.membase + 0x28); + return __raw_readl(up->port.membase + RT288X_DL); } static void au_serial_dl_write(struct uart_8250_port *up, u32 value) { - __raw_writel(value, up->port.membase + 0x28); + __raw_writel(value, up->port.membase + RT288X_DL); } int au_platform_setup(struct plat_serial8250_port *p) -- cgit v1.2.3 From 72fc578b220c417787384ac7c3e235e98f218be6 Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Thu, 11 May 2023 15:10:29 +0300 Subject: serial: 8250_rt288x: Remove unnecessary UART_REG_UNMAPPED As unmapped registers are at the tail of the array, the ARRAY_SIZE() condition will catch them just fine. No need to define special value for them. Convert the arrays to u8 as all entiries are now positive. Signed-off-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230511121029.13128-7-ilpo.jarvinen@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_rt288x.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/tty/serial/8250/8250_rt288x.c b/drivers/tty/serial/8250/8250_rt288x.c index b091a1269bfa..6415ca8d3adf 100644 --- a/drivers/tty/serial/8250/8250_rt288x.c +++ b/drivers/tty/serial/8250/8250_rt288x.c @@ -14,10 +14,8 @@ #define RT288X_DL 0x28 -#define UART_REG_UNMAPPED -1 - /* Au1x00/RT288x UART hardware has a weird register layout */ -static const s8 au_io_in_map[8] = { +static const u8 au_io_in_map[7] = { [UART_RX] = 0, [UART_IER] = 2, [UART_IIR] = 3, @@ -25,18 +23,14 @@ static const s8 au_io_in_map[8] = { [UART_MCR] = 6, [UART_LSR] = 7, [UART_MSR] = 8, - [UART_SCR] = UART_REG_UNMAPPED, }; -static const s8 au_io_out_map[8] = { +static const u8 au_io_out_map[5] = { [UART_TX] = 1, [UART_IER] = 2, [UART_FCR] = 4, [UART_LCR] = 5, [UART_MCR] = 6, - [UART_LSR] = UART_REG_UNMAPPED, - [UART_MSR] = UART_REG_UNMAPPED, - [UART_SCR] = UART_REG_UNMAPPED, }; static unsigned int au_serial_in(struct uart_port *p, int offset) @@ -44,8 +38,7 @@ static unsigned int au_serial_in(struct uart_port *p, int offset) if (offset >= ARRAY_SIZE(au_io_in_map)) return UINT_MAX; offset = au_io_in_map[offset]; - if (offset == UART_REG_UNMAPPED) - return UINT_MAX; + return __raw_readl(p->membase + (offset << p->regshift)); } @@ -54,8 +47,7 @@ static void au_serial_out(struct uart_port *p, int offset, int value) if (offset >= ARRAY_SIZE(au_io_out_map)) return; offset = au_io_out_map[offset]; - if (offset == UART_REG_UNMAPPED) - return; + __raw_writel(value, p->membase + (offset << p->regshift)); } -- cgit v1.2.3 From 6bd6cd29c92401a101993290051fa55078238a52 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 12 May 2023 19:38:09 +0200 Subject: serial: stm32: Ignore return value of uart_remove_one_port() in .remove() Returning early from stm32_usart_serial_remove() results in a resource leak as several cleanup functions are not called. The driver core ignores the return value and there is no possibility to clean up later. uart_remove_one_port() only returns non-zero if there is some inconsistency (i.e. stm32_usart_driver.state[port->line].uart_port == NULL). This should never happen, and even if it does it's a bad idea to exit early in the remove callback without cleaning up. This prepares changing the prototype of struct platform_driver::remove to return void. See commit 5c5a7680e67b ("platform: Provide a remove callback that returns no value") for further details about this quest. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230512173810.131447-2-u.kleine-koenig@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/stm32-usart.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c index 1e38fc9b10c1..e9e11a259621 100644 --- a/drivers/tty/serial/stm32-usart.c +++ b/drivers/tty/serial/stm32-usart.c @@ -1755,13 +1755,10 @@ static int stm32_usart_serial_remove(struct platform_device *pdev) struct uart_port *port = platform_get_drvdata(pdev); struct stm32_port *stm32_port = to_stm32_port(port); const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs; - int err; u32 cr3; pm_runtime_get_sync(&pdev->dev); - err = uart_remove_one_port(&stm32_usart_driver, port); - if (err) - return(err); + uart_remove_one_port(&stm32_usart_driver, port); pm_runtime_disable(&pdev->dev); pm_runtime_set_suspended(&pdev->dev); -- cgit v1.2.3 From d5b3d02d0b107345f2a6ecb5b06f98356f5c97ab Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 12 May 2023 19:38:10 +0200 Subject: serial: Make uart_remove_one_port() return void The return value is only ever used as a return value for remove callbacks of platform drivers. This return value is ignored by the driver core. (The only effect is an error message, but uart_remove_one_port() already emitted one in this case.) So the return value isn't used at all and uart_remove_one_port() can be changed to return void without any loss. Also this better matches the Linux device model as remove functions are not supposed to fail. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230512173810.131447-3-u.kleine-koenig@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/atmel_serial.c | 5 ++--- drivers/tty/serial/clps711x.c | 4 +++- drivers/tty/serial/cpm_uart/cpm_uart_core.c | 5 ++++- drivers/tty/serial/imx.c | 4 +++- drivers/tty/serial/lantiq.c | 4 +++- drivers/tty/serial/serial_core.c | 6 +----- drivers/tty/serial/st-asc.c | 4 +++- drivers/tty/serial/uartlite.c | 12 ++++-------- drivers/tty/serial/xilinx_uartps.c | 5 ++--- include/linux/serial_core.h | 2 +- 10 files changed, 26 insertions(+), 25 deletions(-) diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index 9cd7479b03c0..6e9192f122aa 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -3006,14 +3006,13 @@ static int atmel_serial_remove(struct platform_device *pdev) { struct uart_port *port = platform_get_drvdata(pdev); struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); - int ret = 0; tasklet_kill(&atmel_port->tasklet_rx); tasklet_kill(&atmel_port->tasklet_tx); device_init_wakeup(&pdev->dev, 0); - ret = uart_remove_one_port(&atmel_uart, port); + uart_remove_one_port(&atmel_uart, port); kfree(atmel_port->rx_ring.buf); @@ -3023,7 +3022,7 @@ static int atmel_serial_remove(struct platform_device *pdev) pdev->dev.of_node = NULL; - return ret; + return 0; } static SIMPLE_DEV_PM_OPS(atmel_serial_pm_ops, atmel_serial_suspend, diff --git a/drivers/tty/serial/clps711x.c b/drivers/tty/serial/clps711x.c index e190dce58f46..e49bc4019b50 100644 --- a/drivers/tty/serial/clps711x.c +++ b/drivers/tty/serial/clps711x.c @@ -514,7 +514,9 @@ static int uart_clps711x_remove(struct platform_device *pdev) { struct clps711x_port *s = platform_get_drvdata(pdev); - return uart_remove_one_port(&clps711x_uart, &s->port); + uart_remove_one_port(&clps711x_uart, &s->port); + + return 0; } static const struct of_device_id __maybe_unused clps711x_uart_dt_ids[] = { diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c index 349e7da643f0..66afa9bea6bf 100644 --- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c +++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c @@ -1431,7 +1431,10 @@ static int cpm_uart_probe(struct platform_device *ofdev) static int cpm_uart_remove(struct platform_device *ofdev) { struct uart_cpm_port *pinfo = platform_get_drvdata(ofdev); - return uart_remove_one_port(&cpm_reg, &pinfo->port); + + uart_remove_one_port(&cpm_reg, &pinfo->port); + + return 0; } static const struct of_device_id cpm_uart_match[] = { diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index c5e17569c3ad..b2bf3cb449f4 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -2467,7 +2467,9 @@ static int imx_uart_remove(struct platform_device *pdev) { struct imx_port *sport = platform_get_drvdata(pdev); - return uart_remove_one_port(&imx_uart_uart_driver, &sport->port); + uart_remove_one_port(&imx_uart_uart_driver, &sport->port); + + return 0; } static void imx_uart_restore_context(struct imx_port *sport) diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c index a58e9277dfad..d413f97f7190 100644 --- a/drivers/tty/serial/lantiq.c +++ b/drivers/tty/serial/lantiq.c @@ -889,7 +889,9 @@ static int lqasc_remove(struct platform_device *pdev) { struct uart_port *port = platform_get_drvdata(pdev); - return uart_remove_one_port(&lqasc_reg, port); + uart_remove_one_port(&lqasc_reg, port); + + return 0; } static const struct ltq_soc_data soc_data_lantiq = { diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 54e82f476a2c..4b98d13555c0 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -3154,13 +3154,12 @@ EXPORT_SYMBOL(uart_add_one_port); * This unhooks (and hangs up) the specified port structure from the core * driver. No further calls will be made to the low-level code for this port. */ -int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport) +void uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport) { struct uart_state *state = drv->state + uport->line; struct tty_port *port = &state->port; struct uart_port *uart_port; struct tty_struct *tty; - int ret = 0; mutex_lock(&port_mutex); @@ -3176,7 +3175,6 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport) if (!uart_port) { mutex_unlock(&port->mutex); - ret = -EINVAL; goto out; } uport->flags |= UPF_DEAD; @@ -3219,8 +3217,6 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport) mutex_unlock(&port->mutex); out: mutex_unlock(&port_mutex); - - return ret; } EXPORT_SYMBOL(uart_remove_one_port); diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c index 5215e6910f68..dc2f2051435c 100644 --- a/drivers/tty/serial/st-asc.c +++ b/drivers/tty/serial/st-asc.c @@ -796,7 +796,9 @@ static int asc_serial_remove(struct platform_device *pdev) { struct uart_port *port = platform_get_drvdata(pdev); - return uart_remove_one_port(&asc_uart_driver, port); + uart_remove_one_port(&asc_uart_driver, port); + + return 0; } #ifdef CONFIG_PM_SLEEP diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c index 94584e54ebbe..679574893ebe 100644 --- a/drivers/tty/serial/uartlite.c +++ b/drivers/tty/serial/uartlite.c @@ -685,18 +685,15 @@ static int ulite_assign(struct device *dev, int id, phys_addr_t base, int irq, * * @dev: pointer to device structure */ -static int ulite_release(struct device *dev) +static void ulite_release(struct device *dev) { struct uart_port *port = dev_get_drvdata(dev); - int rc = 0; if (port) { - rc = uart_remove_one_port(&ulite_uart_driver, port); + uart_remove_one_port(&ulite_uart_driver, port); dev_set_drvdata(dev, NULL); port->mapbase = 0; } - - return rc; } /** @@ -900,14 +897,13 @@ static int ulite_remove(struct platform_device *pdev) { struct uart_port *port = dev_get_drvdata(&pdev->dev); struct uartlite_data *pdata = port->private_data; - int rc; clk_disable_unprepare(pdata->clk); - rc = ulite_release(&pdev->dev); + ulite_release(&pdev->dev); pm_runtime_disable(&pdev->dev); pm_runtime_set_suspended(&pdev->dev); pm_runtime_dont_use_autosuspend(&pdev->dev); - return rc; + return 0; } /* work with hotplug and coldplug */ diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index 8e521c69a959..20a751663ef9 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -1670,14 +1670,13 @@ static int cdns_uart_remove(struct platform_device *pdev) { struct uart_port *port = platform_get_drvdata(pdev); struct cdns_uart *cdns_uart_data = port->private_data; - int rc; /* Remove the cdns_uart port from the serial core */ #ifdef CONFIG_COMMON_CLK clk_notifier_unregister(cdns_uart_data->uartclk, &cdns_uart_data->clk_rate_change_nb); #endif - rc = uart_remove_one_port(cdns_uart_data->cdns_uart_driver, port); + uart_remove_one_port(cdns_uart_data->cdns_uart_driver, port); port->mapbase = 0; clk_disable_unprepare(cdns_uart_data->uartclk); clk_disable_unprepare(cdns_uart_data->pclk); @@ -1693,7 +1692,7 @@ static int cdns_uart_remove(struct platform_device *pdev) if (!--instances) uart_unregister_driver(cdns_uart_data->cdns_uart_driver); - return rc; + return 0; } static struct platform_driver cdns_uart_platform_driver = { diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index 66ecec15a1bf..ddcdb5b8523e 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -853,7 +853,7 @@ void uart_console_write(struct uart_port *port, const char *s, int uart_register_driver(struct uart_driver *uart); void uart_unregister_driver(struct uart_driver *uart); int uart_add_one_port(struct uart_driver *reg, struct uart_port *port); -int uart_remove_one_port(struct uart_driver *reg, struct uart_port *port); +void uart_remove_one_port(struct uart_driver *reg, struct uart_port *port); bool uart_match_port(const struct uart_port *port1, const struct uart_port *port2); -- cgit v1.2.3 From b1207d86169d16581101f93b2906cefd0e940ff1 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Thu, 25 May 2023 11:37:52 +0206 Subject: serial: 8250: lock port in startup() callbacks uart_ops startup() callback is called without interrupts disabled and without port->lock locked, relatively late during the boot process (from the call path of console_on_rootfs()). If the device is a console, it was already previously registered and could be actively printing messages. The console printing function serial8250_console_write() modifies the interrupt register (UART_IER) under the port->lock with the pattern: read, clear, restore. Since some startup() callbacks are modifying UART_IER without the port->lock locked, it is possible that the value intended to be written by the startup() callback will get overwritten and be lost. CPU0 CPU1 serial8250_console_write omap_8250_startup -------------------------- ----------------- spin_lock(port->lock) oldval = read(UART_IER) uart_console_write() write(newval, UART_IER) write(oldval, UART_IER) spin_unlock(port->lock) Add port->lock synchronization to the 8250 startup() callbacks where they need to access UART_IER. This avoids racing with serial8250_console_write(). Signed-off-by: John Ogness Link: https://lore.kernel.org/r/20230525093159.223817-2-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_bcm7271.c | 4 ++++ drivers/tty/serial/8250/8250_exar.c | 4 ++++ drivers/tty/serial/8250/8250_omap.c | 3 +++ drivers/tty/serial/8250/8250_port.c | 18 ++++++++++++++++-- 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c index af0e1c070187..d4b05d7ad9e8 100644 --- a/drivers/tty/serial/8250/8250_bcm7271.c +++ b/drivers/tty/serial/8250/8250_bcm7271.c @@ -605,9 +605,13 @@ static int brcmuart_startup(struct uart_port *port) /* * Disable the Receive Data Interrupt because the DMA engine * will handle this. + * + * Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); up->ier &= ~UART_IER_RDI; serial_port_out(port, UART_IER, up->ier); + spin_unlock_irq(&port->lock); priv->tx_running = false; priv->dma.rx_dma = NULL; diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c index b406cba10b0e..077c3ba3539e 100644 --- a/drivers/tty/serial/8250/8250_exar.c +++ b/drivers/tty/serial/8250/8250_exar.c @@ -198,8 +198,12 @@ static int xr17v35x_startup(struct uart_port *port) /* * Make sure all interrups are masked until initialization is * complete and the FIFOs are cleared + * + * Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); serial_port_out(port, UART_IER, 0); + spin_unlock_irq(&port->lock); return serial8250_do_startup(port); } diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 5c093dfcee1d..fbca0692aa51 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -710,8 +710,11 @@ static int omap_8250_startup(struct uart_port *port) up->dma = NULL; } + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); up->ier = UART_IER_RLSI | UART_IER_RDI; serial_out(up, UART_IER, up->ier); + spin_unlock_irq(&port->lock); #ifdef CONFIG_PM up->capabilities |= UART_CAP_RPM; diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 2d8f87f4e40f..ecfcdc02eeaf 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -2150,7 +2150,12 @@ int serial8250_do_startup(struct uart_port *port) serial8250_rpm_get(up); if (port->type == PORT_16C950) { - /* Wake up and initialize UART */ + /* + * Wake up and initialize UART + * + * Synchronize UART_IER access against the console. + */ + spin_lock_irqsave(&port->lock, flags); up->acr = 0; serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B); serial_port_out(port, UART_EFR, UART_EFR_ECB); @@ -2160,12 +2165,19 @@ int serial8250_do_startup(struct uart_port *port) serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B); serial_port_out(port, UART_EFR, UART_EFR_ECB); serial_port_out(port, UART_LCR, 0); + spin_unlock_irqrestore(&port->lock, flags); } if (port->type == PORT_DA830) { - /* Reset the port */ + /* + * Reset the port + * + * Synchronize UART_IER access against the console. + */ + spin_lock_irqsave(&port->lock, flags); serial_port_out(port, UART_IER, 0); serial_port_out(port, UART_DA830_PWREMU_MGMT, 0); + spin_unlock_irqrestore(&port->lock, flags); mdelay(10); /* Enable Tx, Rx and free run mode */ @@ -2276,6 +2288,8 @@ int serial8250_do_startup(struct uart_port *port) * this interrupt whenever the transmitter is idle and * the interrupt is enabled. Delays are necessary to * allow register changes to become visible. + * + * Synchronize UART_IER access against the console. */ spin_lock_irqsave(&port->lock, flags); -- cgit v1.2.3 From abcb0cf1f5b2d99b1d117a4dbce334120e358d6d Mon Sep 17 00:00:00 2001 From: John Ogness Date: Thu, 25 May 2023 11:37:53 +0206 Subject: serial: core: lock port for stop_rx() in uart_suspend_port() The uarts_ops stop_rx() callback expects that the port->lock is taken and interrupts are disabled. Fixes: c9d2325cdb92 ("serial: core: Do stop_rx in suspend path for console if console_suspend is disabled") Signed-off-by: John Ogness Reviewed-by: Douglas Anderson Link: https://lore.kernel.org/r/20230525093159.223817-3-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial_core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 4b98d13555c0..37ad53616372 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2333,8 +2333,11 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport) * able to Re-start_rx later. */ if (!console_suspend_enabled && uart_console(uport)) { - if (uport->ops->start_rx) + if (uport->ops->start_rx) { + spin_lock_irq(&uport->lock); uport->ops->stop_rx(uport); + spin_unlock_irq(&uport->lock); + } goto unlock; } -- cgit v1.2.3 From ca73a892c5bec4b08a2fa22b3015e98ed905abb7 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Thu, 25 May 2023 11:37:54 +0206 Subject: serial: 8250: lock port for stop_rx() in omap8250_irq() The uarts_ops stop_rx() callback expects that the port->lock is taken and interrupts are disabled. Fixes: 1fe0e1fa3209 ("serial: 8250_omap: Handle optional overrun-throttle-ms property") Signed-off-by: John Ogness Reviewed-by: Tony Lindgren Link: https://lore.kernel.org/r/20230525093159.223817-4-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index fbca0692aa51..c17d98161d5e 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -658,7 +658,9 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id) up->ier = port->serial_in(port, UART_IER); if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) { + spin_lock(&port->lock); port->ops->stop_rx(port); + spin_unlock(&port->lock); } else { /* Keep restarting the timer until * the input overrun subsides. -- cgit v1.2.3 From 51e45fba14bf08b66bca764a083c7f2e2ff62f01 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Thu, 25 May 2023 11:37:55 +0206 Subject: serial: core: lock port for start_rx() in uart_resume_port() The only user of the start_rx() callback (qcom_geni) directly calls its own stop_rx() callback. Since stop_rx() requires that the port->lock is taken and interrupts are disabled, the start_rx() callback has the same requirement. Fixes: cfab87c2c271 ("serial: core: Introduce callback for start_rx and do stop_rx in suspend only if this callback implementation is present.") Signed-off-by: John Ogness Reviewed-by: Douglas Anderson Link: https://lore.kernel.org/r/20230525093159.223817-5-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial_core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 37ad53616372..f856c7fae2fd 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2430,8 +2430,11 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport) if (console_suspend_enabled) uart_change_pm(state, UART_PM_STATE_ON); uport->ops->set_termios(uport, &termios, NULL); - if (!console_suspend_enabled && uport->ops->start_rx) + if (!console_suspend_enabled && uport->ops->start_rx) { + spin_lock_irq(&uport->lock); uport->ops->start_rx(uport); + spin_unlock_irq(&uport->lock); + } if (console_suspend_enabled) console_start(uport->cons); } -- cgit v1.2.3 From 87660fb4041f58d5fdfa58f4d3d35385dcc8320d Mon Sep 17 00:00:00 2001 From: John Ogness Date: Thu, 25 May 2023 11:37:56 +0206 Subject: serial: 8250: lock port for rx_dma() callback The rx_dma() callback (omap_8250_rx_dma) accesses UART_IER. This register is modified twice by each console write (serial8250_console_write()) under the port lock. However, not all calls to the rx_dma() callback are under the port lock. Add the missing port locking around rx_dma() callback calls. Add lockdep notation to the omap_8250_rx_dma(). Note that this is not fixing a real problem because: 1. Currently DMA is forced off for 8250_omap consoles. 2. The serial devices are resumed before console printing is enabled. However, adding this locking allows for clean locking semantics for the rx_dma() callback so that lockdep can be used to identify possible problems in the future. It also simplifies synchronization of UART_IER in general by not needing to rely on implementation details such as 1 and 2. Signed-off-by: John Ogness Link: https://lore.kernel.org/r/20230525093159.223817-6-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index c17d98161d5e..3cb9cfa62331 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -728,8 +728,11 @@ static int omap_8250_startup(struct uart_port *port) priv->wer |= OMAP_UART_TX_WAKEUP_EN; serial_out(up, UART_OMAP_WER, priv->wer); - if (up->dma && !(priv->habit & UART_HAS_EFR2)) + if (up->dma && !(priv->habit & UART_HAS_EFR2)) { + spin_lock_irq(&port->lock); up->dma->rx_dma(up); + spin_unlock_irq(&port->lock); + } enable_irq(up->port.irq); @@ -1003,6 +1006,9 @@ static int omap_8250_rx_dma(struct uart_8250_port *p) unsigned long flags; u32 reg; + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&p->port.lock); + if (priv->rx_dma_broken) return -EINVAL; @@ -1736,8 +1742,11 @@ static int omap8250_runtime_resume(struct device *dev) if (up && omap8250_lost_context(up)) omap8250_restore_regs(up); - if (up && up->dma && up->dma->rxchan && !(priv->habit & UART_HAS_EFR2)) + if (up && up->dma && up->dma->rxchan && !(priv->habit & UART_HAS_EFR2)) { + spin_lock_irq(&up->port.lock); omap_8250_rx_dma(up); + spin_unlock_irq(&up->port.lock); + } priv->latency = priv->calc_latency; schedule_work(&priv->qos_work); -- cgit v1.2.3 From 8b45503776b6e2c53abbb217970236ed54bd261b Mon Sep 17 00:00:00 2001 From: John Ogness Date: Thu, 25 May 2023 11:37:57 +0206 Subject: serial: 8250: lock port for omap8250_restore_regs() omap8250_restore_regs() accesses UART_IER. This register is modified twice by each console write (serial8250_console_write()) under the port lock. However, not all calls to omap8250_restore_regs() are under the port lock. Add the missing port locking around omap8250_restore_regs() calls. Add lockdep notation to the omap8250_restore_regs(). Note that this is not fixing a real problem because the serial devices are resumed before console printing is enabled. However, adding this locking allows for clean locking semantics for omap8250_restore_regs() so that lockdep can be used to identify possible problems in the future. It also simplifies synchronization of UART_IER in general by not needing to rely on such implementation details. Signed-off-by: John Ogness Link: https://lore.kernel.org/r/20230525093159.223817-7-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 3cb9cfa62331..34939462fd69 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -309,6 +309,9 @@ static void omap8250_restore_regs(struct uart_8250_port *up) struct uart_8250_dma *dma = up->dma; u8 mcr = serial8250_in_MCR(up); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + if (dma && dma->tx_running) { /* * TCSANOW requests the change to occur immediately however if @@ -1739,8 +1742,11 @@ static int omap8250_runtime_resume(struct device *dev) if (priv->line >= 0) up = serial8250_get_port(priv->line); - if (up && omap8250_lost_context(up)) + if (up && omap8250_lost_context(up)) { + spin_lock_irq(&up->port.lock); omap8250_restore_regs(up); + spin_unlock_irq(&up->port.lock); + } if (up && up->dma && up->dma->rxchan && !(priv->habit & UART_HAS_EFR2)) { spin_lock_irq(&up->port.lock); -- cgit v1.2.3 From 25614735a647693c1260f253dc3ab32127697806 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Thu, 25 May 2023 11:37:58 +0206 Subject: serial: 8250: lock port for UART_IER access in omap8250_irq() omap8250_irq() accesses UART_IER. This register is modified twice by each console write (serial8250_console_write()) under the port lock. omap8250_irq() must also take the port lock to guanentee synchronized access to UART_IER. Since the port lock is already being taken for the stop_rx() callback and since it is safe to call cancel_delayed_work() while holding the port lock, simply extend the port lock region to include UART_IER access. Fixes: 1fe0e1fa3209 ("serial: 8250_omap: Handle optional overrun-throttle-ms property") Signed-off-by: John Ogness Reviewed-by: Tony Lindgren Link: https://lore.kernel.org/r/20230525093159.223817-8-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 34939462fd69..3225c95fde1d 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -659,17 +659,18 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id) if ((lsr & UART_LSR_OE) && up->overrun_backoff_time_ms > 0) { unsigned long delay; + /* Synchronize UART_IER access against the console. */ + spin_lock(&port->lock); up->ier = port->serial_in(port, UART_IER); if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) { - spin_lock(&port->lock); port->ops->stop_rx(port); - spin_unlock(&port->lock); } else { /* Keep restarting the timer until * the input overrun subsides. */ cancel_delayed_work(&up->overrun_backoff); } + spin_unlock(&port->lock); delay = msecs_to_jiffies(up->overrun_backoff_time_ms); schedule_delayed_work(&up->overrun_backoff, delay); -- cgit v1.2.3 From d0b309a5d3f4648fd4fb1a255a7ae9fb98d87be3 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Thu, 25 May 2023 11:37:59 +0206 Subject: serial: 8250: synchronize and annotate UART_IER access The UART_IER register is modified twice by each console write (serial8250_console_write()) under the port lock. Any driver code that accesses UART_IER must do so with the port locked in order to ensure consistent values, even when for read accesses. Add locking, lockdep notation, and/or comments everywhere UART_IER is accessed. The added locking is not fixing a real problem because it occurs where the console is not active. However, adding the locking to these non-critical paths greatly simplifies UART_IER access tracking by establishing a general policy that all UART_IER access is performed with the port locked. Signed-off-by: John Ogness Link: https://lore.kernel.org/r/20230525093159.223817-9-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250.h | 6 ++++ drivers/tty/serial/8250/8250_aspeed_vuart.c | 3 ++ drivers/tty/serial/8250/8250_mtk.c | 9 +++++ drivers/tty/serial/8250/8250_omap.c | 14 ++++++++ drivers/tty/serial/8250/8250_port.c | 53 +++++++++++++++++++++++++++++ 5 files changed, 85 insertions(+) diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index 5418708f4631..471c6bc5f78f 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -179,6 +179,9 @@ static inline void serial_dl_write(struct uart_8250_port *up, u32 value) static inline bool serial8250_set_THRI(struct uart_8250_port *up) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + if (up->ier & UART_IER_THRI) return false; up->ier |= UART_IER_THRI; @@ -188,6 +191,9 @@ static inline bool serial8250_set_THRI(struct uart_8250_port *up) static inline bool serial8250_clear_THRI(struct uart_8250_port *up) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + if (!(up->ier & UART_IER_THRI)) return false; up->ier &= ~UART_IER_THRI; diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c index 9d2a7856784f..4a9e71b2dbbc 100644 --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c @@ -275,6 +275,9 @@ static void __aspeed_vuart_set_throttle(struct uart_8250_port *up, { unsigned char irqs = UART_IER_RLSI | UART_IER_RDI; + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + up->ier &= ~irqs; if (!throttle) up->ier |= irqs; diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index fb1d5ec0940e..aa8e98164d68 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -222,11 +222,17 @@ static void mtk8250_shutdown(struct uart_port *port) static void mtk8250_disable_intrs(struct uart_8250_port *up, int mask) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + serial_out(up, UART_IER, serial_in(up, UART_IER) & (~mask)); } static void mtk8250_enable_intrs(struct uart_8250_port *up, int mask) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + serial_out(up, UART_IER, serial_in(up, UART_IER) | mask); } @@ -235,6 +241,9 @@ static void mtk8250_set_flow_ctrl(struct uart_8250_port *up, int mode) struct uart_port *port = &up->port; int lcr = serial_in(up, UART_LCR); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); serial_out(up, MTK_UART_EFR, UART_EFR_ECB); serial_out(up, UART_LCR, lcr); diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 3225c95fde1d..0498b9b0e4e9 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -533,6 +533,10 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state, u8 efr; pm_runtime_get_sync(port->dev); + + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); efr = serial_in(up, UART_EFR); serial_out(up, UART_EFR, efr | UART_EFR_ECB); @@ -543,6 +547,8 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state, serial_out(up, UART_EFR, efr); serial_out(up, UART_LCR, 0); + spin_unlock_irq(&port->lock); + pm_runtime_mark_last_busy(port->dev); pm_runtime_put_autosuspend(port->dev); } @@ -760,8 +766,11 @@ static void omap_8250_shutdown(struct uart_port *port) if (priv->habit & UART_HAS_EFR2) serial_out(up, UART_OMAP_EFR2, 0x0); + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); up->ier = 0; serial_out(up, UART_IER, 0); + spin_unlock_irq(&port->lock); disable_irq_nosync(up->port.irq); dev_pm_clear_wake_irq(port->dev); @@ -803,6 +812,7 @@ static void omap_8250_unthrottle(struct uart_port *port) pm_runtime_get_sync(port->dev); + /* Synchronize UART_IER access against the console. */ spin_lock_irqsave(&port->lock, flags); priv->throttled = false; if (up->dma) @@ -953,6 +963,7 @@ static void __dma_rx_complete(void *param) struct dma_tx_state state; unsigned long flags; + /* Synchronize UART_IER access against the console. */ spin_lock_irqsave(&p->port.lock, flags); /* @@ -1227,6 +1238,9 @@ static u16 omap_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir, u16 status static void am654_8250_handle_rx_dma(struct uart_8250_port *up, u8 iir, u16 status) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + /* * Queue a new transfer if FIFO has data. */ diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index ecfcdc02eeaf..1321bb42a074 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -539,6 +539,9 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put); */ static int serial8250_em485_init(struct uart_8250_port *p) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&p->port.lock); + if (p->em485) goto deassert_rts; @@ -677,6 +680,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial8250_rpm_get(p); if (p->capabilities & UART_CAP_SLEEP) { + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&p->port.lock); if (p->capabilities & UART_CAP_EFR) { lcr = serial_in(p, UART_LCR); efr = serial_in(p, UART_EFR); @@ -690,6 +695,7 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) serial_out(p, UART_EFR, efr); serial_out(p, UART_LCR, lcr); } + spin_unlock_irq(&p->port.lock); } serial8250_rpm_put(p); @@ -697,6 +703,9 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep) static void serial8250_clear_IER(struct uart_8250_port *up) { + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + if (up->capabilities & UART_CAP_UUE) serial_out(up, UART_IER, UART_IER_UUE); else @@ -969,6 +978,9 @@ static void autoconfig_16550a(struct uart_8250_port *up) unsigned char status1, status2; unsigned int iersave; + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&up->port.lock); + up->port.type = PORT_16550A; up->capabilities |= UART_CAP_FIFO; @@ -1152,6 +1164,8 @@ static void autoconfig(struct uart_8250_port *up) /* * We really do need global IRQs disabled here - we're going to * be frobbing the chips IRQ enable register to see if it exists. + * + * Synchronize UART_IER access against the console. */ spin_lock_irqsave(&port->lock, flags); @@ -1324,7 +1338,10 @@ static void autoconfig_irq(struct uart_8250_port *up) /* forget possible initially masked and pending IRQ */ probe_irq_off(probe_irq_on()); save_mcr = serial8250_in_MCR(up); + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); save_ier = serial_in(up, UART_IER); + spin_unlock_irq(&port->lock); serial8250_out_MCR(up, UART_MCR_OUT1 | UART_MCR_OUT2); irqs = probe_irq_on(); @@ -1336,7 +1353,10 @@ static void autoconfig_irq(struct uart_8250_port *up) serial8250_out_MCR(up, UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2); } + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); serial_out(up, UART_IER, UART_IER_ALL_INTR); + spin_unlock_irq(&port->lock); serial_in(up, UART_LSR); serial_in(up, UART_RX); serial_in(up, UART_IIR); @@ -1346,7 +1366,10 @@ static void autoconfig_irq(struct uart_8250_port *up) irq = probe_irq_off(irqs); serial8250_out_MCR(up, save_mcr); + /* Synchronize UART_IER access against the console. */ + spin_lock_irq(&port->lock); serial_out(up, UART_IER, save_ier); + spin_unlock_irq(&port->lock); if (port->flags & UPF_FOURPORT) outb_p(save_ICP, ICP); @@ -1361,6 +1384,9 @@ static void serial8250_stop_rx(struct uart_port *port) { struct uart_8250_port *up = up_to_u8250p(port); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + serial8250_rpm_get(up); up->ier &= ~(UART_IER_RLSI | UART_IER_RDI); @@ -1380,6 +1406,9 @@ void serial8250_em485_stop_tx(struct uart_8250_port *p) { unsigned char mcr = serial8250_in_MCR(p); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&p->port.lock); + if (p->port.rs485.flags & SER_RS485_RTS_AFTER_SEND) mcr |= UART_MCR_RTS; else @@ -1429,6 +1458,9 @@ static void __stop_tx_rs485(struct uart_8250_port *p, u64 stop_delay) { struct uart_8250_em485 *em485 = p->em485; + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&p->port.lock); + stop_delay += (u64)p->port.rs485.delay_rts_after_send * NSEC_PER_MSEC; /* @@ -1608,6 +1640,9 @@ static void serial8250_start_tx(struct uart_port *port) struct uart_8250_port *up = up_to_u8250p(port); struct uart_8250_em485 *em485 = up->em485; + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + if (!port->x_char && uart_circ_empty(&port->state->xmit)) return; @@ -1635,6 +1670,9 @@ static void serial8250_disable_ms(struct uart_port *port) { struct uart_8250_port *up = up_to_u8250p(port); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + /* no MSR capabilities */ if (up->bugs & UART_BUG_NOMSR) return; @@ -1649,6 +1687,9 @@ static void serial8250_enable_ms(struct uart_port *port) { struct uart_8250_port *up = up_to_u8250p(port); + /* Port locked to synchronize UART_IER access against the console. */ + lockdep_assert_held_once(&port->lock); + /* no MSR capabilities */ if (up->bugs & UART_BUG_NOMSR) return; @@ -2105,6 +2146,14 @@ static void serial8250_put_poll_char(struct uart_port *port, unsigned int ier; struct uart_8250_port *up = up_to_u8250p(port); + /* + * Normally the port is locked to synchronize UART_IER access + * against the console. However, this function is only used by + * KDB/KGDB, where it may not be possible to acquire the port + * lock because all other CPUs are quiesced. The quiescence + * should allow safe lockless usage here. + */ + serial8250_rpm_get(up); /* * First save the IER then disable the interrupts @@ -2440,6 +2489,8 @@ void serial8250_do_shutdown(struct uart_port *port) serial8250_rpm_get(up); /* * Disable interrupts from this port + * + * Synchronize UART_IER access against the console. */ spin_lock_irqsave(&port->lock, flags); up->ier = 0; @@ -2739,6 +2790,8 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios, /* * Ok, we're now changing the port state. Do it with * interrupts disabled. + * + * Synchronize UART_IER access against the console. */ serial8250_rpm_get(up); spin_lock_irqsave(&port->lock, flags); -- cgit v1.2.3 From 0d07703be74fec50eb09213aba69aa33a2631669 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Thu, 25 May 2023 23:01:47 +0200 Subject: serial: Switch i2c drivers back to use .probe() After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new() call-back type"), all drivers being converted to .probe_new() and then 03c835f498b5 ("i2c: Switch .probe() to not take an id parameter") convert back to (the new) .probe() to be able to eventually drop .probe_new() from struct i2c_driver. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230525210147.734737-1-u.kleine-koenig@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/max310x.c | 2 +- drivers/tty/serial/sc16is7xx.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c index 9fee722058f4..997e39449766 100644 --- a/drivers/tty/serial/max310x.c +++ b/drivers/tty/serial/max310x.c @@ -1636,7 +1636,7 @@ static struct i2c_driver max310x_i2c_driver = { .of_match_table = max310x_dt_ids, .pm = &max310x_pm_ops, }, - .probe_new = max310x_i2c_probe, + .probe = max310x_i2c_probe, .remove = max310x_i2c_remove, }; #endif diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index abad091baeea..2e7e7c409cf2 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -1709,7 +1709,7 @@ static struct i2c_driver sc16is7xx_i2c_uart_driver = { .name = SC16IS7XX_NAME, .of_match_table = sc16is7xx_dt_ids, }, - .probe_new = sc16is7xx_i2c_probe, + .probe = sc16is7xx_i2c_probe, .remove = sc16is7xx_i2c_remove, .id_table = sc16is7xx_i2c_id_table, }; -- cgit v1.2.3 From 20ec397d694b05d3e7150e3ba15047c53e7e6b94 Mon Sep 17 00:00:00 2001 From: Sherry Sun Date: Mon, 22 May 2023 10:51:11 +0800 Subject: tty: serial: fsl_lpuart: Check the return value of dmaengine_tx_status Coverity reports the Unchecked return value (CHECKED_RETURN) warning: Calling dmaengine_tx_status without checking return value. So here add the return value check for dmaengine_tx_status() function to make coverity happy. Fixes: cf9aa72d2f91 ("tty: serial: fsl_lpuart: optimize the timer based EOP logic") Signed-off-by: Sherry Sun Link: https://lore.kernel.org/r/20230522025111.3747-1-sherry.sun@nxp.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/fsl_lpuart.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index 0e56fa64b4ce..0718c9f48d21 100644 --- a/drivers/tty/serial/fsl_lpuart.c +++ b/drivers/tty/serial/fsl_lpuart.c @@ -1286,13 +1286,19 @@ static void lpuart_dma_rx_complete(void *arg) static void lpuart_timer_func(struct timer_list *t) { struct lpuart_port *sport = from_timer(sport, t, lpuart_timer); + enum dma_status dmastat; struct dma_chan *chan = sport->dma_rx_chan; struct circ_buf *ring = &sport->rx_ring; struct dma_tx_state state; unsigned long flags; int count; - dmaengine_tx_status(chan, sport->dma_rx_cookie, &state); + dmastat = dmaengine_tx_status(chan, sport->dma_rx_cookie, &state); + if (dmastat == DMA_ERROR) { + dev_err(sport->port.dev, "Rx DMA transfer failed!\n"); + return; + } + ring->head = sport->rx_sgl.length - state.residue; count = CIRC_CNT(ring->head, ring->tail, sport->rx_sgl.length); -- cgit v1.2.3 From 1b997aef4f54a8c590dbef54fbcb84ad9babc496 Mon Sep 17 00:00:00 2001 From: James Hilliard Date: Sun, 21 May 2023 01:50:44 -0600 Subject: serial: Indicate fintek option may also be required for RS232 support The current config comment for SERIAL_8250_FINTEK implies that this option is only needed when one wants to support RS485. As it turns out we also need to enable this option for RS232 support to function correctly on some variants. For example for variants such as the F71869AD attempting to use multiple RS232 ports simultaneously without this option enabled can result in data corruption. Signed-off-by: James Hilliard Link: https://lore.kernel.org/r/20230521075046.3539376-1-james.hilliard1@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/Kconfig | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 5313aa31930f..cf33e858b0be 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -71,14 +71,16 @@ config SERIAL_8250_16550A_VARIANTS console, you can say N here. config SERIAL_8250_FINTEK - bool "Support for Fintek F81216A LPC to 4 UART RS485 API" + bool "Support for Fintek variants" depends on SERIAL_8250 help - Selecting this option will add support for the RS485 capabilities - of the Fintek F81216A LPC to 4 UART. + Selecting this option will add support for the RS232 and RS485 + capabilities of the Fintek F81216A LPC to 4 UART as well similar + variants. If this option is not selected the device will be configured as a - standard 16550A serial port. + standard 16550A serial port, however the device may not function + correctly without this option enabled. If unsure, say N. -- cgit v1.2.3 From d2d4bd217ccd474cfe70bba7bb1b26563249679d Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Wed, 24 May 2023 14:27:52 +0200 Subject: serial: 8250-fsl: Expand description of the MPC83xx UART's misbehaviour After working quite a bit on erratic behaviour of the MPC83xx UART I (think I) understood the problem. Expand the description accoringly to conserve the knowledge for the future. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230524122754.481816-2-u.kleine-koenig@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_fsl.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c index 8adfaa183f77..00f46b9a8b09 100644 --- a/drivers/tty/serial/8250/8250_fsl.c +++ b/drivers/tty/serial/8250/8250_fsl.c @@ -38,7 +38,19 @@ int fsl8250_handle_irq(struct uart_port *port) return 0; } - /* This is the WAR; if last event was BRK, then read and return */ + /* + * For a single break the hardware reports LSR.BI for each character + * time. This is described in the MPC8313E chip errata as "General17". + * A typical break has a duration of 0.3s, with a 115200n8 configuration + * that (theoretically) corresponds to ~3500 interrupts in these 0.3s. + * In practise it's less (around 500) because of hardware + * and software latencies. The workaround recommended by the vendor is + * to read the RX register (to clear LSR.DR and thus prevent a FIFO + * aging interrupt). To prevent the irq from retriggering LSR must not be + * read. (This would clear LSR.BI, hardware would reassert the BI event + * immediately and interrupt the CPU again. The hardware clears LSR.BI + * when the next valid char is read.) + */ if (unlikely(up->lsr_saved_flags & UART_LSR_BI)) { up->lsr_saved_flags &= ~UART_LSR_BI; port->serial_in(port, UART_RX); -- cgit v1.2.3 From c53aab20762255ee03e65dd66b3cba3887ad39d1 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 15 May 2023 08:57:06 +0200 Subject: serial: 8250: omap: Move uart_write() inside PM section If CONFIG_PM is not set (e.g. m68k/allmodconfig): drivers/tty/serial/8250/8250_omap.c:169:13: error: ‘uart_write’ defined but not used [-Werror=unused-function] 169 | static void uart_write(struct omap8250_priv *priv, u32 reg, u32 val) | ^~~~~~~~~~ Fix tis by moving uart_write() inside the existing section protected by #ifdef CONFIG_PM. Reported-by: noreply@ellerman.id.au Link: http://kisskb.ellerman.id.au/kisskb/buildresult/14925095/ Fixes: 398cecc24846e867 ("serial: 8250: omap: Fix imprecise external abort for omap_8250_pm()") Signed-off-by: Geert Uytterhoeven Reviewed-by: Tony Lindgren Link: https://lore.kernel.org/r/20230515065706.1723477-1-geert@linux-m68k.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 0498b9b0e4e9..af47002c3cb3 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -166,11 +166,6 @@ static u32 uart_read(struct omap8250_priv *priv, u32 reg) 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)); -} - /* * Called on runtime PM resume path from omap8250_restore_regs(), and * omap8250_set_mctrl(). @@ -1666,6 +1661,11 @@ static int omap8250_lost_context(struct uart_8250_port *up) return 0; } +static void uart_write(struct omap8250_priv *priv, u32 reg, u32 val) +{ + writel(val, priv->membase + (reg << OMAP_UART_REGSHIFT)); +} + /* TODO: in future, this should happen via API in drivers/reset/ */ static int omap8250_soft_reset(struct device *dev) { -- cgit v1.2.3 From ae62c49c0ceff20dc7c1fad4a5b8f91d64b4f628 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 17 May 2023 22:20:07 +0200 Subject: serial: 8250: omap: convert to modern PM ops The new uart_write() function is only called from suspend/resume code, causing a build warning when those are left out: drivers/tty/serial/8250/8250_omap.c:169:13: error: 'uart_write' defined but not used [-Werror=unused-function] Remove the #ifdefs and use the modern pm_ops/pm_sleep_ops and their wrappers to let the compiler see where it's used but still drop the dead code. Fixes: 398cecc24846 ("serial: 8250: omap: Fix imprecise external abort for omap_8250_pm()") Signed-off-by: Arnd Bergmann Reviewed-by: Tony Lindgren Link: https://lore.kernel.org/r/20230517202012.634386-1-arnd@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index af47002c3cb3..fe26ebc7b834 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -1595,7 +1595,6 @@ static int omap8250_remove(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM_SLEEP static int omap8250_prepare(struct device *dev) { struct omap8250_priv *priv = dev_get_drvdata(dev); @@ -1640,12 +1639,7 @@ static int omap8250_resume(struct device *dev) serial8250_resume_port(priv->line); return 0; } -#else -#define omap8250_prepare NULL -#define omap8250_complete NULL -#endif -#ifdef CONFIG_PM static int omap8250_lost_context(struct uart_8250_port *up) { u32 val; @@ -1773,7 +1767,6 @@ static int omap8250_runtime_resume(struct device *dev) schedule_work(&priv->qos_work); return 0; } -#endif #ifdef CONFIG_SERIAL_8250_OMAP_TTYO_FIXUP static int __init omap8250_console_fixup(void) @@ -1816,17 +1809,17 @@ console_initcall(omap8250_console_fixup); #endif static const struct dev_pm_ops omap8250_dev_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(omap8250_suspend, omap8250_resume) - SET_RUNTIME_PM_OPS(omap8250_runtime_suspend, + SYSTEM_SLEEP_PM_OPS(omap8250_suspend, omap8250_resume) + RUNTIME_PM_OPS(omap8250_runtime_suspend, omap8250_runtime_resume, NULL) - .prepare = omap8250_prepare, - .complete = omap8250_complete, + .prepare = pm_sleep_ptr(omap8250_prepare), + .complete = pm_sleep_ptr(omap8250_complete), }; static struct platform_driver omap8250_platform_driver = { .driver = { .name = "omap8250", - .pm = &omap8250_dev_pm_ops, + .pm = pm_ptr(&omap8250_dev_pm_ops), .of_match_table = omap8250_dt_ids, }, .probe = omap8250_probe, -- cgit v1.2.3 From 84a9582fd203063cd4d301204971ff2cd8327f1a Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Thu, 25 May 2023 14:30:30 +0300 Subject: serial: core: Start managing serial controllers to enable runtime PM We want to enable runtime PM for serial port device drivers in a generic way. To do this, we want to have the serial core layer manage the registered physical serial controller devices. To manage serial controllers, let's set up a struct bus and struct device for the serial core controller as suggested by Greg and Jiri. The serial core controller devices are children of the physical serial port device. The serial core controller device is needed to support multiple different kind of ports connected to single physical serial port device. Let's also set up a struct device for the serial core port. The serial core port instances are children of the serial core controller device. With the serial core port device we can now flush pending TX on the runtime PM resume as suggested by Johan. Suggested-by: Andy Shevchenko Suggested-by: Greg Kroah-Hartman Suggested-by: Jiri Slaby Suggested-by: Johan Hovold Signed-off-by: Tony Lindgren Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20230525113034.46880-1-tony@atomide.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_core.c | 1 + drivers/tty/serial/8250/8250_port.c | 1 + drivers/tty/serial/Makefile | 3 +- drivers/tty/serial/serial_base.h | 46 ++++++++ drivers/tty/serial/serial_base_bus.c | 200 +++++++++++++++++++++++++++++++++++ drivers/tty/serial/serial_core.c | 192 +++++++++++++++++++++++++++++---- drivers/tty/serial/serial_ctrl.c | 68 ++++++++++++ drivers/tty/serial/serial_port.c | 105 ++++++++++++++++++ include/linux/serial_core.h | 5 +- 9 files changed, 598 insertions(+), 23 deletions(-) create mode 100644 drivers/tty/serial/serial_base.h create mode 100644 drivers/tty/serial/serial_base_bus.c create mode 100644 drivers/tty/serial/serial_ctrl.c create mode 100644 drivers/tty/serial/serial_port.c diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 4434c3256a92..914e0e6251bf 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -1039,6 +1039,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up) if (uart->port.dev) uart_remove_one_port(&serial8250_reg, &uart->port); + uart->port.ctrl_id = up->port.ctrl_id; uart->port.iobase = up->port.iobase; uart->port.membase = up->port.membase; uart->port.irq = up->port.irq; diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 1321bb42a074..dfb51a854e77 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -3282,6 +3282,7 @@ void serial8250_init_port(struct uart_8250_port *up) struct uart_port *port = &up->port; spin_lock_init(&port->lock); + port->ctrl_id = 0; port->ops = &serial8250_pops; port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE); diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile index 531ec3a19dae..4f7ab4150ec5 100644 --- a/drivers/tty/serial/Makefile +++ b/drivers/tty/serial/Makefile @@ -3,7 +3,8 @@ # Makefile for the kernel serial device drivers. # -obj-$(CONFIG_SERIAL_CORE) += serial_core.o +obj-$(CONFIG_SERIAL_CORE) += serial_base.o +serial_base-y := serial_core.o serial_base_bus.o serial_ctrl.o serial_port.o obj-$(CONFIG_SERIAL_EARLYCON) += earlycon.o obj-$(CONFIG_SERIAL_EARLYCON_SEMIHOST) += earlycon-semihost.o diff --git a/drivers/tty/serial/serial_base.h b/drivers/tty/serial/serial_base.h new file mode 100644 index 000000000000..9faac0ff6b89 --- /dev/null +++ b/drivers/tty/serial/serial_base.h @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Serial core related functions, serial port device drivers do not need this. + * + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/ + * Author: Tony Lindgren + */ + +#define to_serial_base_ctrl_device(d) container_of((d), struct serial_ctrl_device, dev) +#define to_serial_base_port_device(d) container_of((d), struct serial_port_device, dev) + +struct uart_driver; +struct uart_port; +struct device_driver; +struct device; + +struct serial_ctrl_device { + struct device dev; +}; + +struct serial_port_device { + struct device dev; + struct uart_port *port; +}; + +int serial_base_ctrl_init(void); +void serial_base_ctrl_exit(void); + +int serial_base_port_init(void); +void serial_base_port_exit(void); + +int serial_base_driver_register(struct device_driver *driver); +void serial_base_driver_unregister(struct device_driver *driver); + +struct serial_ctrl_device *serial_base_ctrl_add(struct uart_port *port, + struct device *parent); +struct serial_port_device *serial_base_port_add(struct uart_port *port, + struct serial_ctrl_device *parent); +void serial_base_ctrl_device_remove(struct serial_ctrl_device *ctrl_dev); +void serial_base_port_device_remove(struct serial_port_device *port_dev); + +int serial_ctrl_register_port(struct uart_driver *drv, struct uart_port *port); +void serial_ctrl_unregister_port(struct uart_driver *drv, struct uart_port *port); + +int serial_core_register_port(struct uart_driver *drv, struct uart_port *port); +void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port); diff --git a/drivers/tty/serial/serial_base_bus.c b/drivers/tty/serial/serial_base_bus.c new file mode 100644 index 000000000000..1b37833b8f66 --- /dev/null +++ b/drivers/tty/serial/serial_base_bus.c @@ -0,0 +1,200 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Serial base bus layer for controllers + * + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/ + * Author: Tony Lindgren + * + * The serial core bus manages the serial core controller instances. + */ + +#include +#include +#include +#include +#include +#include + +#include "serial_base.h" + +static int serial_base_match(struct device *dev, struct device_driver *drv) +{ + int len = strlen(drv->name); + + return !strncmp(dev_name(dev), drv->name, len); +} + +static struct bus_type serial_base_bus_type = { + .name = "serial-base", + .match = serial_base_match, +}; + +int serial_base_driver_register(struct device_driver *driver) +{ + driver->bus = &serial_base_bus_type; + + return driver_register(driver); +} + +void serial_base_driver_unregister(struct device_driver *driver) +{ + driver_unregister(driver); +} + +static int serial_base_device_init(struct uart_port *port, + struct device *dev, + struct device *parent_dev, + const struct device_type *type, + void (*release)(struct device *dev), + int id) +{ + device_initialize(dev); + dev->type = type; + dev->parent = parent_dev; + dev->bus = &serial_base_bus_type; + dev->release = release; + + return dev_set_name(dev, "%s.%s.%d", type->name, dev_name(port->dev), id); +} + +static const struct device_type serial_ctrl_type = { + .name = "ctrl", +}; + +static void serial_base_ctrl_release(struct device *dev) +{ + struct serial_ctrl_device *ctrl_dev = to_serial_base_ctrl_device(dev); + + kfree(ctrl_dev); +} + +void serial_base_ctrl_device_remove(struct serial_ctrl_device *ctrl_dev) +{ + if (!ctrl_dev) + return; + + device_del(&ctrl_dev->dev); +} + +struct serial_ctrl_device *serial_base_ctrl_add(struct uart_port *port, + struct device *parent) +{ + struct serial_ctrl_device *ctrl_dev; + int err; + + ctrl_dev = kzalloc(sizeof(*ctrl_dev), GFP_KERNEL); + if (!ctrl_dev) + return ERR_PTR(-ENOMEM); + + err = serial_base_device_init(port, &ctrl_dev->dev, + parent, &serial_ctrl_type, + serial_base_ctrl_release, + port->ctrl_id); + if (err) + goto err_free_ctrl_dev; + + err = device_add(&ctrl_dev->dev); + if (err) + goto err_put_device; + + return ctrl_dev; + +err_put_device: + put_device(&ctrl_dev->dev); +err_free_ctrl_dev: + kfree(ctrl_dev); + + return ERR_PTR(err); +} + +static const struct device_type serial_port_type = { + .name = "port", +}; + +static void serial_base_port_release(struct device *dev) +{ + struct serial_port_device *port_dev = to_serial_base_port_device(dev); + + kfree(port_dev); +} + +struct serial_port_device *serial_base_port_add(struct uart_port *port, + struct serial_ctrl_device *ctrl_dev) +{ + struct serial_port_device *port_dev; + int err; + + port_dev = kzalloc(sizeof(*port_dev), GFP_KERNEL); + if (!port_dev) + return ERR_PTR(-ENOMEM); + + err = serial_base_device_init(port, &port_dev->dev, + &ctrl_dev->dev, &serial_port_type, + serial_base_port_release, + port->line); + if (err) + goto err_free_port_dev; + + port_dev->port = port; + + err = device_add(&port_dev->dev); + if (err) + goto err_put_device; + + return port_dev; + +err_put_device: + put_device(&port_dev->dev); +err_free_port_dev: + kfree(port_dev); + + return ERR_PTR(err); +} + +void serial_base_port_device_remove(struct serial_port_device *port_dev) +{ + if (!port_dev) + return; + + device_del(&port_dev->dev); +} + +static int serial_base_init(void) +{ + int ret; + + ret = bus_register(&serial_base_bus_type); + if (ret) + return ret; + + ret = serial_base_ctrl_init(); + if (ret) + goto err_bus_unregister; + + ret = serial_base_port_init(); + if (ret) + goto err_ctrl_exit; + + return 0; + +err_ctrl_exit: + serial_base_ctrl_exit(); + +err_bus_unregister: + bus_unregister(&serial_base_bus_type); + + return ret; +} +module_init(serial_base_init); + +static void serial_base_exit(void) +{ + serial_base_port_exit(); + serial_base_ctrl_exit(); + bus_unregister(&serial_base_bus_type); +} +module_exit(serial_base_exit); + +MODULE_AUTHOR("Tony Lindgren "); +MODULE_DESCRIPTION("Serial core bus"); +MODULE_LICENSE("GPL"); diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index f856c7fae2fd..29bd5ede0b25 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -31,6 +32,8 @@ #include #include +#include "serial_base.h" + /* * This is used to lock changes in serial line configuration. */ @@ -134,9 +137,30 @@ static void __uart_start(struct tty_struct *tty) { struct uart_state *state = tty->driver_data; struct uart_port *port = state->uart_port; + struct serial_port_device *port_dev; + int err; + + if (!port || port->flags & UPF_DEAD || uart_tx_stopped(port)) + return; + + port_dev = port->port_dev; + + /* Increment the runtime PM usage count for the active check below */ + err = pm_runtime_get(&port_dev->dev); + if (err < 0) { + pm_runtime_put_noidle(&port_dev->dev); + return; + } - if (port && !(port->flags & UPF_DEAD) && !uart_tx_stopped(port)) + /* + * Start TX if enabled, and kick runtime PM. If the device is not + * enabled, serial_port_runtime_resume() calls start_tx() again + * after enabling the device. + */ + if (pm_runtime_active(&port_dev->dev)) port->ops->start_tx(port); + pm_runtime_mark_last_busy(&port_dev->dev); + pm_runtime_put_autosuspend(&port_dev->dev); } static void uart_start(struct tty_struct *tty) @@ -3048,7 +3072,7 @@ static const struct attribute_group tty_dev_attr_group = { }; /** - * uart_add_one_port - attach a driver-defined port structure + * serial_core_add_one_port - attach a driver-defined port structure * @drv: pointer to the uart low level driver structure for this port * @uport: uart port structure to use for this port. * @@ -3057,8 +3081,9 @@ static const struct attribute_group tty_dev_attr_group = { * This allows the driver @drv to register its own uart_port structure with the * core driver. The main purpose is to allow the low level uart drivers to * expand uart_port, rather than having yet more levels of structures. + * Caller must hold port_mutex. */ -int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) +static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *uport) { struct uart_state *state; struct tty_port *port; @@ -3072,7 +3097,6 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) state = drv->state + uport->line; port = &state->port; - mutex_lock(&port_mutex); mutex_lock(&port->mutex); if (state->uart_port) { ret = -EINVAL; @@ -3137,21 +3161,14 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) uport->line); } - /* - * Ensure UPF_DEAD is not set. - */ - uport->flags &= ~UPF_DEAD; - out: mutex_unlock(&port->mutex); - mutex_unlock(&port_mutex); return ret; } -EXPORT_SYMBOL(uart_add_one_port); /** - * uart_remove_one_port - detach a driver defined port structure + * serial_core_remove_one_port - detach a driver defined port structure * @drv: pointer to the uart low level driver structure for this port * @uport: uart port structure for this port * @@ -3159,20 +3176,16 @@ EXPORT_SYMBOL(uart_add_one_port); * * This unhooks (and hangs up) the specified port structure from the core * driver. No further calls will be made to the low-level code for this port. + * Caller must hold port_mutex. */ -void uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport) +static void serial_core_remove_one_port(struct uart_driver *drv, + struct uart_port *uport) { struct uart_state *state = drv->state + uport->line; struct tty_port *port = &state->port; struct uart_port *uart_port; struct tty_struct *tty; - mutex_lock(&port_mutex); - - /* - * Mark the port "dead" - this prevents any opens from - * succeeding while we shut down the port. - */ mutex_lock(&port->mutex); uart_port = uart_port_check(state); if (uart_port != uport) @@ -3183,7 +3196,6 @@ void uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport) mutex_unlock(&port->mutex); goto out; } - uport->flags |= UPF_DEAD; mutex_unlock(&port->mutex); /* @@ -3215,6 +3227,7 @@ void uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport) * Indicate that there isn't a port here anymore. */ uport->type = PORT_UNKNOWN; + uport->port_dev = NULL; mutex_lock(&port->mutex); WARN_ON(atomic_dec_return(&state->refcount) < 0); @@ -3224,7 +3237,6 @@ void uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport) out: mutex_unlock(&port_mutex); } -EXPORT_SYMBOL(uart_remove_one_port); /** * uart_match_port - are the two ports equivalent? @@ -3259,6 +3271,144 @@ bool uart_match_port(const struct uart_port *port1, } EXPORT_SYMBOL(uart_match_port); +static struct serial_ctrl_device * +serial_core_get_ctrl_dev(struct serial_port_device *port_dev) +{ + struct device *dev = &port_dev->dev; + + return to_serial_base_ctrl_device(dev->parent); +} + +/* + * Find a registered serial core controller device if one exists. Returns + * the first device matching the ctrl_id. Caller must hold port_mutex. + */ +static struct serial_ctrl_device *serial_core_ctrl_find(struct uart_driver *drv, + struct device *phys_dev, + int ctrl_id) +{ + struct uart_state *state; + int i; + + lockdep_assert_held(&port_mutex); + + for (i = 0; i < drv->nr; i++) { + state = drv->state + i; + if (!state->uart_port || !state->uart_port->port_dev) + continue; + + if (state->uart_port->dev == phys_dev && + state->uart_port->ctrl_id == ctrl_id) + return serial_core_get_ctrl_dev(state->uart_port->port_dev); + } + + return NULL; +} + +static struct serial_ctrl_device *serial_core_ctrl_device_add(struct uart_port *port) +{ + return serial_base_ctrl_add(port, port->dev); +} + +static int serial_core_port_device_add(struct serial_ctrl_device *ctrl_dev, + struct uart_port *port) +{ + struct serial_port_device *port_dev; + + port_dev = serial_base_port_add(port, ctrl_dev); + if (IS_ERR(port_dev)) + return PTR_ERR(port_dev); + + port->port_dev = port_dev; + + return 0; +} + +/* + * Initialize a serial core port device, and a controller device if needed. + */ +int serial_core_register_port(struct uart_driver *drv, struct uart_port *port) +{ + struct serial_ctrl_device *ctrl_dev, *new_ctrl_dev = NULL; + int ret; + + mutex_lock(&port_mutex); + + /* + * Prevent serial_port_runtime_resume() from trying to use the port + * until serial_core_add_one_port() has completed + */ + port->flags |= UPF_DEAD; + + /* Inititalize a serial core controller device if needed */ + ctrl_dev = serial_core_ctrl_find(drv, port->dev, port->ctrl_id); + if (!ctrl_dev) { + new_ctrl_dev = serial_core_ctrl_device_add(port); + if (!new_ctrl_dev) { + ret = -ENODEV; + goto err_unlock; + } + ctrl_dev = new_ctrl_dev; + } + + /* + * Initialize a serial core port device. Tag the port dead to prevent + * serial_port_runtime_resume() trying to do anything until port has + * been registered. It gets cleared by serial_core_add_one_port(). + */ + ret = serial_core_port_device_add(ctrl_dev, port); + if (ret) + goto err_unregister_ctrl_dev; + + ret = serial_core_add_one_port(drv, port); + if (ret) + goto err_unregister_port_dev; + + port->flags &= ~UPF_DEAD; + + mutex_unlock(&port_mutex); + + return 0; + +err_unregister_port_dev: + serial_base_port_device_remove(port->port_dev); + +err_unregister_ctrl_dev: + serial_base_ctrl_device_remove(new_ctrl_dev); + +err_unlock: + mutex_unlock(&port_mutex); + + return ret; +} + +/* + * Removes a serial core port device, and the related serial core controller + * device if the last instance. + */ +void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port) +{ + struct device *phys_dev = port->dev; + struct serial_port_device *port_dev = port->port_dev; + struct serial_ctrl_device *ctrl_dev = serial_core_get_ctrl_dev(port_dev); + int ctrl_id = port->ctrl_id; + + mutex_lock(&port_mutex); + + port->flags |= UPF_DEAD; + + serial_core_remove_one_port(drv, port); + + /* Note that struct uart_port *port is no longer valid at this point */ + serial_base_port_device_remove(port_dev); + + /* Drop the serial core controller device if no ports are using it */ + if (!serial_core_ctrl_find(drv, phys_dev, ctrl_id)) + serial_base_ctrl_device_remove(ctrl_dev); + + mutex_unlock(&port_mutex); +} + /** * uart_handle_dcd_change - handle a change of carrier detect state * @uport: uart_port structure for the open port diff --git a/drivers/tty/serial/serial_ctrl.c b/drivers/tty/serial/serial_ctrl.c new file mode 100644 index 000000000000..6fcf634425dc --- /dev/null +++ b/drivers/tty/serial/serial_ctrl.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Serial core controller driver + * + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/ + * Author: Tony Lindgren + * + * This driver manages the serial core controller struct device instances. + * The serial core controller devices are children of the physical serial + * port device. + */ + +#include +#include +#include +#include +#include + +#include "serial_base.h" + +static int serial_ctrl_probe(struct device *dev) +{ + pm_runtime_enable(dev); + + return 0; +} + +static int serial_ctrl_remove(struct device *dev) +{ + pm_runtime_disable(dev); + + return 0; +} + +/* + * Serial core controller device init functions. Note that the physical + * serial port device driver may not have completed probe at this point. + */ +int serial_ctrl_register_port(struct uart_driver *drv, struct uart_port *port) +{ + return serial_core_register_port(drv, port); +} + +void serial_ctrl_unregister_port(struct uart_driver *drv, struct uart_port *port) +{ + serial_core_unregister_port(drv, port); +} + +static struct device_driver serial_ctrl_driver = { + .name = "ctrl", + .suppress_bind_attrs = true, + .probe = serial_ctrl_probe, + .remove = serial_ctrl_remove, +}; + +int serial_base_ctrl_init(void) +{ + return serial_base_driver_register(&serial_ctrl_driver); +} + +void serial_base_ctrl_exit(void) +{ + serial_base_driver_unregister(&serial_ctrl_driver); +} + +MODULE_AUTHOR("Tony Lindgren "); +MODULE_DESCRIPTION("Serial core controller driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c new file mode 100644 index 000000000000..862423237007 --- /dev/null +++ b/drivers/tty/serial/serial_port.c @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Serial core port device driver + * + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/ + * Author: Tony Lindgren + */ + +#include +#include +#include +#include +#include + +#include "serial_base.h" + +#define SERIAL_PORT_AUTOSUSPEND_DELAY_MS 500 + +/* Only considers pending TX for now. Caller must take care of locking */ +static int __serial_port_busy(struct uart_port *port) +{ + return !uart_tx_stopped(port) && + uart_circ_chars_pending(&port->state->xmit); +} + +static int serial_port_runtime_resume(struct device *dev) +{ + struct serial_port_device *port_dev = to_serial_base_port_device(dev); + struct uart_port *port; + unsigned long flags; + + port = port_dev->port; + + if (port->flags & UPF_DEAD) + goto out; + + /* Flush any pending TX for the port */ + spin_lock_irqsave(&port->lock, flags); + if (__serial_port_busy(port)) + port->ops->start_tx(port); + spin_unlock_irqrestore(&port->lock, flags); + +out: + pm_runtime_mark_last_busy(dev); + + return 0; +} + +static DEFINE_RUNTIME_DEV_PM_OPS(serial_port_pm, + NULL, serial_port_runtime_resume, NULL); + +static int serial_port_probe(struct device *dev) +{ + pm_runtime_enable(dev); + pm_runtime_set_autosuspend_delay(dev, SERIAL_PORT_AUTOSUSPEND_DELAY_MS); + pm_runtime_use_autosuspend(dev); + + return 0; +} + +static int serial_port_remove(struct device *dev) +{ + pm_runtime_dont_use_autosuspend(dev); + pm_runtime_disable(dev); + + return 0; +} + +/* + * Serial core port device init functions. Note that the physical serial + * port device driver may not have completed probe at this point. + */ +int uart_add_one_port(struct uart_driver *drv, struct uart_port *port) +{ + return serial_ctrl_register_port(drv, port); +} +EXPORT_SYMBOL(uart_add_one_port); + +void uart_remove_one_port(struct uart_driver *drv, struct uart_port *port) +{ + serial_ctrl_unregister_port(drv, port); +} +EXPORT_SYMBOL(uart_remove_one_port); + +static struct device_driver serial_port_driver = { + .name = "port", + .suppress_bind_attrs = true, + .probe = serial_port_probe, + .remove = serial_port_remove, + .pm = pm_ptr(&serial_port_pm), +}; + +int serial_base_port_init(void) +{ + return serial_base_driver_register(&serial_port_driver); +} + +void serial_base_port_exit(void) +{ + serial_base_driver_unregister(&serial_port_driver); +} + +MODULE_AUTHOR("Tony Lindgren "); +MODULE_DESCRIPTION("Serial controller port driver"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index ddcdb5b8523e..6d58c57acdaa 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -28,6 +28,7 @@ struct uart_port; struct serial_struct; +struct serial_port_device; struct device; struct gpio_desc; @@ -458,6 +459,7 @@ struct uart_port { struct serial_rs485 *rs485); int (*iso7816_config)(struct uart_port *, struct serial_iso7816 *iso7816); + int ctrl_id; /* optional serial core controller id */ unsigned int irq; /* irq number */ unsigned long irqflags; /* irq flags */ unsigned int uartclk; /* base uart clock */ @@ -563,7 +565,8 @@ struct uart_port { unsigned int minor; resource_size_t mapbase; /* for ioremap */ resource_size_t mapsize; - struct device *dev; /* parent device */ + struct device *dev; /* serial port physical parent device */ + struct serial_port_device *port_dev; /* serial core port device */ unsigned long sysrq; /* sysrq timeout */ unsigned int sysrq_ch; /* char for sysrq */ -- cgit v1.2.3 From d0a396083e91e3f118c90afa95c91f8c243a6c11 Mon Sep 17 00:00:00 2001 From: Steven Price Date: Thu, 1 Jun 2023 11:55:48 +0100 Subject: serial: core: Don't drop port_mutex in serial_core_remove_one_port Commit 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM") required the caller to hold port_mutex rather than taking it locally. However the mutex_unlock() call wasn't removed causing the mutex to be dropped unexpectly. Remove the call to mutex_unlock() (and fix up the early return) to restore correct behaviour. Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM") Signed-off-by: Steven Price Reviewed-by: Tony Lindgren Link: https://lore.kernel.org/r/20230601105548.29965-1-steven.price@arm.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial_core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 29bd5ede0b25..efda90215ee9 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -3194,7 +3194,7 @@ static void serial_core_remove_one_port(struct uart_driver *drv, if (!uart_port) { mutex_unlock(&port->mutex); - goto out; + return; } mutex_unlock(&port->mutex); @@ -3234,8 +3234,6 @@ static void serial_core_remove_one_port(struct uart_driver *drv, wait_event(state->remove_wait, !atomic_read(&state->refcount)); state->uart_port = NULL; mutex_unlock(&port->mutex); -out: - mutex_unlock(&port_mutex); } /** -- cgit v1.2.3 From 539914240a01c7d68476d10c07b7f4f9a05d38ed Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Thu, 1 Jun 2023 17:14:44 +0300 Subject: serial: core: Fix probing serial_base_bus devices If a physical serial port device driver uses arch_initcall() we fail to probe the serial_base_bus devices and the serial port tx fails. This is because as serial_base_bus uses module_initcall(). Let's fix the issue by changing serial_base_bus to use arch_initcall(). Let's also return an error if a driver attempts to call uart_add_one_port() too early. Reported-by: Marek Szyprowski Tested-by: Marek Szyprowski Closes: https://lore.kernel.org/linux-serial/20230601132012.GB14287@atomide.com/T/#m6a40440fc04d551d27b147da8602e065c982a115 Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM") Signed-off-by: Tony Lindgren Link: https://lore.kernel.org/r/20230601141445.11321-1-tony@atomide.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial_base_bus.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/serial_base_bus.c b/drivers/tty/serial/serial_base_bus.c index 1b37833b8f66..9354af7c11af 100644 --- a/drivers/tty/serial/serial_base_bus.c +++ b/drivers/tty/serial/serial_base_bus.c @@ -17,6 +17,8 @@ #include "serial_base.h" +static bool serial_base_initialized; + static int serial_base_match(struct device *dev, struct device_driver *drv) { int len = strlen(drv->name); @@ -48,6 +50,11 @@ static int serial_base_device_init(struct uart_port *port, void (*release)(struct device *dev), int id) { + if (!serial_base_initialized) { + dev_err(port->dev, "uart_add_one_port() called before arch_initcall()?\n"); + return -EPROBE_DEFER; + } + device_initialize(dev); dev->type = type; dev->parent = parent_dev; @@ -175,6 +182,8 @@ static int serial_base_init(void) if (ret) goto err_ctrl_exit; + serial_base_initialized = true; + return 0; err_ctrl_exit: @@ -185,7 +194,7 @@ err_bus_unregister: return ret; } -module_init(serial_base_init); +arch_initcall(serial_base_init); static void serial_base_exit(void) { -- cgit v1.2.3 From 49c80922ff8115bc86c82e5fc9e6842cd87db689 Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Fri, 2 Jun 2023 10:00:05 +0300 Subject: serial: core: Fix error handling for serial_core_ctrl_device_add() Checking for NULL is incorrect as serial_base_ctrl_add() uses ERR_PTR(). Let's also pass any returned error along, there's no reason to translate all errors to -ENODEV. Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM") Signed-off-by: Tony Lindgren Reviewed-by: Jiri Slaby Link: https://lore.kernel.org/r/20230602070007.59268-1-tony@atomide.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index efda90215ee9..831d033611e6 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -3342,8 +3342,8 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port) ctrl_dev = serial_core_ctrl_find(drv, port->dev, port->ctrl_id); if (!ctrl_dev) { new_ctrl_dev = serial_core_ctrl_device_add(port); - if (!new_ctrl_dev) { - ret = -ENODEV; + if (IS_ERR(new_ctrl_dev)) { + ret = PTR_ERR(new_ctrl_dev); goto err_unlock; } ctrl_dev = new_ctrl_dev; -- cgit v1.2.3 From 66eff0ef528b6d6e9a45b68f6cd969dcbe7b800a Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Mon, 5 Jun 2023 15:08:56 +0200 Subject: powerpc/legacy_serial: Warn about 8250 devices operated without active FSL workarounds If the 8250 driver is built as a module (or built-in without console support) the Freescale specific workaround were silently not activated. Add a warning in this case. Currently CONFIG_SERIAL_8250_FSL=y implies that the function fsl8250_handle_irq() is built-in and can be used. However with the changes of the next commit CONFIG_SERIAL_8250_FSL might be enabled also when the 8250 driver is a module and so more care is needed when fsl8250_handle_irq() is to be used. The code added here is able to handle the new situation already. Signed-off-by: Uwe Kleine-König Message-ID: <20230605130857.85543-2-u.kleine-koenig@pengutronix.de> Signed-off-by: Greg Kroah-Hartman --- arch/powerpc/kernel/legacy_serial.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c index c9ad12461d44..fdbd85aafeb1 100644 --- a/arch/powerpc/kernel/legacy_serial.c +++ b/arch/powerpc/kernel/legacy_serial.c @@ -508,12 +508,16 @@ static void __init fixup_port_irq(int index, port->irq = virq; -#ifdef CONFIG_SERIAL_8250_FSL - if (of_device_is_compatible(np, "fsl,ns16550")) { - port->handle_irq = fsl8250_handle_irq; - port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE); + if (IS_ENABLED(CONFIG_SERIAL_8250) && + of_device_is_compatible(np, "fsl,ns16550")) { + if (IS_REACHABLE(CONFIG_SERIAL_8250)) { + port->handle_irq = fsl8250_handle_irq; + port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE); + } else { + pr_warn_once("Not activating Freescale specific workaround for device %pOFP\n", + np); + } } -#endif } static void __init fixup_port_pio(int index, -- cgit v1.2.3 From 643662d12bc2b38ca693d6707e8da11c4777a09f Mon Sep 17 00:00:00 2001 From: Raphael Gallais-Pou Date: Sun, 4 Jun 2023 10:35:58 +0200 Subject: serial: st-asc: fix typo in property name Changes the property name read in the driver according to the YAML. According to device-tree documentation, property names should not include underscores. Signed-off-by: Raphael Gallais-Pou Reviewed-by: Patrice Chotard Message-ID: <20230604083558.16661-1-rgallaispou@gmail.com> Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/st-asc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c index dc2f2051435c..aa471c9c24d9 100644 --- a/drivers/tty/serial/st-asc.c +++ b/drivers/tty/serial/st-asc.c @@ -754,7 +754,7 @@ static struct asc_port *asc_of_get_asc_port(struct platform_device *pdev) asc_ports[id].hw_flow_control = of_property_read_bool(np, "uart-has-rtscts"); - asc_ports[id].force_m1 = of_property_read_bool(np, "st,force_m1"); + asc_ports[id].force_m1 = of_property_read_bool(np, "st,force-m1"); asc_ports[id].port.line = id; asc_ports[id].rts = NULL; -- cgit v1.2.3 From b6c7ff2693ddc24cdad3b032d969303f7b2ae3da Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Tue, 6 Jun 2023 17:17:45 +0800 Subject: serial: 8250_mtk: Simplify clock sequencing and runtime PM The 8250_mtk driver's runtime PM support has some issues: - The bus clock is enabled (through runtime PM callback) later than a register write - runtime PM resume callback directly called in probe, but no pm_runtime_set_active() call is present - UART PM function calls the callbacks directly, _and_ calls runtime PM API - runtime PM callbacks try to do reference counting, adding yet another count between runtime PM and clocks This fragile setup worked in a way, but broke recently with runtime PM support added to the serial core. The system would hang when the UART console was probed and brought up. Tony provided some potential fixes [1][2], though they were still a bit complicated. The 8250_dw driver, which the 8250_mtk driver might have been based on, has a similar structure but simpler runtime PM usage. Simplify clock sequencing and runtime PM support in the 8250_mtk driver. Specifically, the clock is acquired enabled and assumed to be active, unless toggled through runtime PM suspend/resume. Reference counting is removed and left to the runtime PM core. The serial pm function now only calls the runtime PM API. [1] https://lore.kernel.org/linux-serial/20230602092701.GP14287@atomide.com/ [2] https://lore.kernel.org/linux-serial/20230605061511.GW14287@atomide.com/ Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM") Suggested-by: Tony Lindgren Signed-off-by: Chen-Yu Tsai Reviewed-by: AngeloGioacchino Del Regno Reviewed-by: Tony Lindgren Message-ID: <20230606091747.2031168-1-wenst@chromium.org> Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_mtk.c | 50 ++++++++------------------------------ 1 file changed, 10 insertions(+), 40 deletions(-) diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c index aa8e98164d68..74da5676ce67 100644 --- a/drivers/tty/serial/8250/8250_mtk.c +++ b/drivers/tty/serial/8250/8250_mtk.c @@ -431,12 +431,7 @@ static int __maybe_unused mtk8250_runtime_suspend(struct device *dev) while (serial_in(up, MTK_UART_DEBUG0)); - if (data->clk_count == 0U) { - dev_dbg(dev, "%s clock count is 0\n", __func__); - } else { - clk_disable_unprepare(data->bus_clk); - data->clk_count--; - } + clk_disable_unprepare(data->bus_clk); return 0; } @@ -444,19 +439,8 @@ static int __maybe_unused mtk8250_runtime_suspend(struct device *dev) static int __maybe_unused mtk8250_runtime_resume(struct device *dev) { struct mtk8250_data *data = dev_get_drvdata(dev); - int err; - if (data->clk_count > 0U) { - dev_dbg(dev, "%s clock count is %d\n", __func__, - data->clk_count); - } else { - err = clk_prepare_enable(data->bus_clk); - if (err) { - dev_warn(dev, "Can't enable bus clock\n"); - return err; - } - data->clk_count++; - } + clk_prepare_enable(data->bus_clk); return 0; } @@ -465,14 +449,12 @@ static void mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old) { if (!state) - if (!mtk8250_runtime_resume(port->dev)) - pm_runtime_get_sync(port->dev); + pm_runtime_get_sync(port->dev); serial8250_do_pm(port, state, old); if (state) - if (!pm_runtime_put_sync_suspend(port->dev)) - mtk8250_runtime_suspend(port->dev); + pm_runtime_put_sync_suspend(port->dev); } #ifdef CONFIG_SERIAL_8250_DMA @@ -504,7 +486,7 @@ static int mtk8250_probe_of(struct platform_device *pdev, struct uart_port *p, return 0; } - data->bus_clk = devm_clk_get(&pdev->dev, "bus"); + data->bus_clk = devm_clk_get_enabled(&pdev->dev, "bus"); if (IS_ERR(data->bus_clk)) return PTR_ERR(data->bus_clk); @@ -587,25 +569,16 @@ static int mtk8250_probe(struct platform_device *pdev) platform_set_drvdata(pdev, data); - pm_runtime_enable(&pdev->dev); - err = mtk8250_runtime_resume(&pdev->dev); - if (err) - goto err_pm_disable; - data->line = serial8250_register_8250_port(&uart); - if (data->line < 0) { - err = data->line; - goto err_pm_disable; - } + if (data->line < 0) + return data->line; data->rx_wakeup_irq = platform_get_irq_optional(pdev, 1); - return 0; - -err_pm_disable: - pm_runtime_disable(&pdev->dev); + pm_runtime_set_active(&pdev->dev); + pm_runtime_enable(&pdev->dev); - return err; + return 0; } static int mtk8250_remove(struct platform_device *pdev) @@ -619,9 +592,6 @@ static int mtk8250_remove(struct platform_device *pdev) pm_runtime_disable(&pdev->dev); pm_runtime_put_noidle(&pdev->dev); - if (!pm_runtime_status_suspended(&pdev->dev)) - mtk8250_runtime_suspend(&pdev->dev); - return 0; } -- cgit v1.2.3 From 52861a3be8d6eae5c90963b747c4276ba6059c6b Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 6 Jun 2023 11:26:25 +0300 Subject: serial: core: don't kfree device managed data The put_device() function will call serial_base_ctrl_release() or serial_base_port_release() so these kfrees() are a double free bug. Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM") Signed-off-by: Dan Carpenter Reviewed-by: Tony Lindgren Message-ID: Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial_base_bus.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/tty/serial/serial_base_bus.c b/drivers/tty/serial/serial_base_bus.c index 9354af7c11af..9de9f6fa6481 100644 --- a/drivers/tty/serial/serial_base_bus.c +++ b/drivers/tty/serial/serial_base_bus.c @@ -98,7 +98,7 @@ struct serial_ctrl_device *serial_base_ctrl_add(struct uart_port *port, serial_base_ctrl_release, port->ctrl_id); if (err) - goto err_free_ctrl_dev; + goto err_put_device; err = device_add(&ctrl_dev->dev); if (err) @@ -108,8 +108,6 @@ struct serial_ctrl_device *serial_base_ctrl_add(struct uart_port *port, err_put_device: put_device(&ctrl_dev->dev); -err_free_ctrl_dev: - kfree(ctrl_dev); return ERR_PTR(err); } @@ -140,7 +138,7 @@ struct serial_port_device *serial_base_port_add(struct uart_port *port, serial_base_port_release, port->line); if (err) - goto err_free_port_dev; + goto err_put_device; port_dev->port = port; @@ -152,8 +150,6 @@ struct serial_port_device *serial_base_port_add(struct uart_port *port, err_put_device: put_device(&port_dev->dev); -err_free_port_dev: - kfree(port_dev); return ERR_PTR(err); } -- cgit v1.2.3 From 1eea99c04555e505c0361f70c9232a9f0d88974b Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 9 Jun 2023 15:39:31 +0200 Subject: powerpc/legacy_serial: Handle SERIAL_8250_FSL=n build failures With SERIAL_8250=y and SERIAL_8250_FSL_CONSOLE=n the both IS_ENABLED(CONFIG_SERIAL_8250) and IS_REACHABLE(CONFIG_SERIAL_8250) evaluate to true and so fsl8250_handle_irq() is used. However this function is only available if CONFIG_SERIAL_8250_CONSOLE=y (and thus SERIAL_8250_FSL=y). To prepare SERIAL_8250_FSL becoming tristate and being enabled in more cases, check for IS_REACHABLE(CONFIG_SERIAL_8250_FSL) before making use of fsl8250_handle_irq(). This check is correct with and without the change to make SERIAL_8250_FSL modular. Reported-by: Randy Dunlap Fixes: 66eff0ef528b ("powerpc/legacy_serial: Warn about 8250 devices operated without active FSL workarounds") Signed-off-by: Uwe Kleine-König Acked-by: Randy Dunlap Tested-by: Randy Dunlap # build-tested Message-ID: <20230609133932.786117-2-u.kleine-koenig@pengutronix.de> Signed-off-by: Greg Kroah-Hartman --- arch/powerpc/kernel/legacy_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c index fdbd85aafeb1..6ee65741dbd5 100644 --- a/arch/powerpc/kernel/legacy_serial.c +++ b/arch/powerpc/kernel/legacy_serial.c @@ -510,7 +510,7 @@ static void __init fixup_port_irq(int index, if (IS_ENABLED(CONFIG_SERIAL_8250) && of_device_is_compatible(np, "fsl,ns16550")) { - if (IS_REACHABLE(CONFIG_SERIAL_8250)) { + if (IS_REACHABLE(CONFIG_SERIAL_8250_FSL)) { port->handle_irq = fsl8250_handle_irq; port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE); } else { -- cgit v1.2.3 From d9f59caf94a92f7a54b83766de29028cab9ec5b8 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 9 Jun 2023 15:39:32 +0200 Subject: serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE The need to handle the FSL variant of 8250 in a special way is also present without console support. So soften the dependency for SERIAL_8250_FSL from SERIAL_8250_CONSOLE to SERIAL_8250. To handle SERIAL_8250=m, the FSL code can be modular, too, thus SERIAL_8250_FSL becomes tristate. Compiling 8250_fsl as a module requires adding a module license so this is added, too. While add it also add a appropriate module description. As then SERIAL_OF_PLATFORM=y + SERIAL_8250_FSL=m is a valid combination (if COMPILE_TEST is enabled on a platform that is neither PPC, ARM nor ARM64), the check in 8250_of.c must be weakened a bit. Signed-off-by: Uwe Kleine-König Reviewed-by: Andy Shevchenko Message-ID: <20230609133932.786117-3-u.kleine-koenig@pengutronix.de> Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_fsl.c | 3 +++ drivers/tty/serial/8250/8250_of.c | 2 +- drivers/tty/serial/8250/Kconfig | 6 +++--- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c index 00f46b9a8b09..6af4e1c1210a 100644 --- a/drivers/tty/serial/8250/8250_fsl.c +++ b/drivers/tty/serial/8250/8250_fsl.c @@ -184,3 +184,6 @@ static struct platform_driver fsl8250_platform_driver = { module_platform_driver(fsl8250_platform_driver); #endif + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Handling of Freescale specific 8250 variants"); diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c index c9f6bd7a7038..51329625c48a 100644 --- a/drivers/tty/serial/8250/8250_of.c +++ b/drivers/tty/serial/8250/8250_of.c @@ -177,7 +177,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev, break; } - if (IS_ENABLED(CONFIG_SERIAL_8250_FSL) && + if (IS_REACHABLE(CONFIG_SERIAL_8250_FSL) && (of_device_is_compatible(np, "fsl,ns16550") || of_device_is_compatible(np, "fsl,16550-FIFO64"))) { port->handle_irq = fsl8250_handle_irq; diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index cf33e858b0be..ee17cf5c44c6 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -379,9 +379,9 @@ config SERIAL_8250_BCM2835AUX If unsure, say N. config SERIAL_8250_FSL - bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64) - depends on SERIAL_8250_CONSOLE - default PPC || ARM || ARM64 + tristate "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64) + depends on SERIAL_8250 + default SERIAL_8250 if PPC || ARM || ARM64 help Selecting this option enables a workaround for a break-detection erratum for Freescale 16550 UARTs in the 8250 driver. It also -- cgit v1.2.3 From a9c09546e903f1068acfa38e1ee18bded7114b37 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Sat, 10 Jun 2023 17:59:25 +0200 Subject: tty: serial: samsung_tty: Fix a memory leak in s3c24xx_serial_getclk() in case of error If clk_get_rate() fails, the clk that has just been allocated needs to be freed. Cc: # v3.3+ Reviewed-by: Krzysztof Kozlowski Reviewed-by: Andi Shyti Fixes: 5f5a7a5578c5 ("serial: samsung: switch to clkdev based clock lookup") Signed-off-by: Christophe JAILLET Reviewed-by: Jiri Slaby Message-ID: Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/samsung_tty.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index 2a7520ad3abd..a92a23e1964e 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -1459,8 +1459,12 @@ static unsigned int s3c24xx_serial_getclk(struct s3c24xx_uart_port *ourport, continue; rate = clk_get_rate(clk); - if (!rate) + if (!rate) { + dev_err(ourport->port.dev, + "Failed to get clock rate for %s.\n", clkname); + clk_put(clk); continue; + } if (ourport->info->has_divslot) { unsigned long div = rate / req_baud; -- cgit v1.2.3 From 832e231cff476102e8204a9e7bddfe5c6154a375 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Sat, 10 Jun 2023 17:59:26 +0200 Subject: tty: serial: samsung_tty: Fix a memory leak in s3c24xx_serial_getclk() when iterating clk When the best clk is searched, we iterate over all possible clk. If we find a better match, the previous one, if any, needs to be freed. If a better match has already been found, we still need to free the new one, otherwise it leaks. Cc: # v3.3+ Reviewed-by: Krzysztof Kozlowski Reviewed-by: Andi Shyti Fixes: 5f5a7a5578c5 ("serial: samsung: switch to clkdev based clock lookup") Signed-off-by: Christophe JAILLET Reviewed-by: Jiri Slaby Message-ID: Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/samsung_tty.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index a92a23e1964e..0b37019820b4 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -1490,10 +1490,18 @@ static unsigned int s3c24xx_serial_getclk(struct s3c24xx_uart_port *ourport, calc_deviation = -calc_deviation; if (calc_deviation < deviation) { + /* + * If we find a better clk, release the previous one, if + * any. + */ + if (!IS_ERR(*best_clk)) + clk_put(*best_clk); *best_clk = clk; best_quot = quot; *clk_num = cnt; deviation = calc_deviation; + } else { + clk_put(clk); } } -- cgit v1.2.3 From f3710f5e9e1a68da53202cffba73f4b604f05b15 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Sat, 10 Jun 2023 17:59:27 +0200 Subject: tty: serial: samsung_tty: Use abs() to simplify some code Use abs() instead of hand-writing it. Suggested-by: Walter Harms Signed-off-by: Christophe JAILLET Reviewed-by: Andi Shyti Reviewed-by: Jiri Slaby Message-ID: <7bd165e82ed3675d4ddee343ab373031e995a126.1686412569.git.christophe.jaillet@wanadoo.fr> Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/samsung_tty.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index 0b37019820b4..b29e9dfd81a6 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -1485,9 +1486,7 @@ static unsigned int s3c24xx_serial_getclk(struct s3c24xx_uart_port *ourport, } quot--; - calc_deviation = req_baud - baud; - if (calc_deviation < 0) - calc_deviation = -calc_deviation; + calc_deviation = abs(req_baud - baud); if (calc_deviation < deviation) { /* -- cgit v1.2.3 From 20a41a62618df85f3a2981008edec5cadd785e0a Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Wed, 14 Jun 2023 07:59:19 +0300 Subject: serial: 8250_omap: Use force_suspend and resume for system suspend We should not rely on autosuspend timeout for system suspend. Instead, let's use force_suspend and force_resume functions. Otherwise the serial port controller device may not be idled on suspend. As we are doing a register write on suspend to configure the serial port, we still need to runtime PM resume the port on suspend. While at it, let's switch to pm_runtime_resume_and_get() and check for errors returned. And let's add the missing line break before return to the suspend function while at it. Fixes: 09d8b2bdbc5c ("serial: 8250: omap: Provide ability to enable/disable UART as wakeup source") Signed-off-by: Tony Lindgren Tested-by: Dhruva Gole Message-ID: <20230614045922.4798-1-tony@atomide.com> Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_omap.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index fe26ebc7b834..d48a82f1634e 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -1618,25 +1618,35 @@ static int omap8250_suspend(struct device *dev) { struct omap8250_priv *priv = dev_get_drvdata(dev); struct uart_8250_port *up = serial8250_get_port(priv->line); + int err; serial8250_suspend_port(priv->line); - pm_runtime_get_sync(dev); + err = pm_runtime_resume_and_get(dev); + if (err) + return err; if (!device_may_wakeup(dev)) priv->wer = 0; serial_out(up, UART_OMAP_WER, priv->wer); - pm_runtime_mark_last_busy(dev); - pm_runtime_put_autosuspend(dev); - + err = pm_runtime_force_suspend(dev); flush_work(&priv->qos_work); - return 0; + + return err; } static int omap8250_resume(struct device *dev) { struct omap8250_priv *priv = dev_get_drvdata(dev); + int err; + err = pm_runtime_force_resume(dev); + if (err) + return err; serial8250_resume_port(priv->line); + /* Paired with pm_runtime_resume_and_get() in omap8250_suspend() */ + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + return 0; } -- cgit v1.2.3 From cef09673c35bdffb0866268cedae5994e024ddb8 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 14 Jun 2023 09:36:33 +0300 Subject: serial: core: fix -EPROBE_DEFER handling in init The -EPROBE_DEFER error path in serial_base_device_init() is a bit awkward. Before the call to device_initialize(dev) then we need to manually release all the device resources. And after the call then we need to call put_device() to release the resources. Doing either one wrong will result in a leak or a use after free. So let's wait to return -EPROBE_DEFER until after the call to device_initialize(dev) so that way callers do not have to handle -EPROBE_DEFER as a special case. Now callers can just use put_device() for clean up. The second issue with the -EPROBE_DEFER path is that deferring is not supposed to be a fatal error, but instead it's normal part of the init process and the kernel recovers from it automatically. That means we should not print an error message but just a debug message on this path. Fixes: 539914240a01 ("serial: core: Fix probing serial_base_bus devices") Signed-off-by: Dan Carpenter Reviewed-by: Tony Lindgren Message-ID: <18318adb-ab2c-4dcc-9f96-498a13d16b80@moroto.mountain> Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/serial_base_bus.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/tty/serial/serial_base_bus.c b/drivers/tty/serial/serial_base_bus.c index 9de9f6fa6481..6ff59c89d867 100644 --- a/drivers/tty/serial/serial_base_bus.c +++ b/drivers/tty/serial/serial_base_bus.c @@ -50,17 +50,17 @@ static int serial_base_device_init(struct uart_port *port, void (*release)(struct device *dev), int id) { - if (!serial_base_initialized) { - dev_err(port->dev, "uart_add_one_port() called before arch_initcall()?\n"); - return -EPROBE_DEFER; - } - device_initialize(dev); dev->type = type; dev->parent = parent_dev; dev->bus = &serial_base_bus_type; dev->release = release; + if (!serial_base_initialized) { + dev_dbg(port->dev, "uart_add_one_port() called before arch_initcall()?\n"); + return -EPROBE_DEFER; + } + return dev_set_name(dev, "%s.%s.%d", type->name, dev_name(port->dev), id); } -- cgit v1.2.3 From 4903fde8047a28299d1fc79c1a0dcc255e928f12 Mon Sep 17 00:00:00 2001 From: Hui Li Date: Thu, 6 Apr 2023 10:44:50 +0800 Subject: tty: fix hang on tty device with no_room set It is possible to hang pty devices in this case, the reader was blocking at epoll on master side, the writer was sleeping at wait_woken inside n_tty_write on slave side, and the write buffer on tty_port was full, we found that the reader and writer would never be woken again and blocked forever. The problem was caused by a race between reader and kworker: n_tty_read(reader): n_tty_receive_buf_common(kworker): copy_from_read_buf()| |room = N_TTY_BUF_SIZE - (ldata->read_head - tail) |room <= 0 n_tty_kick_worker() | |ldata->no_room = true After writing to slave device, writer wakes up kworker to flush data on tty_port to reader, and the kworker finds that reader has no room to store data so room <= 0 is met. At this moment, reader consumes all the data on reader buffer and calls n_tty_kick_worker to check ldata->no_room which is false and reader quits reading. Then kworker sets ldata->no_room=true and quits too. If write buffer is not full, writer will wake kworker to flush data again after following writes, but if write buffer is full and writer goes to sleep, kworker will never be woken again and tty device is blocked. This problem can be solved with a check for read buffer size inside n_tty_receive_buf_common, if read buffer is empty and ldata->no_room is true, a call to n_tty_kick_worker is necessary to keep flushing data to reader. Cc: Fixes: 42458f41d08f ("n_tty: Ensure reader restarts worker for next reader") Reviewed-by: Ilpo Järvinen Signed-off-by: Hui Li Message-ID: <1680749090-14106-1-git-send-email-caelli@tencent.com> Signed-off-by: Greg Kroah-Hartman --- drivers/tty/n_tty.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 1c9e5d2ea7de..552e8a741562 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -203,8 +203,8 @@ static void n_tty_kick_worker(struct tty_struct *tty) struct n_tty_data *ldata = tty->disc_data; /* Did the input worker stop? Restart it */ - if (unlikely(ldata->no_room)) { - ldata->no_room = 0; + if (unlikely(READ_ONCE(ldata->no_room))) { + WRITE_ONCE(ldata->no_room, 0); WARN_RATELIMIT(tty->port->itty == NULL, "scheduling with invalid itty\n"); @@ -1697,7 +1697,7 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp, if (overflow && room < 0) ldata->read_head--; room = overflow; - ldata->no_room = flow && !room; + WRITE_ONCE(ldata->no_room, flow && !room); } else overflow = 0; @@ -1728,6 +1728,17 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp, } else n_tty_check_throttle(tty); + if (unlikely(ldata->no_room)) { + /* + * Barrier here is to ensure to read the latest read_tail in + * chars_in_buffer() and to make sure that read_tail is not loaded + * before ldata->no_room is set. + */ + smp_mb(); + if (!chars_in_buffer(tty)) + n_tty_kick_worker(tty); + } + up_read(&tty->termios_rwsem); return rcvd; @@ -2281,8 +2292,14 @@ more_to_be_read: if (time) timeout = time; } - if (old_tail != ldata->read_tail) + if (old_tail != ldata->read_tail) { + /* + * Make sure no_room is not read in n_tty_kick_worker() + * before setting ldata->read_tail in copy_from_read_buf(). + */ + smp_mb(); n_tty_kick_worker(tty); + } up_read(&tty->termios_rwsem); remove_wait_queue(&tty->read_wait, &wait); -- cgit v1.2.3 From 360c11e2258ce4269441a5ab6d43d0b202f4261b Mon Sep 17 00:00:00 2001 From: Michal Sekletar Date: Tue, 13 Jun 2023 19:21:06 +0200 Subject: tty: tty_io: update timestamps on all device nodes User space applications watch for timestamp changes on character device files in order to determine idle time of a given terminal session. For example, "w" program uses this information to populate the IDLE column of its output [1]. Similarly, systemd-logind has optional feature where it uses atime of the tty character device to determine if there was activity on the terminal associated with the logind's session object. If there was no activity for a configured period of time then logind will terminate such session [2]. Now, usually (e.g. bash running on the terminal) the use of the terminal will update timestamps (atime and mtime) on the corresponding terminal character device. However, if access to the terminal, e.g. /dev/pts/0, is performed through magic character device /dev/tty then such access obviously changes the state of the terminal, however timestamps on the device that correspond to the terminal (/dev/pts/0) are not updated. This patch makes sure that we update timestamps on *all* character devices that correspond to the given tty, because outside observers (w, systemd-logind) are maybe checking these timestamps. Obviously, they can not check timestamps on /dev/tty as that has per-process meaning. [1] https://gitlab.com/procps-ng/procps/-/blob/v4.0.0/w.c#L286 [2] https://github.com/systemd/systemd/blob/v252/NEWS#L477 Signed-off-by: Michal Sekletar Message-ID: <20230613172107.78138-1-msekleta@redhat.com> Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_io.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index c84be40fb8df..a505d2c49110 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -101,6 +101,7 @@ #include #include #include +#include #include #include @@ -811,18 +812,26 @@ void start_tty(struct tty_struct *tty) } EXPORT_SYMBOL(start_tty); -static void tty_update_time(struct timespec64 *time) +static void tty_update_time(struct tty_struct *tty, bool mtime) { time64_t sec = ktime_get_real_seconds(); + struct tty_file_private *priv; - /* - * We only care if the two values differ in anything other than the - * lower three bits (i.e every 8 seconds). If so, then we can update - * the time of the tty device, otherwise it could be construded as a - * security leak to let userspace know the exact timing of the tty. - */ - if ((sec ^ time->tv_sec) & ~7) - time->tv_sec = sec; + spin_lock(&tty->files_lock); + list_for_each_entry(priv, &tty->tty_files, list) { + struct inode *inode = file_inode(priv->file); + struct timespec64 *time = mtime ? &inode->i_mtime : &inode->i_atime; + + /* + * We only care if the two values differ in anything other than the + * lower three bits (i.e every 8 seconds). If so, then we can update + * the time of the tty device, otherwise it could be construded as a + * security leak to let userspace know the exact timing of the tty. + */ + if ((sec ^ time->tv_sec) & ~7) + time->tv_sec = sec; + } + spin_unlock(&tty->files_lock); } /* @@ -928,7 +937,7 @@ static ssize_t tty_read(struct kiocb *iocb, struct iov_iter *to) tty_ldisc_deref(ld); if (i > 0) - tty_update_time(&inode->i_atime); + tty_update_time(tty, false); return i; } @@ -1036,7 +1045,7 @@ static inline ssize_t do_tty_write( cond_resched(); } if (written) { - tty_update_time(&file_inode(file)->i_mtime); + tty_update_time(tty, true); ret = written; } out: -- cgit v1.2.3 From e8cc334847dbd204ed4f500b3e3fa899b3766b62 Mon Sep 17 00:00:00 2001 From: Michal Sekletar Date: Tue, 13 Jun 2023 19:21:08 +0200 Subject: selftests: tty: add selftest for tty timestamp updates Add new test case which checks that timestamp updates on actual terminal character device (e.g. /dev/pts/0) happen even if the terminal is accessed via magic /dev/tty file. Signed-off-by: Michal Sekletar Message-ID: <20230613172107.78138-2-msekleta@redhat.com> Signed-off-by: Greg Kroah-Hartman --- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/tty/.gitignore | 2 + tools/testing/selftests/tty/Makefile | 5 ++ tools/testing/selftests/tty/tty_tstamp_update.c | 88 +++++++++++++++++++++++++ 4 files changed, 96 insertions(+) create mode 100644 tools/testing/selftests/tty/.gitignore create mode 100644 tools/testing/selftests/tty/Makefile create mode 100644 tools/testing/selftests/tty/tty_tstamp_update.c diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 90a62cf75008..862f5f9a76a0 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -86,6 +86,7 @@ TARGETS += timers endif TARGETS += tmpfs TARGETS += tpm2 +TARGETS += tty TARGETS += user TARGETS += vDSO TARGETS += mm diff --git a/tools/testing/selftests/tty/.gitignore b/tools/testing/selftests/tty/.gitignore new file mode 100644 index 000000000000..fe70462a4aad --- /dev/null +++ b/tools/testing/selftests/tty/.gitignore @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +tty_tstamp_update diff --git a/tools/testing/selftests/tty/Makefile b/tools/testing/selftests/tty/Makefile new file mode 100644 index 000000000000..50d7027b2ae3 --- /dev/null +++ b/tools/testing/selftests/tty/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 +CFLAGS = -O2 -Wall +TEST_GEN_PROGS := tty_tstamp_update + +include ../lib.mk diff --git a/tools/testing/selftests/tty/tty_tstamp_update.c b/tools/testing/selftests/tty/tty_tstamp_update.c new file mode 100644 index 000000000000..0ee97943dccc --- /dev/null +++ b/tools/testing/selftests/tty/tty_tstamp_update.c @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../kselftest.h" + +#define MIN_TTY_PATH_LEN 8 + +static bool tty_valid(char *tty) +{ + if (strlen(tty) < MIN_TTY_PATH_LEN) + return false; + + if (strncmp(tty, "/dev/tty", MIN_TTY_PATH_LEN) == 0 || + strncmp(tty, "/dev/pts", MIN_TTY_PATH_LEN) == 0) + return true; + + return false; +} + +static int write_dev_tty(void) +{ + FILE *f; + int r = 0; + + f = fopen("/dev/tty", "r+"); + if (!f) + return -errno; + + r = fprintf(f, "hello, world!\n"); + if (r != strlen("hello, world!\n")) + r = -EIO; + + fclose(f); + return r; +} + +int main(int argc, char **argv) +{ + int r; + char tty[PATH_MAX] = {}; + struct stat st1, st2; + + ksft_print_header(); + ksft_set_plan(1); + + r = readlink("/proc/self/fd/0", tty, PATH_MAX); + if (r < 0) + ksft_exit_fail_msg("readlink on /proc/self/fd/0 failed: %m\n"); + + if (!tty_valid(tty)) + ksft_exit_skip("invalid tty path '%s'\n", tty); + + r = stat(tty, &st1); + if (r < 0) + ksft_exit_fail_msg("stat failed on tty path '%s': %m\n", tty); + + /* We need to wait at least 8 seconds in order to observe timestamp change */ + /* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fbf47635315ab308c9b58a1ea0906e711a9228de */ + sleep(10); + + r = write_dev_tty(); + if (r < 0) + ksft_exit_fail_msg("failed to write to /dev/tty: %s\n", + strerror(-r)); + + r = stat(tty, &st2); + if (r < 0) + ksft_exit_fail_msg("stat failed on tty path '%s': %m\n", tty); + + /* We wrote to the terminal so timestamps should have been updated */ + if (st1.st_atim.tv_sec == st2.st_atim.tv_sec && + st1.st_mtim.tv_sec == st2.st_mtim.tv_sec) { + ksft_test_result_fail("tty timestamps not updated\n"); + ksft_exit_fail(); + } + + ksft_test_result_pass( + "timestamps of terminal '%s' updated after write to /dev/tty\n", tty); + return EXIT_SUCCESS; +} -- cgit v1.2.3 From 639949a7031e04c59ec91614eceb9543e9120f43 Mon Sep 17 00:00:00 2001 From: Martin Fuzzey Date: Fri, 16 Jun 2023 12:47:23 +0200 Subject: tty: serial: imx: fix rs485 rx after tx Since commit 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal active high") RS485 reception no longer works after a transmission. The following scenario shows the problem: 1) Open a port in RS485 mode 2) Receive data from remote (OK) 3) Transmit data to remote (OK) 4) Receive data from remote (Nothing received) In RS485 mode, imx_uart_start_tx() calls imx_uart_stop_rx() and, when the transmission is complete, imx_uart_stop_tx() calls imx_uart_start_rx(). Since the above commit imx_uart_stop_rx() now sets the loopback bit but imx_uart_start_rx() does not clear it causing the hardware to remain in loopback mode and not receive external data. Fix this by moving the existing loopback disable code to a helper function and calling it from imx_uart_start_rx() too. Fixes: 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal active high") Cc: stable@vger.kernel.org Signed-off-by: Martin Fuzzey Reviewed-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230616104838.2729694-1-martin.fuzzey@flowbird.group Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/imx.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index b2bf3cb449f4..7341d060f85c 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -369,6 +369,16 @@ static void imx_uart_soft_reset(struct imx_port *sport) sport->idle_counter = 0; } +static void imx_uart_disable_loopback_rs485(struct imx_port *sport) +{ + unsigned int uts; + + /* See SER_RS485_ENABLED/UTS_LOOP comment in imx_uart_probe() */ + uts = imx_uart_readl(sport, imx_uart_uts_reg(sport)); + uts &= ~UTS_LOOP; + imx_uart_writel(sport, uts, imx_uart_uts_reg(sport)); +} + /* called with port.lock taken and irqs off */ static void imx_uart_start_rx(struct uart_port *port) { @@ -390,6 +400,7 @@ static void imx_uart_start_rx(struct uart_port *port) /* Write UCR2 first as it includes RXEN */ imx_uart_writel(sport, ucr2, UCR2); imx_uart_writel(sport, ucr1, UCR1); + imx_uart_disable_loopback_rs485(sport); } /* called with port.lock taken and irqs off */ @@ -1422,7 +1433,7 @@ static int imx_uart_startup(struct uart_port *port) int retval; unsigned long flags; int dma_is_inited = 0; - u32 ucr1, ucr2, ucr3, ucr4, uts; + u32 ucr1, ucr2, ucr3, ucr4; retval = clk_prepare_enable(sport->clk_per); if (retval) @@ -1521,10 +1532,7 @@ static int imx_uart_startup(struct uart_port *port) imx_uart_writel(sport, ucr2, UCR2); } - /* See SER_RS485_ENABLED/UTS_LOOP comment in imx_uart_probe() */ - uts = imx_uart_readl(sport, imx_uart_uts_reg(sport)); - uts &= ~UTS_LOOP; - imx_uart_writel(sport, uts, imx_uart_uts_reg(sport)); + imx_uart_disable_loopback_rs485(sport); spin_unlock_irqrestore(&sport->port.lock, flags); -- cgit v1.2.3 From e0edfdc15863ec80a1d9ac6e174dbccc00206dd0 Mon Sep 17 00:00:00 2001 From: Sherry Sun Date: Mon, 19 Jun 2023 16:06:13 +0800 Subject: tty: serial: fsl_lpuart: add earlycon for imx8ulp platform Add earlycon support for imx8ulp platform. Signed-off-by: Sherry Sun Cc: stable Link: https://lore.kernel.org/r/20230619080613.16522-1-sherry.sun@nxp.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/fsl_lpuart.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index f2a47a8c5b85..e29e9d407e85 100644 --- a/drivers/tty/serial/fsl_lpuart.c +++ b/drivers/tty/serial/fsl_lpuart.c @@ -2724,6 +2724,7 @@ OF_EARLYCON_DECLARE(lpuart, "fsl,vf610-lpuart", lpuart_early_console_setup); OF_EARLYCON_DECLARE(lpuart32, "fsl,ls1021a-lpuart", lpuart32_early_console_setup); OF_EARLYCON_DECLARE(lpuart32, "fsl,ls1028a-lpuart", ls1028a_early_console_setup); OF_EARLYCON_DECLARE(lpuart32, "fsl,imx7ulp-lpuart", lpuart32_imx_early_console_setup); +OF_EARLYCON_DECLARE(lpuart32, "fsl,imx8ulp-lpuart", lpuart32_imx_early_console_setup); OF_EARLYCON_DECLARE(lpuart32, "fsl,imx8qxp-lpuart", lpuart32_imx_early_console_setup); OF_EARLYCON_DECLARE(lpuart32, "fsl,imxrt1050-lpuart", lpuart32_imx_early_console_setup); EARLYCON_DECLARE(lpuart, lpuart_early_console_setup); -- cgit v1.2.3 From 930cbf92db0184e327293d5e7089be0b08d46371 Mon Sep 17 00:00:00 2001 From: Jacky Huang Date: Mon, 19 Jun 2023 03:23:29 +0000 Subject: tty: serial: Add Nuvoton ma35d1 serial driver support This adds UART and console driver for Nuvoton ma35d1 Soc. It supports full-duplex communication, FIFO control, and hardware flow control. Command line set "console=ttyNVT0,115200", NVT means Nuvoton MA35 UART port. The UART driver probe will create path named "/dev/ttyNVTx". Signed-off-by: Jacky Huang Reviewed-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230619032330.233796-2-ychuang570808@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/Kconfig | 23 ++ drivers/tty/serial/Makefile | 1 + drivers/tty/serial/ma35d1_serial.c | 821 +++++++++++++++++++++++++++++++++++++ 3 files changed, 845 insertions(+) create mode 100644 drivers/tty/serial/ma35d1_serial.c diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index 3e3fb377d90d..444a31ff18b5 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -1555,6 +1555,29 @@ config SERIAL_SUNPLUS_CONSOLE you can alter that using a kernel command line option such as "console=ttySUPx". +config SERIAL_NUVOTON_MA35D1 + tristate "Nuvoton MA35D1 family UART support" + depends on ARCH_MA35 || COMPILE_TEST + select SERIAL_CORE + help + This driver supports Nuvoton MA35D1 family UART ports. If you would + like to use them, you must answer Y or M to this option. Note that + for use as console, it must be included in kernel and not as a + module. If you enable this option, Ma35D1 serial ports in the system + will be registered as ttyNVTx. + +config SERIAL_NUVOTON_MA35D1_CONSOLE + bool "Console on a Nuvotn MA35D1 family UART port" + depends on SERIAL_NUVOTON_MA35D1=y + select SERIAL_CORE_CONSOLE + help + Select this options if you'd like to use the UART port0 of the + Nuvoton MA35D1 family as a console. + Even if you say Y here, the currently visible virtual console + (/dev/tty0) will still be used as the system console by default, + but you can alter that using a kernel command line option such as + "console=ttyNVTx". + endmenu config SERIAL_MCTRL_GPIO diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile index 4f7ab4150ec5..d4123469583d 100644 --- a/drivers/tty/serial/Makefile +++ b/drivers/tty/serial/Makefile @@ -94,3 +94,4 @@ obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o +obj-$(CONFIG_SERIAL_NUVOTON_MA35D1) += ma35d1_serial.o diff --git a/drivers/tty/serial/ma35d1_serial.c b/drivers/tty/serial/ma35d1_serial.c new file mode 100644 index 000000000000..2604b4d9fb78 --- /dev/null +++ b/drivers/tty/serial/ma35d1_serial.c @@ -0,0 +1,821 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * MA35D1 serial driver + * Copyright (C) 2023 Nuvoton Technology Corp. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define MA35_UART_NR 17 + +#define MA35_RBR_REG 0x00 +#define MA35_THR_REG 0x00 +#define MA35_IER_REG 0x04 +#define MA35_FCR_REG 0x08 +#define MA35_LCR_REG 0x0C +#define MA35_MCR_REG 0x10 +#define MA35_MSR_REG 0x14 +#define MA35_FSR_REG 0x18 +#define MA35_ISR_REG 0x1C +#define MA35_TOR_REG 0x20 +#define MA35_BAUD_REG 0x24 +#define MA35_ALTCTL_REG 0x2C +#define MA35_FUN_SEL_REG 0x30 +#define MA35_WKCTL_REG 0x40 +#define MA35_WKSTS_REG 0x44 + +/* MA35_IER_REG - Interrupt Enable Register */ +#define MA35_IER_RDA_IEN BIT(0) /* RBR Available Interrupt Enable */ +#define MA35_IER_THRE_IEN BIT(1) /* THR Empty Interrupt Enable */ +#define MA35_IER_RLS_IEN BIT(2) /* RX Line Status Interrupt Enable */ +#define MA35_IER_RTO_IEN BIT(4) /* RX Time-out Interrupt Enable */ +#define MA35_IER_BUFERR_IEN BIT(5) /* Buffer Error Interrupt Enable */ +#define MA35_IER_TIME_OUT_EN BIT(11) /* RX Buffer Time-out Counter Enable */ +#define MA35_IER_AUTO_RTS BIT(12) /* nRTS Auto-flow Control Enable */ +#define MA35_IER_AUTO_CTS BIT(13) /* nCTS Auto-flow Control Enable */ + +/* MA35_FCR_REG - FIFO Control Register */ +#define MA35_FCR_RFR BIT(1) /* RX Field Software Reset */ +#define MA35_FCR_TFR BIT(2) /* TX Field Software Reset */ +#define MA35_FCR_RFITL_MASK GENMASK(7, 4) /* RX FIFO Interrupt Trigger Level */ +#define MA35_FCR_RFITL_1BYTE FIELD_PREP(MA35_FCR_RFITL_MASK, 0) +#define MA35_FCR_RFITL_4BYTES FIELD_PREP(MA35_FCR_RFITL_MASK, 1) +#define MA35_FCR_RFITL_8BYTES FIELD_PREP(MA35_FCR_RFITL_MASK, 2) +#define MA35_FCR_RFITL_14BYTES FIELD_PREP(MA35_FCR_RFITL_MASK, 3) +#define MA35_FCR_RFITL_30BYTES FIELD_PREP(MA35_FCR_RFITL_MASK, 4) +#define MA35_FCR_RTSTL_MASK GENMASK(19, 16) /* nRTS Trigger Level */ +#define MA35_FCR_RTSTL_1BYTE FIELD_PREP(MA35_FCR_RTSTL_MASK, 0) +#define MA35_FCR_RTSTL_4BYTES FIELD_PREP(MA35_FCR_RTSTL_MASK, 1) +#define MA35_FCR_RTSTL_8BYTES FIELD_PREP(MA35_FCR_RTSTL_MASK, 2) +#define MA35_FCR_RTSTL_14BYTES FIELD_PREP(MA35_FCR_RTSTL_MASK, 3) +#define MA35_FCR_RTSTLL_30BYTES FIELD_PREP(MA35_FCR_RTSTL_MASK, 4) + +/* MA35_LCR_REG - Line Control Register */ +#define MA35_LCR_NSB BIT(2) /* Number of “STOP Bit” */ +#define MA35_LCR_PBE BIT(3) /* Parity Bit Enable */ +#define MA35_LCR_EPE BIT(4) /* Even Parity Enable */ +#define MA35_LCR_SPE BIT(5) /* Stick Parity Enable */ +#define MA35_LCR_BREAK BIT(6) /* Break Control */ +#define MA35_LCR_WLS_MASK GENMASK(1, 0) /* Word Length Selection */ +#define MA35_LCR_WLS_5BITS FIELD_PREP(MA35_LCR_WLS_MASK, 0) +#define MA35_LCR_WLS_6BITS FIELD_PREP(MA35_LCR_WLS_MASK, 1) +#define MA35_LCR_WLS_7BITS FIELD_PREP(MA35_LCR_WLS_MASK, 2) +#define MA35_LCR_WLS_8BITS FIELD_PREP(MA35_LCR_WLS_MASK, 3) + +/* MA35_MCR_REG - Modem Control Register */ +#define MA35_MCR_RTS_CTRL BIT(1) /* nRTS Signal Control */ +#define MA35_MCR_RTSACTLV BIT(9) /* nRTS Pin Active Level */ +#define MA35_MCR_RTSSTS BIT(13) /* nRTS Pin Status (Read Only) */ + +/* MA35_MSR_REG - Modem Status Register */ +#define MA35_MSR_CTSDETF BIT(0) /* Detect nCTS State Change Flag */ +#define MA35_MSR_CTSSTS BIT(4) /* nCTS Pin Status (Read Only) */ +#define MA35_MSR_CTSACTLV BIT(8) /* nCTS Pin Active Level */ + +/* MA35_FSR_REG - FIFO Status Register */ +#define MA35_FSR_RX_OVER_IF BIT(0) /* RX Overflow Error Interrupt Flag */ +#define MA35_FSR_PEF BIT(4) /* Parity Error Flag*/ +#define MA35_FSR_FEF BIT(5) /* Framing Error Flag */ +#define MA35_FSR_BIF BIT(6) /* Break Interrupt Flag */ +#define MA35_FSR_RX_EMPTY BIT(14) /* Receiver FIFO Empty (Read Only) */ +#define MA35_FSR_RX_FULL BIT(15) /* Receiver FIFO Full (Read Only) */ +#define MA35_FSR_TX_EMPTY BIT(22) /* Transmitter FIFO Empty (Read Only) */ +#define MA35_FSR_TX_FULL BIT(23) /* Transmitter FIFO Full (Read Only) */ +#define MA35_FSR_TX_OVER_IF BIT(24) /* TX Overflow Error Interrupt Flag */ +#define MA35_FSR_TE_FLAG BIT(28) /* Transmitter Empty Flag (Read Only) */ +#define MA35_FSR_RXPTR_MSK GENMASK(13, 8) /* TX FIFO Pointer mask */ +#define MA35_FSR_TXPTR_MSK GENMASK(21, 16) /* RX FIFO Pointer mask */ + +/* MA35_ISR_REG - Interrupt Status Register */ +#define MA35_ISR_RDA_IF BIT(0) /* RBR Available Interrupt Flag */ +#define MA35_ISR_THRE_IF BIT(1) /* THR Empty Interrupt Flag */ +#define MA35_ISR_RLSIF BIT(2) /* Receive Line Interrupt Flag */ +#define MA35_ISR_MODEMIF BIT(3) /* MODEM Interrupt Flag */ +#define MA35_ISR_RXTO_IF BIT(4) /* RX Time-out Interrupt Flag */ +#define MA35_ISR_BUFEIF BIT(5) /* Buffer Error Interrupt Flag */ +#define MA35_ISR_WK_IF BIT(6) /* UART Wake-up Interrupt Flag */ +#define MA35_ISR_RDAINT BIT(8) /* RBR Available Interrupt Indicator */ +#define MA35_ISR_THRE_INT BIT(9) /* THR Empty Interrupt Indicator */ +#define MA35_ISR_ALL 0xFFFFFFFF + +/* MA35_BAUD_REG - Baud Rate Divider Register */ +#define MA35_BAUD_MODE_MASK GENMASK(29, 28) +#define MA35_BAUD_MODE0 FIELD_PREP(MA35_BAUD_MODE_MASK, 0) +#define MA35_BAUD_MODE1 FIELD_PREP(MA35_BAUD_MODE_MASK, 2) +#define MA35_BAUD_MODE2 FIELD_PREP(MA35_BAUD_MODE_MASK, 3) +#define MA35_BAUD_MASK GENMASK(15, 0) + +/* MA35_ALTCTL_REG - Alternate Control/Status Register */ +#define MA35_ALTCTL_RS485AUD BIT(10) /* RS-485 Auto Direction Function */ + +/* MA35_FUN_SEL_REG - Function Select Register */ +#define MA35_FUN_SEL_MASK GENMASK(2, 0) +#define MA35_FUN_SEL_UART FIELD_PREP(MA35_FUN_SEL_MASK, 0) +#define MA35_FUN_SEL_RS485 FIELD_PREP(MA35_FUN_SEL_MASK, 3) + +/* The constrain for MA35D1 UART baud rate divider */ +#define MA35_BAUD_DIV_MAX 0xFFFF +#define MA35_BAUD_DIV_MIN 11 + +/* UART FIFO depth */ +#define MA35_UART_FIFO_DEPTH 32 +/* UART console clock */ +#define MA35_UART_CONSOLE_CLK (24 * HZ_PER_MHZ) +/* UART register ioremap size */ +#define MA35_UART_REG_SIZE 0x100 +/* Rx Timeout */ +#define MA35_UART_RX_TOUT 0x40 + +#define MA35_IER_CONFIG (MA35_IER_RTO_IEN | MA35_IER_RDA_IEN | \ + MA35_IER_TIME_OUT_EN | MA35_IER_BUFERR_IEN) + +#define MA35_ISR_IF_CHECK (MA35_ISR_RDA_IF | MA35_ISR_RXTO_IF | \ + MA35_ISR_THRE_INT | MA35_ISR_BUFEIF) + +#define MA35_FSR_TX_BOTH_EMPTY (MA35_FSR_TE_FLAG | MA35_FSR_TX_EMPTY) + +static struct uart_driver ma35d1serial_reg; + +struct uart_ma35d1_port { + struct uart_port port; + struct clk *clk; + u16 capabilities; /* port capabilities */ + u8 ier; + u8 lcr; + u8 mcr; + u32 baud_rate; + u32 console_baud_rate; + u32 console_line; + u32 console_int; +}; + +static struct uart_ma35d1_port ma35d1serial_ports[MA35_UART_NR]; + +static struct uart_ma35d1_port *to_ma35d1_uart_port(struct uart_port *uart) +{ + return container_of(uart, struct uart_ma35d1_port, port); +} + +static u32 serial_in(struct uart_ma35d1_port *p, u32 offset) +{ + return readl_relaxed(p->port.membase + offset); +} + +static void serial_out(struct uart_ma35d1_port *p, u32 offset, u32 value) +{ + writel_relaxed(value, p->port.membase + offset); +} + +static void __stop_tx(struct uart_ma35d1_port *p) +{ + u32 ier; + + ier = serial_in(p, MA35_IER_REG); + if (ier & MA35_IER_THRE_IEN) + serial_out(p, MA35_IER_REG, ier & ~MA35_IER_THRE_IEN); +} + +static void ma35d1serial_stop_tx(struct uart_port *port) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + + __stop_tx(up); +} + +static void transmit_chars(struct uart_ma35d1_port *up) +{ + u32 count; + u8 ch; + + if (uart_tx_stopped(&up->port)) { + ma35d1serial_stop_tx(&up->port); + return; + } + count = MA35_UART_FIFO_DEPTH - FIELD_GET(MA35_FSR_TXPTR_MSK, + serial_in(up, MA35_FSR_REG)); + uart_port_tx_limited(&up->port, ch, count, + !(serial_in(up, MA35_FSR_REG) & MA35_FSR_TX_FULL), + serial_out(up, MA35_THR_REG, ch), + ({})); +} + +static void ma35d1serial_start_tx(struct uart_port *port) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + u32 ier; + + ier = serial_in(up, MA35_IER_REG); + serial_out(up, MA35_IER_REG, ier & ~MA35_IER_THRE_IEN); + transmit_chars(up); + serial_out(up, MA35_IER_REG, ier | MA35_IER_THRE_IEN); +} + +static void ma35d1serial_stop_rx(struct uart_port *port) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + u32 ier; + + ier = serial_in(up, MA35_IER_REG); + ier &= ~MA35_IER_RDA_IEN; + serial_out(up, MA35_IER_REG, ier); +} + +static void receive_chars(struct uart_ma35d1_port *up) +{ + int max_count = 256; + u8 ch, flag; + u32 fsr; + + fsr = serial_in(up, MA35_FSR_REG); + do { + flag = TTY_NORMAL; + up->port.icount.rx++; + + if (unlikely(fsr & (MA35_FSR_BIF | MA35_FSR_FEF | + MA35_FSR_PEF | MA35_FSR_RX_OVER_IF))) { + if (fsr & MA35_FSR_BIF) { + up->port.icount.brk++; + if (uart_handle_break(&up->port)) + continue; + } + if (fsr & MA35_FSR_FEF) + up->port.icount.frame++; + if (fsr & MA35_FSR_PEF) + up->port.icount.parity++; + if (fsr & MA35_FSR_RX_OVER_IF) + up->port.icount.overrun++; + + serial_out(up, MA35_FSR_REG, + fsr & (MA35_FSR_BIF | MA35_FSR_FEF | + MA35_FSR_PEF | MA35_FSR_RX_OVER_IF)); + if (fsr & MA35_FSR_BIF) + flag = TTY_BREAK; + else if (fsr & MA35_FSR_PEF) + flag = TTY_PARITY; + else if (fsr & MA35_FSR_FEF) + flag = TTY_FRAME; + } + + ch = serial_in(up, MA35_RBR_REG); + if (uart_handle_sysrq_char(&up->port, ch)) + continue; + + spin_lock(&up->port.lock); + uart_insert_char(&up->port, fsr, MA35_FSR_RX_OVER_IF, ch, flag); + spin_unlock(&up->port.lock); + + fsr = serial_in(up, MA35_FSR_REG); + } while (!(fsr & MA35_FSR_RX_EMPTY) && (max_count-- > 0)); + + spin_lock(&up->port.lock); + tty_flip_buffer_push(&up->port.state->port); + spin_unlock(&up->port.lock); +} + +static irqreturn_t ma35d1serial_interrupt(int irq, void *dev_id) +{ + struct uart_port *port = dev_id; + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + u32 isr, fsr; + + isr = serial_in(up, MA35_ISR_REG); + fsr = serial_in(up, MA35_FSR_REG); + + if (!(isr & MA35_ISR_IF_CHECK)) + return IRQ_NONE; + + if (isr & (MA35_ISR_RDA_IF | MA35_ISR_RXTO_IF)) + receive_chars(up); + if (isr & MA35_ISR_THRE_INT) + transmit_chars(up); + if (fsr & MA35_FSR_TX_OVER_IF) + serial_out(up, MA35_FSR_REG, MA35_FSR_TX_OVER_IF); + + return IRQ_HANDLED; +} + +static u32 ma35d1serial_tx_empty(struct uart_port *port) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + u32 fsr; + + fsr = serial_in(up, MA35_FSR_REG); + if ((fsr & MA35_FSR_TX_BOTH_EMPTY) == MA35_FSR_TX_BOTH_EMPTY) + return TIOCSER_TEMT; + else + return 0; +} + +static u32 ma35d1serial_get_mctrl(struct uart_port *port) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + u32 status; + u32 ret = 0; + + status = serial_in(up, MA35_MSR_REG); + if (!(status & MA35_MSR_CTSSTS)) + ret |= TIOCM_CTS; + return ret; +} + +static void ma35d1serial_set_mctrl(struct uart_port *port, u32 mctrl) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + u32 mcr, msr, ier; + + mcr = serial_in(up, MA35_MCR_REG); + mcr &= ~MA35_MCR_RTS_CTRL; + + if (mctrl & TIOCM_RTS) + mcr |= MA35_MCR_RTSACTLV; + else + mcr &= ~MA35_MCR_RTSACTLV; + + if (up->mcr & UART_MCR_AFE) { + ier = serial_in(up, MA35_IER_REG); + ier |= MA35_IER_AUTO_RTS | MA35_IER_AUTO_CTS; + serial_out(up, MA35_IER_REG, ier); + up->port.flags |= UPF_HARD_FLOW; + } else { + ier = serial_in(up, MA35_IER_REG); + ier &= ~(MA35_IER_AUTO_RTS | MA35_IER_AUTO_CTS); + serial_out(up, MA35_IER_REG, ier); + up->port.flags &= ~UPF_HARD_FLOW; + } + + msr = serial_in(up, MA35_MSR_REG); + msr |= MA35_MSR_CTSACTLV; + serial_out(up, MA35_MSR_REG, msr); + serial_out(up, MA35_MCR_REG, mcr); +} + +static void ma35d1serial_break_ctl(struct uart_port *port, int break_state) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + unsigned long flags; + u32 lcr; + + spin_lock_irqsave(&up->port.lock, flags); + lcr = serial_in(up, MA35_LCR_REG); + if (break_state != 0) + lcr |= MA35_LCR_BREAK; + else + lcr &= ~MA35_LCR_BREAK; + serial_out(up, MA35_LCR_REG, lcr); + spin_unlock_irqrestore(&up->port.lock, flags); +} + +static int ma35d1serial_startup(struct uart_port *port) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + u32 fcr; + int retval; + + /* Reset FIFO */ + serial_out(up, MA35_FCR_REG, MA35_FCR_TFR | MA35_FCR_RFR); + + /* Clear pending interrupts */ + serial_out(up, MA35_ISR_REG, MA35_ISR_ALL); + + retval = request_irq(port->irq, ma35d1serial_interrupt, 0, + dev_name(port->dev), port); + if (retval) { + dev_err(up->port.dev, "request irq failed.\n"); + return retval; + } + + fcr = serial_in(up, MA35_FCR_REG); + fcr |= MA35_FCR_RFITL_4BYTES | MA35_FCR_RTSTL_8BYTES; + serial_out(up, MA35_FCR_REG, fcr); + serial_out(up, MA35_LCR_REG, MA35_LCR_WLS_8BITS); + serial_out(up, MA35_TOR_REG, MA35_UART_RX_TOUT); + serial_out(up, MA35_IER_REG, MA35_IER_CONFIG); + return 0; +} + +static void ma35d1serial_shutdown(struct uart_port *port) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + + serial_out(up, MA35_IER_REG, 0); + free_irq(port->irq, port); +} + +static void ma35d1serial_set_termios(struct uart_port *port, + struct ktermios *termios, + const struct ktermios *old) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + unsigned long flags; + u32 baud, quot; + u32 lcr = 0; + + lcr = UART_LCR_WLEN(tty_get_char_size(termios->c_cflag)); + + if (termios->c_cflag & CSTOPB) + lcr |= MA35_LCR_NSB; + if (termios->c_cflag & PARENB) + lcr |= MA35_LCR_PBE; + if (!(termios->c_cflag & PARODD)) + lcr |= MA35_LCR_EPE; + if (termios->c_cflag & CMSPAR) + lcr |= MA35_LCR_SPE; + + baud = uart_get_baud_rate(port, termios, old, + port->uartclk / MA35_BAUD_DIV_MAX, + port->uartclk / MA35_BAUD_DIV_MIN); + + /* MA35D1 UART baud rate equation: baudrate = UART_CLK / (quot + 2) */ + quot = (port->uartclk / baud) - 2; + + /* + * Ok, we're now changing the port state. Do it with + * interrupts disabled. + */ + spin_lock_irqsave(&up->port.lock, flags); + + up->port.read_status_mask = MA35_FSR_RX_OVER_IF; + if (termios->c_iflag & INPCK) + up->port.read_status_mask |= MA35_FSR_FEF | MA35_FSR_PEF; + if (termios->c_iflag & (BRKINT | PARMRK)) + up->port.read_status_mask |= MA35_FSR_BIF; + + /* Characteres to ignore */ + up->port.ignore_status_mask = 0; + if (termios->c_iflag & IGNPAR) + up->port.ignore_status_mask |= MA35_FSR_FEF | MA35_FSR_PEF; + if (termios->c_iflag & IGNBRK) { + up->port.ignore_status_mask |= MA35_FSR_BIF; + /* + * If we're ignoring parity and break indicators, + * ignore overruns too (for real raw support). + */ + if (termios->c_iflag & IGNPAR) + up->port.ignore_status_mask |= MA35_FSR_RX_OVER_IF; + } + if (termios->c_cflag & CRTSCTS) + up->mcr |= UART_MCR_AFE; + else + up->mcr &= ~UART_MCR_AFE; + + uart_update_timeout(port, termios->c_cflag, baud); + + ma35d1serial_set_mctrl(&up->port, up->port.mctrl); + + serial_out(up, MA35_BAUD_REG, MA35_BAUD_MODE2 | FIELD_PREP(MA35_BAUD_MASK, quot)); + + serial_out(up, MA35_LCR_REG, lcr); + + spin_unlock_irqrestore(&up->port.lock, flags); +} + +static const char *ma35d1serial_type(struct uart_port *port) +{ + return "ma35d1-uart"; +} + +static void ma35d1serial_config_port(struct uart_port *port, int flags) +{ + /* + * Driver core for serial ports forces a non-zero value for port type. + * Write an arbitrary value here to accommodate the serial core driver, + * as ID part of UAPI is redundant. + */ + port->type = 1; +} + +static int ma35d1serial_verify_port(struct uart_port *port, struct serial_struct *ser) +{ + if (port->type != PORT_UNKNOWN && ser->type != 1) + return -EINVAL; + + return 0; +} + +static const struct uart_ops ma35d1serial_ops = { + .tx_empty = ma35d1serial_tx_empty, + .set_mctrl = ma35d1serial_set_mctrl, + .get_mctrl = ma35d1serial_get_mctrl, + .stop_tx = ma35d1serial_stop_tx, + .start_tx = ma35d1serial_start_tx, + .stop_rx = ma35d1serial_stop_rx, + .break_ctl = ma35d1serial_break_ctl, + .startup = ma35d1serial_startup, + .shutdown = ma35d1serial_shutdown, + .set_termios = ma35d1serial_set_termios, + .type = ma35d1serial_type, + .config_port = ma35d1serial_config_port, + .verify_port = ma35d1serial_verify_port, +}; + +static const struct of_device_id ma35d1_serial_of_match[] = { + { .compatible = "nuvoton,ma35d1-uart" }, + {}, +}; +MODULE_DEVICE_TABLE(of, ma35d1_serial_of_match); + +#ifdef CONFIG_SERIAL_NUVOTON_MA35D1_CONSOLE + +static struct device_node *ma35d1serial_uart_nodes[MA35_UART_NR]; + +static void wait_for_xmitr(struct uart_ma35d1_port *up) +{ + unsigned int reg = 0; + + read_poll_timeout_atomic(serial_in, reg, reg & MA35_FSR_TX_EMPTY, + 1, 10000, false, + up, MA35_FSR_REG); +} + +static void ma35d1serial_console_putchar(struct uart_port *port, unsigned char ch) +{ + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + + wait_for_xmitr(up); + serial_out(up, MA35_THR_REG, ch); +} + +/* + * Print a string to the serial port trying not to disturb + * any possible real use of the port... + * + * The console_lock must be held when we get here. + */ +static void ma35d1serial_console_write(struct console *co, const char *s, u32 count) +{ + struct uart_ma35d1_port *up = &ma35d1serial_ports[co->index]; + unsigned long flags; + int locked = 1; + u32 ier; + + if (up->port.sysrq) + locked = 0; + else if (oops_in_progress) + locked = spin_trylock_irqsave(&up->port.lock, flags); + else + spin_lock_irqsave(&up->port.lock, flags); + + /* + * First save the IER then disable the interrupts + */ + ier = serial_in(up, MA35_IER_REG); + serial_out(up, MA35_IER_REG, 0); + + uart_console_write(&up->port, s, count, ma35d1serial_console_putchar); + + wait_for_xmitr(up); + serial_out(up, MA35_IER_REG, ier); + + if (locked) + spin_unlock_irqrestore(&up->port.lock, flags); +} + +static int __init ma35d1serial_console_setup(struct console *co, char *options) +{ + struct device_node *np; + struct uart_ma35d1_port *p; + u32 val32[4]; + struct uart_port *port; + int baud = 115200; + int bits = 8; + int parity = 'n'; + int flow = 'n'; + + if ((co->index < 0) || (co->index >= MA35_UART_NR)) { + pr_debug("Console Port%x out of range\n", co->index); + return -EINVAL; + } + + np = ma35d1serial_uart_nodes[co->index]; + p = &ma35d1serial_ports[co->index]; + if (!np || !p) + return -ENODEV; + + if (of_property_read_u32_array(np, "reg", val32, ARRAY_SIZE(val32)) != 0) + return -EINVAL; + + p->port.iobase = val32[1]; + p->port.membase = ioremap(p->port.iobase, MA35_UART_REG_SIZE); + if (!p->port.membase) + return -ENOMEM; + + p->port.ops = &ma35d1serial_ops; + p->port.line = 0; + p->port.uartclk = MA35_UART_CONSOLE_CLK; + + port = &ma35d1serial_ports[co->index].port; + + if (options) + uart_parse_options(options, &baud, &parity, &bits, &flow); + + return uart_set_options(port, co, baud, parity, bits, flow); +} + +static struct console ma35d1serial_console = { + .name = "ttyNVT", + .write = ma35d1serial_console_write, + .device = uart_console_device, + .setup = ma35d1serial_console_setup, + .flags = CON_PRINTBUFFER | CON_ENABLED, + .index = -1, + .data = &ma35d1serial_reg, +}; + +static void ma35d1serial_console_init_port(void) +{ + u32 i = 0; + struct device_node *np; + + for_each_matching_node(np, ma35d1_serial_of_match) { + if (ma35d1serial_uart_nodes[i] == NULL) { + of_node_get(np); + ma35d1serial_uart_nodes[i] = np; + i++; + if (i == MA35_UART_NR) + break; + } + } +} + +static int __init ma35d1serial_console_init(void) +{ + ma35d1serial_console_init_port(); + register_console(&ma35d1serial_console); + return 0; +} +console_initcall(ma35d1serial_console_init); + +#define MA35D1SERIAL_CONSOLE (&ma35d1serial_console) +#else +#define MA35D1SERIAL_CONSOLE NULL +#endif + +static struct uart_driver ma35d1serial_reg = { + .owner = THIS_MODULE, + .driver_name = "serial", + .dev_name = "ttyNVT", + .major = TTY_MAJOR, + .minor = 64, + .cons = MA35D1SERIAL_CONSOLE, + .nr = MA35_UART_NR, +}; + +/* + * Register a set of serial devices attached to a platform device. + * The list is terminated with a zero flags entry, which means we expect + * all entries to have at least UPF_BOOT_AUTOCONF set. + */ +static int ma35d1serial_probe(struct platform_device *pdev) +{ + struct resource *res_mem; + struct uart_ma35d1_port *up; + int ret = 0; + + if (pdev->dev.of_node) { + ret = of_alias_get_id(pdev->dev.of_node, "serial"); + if (ret < 0) { + dev_err(&pdev->dev, "failed to get alias/pdev id, errno %d\n", ret); + return ret; + } + } + up = &ma35d1serial_ports[ret]; + up->port.line = ret; + res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res_mem) + return -ENODEV; + + up->port.iobase = res_mem->start; + up->port.membase = ioremap(up->port.iobase, MA35_UART_REG_SIZE); + up->port.ops = &ma35d1serial_ops; + + spin_lock_init(&up->port.lock); + + up->clk = of_clk_get(pdev->dev.of_node, 0); + if (IS_ERR(up->clk)) { + ret = PTR_ERR(up->clk); + dev_err(&pdev->dev, "failed to get core clk: %d\n", ret); + goto err_iounmap; + } + + ret = clk_prepare_enable(up->clk); + if (ret) + goto err_iounmap; + + if (up->port.line != 0) + up->port.uartclk = clk_get_rate(up->clk); + + ret = platform_get_irq(pdev, 0); + if (ret < 0) + goto err_clk_disable; + + up->port.irq = ret; + up->port.dev = &pdev->dev; + up->port.flags = UPF_BOOT_AUTOCONF; + + platform_set_drvdata(pdev, up); + + ret = uart_add_one_port(&ma35d1serial_reg, &up->port); + if (ret < 0) + goto err_free_irq; + + return 0; + +err_free_irq: + free_irq(up->port.irq, &up->port); + +err_clk_disable: + clk_disable_unprepare(up->clk); + +err_iounmap: + iounmap(up->port.membase); + return ret; +} + +/* + * Remove serial ports registered against a platform device. + */ +static int ma35d1serial_remove(struct platform_device *dev) +{ + struct uart_port *port = platform_get_drvdata(dev); + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + + uart_remove_one_port(&ma35d1serial_reg, port); + clk_disable_unprepare(up->clk); + return 0; +} + +static int ma35d1serial_suspend(struct platform_device *dev, pm_message_t state) +{ + struct uart_port *port = platform_get_drvdata(dev); + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + + uart_suspend_port(&ma35d1serial_reg, &up->port); + if (up->port.line == 0) { + up->console_baud_rate = serial_in(up, MA35_BAUD_REG); + up->console_line = serial_in(up, MA35_LCR_REG); + up->console_int = serial_in(up, MA35_IER_REG); + } + return 0; +} + +static int ma35d1serial_resume(struct platform_device *dev) +{ + struct uart_port *port = platform_get_drvdata(dev); + struct uart_ma35d1_port *up = to_ma35d1_uart_port(port); + + if (up->port.line == 0) { + serial_out(up, MA35_BAUD_REG, up->console_baud_rate); + serial_out(up, MA35_LCR_REG, up->console_line); + serial_out(up, MA35_IER_REG, up->console_int); + } + uart_resume_port(&ma35d1serial_reg, &up->port); + return 0; +} + +static struct platform_driver ma35d1serial_driver = { + .probe = ma35d1serial_probe, + .remove = ma35d1serial_remove, + .suspend = ma35d1serial_suspend, + .resume = ma35d1serial_resume, + .driver = { + .name = "ma35d1-uart", + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(ma35d1_serial_of_match), + }, +}; + +static int __init ma35d1serial_init(void) +{ + int ret; + + ret = uart_register_driver(&ma35d1serial_reg); + if (ret) + return ret; + + ret = platform_driver_register(&ma35d1serial_driver); + if (ret) + uart_unregister_driver(&ma35d1serial_reg); + + return ret; +} + +static void __exit ma35d1serial_exit(void) +{ + platform_driver_unregister(&ma35d1serial_driver); + uart_unregister_driver(&ma35d1serial_reg); +} + +module_init(ma35d1serial_init); +module_exit(ma35d1serial_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("MA35D1 serial driver"); -- cgit v1.2.3 From 27a826837ec9a3e94cc44bd9328b8289b0fcecd7 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 19 Jun 2023 12:45:17 +0300 Subject: serial: atmel: don't enable IRQs prematurely The atmel_complete_tx_dma() function disables IRQs at the start of the function by calling spin_lock_irqsave(&port->lock, flags); There is no need to disable them a second time using the spin_lock_irq() function and, in fact, doing so is a bug because it will enable IRQs prematurely when we call spin_unlock_irq(). Just use spin_lock/unlock() instead without disabling or enabling IRQs. Fixes: 08f738be88bb ("serial: at91: add tx dma support") Signed-off-by: Dan Carpenter Reviewed-by: Jiri Slaby Acked-by: Richard Genoud Link: https://lore.kernel.org/r/cb7c39a9-c004-4673-92e1-be4e34b85368@moroto.mountain Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/atmel_serial.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index 6e9192f122aa..3467a875641a 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -868,11 +868,11 @@ static void atmel_complete_tx_dma(void *arg) dmaengine_terminate_all(chan); uart_xmit_advance(port, atmel_port->tx_len); - spin_lock_irq(&atmel_port->lock_tx); + spin_lock(&atmel_port->lock_tx); async_tx_ack(atmel_port->desc_tx); atmel_port->cookie_tx = -EINVAL; atmel_port->desc_tx = NULL; - spin_unlock_irq(&atmel_port->lock_tx); + spin_unlock(&atmel_port->lock_tx); if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) uart_write_wakeup(port); -- cgit v1.2.3 From a82d62f708545d22859584e0e0620da8e3759bbc Mon Sep 17 00:00:00 2001 From: Jiaqing Zhao Date: Mon, 19 Jun 2023 15:57:44 +0000 Subject: Revert "8250: add support for ASIX devices with a FIFO bug" This reverts commit eb26dfe8aa7eeb5a5aa0b7574550125f8aa4c3b3. Commit eb26dfe8aa7e ("8250: add support for ASIX devices with a FIFO bug") merged on Jul 13, 2012 adds a quirk for PCI_VENDOR_ID_ASIX (0x9710). But that ID is the same as PCI_VENDOR_ID_NETMOS defined in 1f8b061050c7 ("[PATCH] Netmos parallel/serial/combo support") merged on Mar 28, 2005. In pci_serial_quirks array, the NetMos entry always takes precedence over the ASIX entry even since it was initially merged, code in that commit is always unreachable. In my tests, adding the FIFO workaround to pci_netmos_init() makes no difference, and the vendor driver also does not have such workaround. Given that the code was never used for over a decade, it's safe to revert it. Also, the real PCI_VENDOR_ID_ASIX should be 0x125b, which is used on their newer AX99100 PCIe serial controllers released on 2016. The FIFO workaround should not be intended for these newer controllers, and it was never implemented in vendor driver. Fixes: eb26dfe8aa7e ("8250: add support for ASIX devices with a FIFO bug") Cc: stable Signed-off-by: Jiaqing Zhao Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20230619155743.827859-1-jiaqing.zhao@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250.h | 1 - drivers/tty/serial/8250/8250_pci.c | 19 ------------------- drivers/tty/serial/8250/8250_port.c | 11 +++-------- include/linux/serial_8250.h | 1 - 4 files changed, 3 insertions(+), 29 deletions(-) diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index 471c6bc5f78f..1aa3e55c8b47 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -91,7 +91,6 @@ struct serial8250_config { #define UART_BUG_TXEN BIT(1) /* UART has buggy TX IIR status */ #define UART_BUG_NOMSR BIT(2) /* UART has buggy MSR status bits (Au1x00) */ #define UART_BUG_THRE BIT(3) /* UART has buggy THRE reassertion */ -#define UART_BUG_PARITY BIT(4) /* UART mishandles parity if FIFO enabled */ #define UART_BUG_TXRACE BIT(5) /* UART Tx fails to set remote DR */ diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c index e80c4f6551a1..d2d547b5da95 100644 --- a/drivers/tty/serial/8250/8250_pci.c +++ b/drivers/tty/serial/8250/8250_pci.c @@ -1232,14 +1232,6 @@ static int pci_oxsemi_tornado_setup(struct serial_private *priv, return pci_default_setup(priv, board, up, idx); } -static int pci_asix_setup(struct serial_private *priv, - const struct pciserial_board *board, - struct uart_8250_port *port, int idx) -{ - port->bugs |= UART_BUG_PARITY; - return pci_default_setup(priv, board, port, idx); -} - #define QPCR_TEST_FOR1 0x3F #define QPCR_TEST_GET1 0x00 #define QPCR_TEST_FOR2 0x40 @@ -1955,7 +1947,6 @@ pci_moxa_setup(struct serial_private *priv, #define PCI_DEVICE_ID_WCH_CH355_4S 0x7173 #define PCI_VENDOR_ID_AGESTAR 0x5372 #define PCI_DEVICE_ID_AGESTAR_9375 0x6872 -#define PCI_VENDOR_ID_ASIX 0x9710 #define PCI_DEVICE_ID_BROADCOM_TRUMANAGE 0x160a #define PCI_DEVICE_ID_AMCC_ADDIDATA_APCI7800 0x818e @@ -2600,16 +2591,6 @@ static struct pci_serial_quirk pci_serial_quirks[] = { .exit = pci_wch_ch38x_exit, .setup = pci_wch_ch38x_setup, }, - /* - * ASIX devices with FIFO bug - */ - { - .vendor = PCI_VENDOR_ID_ASIX, - .device = PCI_ANY_ID, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .setup = pci_asix_setup, - }, /* * Broadcom TruManage (NetXtreme) */ diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index dfb51a854e77..16aeb1420137 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -2632,11 +2632,8 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up, if (c_cflag & CSTOPB) cval |= UART_LCR_STOP; - if (c_cflag & PARENB) { + if (c_cflag & PARENB) cval |= UART_LCR_PARITY; - if (up->bugs & UART_BUG_PARITY) - up->fifo_bug = true; - } if (!(c_cflag & PARODD)) cval |= UART_LCR_EPAR; if (c_cflag & CMSPAR) @@ -2799,8 +2796,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios, up->lcr = cval; /* Save computed LCR */ if (up->capabilities & UART_CAP_FIFO && port->fifosize > 1) { - /* NOTE: If fifo_bug is not set, a user can set RX_trigger. */ - if ((baud < 2400 && !up->dma) || up->fifo_bug) { + if (baud < 2400 && !up->dma) { up->fcr &= ~UART_FCR_TRIGGER_MASK; up->fcr |= UART_FCR_TRIGGER_1; } @@ -3131,8 +3127,7 @@ static int do_set_rxtrig(struct tty_port *port, unsigned char bytes) struct uart_8250_port *up = up_to_u8250p(uport); int rxtrig; - if (!(up->capabilities & UART_CAP_FIFO) || uport->fifosize <= 1 || - up->fifo_bug) + if (!(up->capabilities & UART_CAP_FIFO) || uport->fifosize <= 1) return -EINVAL; rxtrig = bytes_to_fcr_rxtrig(up, bytes); diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h index eb44420b39ec..be65de65fe61 100644 --- a/include/linux/serial_8250.h +++ b/include/linux/serial_8250.h @@ -127,7 +127,6 @@ struct uart_8250_port { struct list_head list; /* ports on this IRQ */ u32 capabilities; /* port capabilities */ u16 bugs; /* port bugs */ - bool fifo_bug; /* min RX trigger if enabled */ unsigned int tx_loadsz; /* transmit fifo load size */ unsigned char acr; unsigned char fcr; -- cgit v1.2.3 From 777e456852f729fa2942187a43a8eafbaecc6876 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 21 Jun 2023 12:16:06 +0200 Subject: tty_audit: use TASK_COMM_LEN for task comm This is the preferred way of declaring an array for get_task_comm(). Signed-off-by: Jiri Slaby (SUSE) Link: https://lore.kernel.org/r/20230621101611.10580-2-jirislaby@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_audit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index ca7afd7b2716..b98b1aef5f6f 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -69,7 +69,7 @@ static void tty_audit_log(const char *description, dev_t dev, ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_TTY); if (ab) { - char name[sizeof(current->comm)]; + char name[TASK_COMM_LEN]; audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d" " minor=%d comm=", description, pid, uid, -- cgit v1.2.3 From e1488513abee9f34a7d0f3bd57ee65d2a7e13426 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 21 Jun 2023 12:16:07 +0200 Subject: tty_audit: use kzalloc() in tty_audit_buf_alloc() tty_audit_buf_alloc() manually erases most of the entries after kmalloc(). So use kzalloc() and remove the manual sets to zero. That way, we are sure that we do not omit anything. Signed-off-by: Jiri Slaby (SUSE) Link: https://lore.kernel.org/r/20230621101611.10580-3-jirislaby@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_audit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index b98b1aef5f6f..43f34465b9df 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -33,16 +33,16 @@ static struct tty_audit_buf *tty_audit_buf_alloc(void) { struct tty_audit_buf *buf; - buf = kmalloc(sizeof(*buf), GFP_KERNEL); + buf = kzalloc(sizeof(*buf), GFP_KERNEL); if (!buf) goto err; + buf->data = kmalloc(N_TTY_BUF_SIZE, GFP_KERNEL); if (!buf->data) goto err_buf; + mutex_init(&buf->mutex); - buf->dev = MKDEV(0, 0); - buf->icanon = 0; - buf->valid = 0; + return buf; err_buf: -- cgit v1.2.3 From 94f94810efc4b4b337c9f2abc8b0188cef5ccb9b Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 21 Jun 2023 12:16:08 +0200 Subject: tty_audit: invert the condition in tty_audit_log() If we cannot obtain an audit buffer in tty_audit_log(), simply return from the function. Apart this is mostly preferred in the kernel, it allows to merge the split audit string while still keeping it readable. Signed-off-by: Jiri Slaby (SUSE) Link: https://lore.kernel.org/r/20230621101611.10580-4-jirislaby@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_audit.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index 43f34465b9df..c6c27acad2ad 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -66,20 +66,20 @@ static void tty_audit_log(const char *description, dev_t dev, uid_t uid = from_kuid(&init_user_ns, task_uid(current)); uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(current)); unsigned int sessionid = audit_get_sessionid(current); + char name[TASK_COMM_LEN]; ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_TTY); - if (ab) { - char name[TASK_COMM_LEN]; - - audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d" - " minor=%d comm=", description, pid, uid, - loginuid, sessionid, MAJOR(dev), MINOR(dev)); - get_task_comm(name, current); - audit_log_untrustedstring(ab, name); - audit_log_format(ab, " data="); - audit_log_n_hex(ab, data, size); - audit_log_end(ab); - } + if (!ab) + return; + + audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d minor=%d comm=", + description, pid, uid, loginuid, sessionid, + MAJOR(dev), MINOR(dev)); + get_task_comm(name, current); + audit_log_untrustedstring(ab, name); + audit_log_format(ab, " data="); + audit_log_n_hex(ab, data, size); + audit_log_end(ab); } /* -- cgit v1.2.3 From 3e540a7bad8534a9b8845defa051834c9694d86c Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 21 Jun 2023 12:16:09 +0200 Subject: tty_audit: make icanon a bool Use bool for tty_audit_buf::icanon in favor of ugly bitfields. And get rid of "!!" as that is completely unnecessary. Signed-off-by: Jiri Slaby (SUSE) Link: https://lore.kernel.org/r/20230621101611.10580-5-jirislaby@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_audit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index c6c27acad2ad..6cf8b63368ee 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -15,7 +15,7 @@ struct tty_audit_buf { struct mutex mutex; /* Protects all data below */ dev_t dev; /* The TTY which the data is from */ - unsigned icanon:1; + bool icanon; size_t valid; unsigned char *data; /* Allocated size N_TTY_BUF_SIZE */ }; @@ -202,8 +202,8 @@ static struct tty_audit_buf *tty_audit_buf_get(void) void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size) { struct tty_audit_buf *buf; - unsigned int icanon = !!L_ICANON(tty); unsigned int audit_tty; + bool icanon = L_ICANON(tty); dev_t dev; audit_tty = READ_ONCE(current->signal->audit_tty); -- cgit v1.2.3 From e64ed44bce43e003dd154c0bc418a431c15bdf77 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 21 Jun 2023 12:16:10 +0200 Subject: tty_audit: make tty pointers in exposed functions const Both tty_audit_add_data() and tty_audit_tiocsti() need only to read from the tty struct, so make the tty parameters of them both const. This aids the compiler a bit. Signed-off-by: Jiri Slaby (SUSE) Link: https://lore.kernel.org/r/20230621101611.10580-6-jirislaby@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty.h | 11 ++++++----- drivers/tty/tty_audit.c | 5 +++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h index 1e0d80e98d26..89769a1f1f97 100644 --- a/drivers/tty/tty.h +++ b/drivers/tty/tty.h @@ -99,14 +99,15 @@ extern int tty_ldisc_autoload; /* tty_audit.c */ #ifdef CONFIG_AUDIT -void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size); -void tty_audit_tiocsti(struct tty_struct *tty, char ch); +void tty_audit_add_data(const struct tty_struct *tty, const void *data, + size_t size); +void tty_audit_tiocsti(const struct tty_struct *tty, char ch); #else -static inline void tty_audit_add_data(struct tty_struct *tty, const void *data, - size_t size) +static inline void tty_audit_add_data(const struct tty_struct *tty, + const void *data, size_t size) { } -static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch) +static inline void tty_audit_tiocsti(const struct tty_struct *tty, char ch) { } #endif diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index 6cf8b63368ee..5cbe28ac1763 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -134,7 +134,7 @@ void tty_audit_fork(struct signal_struct *sig) /* * tty_audit_tiocsti - Log TIOCSTI */ -void tty_audit_tiocsti(struct tty_struct *tty, char ch) +void tty_audit_tiocsti(const struct tty_struct *tty, char ch) { dev_t dev; @@ -199,7 +199,8 @@ static struct tty_audit_buf *tty_audit_buf_get(void) * * Audit @data of @size from @tty, if necessary. */ -void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size) +void tty_audit_add_data(const struct tty_struct *tty, const void *data, + size_t size) { struct tty_audit_buf *buf; unsigned int audit_tty; -- cgit v1.2.3 From e534755c9412be07f579acd2947401a9f87a33c8 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 21 Jun 2023 12:16:11 +0200 Subject: tty_audit: make data of tty_audit_log() const 'data' are only read (passed down to audit_log_n_hex()), so they can be const -- the same what is expected in audit_log_n_hex(). Only a minor cleanup to be consistent. Signed-off-by: Jiri Slaby (SUSE) Link: https://lore.kernel.org/r/20230621101611.10580-7-jirislaby@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_audit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index 5cbe28ac1763..24d010589379 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -59,7 +59,7 @@ static void tty_audit_buf_free(struct tty_audit_buf *buf) } static void tty_audit_log(const char *description, dev_t dev, - unsigned char *data, size_t size) + const unsigned char *data, size_t size) { struct audit_buffer *ab; pid_t pid = task_pid_nr(current); -- cgit v1.2.3