From f956052e00de211b5c9ebaa1958366c23f82ee9e Mon Sep 17 00:00:00 2001 From: Jeongjun Park Date: Fri, 11 Oct 2024 02:46:19 +0900 Subject: [PATCH 01/12] vt: prevent kernel-infoleak in con_font_get() font.data may not initialize all memory spaces depending on the implementation of vc->vc_sw->con_font_get. This may cause info-leak, so to prevent this, it is safest to modify it to initialize the allocated memory space to 0, and it generally does not affect the overall performance of the system. Cc: stable@vger.kernel.org Reported-by: syzbot+955da2d57931604ee691@syzkaller.appspotmail.com Fixes: 05e2600cb0a4 ("VT: Bump font size limitation to 64x128 pixels") Signed-off-by: Jeongjun Park Link: https://lore.kernel.org/r/20241010174619.59662-1-aha310510@gmail.com Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/vt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index cd87e3d1291e..96842ce817af 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -4726,7 +4726,7 @@ static int con_font_get(struct vc_data *vc, struct console_font_op *op) return -EINVAL; if (op->data) { - font.data = kvmalloc(max_font_size, GFP_KERNEL); + font.data = kvzalloc(max_font_size, GFP_KERNEL); if (!font.data) return -ENOMEM; } else From 9462f4ca56e7d2430fdb6dcc8498244acbfc4489 Mon Sep 17 00:00:00 2001 From: Longlong Xia Date: Thu, 26 Sep 2024 21:02:13 +0800 Subject: [PATCH 02/12] tty: n_gsm: Fix use-after-free in gsm_cleanup_mux BUG: KASAN: slab-use-after-free in gsm_cleanup_mux+0x77b/0x7b0 drivers/tty/n_gsm.c:3160 [n_gsm] Read of size 8 at addr ffff88815fe99c00 by task poc/3379 CPU: 0 UID: 0 PID: 3379 Comm: poc Not tainted 6.11.0+ #56 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020 Call Trace: gsm_cleanup_mux+0x77b/0x7b0 drivers/tty/n_gsm.c:3160 [n_gsm] __pfx_gsm_cleanup_mux+0x10/0x10 drivers/tty/n_gsm.c:3124 [n_gsm] __pfx_sched_clock_cpu+0x10/0x10 kernel/sched/clock.c:389 update_load_avg+0x1c1/0x27b0 kernel/sched/fair.c:4500 __pfx_min_vruntime_cb_rotate+0x10/0x10 kernel/sched/fair.c:846 __rb_insert_augmented+0x492/0xbf0 lib/rbtree.c:161 gsmld_ioctl+0x395/0x1450 drivers/tty/n_gsm.c:3408 [n_gsm] _raw_spin_lock_irqsave+0x92/0xf0 arch/x86/include/asm/atomic.h:107 __pfx_gsmld_ioctl+0x10/0x10 drivers/tty/n_gsm.c:3822 [n_gsm] ktime_get+0x5e/0x140 kernel/time/timekeeping.c:195 ldsem_down_read+0x94/0x4e0 arch/x86/include/asm/atomic64_64.h:79 __pfx_ldsem_down_read+0x10/0x10 drivers/tty/tty_ldsem.c:338 __pfx_do_vfs_ioctl+0x10/0x10 fs/ioctl.c:805 tty_ioctl+0x643/0x1100 drivers/tty/tty_io.c:2818 Allocated by task 65: gsm_data_alloc.constprop.0+0x27/0x190 drivers/tty/n_gsm.c:926 [n_gsm] gsm_send+0x2c/0x580 drivers/tty/n_gsm.c:819 [n_gsm] gsm1_receive+0x547/0xad0 drivers/tty/n_gsm.c:3038 [n_gsm] gsmld_receive_buf+0x176/0x280 drivers/tty/n_gsm.c:3609 [n_gsm] tty_ldisc_receive_buf+0x101/0x1e0 drivers/tty/tty_buffer.c:391 tty_port_default_receive_buf+0x61/0xa0 drivers/tty/tty_port.c:39 flush_to_ldisc+0x1b0/0x750 drivers/tty/tty_buffer.c:445 process_scheduled_works+0x2b0/0x10d0 kernel/workqueue.c:3229 worker_thread+0x3dc/0x950 kernel/workqueue.c:3391 kthread+0x2a3/0x370 kernel/kthread.c:389 ret_from_fork+0x2d/0x70 arch/x86/kernel/process.c:147 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:257 Freed by task 3367: kfree+0x126/0x420 mm/slub.c:4580 gsm_cleanup_mux+0x36c/0x7b0 drivers/tty/n_gsm.c:3160 [n_gsm] gsmld_ioctl+0x395/0x1450 drivers/tty/n_gsm.c:3408 [n_gsm] tty_ioctl+0x643/0x1100 drivers/tty/tty_io.c:2818 [Analysis] gsm_msg on the tx_ctrl_list or tx_data_list of gsm_mux can be freed by multi threads through ioctl,which leads to the occurrence of uaf. Protect it by gsm tx lock. Signed-off-by: Longlong Xia Cc: stable Suggested-by: Jiri Slaby Link: https://lore.kernel.org/r/20240926130213.531959-1-xialonglong@kylinos.cn Signed-off-by: Greg Kroah-Hartman --- drivers/tty/n_gsm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 5d37a0984916..252849910588 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -3157,6 +3157,8 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc) mutex_unlock(&gsm->mutex); /* Now wipe the queues */ tty_ldisc_flush(gsm->tty); + + guard(spinlock_irqsave)(&gsm->tx_lock); list_for_each_entry_safe(txq, ntxq, &gsm->tx_ctrl_list, list) kfree(txq); INIT_LIST_HEAD(&gsm->tx_ctrl_list); From 40d7903386df4d18f04d90510ba90eedee260085 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Wed, 2 Oct 2024 20:40:38 +0200 Subject: [PATCH 03/12] serial: imx: Update mctrl old_status on RTSD interrupt When sending data using DMA at high baudrate (4 Mbdps in local test case) to a device with small RX buffer which keeps asserting RTS after every received byte, it is possible that the iMX UART driver would not recognize the falling edge of RTS input signal and get stuck, unable to transmit any more data. This condition happens when the following sequence of events occur: - imx_uart_mctrl_check() is called at some point and takes a snapshot of UART control signal status into sport->old_status using imx_uart_get_hwmctrl(). The RTSS/TIOCM_CTS bit is of interest here (*). - DMA transfer occurs, the remote device asserts RTS signal after each byte. The i.MX UART driver recognizes each such RTS signal change, raises an interrupt with USR1 register RTSD bit set, which leads to invocation of __imx_uart_rtsint(), which calls uart_handle_cts_change(). - If the RTS signal is deasserted, uart_handle_cts_change() clears port->hw_stopped and unblocks the port for further data transfers. - If the RTS is asserted, uart_handle_cts_change() sets port->hw_stopped and blocks the port for further data transfers. This may occur as the last interrupt of a transfer, which means port->hw_stopped remains set and the port remains blocked (**). - Any further data transfer attempts will trigger imx_uart_mctrl_check(), which will read current status of UART control signals by calling imx_uart_get_hwmctrl() (***) and compare it with sport->old_status . - If current status differs from sport->old_status for RTS signal, uart_handle_cts_change() is called and possibly unblocks the port by clearing port->hw_stopped . - If current status does not differ from sport->old_status for RTS signal, no action occurs. This may occur in case prior snapshot (*) was taken before any transfer so the RTS is deasserted, current snapshot (***) was taken after a transfer and therefore RTS is deasserted again, which means current status and sport->old_status are identical. In case (**) triggered when RTS got asserted, and made port->hw_stopped set, the port->hw_stopped will remain set because no change on RTS line is recognized by this driver and uart_handle_cts_change() is not called from here to unblock the port->hw_stopped. Update sport->old_status in __imx_uart_rtsint() accordingly to make imx_uart_mctrl_check() detect such RTS change. Note that TIOCM_CAR and TIOCM_RI bits in sport->old_status do not suffer from this problem. Fixes: ceca629e0b48 ("[ARM] 2971/1: i.MX uart handle rts irq") Cc: stable Reviewed-by: Esben Haabendal Signed-off-by: Marek Vasut Link: https://lore.kernel.org/r/20241002184133.19427-1-marex@denx.de Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/imx.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 67d4a72eda77..90974d338f3c 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -762,6 +762,21 @@ static irqreturn_t __imx_uart_rtsint(int irq, void *dev_id) imx_uart_writel(sport, USR1_RTSD, USR1); usr1 = imx_uart_readl(sport, USR1) & USR1_RTSS; + /* + * Update sport->old_status here, so any follow-up calls to + * imx_uart_mctrl_check() will be able to recognize that RTS + * state changed since last imx_uart_mctrl_check() call. + * + * In case RTS has been detected as asserted here and later on + * deasserted by the time imx_uart_mctrl_check() was called, + * imx_uart_mctrl_check() can detect the RTS state change and + * trigger uart_handle_cts_change() to unblock the port for + * further TX transfers. + */ + if (usr1 & USR1_RTSS) + sport->old_status |= TIOCM_CTS; + else + sport->old_status &= ~TIOCM_CTS; uart_handle_cts_change(&sport->port, usr1); wake_up_interruptible(&sport->port.state->port.delta_msr_wait); From 4bef7c6f299910f19876ad8e7f5897514855f1d2 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Oct 2024 16:51:02 +0200 Subject: [PATCH 04/12] serial: qcom-geni: fix polled console initialisation The polled console (KGDB/KDB) implementation must not call port setup unconditionally as the port may already be in use by the console or a getty. Only make sure that the receiver is enabled, but do not enable any device interrupts. Fixes: d8851a96ba25 ("tty: serial: qcom-geni-serial: Add a poll_init() function") Cc: stable@vger.kernel.org # 6.4 Cc: Douglas Anderson Signed-off-by: Johan Hovold Reviewed-by: Douglas Anderson Link: https://lore.kernel.org/r/20241009145110.16847-2-johan+linaro@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/qcom_geni_serial.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 6f0db310cf69..c237c9d107cd 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -147,6 +147,7 @@ static struct uart_driver qcom_geni_uart_driver; static void __qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport); static void qcom_geni_serial_cancel_tx_cmd(struct uart_port *uport); +static int qcom_geni_serial_port_setup(struct uart_port *uport); static inline struct qcom_geni_serial_port *to_dev_port(struct uart_port *uport) { @@ -395,6 +396,23 @@ static void qcom_geni_serial_poll_put_char(struct uart_port *uport, writel(c, uport->membase + SE_GENI_TX_FIFOn); qcom_geni_serial_poll_tx_done(uport); } + +static int qcom_geni_serial_poll_init(struct uart_port *uport) +{ + struct qcom_geni_serial_port *port = to_dev_port(uport); + int ret; + + if (!port->setup) { + ret = qcom_geni_serial_port_setup(uport); + if (ret) + return ret; + } + + if (!qcom_geni_serial_secondary_active(uport)) + geni_se_setup_s_cmd(&port->se, UART_START_READ, 0); + + return 0; +} #endif #ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE @@ -1582,7 +1600,7 @@ static const struct uart_ops qcom_geni_console_pops = { #ifdef CONFIG_CONSOLE_POLL .poll_get_char = qcom_geni_serial_get_char, .poll_put_char = qcom_geni_serial_poll_put_char, - .poll_init = qcom_geni_serial_port_setup, + .poll_init = qcom_geni_serial_poll_init, #endif .pm = qcom_geni_serial_pm, }; From 19df76662a33d2f2fc41a66607cb8285fc02d6ec Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Oct 2024 16:51:03 +0200 Subject: [PATCH 05/12] serial: qcom-geni: revert broken hibernation support This reverts commit 35781d8356a2eecaa6074ceeb80ee22e252fcdae. Hibernation is not supported on Qualcomm platforms with mainline kernels yet a broken vendor implementation for the GENI serial driver made it upstream. This is effectively dead code that cannot be tested and should just be removed, but if these paths were ever hit for an open non-console port they would crash the machine as the driver would fail to enable clocks during restore() (i.e. all ports would have to be closed by drivers and user space before hibernating the system to avoid this as a comment in the code hinted at). The broken implementation also added a random call to enable the receiver in the port setup code where it does not belong and which enables the receiver prematurely for console ports. Fixes: 35781d8356a2 ("tty: serial: qcom-geni-serial: Add support for Hibernation feature") Cc: stable@vger.kernel.org # 6.2 Cc: Aniket Randive Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20241009145110.16847-3-johan+linaro@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/qcom_geni_serial.c | 41 ++------------------------- 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index c237c9d107cd..2e4a5361f137 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -1170,7 +1170,6 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport) false, true, true); geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2); geni_se_select_mode(&port->se, port->dev_data->mode); - qcom_geni_serial_start_rx(uport); port->setup = true; return 0; @@ -1799,38 +1798,6 @@ static int qcom_geni_serial_sys_resume(struct device *dev) return ret; } -static int qcom_geni_serial_sys_hib_resume(struct device *dev) -{ - int ret = 0; - struct uart_port *uport; - struct qcom_geni_private_data *private_data; - struct qcom_geni_serial_port *port = dev_get_drvdata(dev); - - uport = &port->uport; - private_data = uport->private_data; - - if (uart_console(uport)) { - geni_icc_set_tag(&port->se, QCOM_ICC_TAG_ALWAYS); - geni_icc_set_bw(&port->se); - ret = uart_resume_port(private_data->drv, uport); - /* - * For hibernation usecase clients for - * console UART won't call port setup during restore, - * hence call port setup for console uart. - */ - qcom_geni_serial_port_setup(uport); - } else { - /* - * Peripheral register settings are lost during hibernation. - * Update setup flag such that port setup happens again - * during next session. Clients of HS-UART will close and - * open the port during hibernation. - */ - port->setup = false; - } - return ret; -} - static const struct qcom_geni_device_data qcom_geni_console_data = { .console = true, .mode = GENI_SE_FIFO, @@ -1842,12 +1809,8 @@ static const struct qcom_geni_device_data qcom_geni_uart_data = { }; static const struct dev_pm_ops qcom_geni_serial_pm_ops = { - .suspend = pm_sleep_ptr(qcom_geni_serial_sys_suspend), - .resume = pm_sleep_ptr(qcom_geni_serial_sys_resume), - .freeze = pm_sleep_ptr(qcom_geni_serial_sys_suspend), - .poweroff = pm_sleep_ptr(qcom_geni_serial_sys_suspend), - .restore = pm_sleep_ptr(qcom_geni_serial_sys_hib_resume), - .thaw = pm_sleep_ptr(qcom_geni_serial_sys_hib_resume), + SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_sys_suspend, + qcom_geni_serial_sys_resume) }; static const struct of_device_id qcom_geni_serial_match_table[] = { From 23f5f5debcaac1399cfeacec215278bf6dbc1d11 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Oct 2024 16:51:04 +0200 Subject: [PATCH 06/12] serial: qcom-geni: fix shutdown race A commit adding back the stopping of tx on port shutdown failed to add back the locking which had also been removed by commit e83766334f96 ("tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown"). Holding the port lock is needed to serialise against the console code, which may update the interrupt enable register and access the port state. Fixes: d8aca2f96813 ("tty: serial: qcom-geni-serial: stop operations in progress at shutdown") Fixes: 947cc4ecc06c ("serial: qcom-geni: fix soft lockup on sw flow control and suspend") Cc: stable@vger.kernel.org # 6.3 Reviewed-by: Bartosz Golaszewski Signed-off-by: Johan Hovold Reviewed-by: Douglas Anderson Link: https://lore.kernel.org/r/20241009145110.16847-4-johan+linaro@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/qcom_geni_serial.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 2e4a5361f137..87cd974b76bf 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -1114,10 +1114,12 @@ static void qcom_geni_serial_shutdown(struct uart_port *uport) { disable_irq(uport->irq); + uart_port_lock_irq(uport); qcom_geni_serial_stop_tx(uport); qcom_geni_serial_stop_rx(uport); qcom_geni_serial_cancel_tx_cmd(uport); + uart_port_unlock_irq(uport); } static void qcom_geni_serial_flush_buffer(struct uart_port *uport) From 23ee4a25661c33e6381d41e848a9060ed6d72845 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Oct 2024 16:51:05 +0200 Subject: [PATCH 07/12] serial: qcom-geni: fix dma rx cancellation Make sure to wait for the DMA transfer to complete when cancelling the rx command on stop_rx(). This specifically prevents the DMA completion interrupt from firing after rx has been restarted, something which can lead to an IOMMU fault and hosed rx when the interrupt handler unmaps the DMA buffer for the new command: qcom_geni_serial 988000.serial: serial engine reports 0 RX bytes in! arm-smmu 15000000.iommu: FSR = 00000402 [Format=2 TF], SID=0x563 arm-smmu 15000000.iommu: FSYNR0 = 00210013 [S1CBNDX=33 WNR PLVL=3] Bluetooth: hci0: command 0xfc00 tx timeout Bluetooth: hci0: Reading QCA version information failed (-110) Also add the missing state machine reset which is needed in case cancellation fails. Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA") Cc: stable@vger.kernel.org # 6.3 Cc: Bartosz Golaszewski Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20241009145110.16847-5-johan+linaro@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/qcom_geni_serial.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 87cd974b76bf..aaf24bd037a7 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -805,17 +805,27 @@ static void qcom_geni_serial_start_rx_fifo(struct uart_port *uport) static void qcom_geni_serial_stop_rx_dma(struct uart_port *uport) { struct qcom_geni_serial_port *port = to_dev_port(uport); + bool done; if (!qcom_geni_serial_secondary_active(uport)) return; geni_se_cancel_s_cmd(&port->se); - qcom_geni_serial_poll_bit(uport, SE_GENI_S_IRQ_STATUS, - S_CMD_CANCEL_EN, true); - - if (qcom_geni_serial_secondary_active(uport)) + done = qcom_geni_serial_poll_bit(uport, SE_DMA_RX_IRQ_STAT, + RX_EOT, true); + if (done) { + writel(RX_EOT | RX_DMA_DONE, + uport->membase + SE_DMA_RX_IRQ_CLR); + } else { qcom_geni_serial_abort_rx(uport); + writel(1, uport->membase + SE_DMA_RX_FSM_RST); + qcom_geni_serial_poll_bit(uport, SE_DMA_RX_IRQ_STAT, + RX_RESET_DONE, true); + writel(RX_RESET_DONE | RX_DMA_DONE, + uport->membase + SE_DMA_RX_IRQ_CLR); + } + if (port->rx_dma_addr) { geni_se_rx_dma_unprep(&port->se, port->rx_dma_addr, DMA_RX_BUF_SIZE); From fa103d2599e11e802c818684cff821baefe7f206 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Oct 2024 16:51:06 +0200 Subject: [PATCH 08/12] serial: qcom-geni: fix receiver enable The receiver is supposed to be enabled in the startup() callback and not in set_termios() which is called also during console setup. This specifically avoids accepting input before the port has been opened (and interrupts enabled), something which can also break the GENI firmware (cancel fails and after abort, the "stale" counter handling appears to be broken so that later input is not processed until twelve chars have been received). There also does not appear to be any need to keep the receiver disabled while updating the port settings. Since commit 6f3c3cafb115 ("serial: qcom-geni: disable interrupts during console writes") the calls to manipulate the secondary interrupts, which were done without holding the port lock, can also lead to the receiver being left disabled when set_termios() races with the console code (e.g. when init opens the tty during boot). This can manifest itself as a serial getty not accepting input. The calls to stop and start rx in set_termios() can similarly race with DMA completion and, for example, cause the DMA buffer to be unmapped twice or the mapping to be leaked. Fix this by only enabling the receiver during startup and while holding the port lock to avoid racing with the console code. Fixes: 6f3c3cafb115 ("serial: qcom-geni: disable interrupts during console writes") Fixes: 2aaa43c70778 ("tty: serial: qcom-geni-serial: add support for serial engine DMA") Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP") Cc: stable@vger.kernel.org # 6.3 Cc: Bartosz Golaszewski Signed-off-by: Johan Hovold Reviewed-by: Douglas Anderson Link: https://lore.kernel.org/r/20241009145110.16847-6-johan+linaro@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/qcom_geni_serial.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index aaf24bd037a7..6c4349ea5720 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -1197,6 +1197,11 @@ static int qcom_geni_serial_startup(struct uart_port *uport) if (ret) return ret; } + + uart_port_lock_irq(uport); + qcom_geni_serial_start_rx(uport); + uart_port_unlock_irq(uport); + enable_irq(uport->irq); return 0; @@ -1282,7 +1287,6 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, unsigned int avg_bw_core; unsigned long timeout; - qcom_geni_serial_stop_rx(uport); /* baud rate */ baud = uart_get_baud_rate(uport, termios, old, 300, 4000000); @@ -1298,7 +1302,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, dev_err(port->se.dev, "Couldn't find suitable clock rate for %u\n", baud * sampling_rate); - goto out_restart_rx; + return; } dev_dbg(port->se.dev, "desired_rate = %u, clk_rate = %lu, clk_div = %u\n", @@ -1389,8 +1393,6 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, writel(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN); writel(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG); writel(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG); -out_restart_rx: - qcom_geni_serial_start_rx(uport); } #ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE From c657243ae12000dc57e3648b0ddd30da9ffd1f14 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Oct 2024 16:51:07 +0200 Subject: [PATCH 09/12] serial: qcom-geni: fix rx cancel dma status bit Cancelling an rx command is signalled using bit 14 of the rx DMA status register and not bit 11. This bit is currently unused, but this error becomes apparent, for example, when tracing the status register when closing the port. Fixes: eddac5af0654 ("soc: qcom: Add GENI based QUP Wrapper driver") Reviewed-by: Douglas Anderson Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20241009145110.16847-7-johan+linaro@kernel.org Signed-off-by: Greg Kroah-Hartman --- include/linux/soc/qcom/geni-se.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h index c3bca9c0bf2c..2996a3c28ef3 100644 --- a/include/linux/soc/qcom/geni-se.h +++ b/include/linux/soc/qcom/geni-se.h @@ -258,8 +258,8 @@ struct geni_se { #define RX_DMA_PARITY_ERR BIT(5) #define RX_DMA_BREAK GENMASK(8, 7) #define RX_GENI_GP_IRQ GENMASK(10, 5) -#define RX_GENI_CANCEL_IRQ BIT(11) #define RX_GENI_GP_IRQ_EXT GENMASK(13, 12) +#define RX_GENI_CANCEL_IRQ BIT(14) /* SE_HW_PARAM_0 fields */ #define TX_FIFO_WIDTH_MSK GENMASK(29, 24) From 8173d74ac12bf231fa3a22ae0ad9d56c2abc4618 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Oct 2024 16:51:08 +0200 Subject: [PATCH 10/12] serial: qcom-geni: drop flip buffer WARN() Drop the unnecessary WARN() in case the TTY buffers are ever full in favour of a rate limited dev_err() which doesn't kill the machine when panic_on_warn is set. Reviewed-by: Douglas Anderson Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20241009145110.16847-8-johan+linaro@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/qcom_geni_serial.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 6c4349ea5720..22e468065666 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -588,9 +588,8 @@ static void handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop) ret = tty_insert_flip_string(tport, port->rx_buf, bytes); if (ret != bytes) { - dev_err(uport->dev, "%s:Unable to push data ret %d_bytes %d\n", - __func__, ret, bytes); - WARN_ON_ONCE(1); + dev_err_ratelimited(uport->dev, "failed to push data (%d < %u)\n", + ret, bytes); } uport->icount.rx += ret; tty_flip_buffer_push(tport); From 4cf4b344c1e1a0682d76f3ec9da90a714e986e90 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Oct 2024 16:51:09 +0200 Subject: [PATCH 11/12] serial: qcom-geni: drop unused receive parameter Serial drivers should not be dropping characters themselves, but at least drop the unused 'drop' parameter from the receive handler for now. Reviewed-by: Douglas Anderson Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20241009145110.16847-9-johan+linaro@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/qcom_geni_serial.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 22e468065666..9dd304cdcd86 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -580,7 +580,7 @@ static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop) } #endif /* CONFIG_SERIAL_QCOM_GENI_CONSOLE */ -static void handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop) +static void handle_rx_uart(struct uart_port *uport, u32 bytes) { struct qcom_geni_serial_port *port = to_dev_port(uport); struct tty_port *tport = &uport->state->port; @@ -873,7 +873,7 @@ static void qcom_geni_serial_handle_rx_dma(struct uart_port *uport, bool drop) } if (!drop) - handle_rx_uart(uport, rx_in, drop); + handle_rx_uart(uport, rx_in); ret = geni_se_rx_dma_prep(&port->se, port->rx_buf, DMA_RX_BUF_SIZE, From be847a3a8d4ec4bf270c2455376d11f2de61bfb3 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Oct 2024 16:51:10 +0200 Subject: [PATCH 12/12] serial: qcom-geni: rename suspend functions Drop the unnecessary "_sys" infix from the suspend PM ops. Signed-off-by: Johan Hovold Reviewed-by: Douglas Anderson Link: https://lore.kernel.org/r/20241009145110.16847-10-johan+linaro@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/qcom_geni_serial.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 9dd304cdcd86..5dfe4e599ad6 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -1779,7 +1779,7 @@ static void qcom_geni_serial_remove(struct platform_device *pdev) uart_remove_one_port(drv, &port->uport); } -static int qcom_geni_serial_sys_suspend(struct device *dev) +static int qcom_geni_serial_suspend(struct device *dev) { struct qcom_geni_serial_port *port = dev_get_drvdata(dev); struct uart_port *uport = &port->uport; @@ -1796,7 +1796,7 @@ static int qcom_geni_serial_sys_suspend(struct device *dev) return uart_suspend_port(private_data->drv, uport); } -static int qcom_geni_serial_sys_resume(struct device *dev) +static int qcom_geni_serial_resume(struct device *dev) { int ret; struct qcom_geni_serial_port *port = dev_get_drvdata(dev); @@ -1822,8 +1822,7 @@ static const struct qcom_geni_device_data qcom_geni_uart_data = { }; static const struct dev_pm_ops qcom_geni_serial_pm_ops = { - SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_sys_suspend, - qcom_geni_serial_sys_resume) + SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_suspend, qcom_geni_serial_resume) }; static const struct of_device_id qcom_geni_serial_match_table[] = {