lwn.git/drivers/tty/serial/imx.c, branch docs-6.8

serial: imx: Put DMA enabled UART in separate lock subclass

2023-10-03T12:57:49+00:00

Lockdep complains about possible circular locking dependencies when the i.MX SDMA driver issues console messages under its spinlock. While the SDMA driver calls back into the UART when issuing a message, the i.MX UART driver will never call back into the SDMA driver for this UART, because DMA is explicitly not used for UARTs providing the console. To avoid the lockdep warnings put the UART port lock for console devices into a separate subclass. This fixes possible deadlock warnings like the following which was provoked by adding a printk to the i.MX SDMA driver at a place where the driver holds its spinlock. ====================================================== WARNING: possible circular locking dependency detected 6.6.0-rc3-00045-g517852be693b-dirty #110 Not tainted ------------------------------------------------------ swapper/0/0 is trying to acquire lock: c1818e04 (console_owner){-...}-{0:0}, at: console_flush_all+0x1c4/0x634 but task is already holding lock: c44649e0 (&vc->lock){-...}-{3:3}, at: sdma_int_handler+0xc4/0x368 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (&vc->lock){-...}-{3:3}: _raw_spin_lock_irqsave+0x4c/0x68 sdma_prep_dma_cyclic+0x1a8/0x21c imx_uart_startup+0x44c/0x5d4 uart_startup+0x120/0x2b0 uart_port_activate+0x44/0x98 tty_port_open+0x80/0xd0 uart_open+0x18/0x20 tty_open+0x120/0x664 chrdev_open+0xc0/0x214 do_dentry_open+0x1d0/0x544 path_openat+0xbb0/0xea0 do_filp_open+0x5c/0xd4 do_sys_openat2+0xb8/0xf0 sys_openat+0x8c/0xd8 ret_fast_syscall+0x0/0x1c -> #1 (&port_lock_key){-.-.}-{3:3}: _raw_spin_lock_irqsave+0x4c/0x68 imx_uart_console_write+0x164/0x1a0 console_flush_all+0x220/0x634 console_unlock+0x64/0x164 vprintk_emit+0xb0/0x390 vprintk_default+0x24/0x2c _printk+0x2c/0x5c register_console+0x244/0x478 serial_core_register_port+0x5c4/0x618 imx_uart_probe+0x4e0/0x7d4 platform_probe+0x58/0xb0 really_probe+0xc4/0x2e0 __driver_probe_device+0x84/0x1a0 driver_probe_device+0x2c/0x108 __driver_attach+0x94/0x17c bus_for_each_dev+0x7c/0xd0 bus_add_driver+0xc4/0x1cc driver_register+0x7c/0x114 imx_uart_init+0x20/0x40 do_one_initcall+0x7c/0x3c4 kernel_init_freeable+0x17c/0x228 kernel_init+0x14/0x140 ret_from_fork+0x14/0x24 -> #0 (console_owner){-...}-{0:0}: __lock_acquire+0x14b0/0x29a0 lock_acquire.part.0+0xb4/0x264 console_flush_all+0x20c/0x634 console_unlock+0x64/0x164 vprintk_emit+0xb0/0x390 vprintk_default+0x24/0x2c _printk+0x2c/0x5c sdma_int_handler+0xcc/0x368 __handle_irq_event_percpu+0x94/0x2d0 handle_irq_event+0x38/0xd0 handle_fasteoi_irq+0x98/0x248 handle_irq_desc+0x1c/0x2c gic_handle_irq+0x6c/0x90 generic_handle_arch_irq+0x2c/0x64 __irq_svc+0x90/0xbc cpuidle_enter_state+0x1a0/0x4f4 cpuidle_enter+0x30/0x40 do_idle+0x210/0x2b4 cpu_startup_entry+0x28/0x2c rest_init+0xd0/0x184 arch_post_acpi_subsys_init+0x0/0x8 other info that might help us debug this: Chain exists of: console_owner --> &port_lock_key --> &vc->lock Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&vc->lock); lock(&port_lock_key); lock(&vc->lock); lock(console_owner); *** DEADLOCK *** 3 locks held by swapper/0/0: #0: c44649e0 (&vc->lock){-...}-{3:3}, at: sdma_int_handler+0xc4/0x368 #1: c1818d50 (console_lock){+.+.}-{0:0}, at: vprintk_default+0x24/0x2c #2: c1818d08 (console_srcu){....}-{0:0}, at: console_flush_all+0x44/0x634 stack backtrace: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.6.0-rc3-00045-g517852be693b-dirty #110 Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) unwind_backtrace from show_stack+0x10/0x14 show_stack from dump_stack_lvl+0x60/0x90 dump_stack_lvl from check_noncircular+0x184/0x1b8 check_noncircular from __lock_acquire+0x14b0/0x29a0 __lock_acquire from lock_acquire.part.0+0xb4/0x264 lock_acquire.part.0 from console_flush_all+0x20c/0x634 console_flush_all from console_unlock+0x64/0x164 console_unlock from vprintk_emit+0xb0/0x390 vprintk_emit from vprintk_default+0x24/0x2c vprintk_default from _printk+0x2c/0x5c _printk from sdma_int_handler+0xcc/0x368 sdma_int_handler from __handle_irq_event_percpu+0x94/0x2d0 __handle_irq_event_percpu from handle_irq_event+0x38/0xd0 handle_irq_event from handle_fasteoi_irq+0x98/0x248 handle_fasteoi_irq from handle_irq_desc+0x1c/0x2c handle_irq_desc from gic_handle_irq+0x6c/0x90 gic_handle_irq from generic_handle_arch_irq+0x2c/0x64 generic_handle_arch_irq from __irq_svc+0x90/0xbc Exception stack(0xc1801ee8 to 0xc1801f30) 1ee0: ffffffff ffffffff 00000001 00030349 00000000 00000012 1f00: 00000000 d7e45f4b 00000012 00000000 d7e16d63 c1810828 00000000 c1801f38 1f20: c108125c c1081260 60010013 ffffffff __irq_svc from cpuidle_enter_state+0x1a0/0x4f4 cpuidle_enter_state from cpuidle_enter+0x30/0x40 cpuidle_enter from do_idle+0x210/0x2b4 do_idle from cpu_startup_entry+0x28/0x2c cpu_startup_entry from rest_init+0xd0/0x184 rest_init from arch_post_acpi_subsys_init+0x0/0x8 Reported-by: Tim van der Staaij Signed-off-by: Sascha Hauer Link: https://lore.kernel.org/r/20230928064320.711603-1-s.hauer@pengutronix.de Signed-off-by: Greg Kroah-Hartman

serial: imx: Use port lock wrappers

2023-09-18T09:18:11+00:00

When a serial port is used for kernel console output, then all modifications to the UART registers which are done from other contexts, e.g. getty, termios, are interference points for the kernel console. So far this has been ignored and the printk output is based on the principle of hope. The rework of the console infrastructure which aims to support threaded and atomic consoles, requires to mark sections which modify the UART registers as unsafe. This allows the atomic write function to make informed decisions and eventually to restore operational state. It also allows to prevent the regular UART code from modifying UART registers while printk output is in progress. All modifications of UART registers are guarded by the UART port lock, which provides an obvious synchronization point with the console infrastructure. To avoid adding this functionality to all UART drivers, wrap the spin_[un]lock*() invocations for uart_port::lock into helper functions which just contain the spin_[un]lock*() invocations for now. In a subsequent step these helpers will gain the console synchronization mechanisms. Converted with coccinelle. No functional change. Signed-off-by: Thomas Gleixner Signed-off-by: John Ogness Link: https://lore.kernel.org/r/20230914183831.587273-30-john.ogness@linutronix.de Signed-off-by: Greg Kroah-Hartman

serial: imx: Simplify compatibility handling

2023-09-18T09:14:43+00:00

Three of the four entries of imx_uart_devdata[] use .uts_reg = IMX21_UTS. The difference in the .devtype member isn't relevant, the only thing that matters is if is equal to IMX1_UART. So use an entry with .devtype = IMX21_UART on all platforms but i.MX1. There is no need to have the dev types in an array, so split them up in two separate variables. The fsl,imx53-uart devinfo can go away because in the binding and also the dts files all fsl,imx53-uart devices also are compatible to fsl,imx21-uart. That's not the case for fsl,imx6q-uart (which is a bit strange IMHO), so the fsl,imx6q-uart must stay around. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230911085451.628798-1-u.kleine-koenig@pengutronix.de Signed-off-by: Greg Kroah-Hartman

serial: imx: Use devm_platform_get_and_ioremap_resource()

2023-07-25T18:26:07+00:00

Convert platform_get_resource(), devm_ioremap_resource() to a single call to devm_platform_get_and_ioremap_resource(), as this is exactly what this function does. Signed-off-by: Yangtao Li Link: https://lore.kernel.org/r/20230712062853.11007-14-frank.li@vivo.com Signed-off-by: Greg Kroah-Hartman

tty: Explicitly include correct DT includes

2023-07-25T18:19:05+00:00

The DT of_device.h and of_platform.h date back to the separate of_platform_bus_type before it as merged into the regular platform bus. As part of that merge prepping Arm DT support 13 years ago, they "temporarily" include each other. They also include platform_device.h and of.h. As a result, there's a pretty much random mix of those include files used throughout the tree. In order to detangle these headers and replace the implicit includes with struct declarations, users need to explicitly include the correct includes. Signed-off-by: Rob Herring Acked-by: Uwe Kleine-König # for imx Link: https://lore.kernel.org/r/20230724205440.767071-1-robh@kernel.org Signed-off-by: Greg Kroah-Hartman

tty: serial: imx: fix rs485 rx after tx

2023-06-19T13:41:13+00:00

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

serial: Make uart_remove_one_port() return void

2023-05-13T10:48:16+00:00

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

serial: imx: remove unused imx_uart_is_imx* functions

2023-03-29T08:53:46+00:00

clang with W=1 reports drivers/tty/serial/imx.c:292:19: error: unused function 'imx_uart_is_imx21' [-Werror,-Wunused-function] static inline int imx_uart_is_imx21(struct imx_port *sport) ^ drivers/tty/serial/imx.c:297:19: error: unused function 'imx_uart_is_imx53' [-Werror,-Wunused-function] static inline int imx_uart_is_imx53(struct imx_port *sport) ^ drivers/tty/serial/imx.c:302:19: error: unused function 'imx_uart_is_imx6q' [-Werror,-Wunused-function] static inline int imx_uart_is_imx6q(struct imx_port *sport) ^ These static functions are not used, so remove them. Signed-off-by: Tom Rix Reviewed-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230317205710.1672232-1-trix@redhat.com Signed-off-by: Greg Kroah-Hartman

serial: Use of_property_read_bool() for boolean properties

2023-03-16T12:02:14+00:00

It is preferred to use typed property access functions (i.e. of_property_read_ functions) rather than low-level of_get_property/of_find_property functions for reading properties. Convert reading boolean properties to to of_property_read_bool(). Signed-off-by: Rob Herring Acked-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230310144727.1545699-1-robh@kernel.org Signed-off-by: Greg Kroah-Hartman

serial: Use of_property_present() for testing DT property presence

2023-03-16T12:02:11+00:00

It is preferred to use typed property access functions (i.e. of_property_read_ functions) rather than low-level of_get_property/of_find_property functions for reading properties. As part of this, convert of_get_property/of_find_property calls to the recently added of_property_present() helper when we just want to test for presence of a property and nothing more. Signed-off-by: Rob Herring Acked-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20230310144727.1545630-1-robh@kernel.org Signed-off-by: Greg Kroah-Hartman