The uart_unlock_and_check_sysrq() helper can be used to defer processing
of sysrq until the interrupt handler has released the port lock and is
about to return.
Since commit 81e2073c17 ("genirq: Disable interrupts for force
threaded handlers") interrupt handlers that are not explicitly requested
as threaded are always called with interrupts disabled and there is no
need to save the interrupt state when taking the port lock.
Instead of adding another sysrq helper for when the interrupt state has
not needlessly been saved, drop the state parameter from
uart_unlock_and_check_sysrq() and update its callers to no longer
explicitly disable interrupts in their interrupt handlers.
Cc: Joel Stanley <joel@jms.id.au>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
Link: https://lore.kernel.org/r/20210416140557.25177-2-johan@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Instead of using the array-of-pointers trick to avoid having gcc mess up
the earlycon array stride, specify type alignment when declaring entries
to prevent gcc from increasing alignment.
This is essentially an alternative (one-line) fix to the problem
addressed by commit dd709e72cb ("earlycon: Use a pointer table to fix
__earlycon_table stride").
gcc can increase the alignment of larger objects with static extent as
an optimisation, but this can be suppressed by using the aligned
attribute when declaring variables.
Note that we have been relying on this behaviour for kernel parameters
for 16 years and it indeed hasn't changed since the introduction of the
aligned attribute in gcc-3.1.
Signed-off-by: Johan Hovold <johan@kernel.org>
Link: https://lore.kernel.org/r/20201123102319.8090-3-johan@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Fix the port-lock initialisation regression introduced by commit
a3cb39d258 ("serial: core: Allow detach and attach serial device for
console") by making sure that the lock is again initialised during
console setup.
The console may be registered before the serial controller has been
probed in which case the port lock needs to be initialised during
console setup by a call to uart_set_options(). The console-detach
changes introduced a regression in several drivers by effectively
removing that initialisation by not initialising the lock when the port
is used as a console (which is always the case during console setup).
Add back the early lock initialisation and instead use a new
console-reinit flag to handle the case where a console is being
re-attached through sysfs.
The question whether the console-detach interface should have been added
in the first place is left for another discussion.
Note that the console-enabled check in uart_set_options() is not
redundant because of kgdboc, which can end up reinitialising an already
enabled console (see commit 42b6a1baa3 ("serial_core: Don't
re-initialize a previously initialized spinlock.")).
Fixes: a3cb39d258 ("serial: core: Allow detach and attach serial device for console")
Cc: stable <stable@vger.kernel.org> # 5.7
Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20200909143101.15389-3-johan@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit 8e20fc3917 ("serial_core: Move sysrq functions from header
file") converted the inline sysrq helpers to exported functions which
are now called for every received character, interrupt and break signal
also on systems without CONFIG_MAGIC_SYSRQ_SERIAL instead of being
optimised away by the compiler.
Inlining these helpers again also avoids the function call overhead when
CONFIG_MAGIC_SYSRQ_SERIAL is enabled (e.g. when the port is not used as
a console).
Fixes: 8e20fc3917 ("serial_core: Move sysrq functions from header file")
Cc: Dmitry Safonov <0x7f454c46@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
Cc: stable <stable@vger.kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>
Link: https://lore.kernel.org/r/20200610152232.16925-3-johan@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit e8759ad17d ("serial: uapi: Add support for bus termination")
introduced the ability to enable rs485 bus termination from user space.
So far the feature is only used by a single driver, 8250_exar.c, using a
hardcoded GPIO pin specific to Siemens IOT2040 products.
Provide for a more generic solution by allowing specification of an
rs485 bus termination GPIO pin in the device tree: Amend the serial
core to retrieve the GPIO from the device tree (or ACPI table) and amend
the default ->rs485_config() callback for 8250 drivers to change the
GPIO on request from user space.
Perhaps 8250_exar.c can be converted to the generic approach in a
follow-up patch.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Link: https://lore.kernel.org/r/94c6c800d1ca9fa04766dd1d43a8272c5ad4bedd.1589811297.git.lukas@wunner.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We're about to amend uart_get_rs485_mode() to support a GPIO pin for
rs485 bus termination. Retrieving the GPIO descriptor may fail, so
allow uart_get_rs485_mode() to return an errno and change all callers
to check for failure.
The GPIO descriptor is going to be stored in struct uart_port. Pass
that struct to uart_get_rs485_mode() in lieu of a struct device and
struct serial_rs485, both of which are directly accessible from struct
uart_port.
A few drivers call uart_get_rs485_mode() before setting the struct
device pointer in struct uart_port. Shuffle those calls around where
necessary.
[Heiko Stuebner did the ar933x_uart.c portion, hence his Signed-off-by.]
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Link: https://lore.kernel.org/r/271e814af4b0db3bffbbb74abf2b46b75add4516.1589285873.git.lukas@wunner.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Refactor uart_unlock_and_check_sysrq() to:
- explicitly show that we release a port lock which makes
static analyzers happy:
CHECK drivers/tty/serial/serial_core.c
.../serial_core.c:3290:17: warning: context imbalance in 'uart_unlock_and_check_sysrq' - unexpected unlock
- use flags instead of irqflags to avoid confusion with IRQ flags
- provide one return point
- be more compact
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>
Link: https://lore.kernel.org/r/20200310174337.74109-4-andriy.shevchenko@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Many embedded boards have a disconnected TTL level serial which can
generate some garbage that can lead to spurious false sysrq detects.
Currently, sysrq can be either completely disabled for serial console
or always disabled (with CONFIG_MAGIC_SYSRQ_SERIAL), since
commit 732dbf3a61 ("serial: do not accept sysrq characters via serial port")
At Arista, we have such boards that can generate BREAK and random
garbage. While disabling sysrq for serial console would solve
the problem with spurious false sysrq triggers, it's also desirable
to have a way to enable sysrq back.
As a measure of balance between on and off options, add
MAGIC_SYSRQ_SERIAL_SEQUENCE which is a string sequence that can enable
sysrq if it follows BREAK on a serial line. The longer the string - the
less likely it may be in the garbage.
Having the way to enable sysrq was beneficial to debug lockups with
a manual investigation in field and on the other side preventing false
sysrq detections.
Based-on-patch-by: Vasiliy Khoruzhick <vasilykh@arista.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
Link: https://lore.kernel.org/r/20200302175135.269397-3-dima@arista.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
It should remove the align-padding before @name.
[yes, there's a "hole" in the structure now, but that's fine, no one
cares. If they do care, the whole thing should be restructured using
pahole to find a better ordering. Removing this field is good as some
drivers have been known to abuse it for other things when they shouldn't
have been doing that. -- gregkh]
Signed-off-by: Dmitry Safonov <dima@arista.com>
Link: https://lore.kernel.org/r/20200114171912.261787-4-dima@arista.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The SUPPORT_SYSRQ is messy: every .c source should define it before
including "serial_core.h" if sysrq is supported or struct uart_port will
differ in sizes. Also this prevents moving to serial_core.c functions:
uart_handle_sysrq_char(), uart_prepare_sysrq_char(),
uart_unlock_and_check_sysrq().
It doesn't save many bytes in the structure, and a better way to reduce
it's size would be making rs485 and iso7816 pointers.
Introduce `has_sysrq` member to be used by serial line drivers further.
Signed-off-by: Dmitry Safonov <dima@arista.com>
Link: https://lore.kernel.org/r/20191213000657.931618-4-dima@arista.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
At the current place members those follow are:
: upf_t flags;
: upstat_t status;
: int hw_stopped;
: unsigned int mctrl;
: unsigned int timeout;
: unsigned int type;
: const struct uart_ops *ops;
Together, they give (*ops) 8-byte align on 64-bit platforms.
And `sysrq_ch` introduces 4-byte padding.
On the other side, above:
: struct device *dev;
: unsigned char hub6;
: unsigned char suspended;
: unsigned char unused[2];
: const char *name;
Adds another 4-byte padding.
Moving sysrq members just before `hub6` allows to save 8 bytes
per-uart_port on 64-bit platforms:
On my gcc, x86_64 sizeof(struct uart_port) goes from 528 to 520.
Signed-off-by: Dmitry Safonov <dima@arista.com>
Link: https://lore.kernel.org/r/20191213000657.931618-3-dima@arista.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Based on 1 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version this program is distributed in the
hope that it will be useful but without any warranty without even
the implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details you
should have received a copy of the gnu general public license along
with this program if not write to the free software foundation inc
59 temple place suite 330 boston ma 02111 1307 usa
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 1334 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Reviewed-by: Richard Fontana <rfontana@redhat.com>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070033.113240726@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The converted files are focused at the Kernel internal API,
so, this is a good candidate for the kernel API set of books.
The conversion is actually:
- add blank lines and identation in order to identify paragraphs;
- fix tables markups;
- add some lists markups;
- mark literal blocks;
- adjust title markups.
At its new index.rst, let's add a :orphan: while this is not linked to
the main index.rst file, in order to avoid build warnings.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
In the static inline function uart_handle_break() in serial_core.h we
dereference port->cons. That gives an error unless console.h is also
included.
This error hasn't shown up till now because everyone who has defined
SUPPORT_SYSRQ has also included console.h, but it's a bit ugly to make
this requirement. Let's make the include explicit.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Right now serial drivers process sysrq keys deep in their character
receiving code. This means that they've already grabbed their
port->lock spinlock. This can end up getting in the way if we've go
to do serial stuff (especially kgdb) in response to the sysrq.
Serial drivers have various hacks in them to handle this. Looking at
'8250_port.c' you can see that the console_write() skips locking if
we're in the sysrq handler. Looking at 'msm_serial.c' you can see
that the port lock is dropped around uart_handle_sysrq_char().
It turns out that these hacks aren't exactly perfect. If you have
lockdep turned on and use something like the 8250_port hack you'll get
a splat that looks like:
WARNING: possible circular locking dependency detected
[...] is trying to acquire lock:
... (console_owner){-.-.}, at: console_unlock+0x2e0/0x5e4
but task is already holding lock:
... (&port_lock_key){-.-.}, at: serial8250_handle_irq+0x30/0xe4
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&port_lock_key){-.-.}:
_raw_spin_lock_irqsave+0x58/0x70
serial8250_console_write+0xa8/0x250
univ8250_console_write+0x40/0x4c
console_unlock+0x528/0x5e4
register_console+0x2c4/0x3b0
uart_add_one_port+0x350/0x478
serial8250_register_8250_port+0x350/0x3a8
dw8250_probe+0x67c/0x754
platform_drv_probe+0x58/0xa4
really_probe+0x150/0x294
driver_probe_device+0xac/0xe8
__driver_attach+0x98/0xd0
bus_for_each_dev+0x84/0xc8
driver_attach+0x2c/0x34
bus_add_driver+0xf0/0x1ec
driver_register+0xb4/0x100
__platform_driver_register+0x60/0x6c
dw8250_platform_driver_init+0x20/0x28
...
-> #0 (console_owner){-.-.}:
lock_acquire+0x1e8/0x214
console_unlock+0x35c/0x5e4
vprintk_emit+0x230/0x274
vprintk_default+0x7c/0x84
vprintk_func+0x190/0x1bc
printk+0x80/0xa0
__handle_sysrq+0x104/0x21c
handle_sysrq+0x30/0x3c
serial8250_read_char+0x15c/0x18c
serial8250_rx_chars+0x34/0x74
serial8250_handle_irq+0x9c/0xe4
dw8250_handle_irq+0x98/0xcc
serial8250_interrupt+0x50/0xe8
...
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&port_lock_key);
lock(console_owner);
lock(&port_lock_key);
lock(console_owner);
*** DEADLOCK ***
The hack used in 'msm_serial.c' doesn't cause the above splats but it
seems a bit ugly to unlock / lock our spinlock deep in our irq
handler.
It seems like we could defer processing the sysrq until the end of the
interrupt handler right after we've unlocked the port. With this
scheme if a whole batch of sysrq characters comes in one irq then we
won't handle them all, but that seems like it should be a fine
compromise.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This reverts commit c550f01c81.
Turns out the samsung tty driver is mucking around in the "unused" port
fields and this patch breaks that code :(
So we need to fix that driver up before this can be accepted.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Steve Sakoman <steve@sakoman.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
