The variable linestate being assigned a value that is never read, the
following continue statement jumps to the end of the while-loop and then
it is re-assigned a new value. The assignment is redundant and can be
removed.
Cleans up clang scan build warning:
drivers/tty/serial/jsm/jsm_cls.c:398:4: warning: Value stored
to 'linestatus' is never read [deadcode.DeadStores]
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Link: https://lore.kernel.org/r/20240216121732.2106445-1-colin.i.king@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Variable hsu_rate is being checked for an upper limit and is assigned
a value that is never read. The if statement and assignment are
redundant and can be removed.
Cleans up clang scan build warning:
drivers/tty/serial/lpc32xx_hs.c:237:3: warning: Value stored
to 'hsu_rate' is never read [deadcode.DeadStores]
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Link: https://lore.kernel.org/r/20240215232944.2075789-1-colin.i.king@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Replace custom unit definitions that are available via units.h.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://lore.kernel.org/r/20240215160234.653305-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
clang warns about explicitly casting between incompatible function
pointers:
drivers/tty/hvc/hvc_iucv.c:1100:23: error: cast from 'void (*)(const void *)' to 'void (*)(struct device *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
1100 | priv->dev->release = (void (*)(struct device *)) kfree;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Add a separate function to handle this correctly.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240213101756.461701-1-arnd@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Corrected the doc of vc_sanitize_unicode() and vc_translate_unicode(),
tightly coupled functions which parse UTF-8 byte sequences.
1. Desc. of @rescan corresponded to the meaning of the return value -1.
Corrected + added "Return:" section.
2. Replaced the ambiguous "character" with "code point" or "byte".
Signed-off-by: Roman Žilka <roman.zilka@gmail.com>
Link: https://lore.kernel.org/r/bee9faa8-0ea7-4411-bf77-3cb2e06385c7@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
0x0d00ff81 and 0x0800f501 are bitmasks of ASCII characters. Spell them
explicitly using BIT() + ASCII constants. GENMASK() is used for the
9-bit range in CTRL_ACTION.
This also modifies the 'if' checking if the masks should be applied.
>From a "random" ' ' to the actual size of the bitmasks' type.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-23-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
There are still numbers used for ASCII characters in vt_console_print().
As we have an ASCII enum now, use the constant names from the enum
instead.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-22-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To be uniform in the 'c' handling, use switch-case (with ranges) even in
the ESgetpars case in do_con_trol().
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-21-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To be uniform in the 'c' handling, use switch-case (with ranges) even in
the ESnonstd case in do_con_trol().
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-20-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The code to reset the vc parameter parsing is repeated on two locations.
Create a helper vc_reset_params() and use it on both of them.
And instead of a 'for' loop to clear the array of parameters, use
simpler memset().
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-19-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
In do_con_trol()'s ESsquare case, there is already a switch (c). It is
preceded by an 'if (c == '[')'. Despite this 'if' handles a state
transition and not a modifier, move it as one of the switch cases. This
makes all the 'c' decision making more obvious there.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-18-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Given all the ANSI control states are sequential in the vc_ctl_state
enum, we can define first/last constants and use them in
ansi_control_string(). It makes the test simple and allows for removal
of the 'if' (which was unnecessary at all -- the 'return' should have
returned the 'if' content directly anyway).
And remove the useless comment -- it's clear from the function
prototype.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-17-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The enum for states is currently compact and undocumented. Put each
definition on a separate line and document them all using kernel-doc.
Document the same on the use sites.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-16-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Similar to previous moves, move also "CSI ..." (i.e. vc_priv == EPecma)
handling to a separate function.
This is the last large move of code out of do_con_trol(). And despite it
is still 151 lines of code (down from 407!), it is now quite easy to
folllow the transitions of the state machine in there. ESnonstd and
ESpalette handling still can be moved away, but it won't improve that
much.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-15-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The handling of "CSI ? ..." (i.e. vc_priv == EPdec) can be easily moved
out of do_con_trol() into a separate function. This again increases
readability of do_con_trol().
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-14-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Similar to the ASCII handling, the ESC handling can be easily moved away
from do_con_trol(). So create a new handle_esc() for that.
And add a comment with an example.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-13-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To make the do_con_trol() a bit more understandable, extract the ASCII
handling (the switch-case) to a separate function.
Other nested switch-cases will follow in the next patches.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-12-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
These functions expect u8 as the control character. Switch the type from
'int' appropriately. The caller passing the value (do_con_write()) is
fixed as well.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-11-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Some cases of the CSI switch are stuffed on one line. Put them all to a
separate line as is dictated by the coding style (and for better
readability).
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-10-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
It follows naming of other similar functions. RSB stands here for Right
Square Bracket as (obviously) ']' cannot be in the function name.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-9-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Decrypt the constant values by proper enum names. This time in
setterm_command() (to be renamed to csi_RSB() in the next patches).
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-8-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CSIs without [<=>?] modifiers (ECMA) are handled in the switch-case
below this DEC switch+case handler. So move this ECMA CSI+n there too as
it fits there better.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-7-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
vc_data::vc_priv is _always_ assigned before the ESgetpars case is
entered (in ESsquare). Therefore, there is no need to reset it when
leaving the ESgetpars case. Note the state is set to ESnormal few lines
above, so ESgetpars is entered only by the next CSI.
Therefore, this obfuscation can be removed.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-6-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The DEC and ECMA handling of CSI+h/l is needlessly complicated. Split
these two, so that DEC is handled when the state is EPdec ('CSI ?' was
seen) and ECMA is handled in the EPecma state (no '?').
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-5-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Decrypt the constant values by proper enum names. This time in
set_mode().
Define two of them as DEC ('CSI ?') is about to be split away in the
next patches.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-3-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* make the parameter unsigned, as it is expected to be unsigned,
* make the computation easier to follow -- step-by-step, and
* don't use 85 / 2 which is only a reduced form of 255 / 6 (by a factor
3). Unlike the former, the latter can be understood.
Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20240202065608.14019-2-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that the driver core can properly handle constant struct bus_type,
move the serial_base_bus_type variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Ricardo B. Marliere" <ricardo@marliere.net>
Link: https://lore.kernel.org/r/20240203-bus_cleanup-tty-v1-2-86b698c82efe@marliere.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that the driver core can properly handle constant struct bus_type,
move the serdev_bus_type variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Ricardo B. Marliere" <ricardo@marliere.net>
Link: https://lore.kernel.org/r/20240203-bus_cleanup-tty-v1-1-86b698c82efe@marliere.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If there is a problem after resetting a port, the do/while() loop that
checks the default value of DIVLSB register may run forever and spam the
I2C bus.
Add a delay before each read of DIVLSB, and a maximum number of tries to
prevent that situation from happening.
Also fail probe if port reset is unsuccessful.
Fixes: 10d8b34a42 ("serial: max310x: Driver rework")
Cc: stable@vger.kernel.org
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20240116213001.3691629-5-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Some people are seeing a warning similar to this when using a crystal:
max310x 11-006c: clock is not stable yet
The datasheet doesn't mention the maximum time to wait for the clock to be
stable when using a crystal, and it seems that the 10ms delay in the driver
is not always sufficient.
Jan Kundrát reported that it took three tries (each separated by 10ms) to
get a stable clock.
Modify behavior to check stable clock ready bit multiple times (20), and
waiting 10ms between each try.
Note: the first draft of the driver originally used a 50ms delay, without
checking the clock stable bit.
Then a loop with 1000 retries was implemented, each time reading the clock
stable bit.
Fixes: 4cf9a888fd ("serial: max310x: Check the clock readiness")
Cc: stable@vger.kernel.org
Suggested-by: Jan Kundrát <jan.kundrat@cesnet.cz>
Link: https://www.spinics.net/lists/linux-serial/msg35773.html
Link: https://lore.kernel.org/all/20240110174015.6f20195fde08e5c9e64e5675@hugovil.com/raw
Link: e5dfe3e4a7
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20240116213001.3691629-3-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If regmap_read() returns a non-zero value, the 'val' variable can be left
uninitialized.
Clear it before calling regmap_read() to make sure we properly detect
the clock ready bit.
Fixes: 4cf9a888fd ("serial: max310x: Check the clock readiness")
Cc: stable@vger.kernel.org
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20240116213001.3691629-2-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
In uart_tiocmget():
result = uport->mctrl;
uart_port_lock_irq(uport);
result |= uport->ops->get_mctrl(uport);
uart_port_unlock_irq(uport);
...
return result;
In uart_update_mctrl():
uart_port_lock_irqsave(port, &flags);
...
port->mctrl = (old & ~clear) | set;
...
port->ops->set_mctrl(port, port->mctrl);
...
uart_port_unlock_irqrestore(port, flags);
An atomicity violation is identified due to the concurrent execution of
uart_tiocmget() and uart_update_mctrl(). After assigning
result = uport->mctrl, the mctrl value may change in uart_update_mctrl(),
leading to a mismatch between the value returned by
uport->ops->get_mctrl(uport) and the mctrl value previously read.
This can result in uart_tiocmget() returning an incorrect value.
This possible bug is found by an experimental static analysis tool
developed by our team, BassCheck[1]. This tool analyzes the locking APIs
to extract function pairs that can be concurrently executed, and then
analyzes the instructions in the paired functions to identify possible
concurrency bugs including data races and atomicity violations. The above
possible bug is reported when our tool analyzes the source code of
Linux 5.17.
To address this issue, it is suggested to move the line
result = uport->mctrl inside the uart_port_lock block to ensure atomicity
and prevent the mctrl value from being altered during the execution of
uart_tiocmget(). With this patch applied, our tool no longer reports the
bug, with the kernel configuration allyesconfig for x86_64. Due to the
absence of the requisite hardware, we are unable to conduct runtime
testing of the patch. Therefore, our verification is solely based on code
logic analysis.
[1] https://sites.google.com/view/basscheck/
Fixes: c5f4644e6c ("[PATCH] Serial: Adjust serial locking")
Cc: stable@vger.kernel.org
Signed-off-by: Gui-Dong Han <2045gemini@gmail.com>
Link: https://lore.kernel.org/r/20240112113624.17048-1-2045gemini@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
These > comparisons should be >= to prevent writing one element beyond
the end of the rx_buff[] array. The rx_buff[] buffer has RX_BUF_SIZE
elements. Fix the buffer overflow.
Fixes: aba8290f36 ("8250: microchip: pci1xxxx: Add Burst mode reception support in uart driver for writing into FIFO")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/r/ZZ7vIfj7Jgh-pJn8@moroto
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add rs485 support to uartps driver. Use either rts-gpios or RTS
to control RS485 phy as driver or a receiver.
Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
Link: https://lore.kernel.org/r/20240123061655.2150946-4-manikanta.guntupalli@amd.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The current clock input is set to 62.5 MHz for supporting fractional
divider, which enables generation of an acceptable baud rate from any
frequency. With the current clock input the baud rate range is limited
to 3.9 Mbps. Hence, the current range is extended to support 4 Mbps
with Burst mode operation. Divisor calculation for a given baud rate is
updated as the sampling rate is reduced from 16 to 8 for 4 Mbps.
Signed-off-by: Rengarajan S <rengarajan.s@microchip.com>
Link: https://lore.kernel.org/r/20240125100619.154873-1-rengarajan.s@microchip.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
pci1xxxx_handle_irq reads the burst status and checks if the FIFO
is empty and is ready to accept the incoming data. The handling is
done in pci1xxxx_tx_burst where each transaction processes data in
block of DWORDs, while any remaining bytes are processed individually,
one byte at a time.
Signed-off-by: Rengarajan S <rengarajan.s@microchip.com>
Link: https://lore.kernel.org/r/20240125100006.153342-1-rengarajan.s@microchip.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/tty/serial/serial_txx9.c:933:12: error: no previous prototype for ‘early_serial_txx9_setup’ [-Werror=missing-prototypes]
933 | int __init early_serial_txx9_setup(struct uart_port *port)
| ^~~~~~~~~~~~~~~~~~~~~~~
This function is called from arch/mips/txx9/generic/setup.c, and does
have a forward declaration in arch/mips/include/asm/txx9/generic.h.
As the TXX9 serial driver does not support compile-testing, and thus can
only be built on MIPS, fix this by including the MIPS-only header file.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/818be2380061c19fe65819f7b7f10ab6e7aaa082.1706040343.git.geert+renesas@glider.be
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This SoC family was destined for server use, featuring Qualcomm's very
interesting Kryo cores (before "Kryo" became a marketing term for Arm
cores with small modifications). It did however not leave the labs of
Qualcomm and presumably some partners, nor was it ever productized.
Remove the workarounds, as they are long obsolete.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Link: https://lore.kernel.org/r/20240122-topic-qdf_cleanup_tty-v1-1-0415503184be@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Since the driver was introduced the port features flags never extended.
As we don't expect more flags soon that would bypass the first
cacheline of ``struct s3c24xx_uart_info``, change the type of
``has_divslot`` to bool. Bitfields operations incur performance penalty
when set or read as compared to direct types.
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://lore.kernel.org/r/20240119104526.1221243-19-tudor.ambarus@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
<linux/serial_s3c.h> provides a clock selection pool of maximum 4 clocks.
Update the driver to consider a pool selection of maximum 8 clocks.
u8 is large enough to allow more clocks than are supported by the driver
now, and not too big to cause spanning of ``struct s3c24xx_uart_info``
through 2 cachelines when compiled for arm64. The goal is to reduce the
memory footprint of ``struct s3c24xx_uart_info``.
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://lore.kernel.org/r/20240119104526.1221243-18-tudor.ambarus@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Change the return type of the s3c24xx_serial_rx_fifocnt() method to
``unsigned int`` as the method only returns the fifo size and does not
handle error codes.
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://lore.kernel.org/r/20240119104526.1221243-17-tudor.ambarus@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
s3c24xx_serial_console_txrdy() returned just 0 or 1 to indicate whether
the TX is empty or not. Change its return type to bool.
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://lore.kernel.org/r/20240119104526.1221243-16-tudor.ambarus@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
