On MediaTek SoCs, the UART IP is 16550A compatible, but there are some
specific quirks: we are declaring a register shift of 2, but this is
only valid for the majority of the registers, as there are some that
are out of the standard layout.
Specifically, this driver is using definitions from serial_reg.h, where
we have a UART_EFR register defined as 2: this results in a 0x8 offset,
but there we have the FCR register instead.
The right offset for the EFR register on MediaTek UART is at 0x98,
so, following the decimal definition convention in serial_reg.h and
accounting for the register left shift of two, add and use the correct
register address for this IP, defined as decimal 38, so that the final
calculation results in (0x26 << 2) = 0x98.
Fixes: bdbd0a7f8f ("serial: 8250-mtk: modify baudrate setting")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20220427132328.228297-2-angelogioacchino.delregno@collabora.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
It's horrid and if we inline it into callers, we can get rid of a lot of
sugar around.
So:
* x_char handling becomes a single iowrite8.
* xmit->buf handling is a single loop simply writing characters one by
one directly from the buf instead of complex cnt_to_end computations.
Until the buf is empty or fifo size is reached.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503080808.28332-2-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When x_char is to be sent, the TX path overwrites whatever is in the
circular buffer at offset 0 with x_char and sends it using
pch_uart_hal_write(). I don't understand how this was supposed to work
if xmit->buf[0] already contained some character. It must have been
lost.
Remove this whole pop_tx_x() concept and do the work directly in the
callers. (Without printing anything using dev_dbg().)
Cc: <stable@vger.kernel.org>
Fixes: 3c6a483275 (Serial: EG20T: add PCH_UART driver)
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503080808.28332-1-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
'count' is zero in the pop_tx()'s comparison against 'size'. So the 'if'
tries to find out if 'size' is negative or zero and returns in that
case. But it cannot be negative, due to previous (size < 0) check in the
caller: handle_tx().
So simply move this check from pop_tx() to handle_tx(). Now it's clear
that pop_tx() is called only if fifo_size is non-zero.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503080613.27601-2-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
pic32_uart_startup() disables interrupts by local_irq_save(). But the
function never enables them. The serial core only holds a mutex, so irqs
are not restored.
So how could this driver work? This irq handling was already present in
the driver's initial commit 157b939470 (serial: pic32_uart: Add PIC32
UART driver).
So is it a candidate for removal? Anyone has a contact to the author:
Andrei Pistirica (I believe the one below -- @microchip.com -- will
bounce)? Or to someone else @microchip.com?
Cc: Andrei Pistirica <andrei.pistirica@microchip.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-12-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
struct pic32_sport contains built-up names for irqs. These are freed
only in error path of pic32_uart_startup(). And even there, the freeing
happens before free_irq().
So fix this by:
* moving frees after free_irq(), and
* add frees to pic32_uart_shutdown() -- the opposite of
pic32_uart_startup().
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-11-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
'c' is not in wrapped in parentheses in the to_pic32_sport() macro, so
it might be problematic wrt macro expansion. Using an inline is always
safer in these cases. Both type-wise and macro-expansion-wise. So switch
the macro to an inline.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-8-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
struct pic32_sport::ref_clk is only set, but not read. That means we can
remove it. And when we do so, pic32_enable_clock() and
pic32_disable_clock() are simple wrappers around clk_prepare_enable()
and clk_disable_unprepare() respectively. So we can remove the former
two from the code and replace it by the latter two.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20220503063122.20957-5-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The base baud value reported is supposed to be the highest baud rate
that can be set for a serial port. The SiFive FU740-C000 SOC's on-chip
UART supports baud rates of up to 1/16 of the input clock rate, which is
the bus clock `tlclk'[1], often at 130MHz in the case of the HiFive
Unmatched board.
However the sifive UART driver reports a fixed value of 115200 instead:
10010000.serial: ttySIF0 at MMIO 0x10010000 (irq = 1, base_baud = 115200) is a SiFive UART v0
10011000.serial: ttySIF1 at MMIO 0x10011000 (irq = 2, base_baud = 115200) is a SiFive UART v0
even though we already support setting higher baud rates, e.g.:
$ tty
/dev/ttySIF1
$ stty speed
230400
The baud base value is computed by the serial core by dividing the UART
clock recorded in `struct uart_port' by 16, which is also the minimum
value of the clock divider supported, so correct the baud base value
reported by setting the UART clock recorded to the input clock rate
rather than 115200:
10010000.serial: ttySIF0 at MMIO 0x10010000 (irq = 1, base_baud = 8125000) is a SiFive UART v0
10011000.serial: ttySIF1 at MMIO 0x10011000 (irq = 2, base_baud = 8125000) is a SiFive UART v0
References:
[1] "SiFive FU740-C000 Manual", v1p3, SiFive, Inc., August 13, 2021,
Section 16.9 "Baud Rate Divisor Register (div)", pp.143-144
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Fixes: 1f1496a923 ("riscv: Fix sifive serial driver")
Link: https://lore.kernel.org/r/alpine.DEB.2.21.2204291656280.9383@angie.orcam.me.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Oxford Semiconductor PCIe (Tornado) 950 serial port devices are driven
by a fixed 62.5MHz clock input derived from the 100MHz PCI Express clock.
We currently drive the device using its default oversampling rate of 16
and the clock prescaler disabled, consequently yielding the baud base of
3906250. This base is inadequate for some of the high-speed baud rates
such as 460800bps, for which the closest rate possible can be obtained
by dividing the baud base by 8, yielding the baud rate of 488281.25bps,
which is off by 5.9638%. This is enough for data communication to break
with the remote end talking actual 460800bps, where missed stop bits
have been observed.
We can do better however, by taking advantage of a reduced oversampling
rate, which can be set to any integer value from 4 to 16 inclusive by
programming the TCR register, and by using the clock prescaler, which
can be set to any value from 1 to 63.875 in increments of 0.125 in the
CPR/CPR2 register pair. The prescaler has to be explicitly enabled
though by setting bit 7 in the MCR or otherwise it is bypassed (in the
enhanced mode that we enable) as if the value of 1 was used.
Make use of these features then as follows:
- Set the baud base to 15625000, reflecting the minimum oversampling
rate of 4 with the clock prescaler and divisor both set to 1.
- Override the `set_mctrl' and set the MCR shadow there so as to have
MCR[7] always set and have the 8250 core propagate these settings.
- Override the `get_divisor' handler and determine a good combination of
parameters by using a lookup table with predetermined value pairs of
the oversampling rate and the clock prescaler and finding a pair that
divides the input clock such that the quotient, when rounded to the
nearest integer, deviates the least from the exact result. Calculate
the clock divisor accordingly.
Scale the resulting oversampling rate (only by powers of two) if
possible so as to maximise it, reducing the divisor accordingly, and
avoid a divisor overflow for very low baud rates by scaling the
oversampling rate and/or the prescaler even if that causes some
accuracy loss.
Also handle the historic spd_cust feature so as to allow one to set
all the three parameters manually to arbitrary values, by keeping the
low 16 bits for the divisor and then putting TCR in bits 19:16 and
CPR/CPR2 in bits 28:20, sanitising the bit pattern supplied such as
to clamp CPR/CPR2 values between 0.000 and 0.875 inclusive to 33.875.
This preserves compatibility with any existing setups, that is where
requesting a custom divisor that only has any bits set among the low
16 the oversampling rate of 16 and the clock prescaler of 33.875 will
be used as with the original 8250.
Finally abuse the `frac' argument to store the determined bit patterns
for the TCR, CPR and CPR2 registers.
- Override the `set_divisor' handler so as to set the TCR, CPR and CPR2
registers from the `frac' value supplied. Set the divisor as usual.
With the baud base set to 15625000 and the unsigned 16-bit UART_DIV_MAX
limitation imposed by `serial8250_get_baud_rate' standard baud rates
below 300bps become unavailable in the regular way, e.g. the rate of
200bps requires the baud base to be divided by 78125 and that is beyond
the unsigned 16-bit range. The historic spd_cust feature can still be
used to obtain such rates if so required.
See Documentation/tty/device_drivers/oxsemi-tornado.rst for more details.
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Link: https://lore.kernel.org/r/alpine.DEB.2.21.2204181519450.9383@angie.orcam.me.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Make ICR access helpers available outside 8250_port.c, however retain
them as ordinary static functions so as not to regress code generation.
This is because `serial_icr_write' is currently automatically inlined by
GCC, however `serial_icr_read' is not. Making them both static inline
would grow code produced, e.g.:
$ i386-linux-gnu-size --format=gnu 8250_port-{old,new}.o
text data bss total filename
15065 3378 0 18443 8250_port-old.o
15289 3378 0 18667 8250_port-new.o
and:
$ riscv64-linux-gnu-size --format=gnu 8250_port-{old,new}.o
text data bss total filename
16980 5306 0 22286 8250_port-old.o
17124 5306 0 22430 8250_port-new.o
while making them external would needlessly add a new module interface
and lose the benefit from `serial_icr_write' getting inlined outside
8250_port.o.
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Link: https://lore.kernel.org/r/alpine.DEB.2.21.2204181517500.9383@angie.orcam.me.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The EndRun PTP/1588 dual serial port device is based on the Oxford
Semiconductor OXPCIe952 UART device with the PCI vendor:device ID set
for EndRun Technologies and uses the same sequence to determine the
number of ports available. Despite that we have duplicate code
specific to the EndRun device.
Remove redundant code then and factor out OxSemi Tornado device
detection.
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Link: https://lore.kernel.org/r/alpine.DEB.2.21.2204181516220.9383@angie.orcam.me.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
