| Age | Commit message (Collapse) | Author |
|---|
|
We need the tty/serial fixes in here as well
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 <stable@kernel.org>
Reviewed-by: Esben Haabendal <esben@geanix.com>
Signed-off-by: Marek Vasut <marex@denx.de>
Link: https://lore.kernel.org/r/20241002184133.19427-1-marex@denx.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Implements the necessary callbacks to switch the imx console driver to
perform as an nbcon console.
Add implementations for the nbcon consoles (write_atomic, write_thread,
driver_enter, driver_exit) and add CON_NBCON to the initial flags.
The legacy code is kept in order to easily switch back to legacy mode
by defining CONFIG_SERIAL_IMX_LEGACY_CONSOLE.
Signed-off-by: Esben Haabendal <esben@geanix.com>
Link: https://lore.kernel.org/r/20240913-serial-imx-nbcon-v3-1-4c627302335b@geanix.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Comments regarding status of port.lock on internal functions is useful when
reviewing correct handling of registers that must be protected by this
lock.
Signed-off-by: Esben Haabendal <esben@geanix.com>
Link: https://lore.kernel.org/r/20240913-serial-imx-lockfix-v1-2-4d102746c89d@geanix.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The port lock needs to be held when doing read-modify-write on UCR1 and
UCR3.
Signed-off-by: Esben Haabendal <esben@geanix.com>
Link: https://lore.kernel.org/r/20240913-serial-imx-lockfix-v1-1-4d102746c89d@geanix.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
After commit 0edb555a65d1 ("platform: Make platform_driver::remove()
return void") .remove() is (again) the right callback to implement for
platform drivers.
Convert all platform drivers below drivers/tty/serial to use .remove(),
with the eventual goal to drop struct platform_driver::remove_new(). As
.remove() and .remove_new() have the same prototypes, conversion is done
by just changing the structure member name in the driver initializer.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Link: https://lore.kernel.org/r/20241007205803.444994-7-u.kleine-koenig@baylibre.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty
Pull tty / serial updates from Greg KH:
"Here is a small set of tty and serial driver updates for 6.11-rc1. Not
much happened this cycle, unlike the previous kernel release which had
lots of "excitement" in this part of the kernel. Included in here are
the following changes:
- dt binding updates for new platforms
- 8250 driver updates
- various small serial driver fixes and updates
- printk/console naming and matching attempt #2 (was reverted for
6.10-final, should be good to go this time around, acked by the
relevant maintainers).
All of these have been in linux-next for a while with no reported
issues"
* tag 'tty-6.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty: (22 commits)
Documentation: kernel-parameters: Add DEVNAME:0.0 format for serial ports
serial: core: Add serial_base_match_and_update_preferred_console()
printk: Add match_devname_and_update_preferred_console()
serial: sc16is7xx: hardware reset chip if reset-gpios is defined in DT
dt-bindings: serial: sc16is7xx: add reset-gpios
dt-bindings: serial: vt8500-uart: convert to json-schema
serial: 8250_platform: Explicitly show we initialise ISA ports only once
tty: add missing MODULE_DESCRIPTION() macros
dt-bindings: serial: mediatek,uart: add MT7988
serial: sh-sci: Add support for RZ/V2H(P) SoC
dt-bindings: serial: Add documentation for Renesas RZ/V2H(P) (R9A09G057) SCIF support
dt-bindings: serial: renesas,scif: Make 'interrupt-names' property as required
dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
dt-bindings: serial: renesas,scif: Move ref for serial.yaml at the end
riscv: dts: starfive: jh7110: Add the core reset and jh7110 compatible for uarts
serial: 8250_dw: Use reset array API to get resets
dt-bindings: serial: snps-dw-apb-uart: Add one more reset signal for StarFive JH7110 SoC
serial: 8250: Extract platform driver
serial: 8250: Extract RSA bits
serial: imx: stop casting struct uart_port to struct imx_port
...
|
|
If a process is killed while writing to a /dev/ttymxc* device in RS485
mode, we observe that the RTS signal is left high, thus making it
impossible for other devices to transmit anything.
Moreover, the ->tx_state variable is left in state SEND, which means
that when one next opens the device and configures baud rate etc., the
initialization code in imx_uart_set_termios dutifully ensures the RTS
pin is pulled down, but since ->tx_state is already SEND, the logic in
imx_uart_start_tx() does not in fact pull the pin high before
transmitting, so nothing actually gets on the wire on the other side
of the transceiver. Only when that transmission is allowed to complete
is the state machine then back in a consistent state.
This is completely reproducible by doing something as simple as
seq 10000 > /dev/ttymxc0
and hitting ctrl-C, and watching with a logic analyzer.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: stable <stable@kernel.org>
Reviewed-by: Marek Vasut <marex@denx.de>
Link: https://lore.kernel.org/r/20240625184206.508837-1-linux@rasmusvillemoes.dk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
With commit a81dbd0463ec ("serial: imx: set receiver level before
starting uart") we set the receiver level to its default value. This
caused a regression when using SDMA, where the receiver level is 9
instead of 8 (default). This change will first check if the receiver
level is zero and only then set it to the default. This still avoids the
interrupt storm when the receiver level is zero.
Fixes: a81dbd0463ec ("serial: imx: set receiver level before starting uart")
Cc: stable <stable@kernel.org>
Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Link: https://lore.kernel.org/r/20240703112543.148304-1-eichest@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
This resolves the merge issues in the 8250 code due to some reverts in
6.10-rc6 in the console changes.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Set the receiver level to something > 0 before calling imx_uart_start_rx
in rs485_config. This is necessary to avoid an interrupt storm that
might prevent the system from booting. This was seen on an i.MX7 device
when the rs485-rts-active-low property was active in the device tree.
Fixes: 6d215f83e5fc ("serial: imx: warn user when using unsupported configuration")
Cc: stable <stable@kernel.org>
Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Link: https://lore.kernel.org/r/20240621153829.183780-1-eichest@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
struct imx_port does have a struct uart_port as its first member, so
the current code works, but it is not how kernel code is usually
written.
Similar to many other serial drivers, introduce and use a
to_imx_port() helper based on container_of(). No functional change.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
Link: https://lore.kernel.org/r/20240528094022.2161066-1-linux@rasmusvillemoes.dk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
At the default TX trigger level of 2 in non-DMA mode (meaning that an
interrupt is generated when less than 2 characters are left in the
FIFO), we have observed frequent buffer underruns at 115200 Baud on an
i.MX8M Nano. This can cause communication issues if the receiving side
expects a continuous transfer.
Increasing the level to 8 makes the UART trigger an interrupt earlier,
giving the kernel enough time to refill the FIFO, at the cost of
triggering one interrupt per ~24 instead of ~30 bytes of transmitted
data (as the i.MX UART has a 32 byte FIFO).
Signed-off-by: Michael Krummsdorf <michael.krummsdorf@tq-group.com>
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Link: https://lore.kernel.org/r/20240508133744.35858-1-matthias.schiffer@ew.tq-group.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
By waiting at most 1 second for USR2_TXDC to be set, we avoid a potential
deadlock.
In case of the timeout, there is not much we can do, so we simply ignore
the transmitter state and optimistically try to continue.
Signed-off-by: Esben Haabendal <esben@geanix.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
Link: https://lore.kernel.org/r/919647898c337a46604edcabaf13d42d80c0915d.1712837613.git.esben@geanix.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Switch from struct circ_buf to proper kfifo. kfifo provides much better
API, esp. when wrap-around of the buffer needs to be taken into account.
Look at pl011_dma_tx_refill() or cpm_uart_tx_pump() changes for example.
Kfifo API can also fill in scatter-gather DMA structures, so it easier
for that use case too. Look at lpuart_dma_tx() for example. Note that
not all drivers can be converted to that (like atmel_serial), they
handle DMA specially.
Note that usb-serial uses kfifo for TX for ages.
omap needed a bit more care as it needs to put a char into FIFO to start
the DMA transfer when OMAP_DMA_TX_KICK is set. In that case, we have to
do kfifo_dma_out_prepare twice: once to find out the tx_size (to find
out if it is worths to do DMA at all -- size >= 4), the second time for
the actual transfer.
All traces of circ_buf are removed from serial_core.h (and its struct
uart_state).
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Al Cooper <alcooperx@gmail.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
Cc: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Vineet Gupta <vgupta@kernel.org>
Cc: Richard Genoud <richard.genoud@gmail.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
Cc: Alexander Shiyan <shc_work@mail.ru>
Cc: Baruch Siach <baruch@tkos.co.il>
Cc: Maciej W. Rozycki <macro@orcam.me.uk>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: Taichi Sugaya <sugaya.taichi@socionext.com>
Cc: Takao Orito <orito.takao@socionext.com>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Pali Rohár <pali@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Orson Zhai <orsonzhai@gmail.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Hammer Hsieh <hammerh0314@gmail.com>
Cc: Peter Korsgaard <jacmet@sunsite.dk>
Cc: Timur Tabi <timur@kernel.org>
Cc: Michal Simek <michal.simek@amd.com>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Christian König <christian.koenig@amd.com>
Link: https://lore.kernel.org/r/20240405060826.2521-13-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
When about to transmit the function imx_uart_start_tx is called and in
some RS485 configurations this function will call imx_uart_stop_rx. The
problem is that imx_uart_stop_rx will enable loopback in order to
release the RS485 bus, but when loopback is enabled transmitted data
will just be looped to RX.
This patch fixes the above problem by not enabling loopback when about
to transmit.
This driver now works well when used for RS485 half duplex master
configurations.
Fixes: 79d0224f6bf2 ("tty: serial: imx: Handle RS485 DE signal active high")
Cc: stable <stable@kernel.org>
Signed-off-by: Rickard x Andersson <rickaran@axis.com>
Tested-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Link: https://lore.kernel.org/r/20240221115304.509811-1-rickaran@axis.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
If the imx driver cannot support RS485 it nullifies the ports
rs485_supported structure. But it still calls uart_get_rs485_mode() which
may set the RS485_ENABLED flag nevertheless.
This may lead to an attempt to configure RS485 even if it is not supported
when the flag is evaluated in uart_configure_port() at port startup.
Avoid this by bailing out of uart_get_rs485_mode() if the RS485_ENABLED
flag is not supported by the caller.
With this fix a check for RTS availability is now obsolete in the imx
driver, since it can not evaluate to true any more. So remove this check.
Furthermore the explicit nullifcation of rs485_supported is not needed,
since the memory has already been set to zeros at allocation. So remove
this, too.
Fixes: 00d7a00e2a6f ("serial: imx: Fill in rs485_supported")
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: <stable@vger.kernel.org>
Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://lore.kernel.org/r/20240103061818.564-6-l.sanfilippo@kunbus.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Both the imx and stm32 driver set the rx-during-tx GPIO in rs485_config().
Since this function is called with the port lock held, this can be a
problem in case that setting the GPIO line can sleep (e.g. if a GPIO
expander is used which is connected via SPI or I2C).
Avoid this issue by moving the GPIO setting outside of the port lock into
the serial core and thus making it a generic feature.
Also with commit c54d48543689 ("serial: stm32: Add support for rs485
RX_DURING_TX output GPIO") the SER_RS485_RX_DURING_TX flag is only set if a
rx-during-tx GPIO is _not_ available, which is wrong. Fix this, too.
Furthermore reset old GPIO settings in case that changing the RS485
configuration failed.
Fixes: c54d48543689 ("serial: stm32: Add support for rs485 RX_DURING_TX output GPIO")
Fixes: ca530cfa968c ("serial: imx: Add support for RS485 RX_DURING_TX output GPIO")
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: <stable@vger.kernel.org>
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Link: https://lore.kernel.org/r/20240103061818.564-2-l.sanfilippo@kunbus.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
There are register accesses in the function imx_uart_rs485_config(). The
clock must be enabled for these accesses. This was ensured by calling it
via the function uart_rs485_config() in the probe() function within the
range where the clock is enabled. With the commit 7c7f9bc986e6 ("serial:
Deassert Transmit Enable on probe in driver-specific way") it was removed
from the probe() function and is now only called through the function
uart_add_one_port() which is located at the end of the probe() function.
But the clock is already switched off in this area. To ensure that the
clock is enabled during register access, move the disabling of the clock
to the very end of the probe() function. To avoid leaking enabled clocks
on error also add an error path for exiting with disabling the clock.
Fixes: 7c7f9bc986e6 ("serial: Deassert Transmit Enable on probe in driver-specific way")
Cc: stable <stable@kernel.org>
Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Link: https://lore.kernel.org/r/20231226113647.39376-1-cniedermaier@dh-electronics.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Correct the clock error message by changing the clock name.
Fixes: 1e512d45332b ("serial: imx: add error messages when .probe fails")
Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20231224093209.2612-1-cniedermaier@dh-electronics.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
When using the serial port as RS485 port, the tx statemachine is used to
control the RTS pin to drive the RS485 transceiver TX_EN pin. When the
TTY port is closed in the middle of a transmission (for instance during
userland application crash), imx_uart_shutdown disables the interface
and disables the Transmission Complete interrupt. afer that,
imx_uart_stop_tx bails on an incomplete transmission, to be retriggered
by the TC interrupt. This interrupt is disabled and therefore the tx
statemachine never transitions out of SEND. The statemachine is in
deadlock now, and the TX_EN remains low, making the interface useless.
imx_uart_stop_tx now checks for incomplete transmission AND whether TC
interrupts are enabled before bailing to be retriggered. This makes sure
the state machine handling is reached, and is properly set to
WAIT_AFTER_SEND.
Fixes: cb1a60923609 ("serial: imx: implement rts delaying for rs485")
Signed-off-by: Paul Geurts <paul_geurts@live.nl>
Tested-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Tested-by: Eberhard Stoll <eberhard.stoll@gmx.de>
Link: https://lore.kernel.org/r/AM0PR09MB26758F651BC1B742EB45775995B8A@AM0PR09MB2675.eurprd09.prod.outlook.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
dma_request_slave_channel() is deprecated. dma_request_chan() should
be used directly instead.
Switch to the preferred function and update the error handling accordingly.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Link: https://lore.kernel.org/r/a46b493c6b5cfa09417e3e138e304fd01b61e748.1700410346.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20231110152927.70601-18-u.kleine-koenig@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 <Tim.vanderstaaij@zigngroup.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Link: https://lore.kernel.org/r/20230928064320.711603-1-s.hauer@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 <tglx@linutronix.de>
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Link: https://lore.kernel.org/r/20230914183831.587273-30-john.ogness@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20230911085451.628798-1-u.kleine-koenig@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 <frank.li@vivo.com>
Link: https://lore.kernel.org/r/20230712062853.11007-14-frank.li@vivo.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 <robh@kernel.org>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> # for imx
Link: https://lore.kernel.org/r/20230724205440.767071-1-robh@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 <martin.fuzzey@flowbird.group>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://lore.kernel.org/r/20230616104838.2729694-1-martin.fuzzey@flowbird.group
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20230512173810.131447-3-u.kleine-koenig@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
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 <trix@redhat.com>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20230317205710.1672232-1-trix@redhat.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
It is preferred to use typed property access functions (i.e.
of_property_read_<type> 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 <robh@kernel.org>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20230310144727.1545699-1-robh@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
It is preferred to use typed property access functions (i.e.
of_property_read_<type> 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 <robh@kernel.org>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20230310144727.1545630-1-robh@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
There is no point in casting a struct uart_port to a struct imx_port
just to access the first member of the latter (a struct uart_port).
This introduces no code changes.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://lore.kernel.org/r/20230302115417.1860210-1-u.kleine-koenig@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
cpp_check reports
drivers/tty/serial/imx.c:1207:15: style: Condition 'r_bytes>0' is always true [knownConditionTrueFalse]
if (r_bytes > 0) {
r_byte is set to
r_bytes = rx_ring->head - rx_ring->tail;
The head - tail calculation is also done by the earlier check
if (rx_ring->head <= sg_dma_len(sgl) &&
rx_ring->head > rx_ring->tail) {
so r_bytes will always be > 0, so the second check is not needed.
Signed-off-by: Tom Rix <trix@redhat.com>
Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Link: https://lore.kernel.org/r/20230211154550.2130670-1-trix@redhat.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Neither registers shadowing is functionally needed as all the registers are
read-write, nor the shadowing makes much sense for speed-up, as most speed
critical reads/writes (of data Rx/Tx registers) are not shadowed anyway.
Moreover, the shadowing code is obviously pure overhead on the write path.
Get rid of the shadowing code and variables due to above considerations.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Link: https://lore.kernel.org/r/20230201141603.4205-1-sorganov@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The 'rx' is chip register, similar to 'usr2', so let it be of 'u32' type as
well.
Move 'flg' to be FIFO read loop local as it's not used outside.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Link: https://lore.kernel.org/r/20230201142700.4346-8-sorganov@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The chip provides all the needed bits in the URXD0 register that we read
anyway for data, so get rid of reading USR2 and use only URXD0 bits
instead.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Link: https://lore.kernel.org/r/20230201142700.4346-7-sorganov@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
There is no need to read USR2 twice at every loop iteration: get rid of the
second read.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Link: https://lore.kernel.org/r/20230201142700.4346-6-sorganov@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
There is no reason to prematurely break out of FIFO reading loop, and it
might cause needless reenters into ISR, so keep reading until FIFO is
empty.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Link: https://lore.kernel.org/r/20230201142700.4346-5-sorganov@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Do not call uart_handle_sysrq_char() if we got any receive error along with
the character, as we don't want random junk to be considered a sysrq.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Link: https://lore.kernel.org/r/20230201142700.4346-4-sorganov@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Check if hardware Rx flood is in progress, and issue soft reset to UART to
stop the flood.
A way to reproduce the flood (checked on iMX6SX) is: open iMX UART at 9600
8N1, and from external source send 0xf0 char at 115200 8N1. In about 90% of
cases this starts a flood of "receiving" of 0xff characters by the iMX UART
that is terminated by any activity on RxD line, or could be stopped by
issuing soft reset to the UART (just stop/start of RX does not help). Note
that in essence what we did here is sending isolated start bit about 2.4
times shorter than it is to be if issued on the UART configured baud rate.
There was earlier attempt to fix similar issue in: 'commit
b38cb7d25711 ("serial: imx: Disable new features of autobaud detection")',
but apparently it only gets harder to reproduce the issue after that
commit.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Link: https://lore.kernel.org/r/20230201142700.4346-3-sorganov@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
We perform soft reset in 2 places, slightly differently for no sufficient
reasons, so move more generic variant to a function, and re-use the code.
Out of 2 repeat counters, 10 and 100, select 10, as the code works at
interrupts disabled, and in practice the reset happens immediately.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Link: https://lore.kernel.org/r/20230201142700.4346-2-sorganov@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
There maybe pending USR interrupt before requesting irq, however
uart_add_one_port has not executed, so there will be kernel panic:
[ 0.795668] Unable to handle kernel NULL pointer dereference at virtual addre
ss 0000000000000080
[ 0.802701] Mem abort info:
[ 0.805367] ESR = 0x0000000096000004
[ 0.808950] EC = 0x25: DABT (current EL), IL = 32 bits
[ 0.814033] SET = 0, FnV = 0
[ 0.816950] EA = 0, S1PTW = 0
[ 0.819950] FSC = 0x04: level 0 translation fault
[ 0.824617] Data abort info:
[ 0.827367] ISV = 0, ISS = 0x00000004
[ 0.831033] CM = 0, WnR = 0
[ 0.833866] [0000000000000080] user address but active_mm is swapper
[ 0.839951] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[ 0.845953] Modules linked in:
[ 0.848869] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.1+g56321e101aca #1
[ 0.855617] Hardware name: Freescale i.MX8MP EVK (DT)
[ 0.860452] pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 0.867117] pc : __imx_uart_rxint.constprop.0+0x11c/0x2c0
[ 0.872283] lr : imx_uart_int+0xf8/0x1ec
The issue only happends in the inmate linux when Jailhouse hypervisor
enabled. The test procedure is:
while true; do
jailhouse enable imx8mp.cell
jailhouse cell linux xxxx
sleep 10
jailhouse cell destroy 1
jailhouse disable
sleep 5
done
And during the upper test, press keys to the 2nd linux console.
When `jailhouse cell destroy 1`, the 2nd linux has no chance to put
the uart to a quiese state, so USR1/2 may has pending interrupts. Then
when `jailhosue cell linux xx` to start 2nd linux again, the issue
trigger.
In order to disable irqs before requesting them, both UCR1 and UCR2 irqs
should be disabled, so here fix that, disable the Ageing Timer interrupt
in UCR2 as UCR1 does.
Fixes: 8a61f0c70ae6 ("serial: imx: Disable irqs before requesting them")
Suggested-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: Sherry Sun <sherry.sun@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
Acked-by: Jason Liu <jason.hui.liu@nxp.com>
Link: https://lore.kernel.org/r/20230206013016.29352-1-sherry.sun@nxp.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Convert uart_handle_cts_change() to bool which is more appropriate
than unsigned int.
Rename status to active to better describe what the parameter means.
While at it, make the comment about the active parameter easier to
parse.
Cleanup callsites from operations that are not necessary with bool.
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://lore.kernel.org/r/20230117090358.4796-10-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
If a RX_DURING_TX GPIO is defined by the DT property "rs485-rx-during-tx-gpios"
this patch switches this GPIO accordingly to the RS485 flag RX_DURING_TX in user
space. In addition, the i.MX UART receiver is no longer turned on and off during
sending, because now the hardware is responsible for connecting or disconnecting
RX during TX controlled by this GPIO.
Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Link: https://lore.kernel.org/r/20221202104127.122761-3-cniedermaier@dh-electronics.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The comment in imx_uart_shutdown() says to disable the break condition,
but it doesn't actually do that, here fix this by disabling UCR1_SNDBRK
when closing the uart port like other uart drivers do.
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Link: https://lore.kernel.org/r/20221214031137.28815-4-sherry.sun@nxp.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
We need the tty/serial fixes in here as well.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The following warning is seen with non-console UART instance when
system hibernates.
[ 37.371969] ------------[ cut here ]------------
[ 37.376599] uart3_root_clk already disabled
[ 37.380810] WARNING: CPU: 0 PID: 296 at drivers/clk/clk.c:952 clk_core_disable+0xa4/0xb0
...
[ 37.506986] Call trace:
[ 37.509432] clk_core_disable+0xa4/0xb0
[ 37.513270] clk_disable+0x34/0x50
[ 37.516672] imx_uart_thaw+0x38/0x5c
[ 37.520250] platform_pm_thaw+0x30/0x6c
[ 37.524089] dpm_run_callback.constprop.0+0x3c/0xd4
[ 37.528972] device_resume+0x7c/0x160
[ 37.532633] dpm_resume+0xe8/0x230
[ 37.536036] hibernation_snapshot+0x288/0x430
[ 37.540397] hibernate+0x10c/0x2e0
[ 37.543798] state_store+0xc4/0xd0
[ 37.547203] kobj_attr_store+0x1c/0x30
[ 37.550953] sysfs_kf_write+0x48/0x60
[ 37.554619] kernfs_fop_write_iter+0x118/0x1ac
[ 37.559063] new_sync_write+0xe8/0x184
[ 37.562812] vfs_write+0x230/0x290
[ 37.566214] ksys_write+0x68/0xf4
[ 37.569529] __arm64_sys_write+0x20/0x2c
[ 37.573452] invoke_syscall.constprop.0+0x50/0xf0
[ 37.578156] do_el0_svc+0x11c/0x150
[ 37.581648] el0_svc+0x30/0x140
[ 37.584792] el0t_64_sync_handler+0xe8/0xf0
[ 37.588976] el0t_64_sync+0x1a0/0x1a4
[ 37.592639] ---[ end trace 56e22eec54676d75 ]---
On hibernating, pm core calls into related hooks in sequence like:
.freeze
.freeze_noirq
.thaw_noirq
.thaw
With .thaw_noirq hook being absent, the clock will be disabled in a
unbalanced call which results the warning above.
imx_uart_freeze()
clk_prepare_enable()
imx_uart_suspend_noirq()
clk_disable()
imx_uart_thaw
clk_disable_unprepare()
Adding the missing .thaw_noirq hook as imx_uart_resume_noirq() will have
the call sequence corrected as below and thus fix the warning.
imx_uart_freeze()
clk_prepare_enable()
imx_uart_suspend_noirq()
clk_disable()
imx_uart_resume_noirq()
clk_enable()
imx_uart_thaw
clk_disable_unprepare()
Fixes: 09df0b3464e5 ("serial: imx: fix endless loop during suspend")
Reviewed-by: Martin Kaiser <martin@kaiser.cx>
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Link: https://lore.kernel.org/r/20221012121353.2346280-1-shawn.guo@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The default polarity of RS485 DE signal is active high. This driver does
not handle such case properly. Currently, when a pin is multiplexed as a
UART CTS_B on boot, this pin is pulled HIGH by the i.MX UART CTS circuit,
which activates DE signal on the RS485 transceiver and thus behave as if
the RS485 was transmitting data, so the system blocks the RS485 bus when
it starts and until user application takes over. This behavior is not OK.
The problem consists of two separate parts.
First, the i.MX UART IP requires UCR1 UARTEN and UCR2 RXEN to be set for
UCR2 CTSC and CTS bits to have any effect. The UCR2 CTSC bit permits the
driver to set CTS (RTS_B or RS485 DE signal) to either level sychronous
to the internal UART IP clock. Compared to other options, like GPIO CTS
control, this has the benefit of being synchronous to the UART IP clock
and thus without glitches or bus delays. The reason for the CTS design
is likely because when the Receiver is disabled, the UART IP can never
indicate that it is ready to receive data by assering CTS signal, so
the CTS is always pulled HIGH by default.
When the port is closed by user space, imx_uart_stop_rx() clears UCR2
RXEN bit, and imx_uart_shutdown() clears UCR1 UARTEN bit. This disables
UART Receiver and UART itself, and forces CTS signal HIGH, which leads
to the RS485 bus being blocked because RS485 DE is incorrectly active.
The proposed solution for this problem is to keep the Receiver running
even after the port is closed, but in loopback mode. This disconnects
the RX FIFO input from the RXD external signal, and since UCR2 TXEN is
cleared, the UART Transmitter is disabled, so nothing can feed data in
the RX FIFO. Because the Receiver is still enabled, the UCR2 CTSC and
CTS bits still have effect and the CTS (RS485 DE) control is retained.
Note that in case of RS485 DE signal active low, there is no problem and
no special handling is necessary. The CTS signal defaults to HIGH, thus
the RS485 is by default set to Receive and the bus is not blocked.
Note that while there is the possibility to control CTS using GPIO with
either CTS polarity, this has the downside of not being synchronous to
the UART IP clock and thus glitchy and susceptible to slow DE switching.
Second, on boot, before the UART driver probe callback is called, the
driver core triggers pinctrl_init_done() and configures the IOMUXC to
default state. At this point, UCR1 UARTEN and UCR2 RXEN are both still
cleared, but UART CTS_B (RS485 DE) is configured as CTS function, thus
the RTS signal is pulled HIGH by the UART IP CTS circuit.
One part of the solution here is to enable UCR1 UARTEN and UCR2 RXEN and
UTS loopback in this driver probe callback, thus unblocking the CTSC and
CTS control early on. But this is still too late, since the pin control
is already configured and CTS has been pulled HIGH for a short period
of time.
When Linux kernel boots and this driver is bound, the pin control is set
to special "init" state if the state is available, and driver can switch
the "default" state afterward when ready. This state can be used to set
the CTS line as a GPIO in DT temporarily, and a GPIO hog can force such
GPIO to LOW, thus keeping the RS485 DE line LOW early on boot. Once the
driver takes over and UCR1 UARTEN and UCR2 RXEN and UTS loopback are all
enabled, the driver can switch to "default" pin control state and control
the CTS line as function instead. DT binding example is below:
"
&gpio6 {
rts-init-hog {
gpio-hog;
gpios = <5 0>;
output-low;
line-name = "rs485-de";
};
};
&uart5 { /* DHCOM UART2 */
pinctrl-0 = <&pinctrl_uart5>;
pinctrl-1 = <&pinctrl_uart5_init>;
pinctrl-names = "default", "init";
...
};
pinctrl_uart5_init: uart5-init-grp {
fsl,pins = <
...
MX6QDL_PAD_CSI0_DAT19__GPIO6_IO05 0x30b1
>;
};
pinctrl_uart5: uart5-grp {
fsl,pins = <
...
MX6QDL_PAD_CSI0_DAT19__UART5_CTS_B 0x30b1
>;
};
"
Tested-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Reviewed-by: Fabio Estevam <festevam@denx.de>
Signed-off-by: Marek Vasut <marex@denx.de>
Link: https://lore.kernel.org/r/20220929144400.13571-1-marex@denx.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
