* [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs
@ 2021-06-10 18:38 Maciej W. Rozycki
2021-06-10 18:38 ` [PATCH 1/4] serial: 8250: Dissociate 4MHz Titan ports from Oxford ports Maciej W. Rozycki
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2021-06-10 18:38 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-serial, linux-kernel
Hi,
In the course of verifying support for SMSC FDC37M817 Super I/O chip's
data rate of 460800bps with an EXSYS EX-44072 option card based on the
Oxford Semiconductor OXPCIe952 device I have observed the inability to
transfer any data correctly at this rate between the two devices. Lower
rates, including 230400bps, appeared to work correctly, also with other
kinds of serial ports referred to with my previous patch series, which
strongly indicated something being wrong with the Oxford device.
In the end I have tracked the issue down to our baud base set to 4000000
for the device being off by 2.4%. Enough for an incorrect divisor value
of 9 to be chosen to yield the bit rate of 460800bps being particularly
inaccurate for the baud base selected and caused the actual rate of
434027.78bps of being used, -5.8% off. Consequently the ten bits of data
sent with every 8-bit character were enough for the drift to accumulate up
to the two ends to get out of sync, with the stop bit interpreted as bit 7
of data. Obviously whoever wrote this code never actually tried higher
data rates, or only connected Oxford devices to each other causing the
systematic errors at both ends to cancel each other.
With the baud base corrected to the chip's default of 3906250 for the 650
mode which we use (the reset default for the initial 450 mode is 115314,
approximated for maximum backwards compatibility with legacy OS drivers by
dividing the device's 62.5MHz clock by 33.875), the new calculated divisor
value and the new actual bit rate became 8 and 488281.25bps respectively.
Now +5.96% off, so the stop bit could be missed causing data corruption
with continuous data streams, but at least that could be worked around by
using two stop bits instead. Not a good solution though.
So I chose to implement proper clock handling for the chip. The bit rate
with this device is worked out from the 62.5MHz clock first by choosing an
oversampling rate between 4 and 16 inclusive, then by the clock prescaler
between 1 and 63.875 in increments of 0.125, and finally a 16-bit unsigned
divisor, all of which divide the input clock by the respective value.
By choosing the right values of these three parameters either exact or
highly-accurate actual bit rates can be programmed for standard and many
non-standard rates from 1bps up to 15625000bps, e.g. for the data rate of
460800bps concerned here I was able to get the accuracy of 0.0064% by
choosing the values of 7, 3.875, and 5 respectively for the oversampling
rate, the clock prescaler, and the clock divisor.
Additionally even with my considerably mighty POWER9 box I have observed
frequent input overruns with the bit rates of 460800bps and higher, and I
have noticed we have the receive interrupt trigger level set particularly
high in terms of FIFO usage percentage for 16C950 UARTs and then we don't
make the levels configurable. Lowering the default to a saner value made
the overruns go away altogether for rates below 921600bps. As I've only
verified these changes in terminal environment rather than with modems I
could not make use of hardware flow control which this chip supports and
which I presume would prevent overruns from happening even with higher bit
rates.
There's more that could be done here, for example we don't make use of
the 950 mode where FIFO trigger levels can be fine-tuned in increments of
1, which, interestingly, could help with the lower rate modes as reception
is quite choppy with them, owing to the minimum receive interrupt trigger
level of 16 in the 650 mode. I gave the 950 mode a try, but it made the
chip freeze frequently until new data was received, so clearly I must have
missed something in the chip's configuration, which I did not investigate.
Something for a different time perhaps then.
I have verified these changes for standard termios rates between 300bps
and 460800bps with my WTI CPM-800 site manager device and my Malta board's
serial ports, as suitable, all working flawlessly in terminal mode now.
I have verified standard termios rates between 500000bps and 4000000bps as
well, however for the lack of other high-speed hardware with a pair of
Oxford devices only. Except for input overruns noted above and growing in
numbers as the rate increased rates of up to 3500000bps worked flawlessly.
In particular the rate of 576000bps, still without input overruns, gave
this nice feeling as if working with a virtual terminal rather than over a
serial line!
Conversely the rate of 4000000bps showed significant data corruption,
happening randomly, i.e. some characters went through just fine, while
other ones became garbled in no particular pattern, unlike with the rate
inaccuracy described above. Also with no input overruns whatsoever. I
have double-checked that all the three parameters making up the bit rate
from the clock rate have been programmed correctly.
Therefore I have concluded this is not an issue with my change (or indeed
any other part the driver) and it is simply that the rate has exceeded
either the maximum frequency the EX-44072 board's serial transceivers
support (I haven't checked the chip types used and I can't persuade myself
to disassemble the system just to have a look at the board again), or the
bandwidth of the transmission line used (a flat 8-core telephone cable of
a standard Cisco console cable assembly). Not an issue to be addressed in
software and I find it rather astonishing anyway it worked so well for up
to 3.5MHz already!
I have no modems, so I couldn't verify DCE interoperation, but I don't
expect issues with the bit rates being more accurate now, or the default
FIFO receiver trigger level tweaked to be more conservative.
Finally the 16-bit UART_DIV_MAX limitation of the baud rate requested
with `serial8250_get_baud_rate' makes the standard rates of 200bps and
lower inaccessible in the regular way with the baud base of 15625000.
That could be avoided by tweaking our 8250 driver core appropriately, but
I have figured out with modern serial port usage that would not be the
best use of my time. Someone who does have a real need to use an Oxford
device at these low rates can step in and make the necessary chances.
Meanwhile I took advantage of the ancient spd_cust feature we thankfully
continue supporting and actually did verify not only the standard rates
between 50bps and 200bps, but the rates of 4bps and 2bps as well, using my
old x86 server's serial port with the baud base of 115200. That was,
ahem, an interesting experience both by itself and also with 2bps, which
revealed a phenomenon with SMSC Super I/O ports not working as documented
(already noted in the preceding patch series). Eventually I verified the
2bps rate with a plain ISA multi I/O card and its 16450 UART my EISA 486
box has as the remote console, which does support the divisor value of
57600 required.
See individual change descriptions for further details including figures.
Please apply.
Maciej
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] serial: 8250: Dissociate 4MHz Titan ports from Oxford ports
2021-06-10 18:38 [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Maciej W. Rozycki
@ 2021-06-10 18:38 ` Maciej W. Rozycki
2021-06-10 18:39 ` [PATCH 2/4] serial: 8250: Correct the clock for OxSemi PCIe devices Maciej W. Rozycki
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2021-06-10 18:38 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-serial, linux-kernel
Oxford Semiconductor PCIe (Tornado) serial port devices have their baud
base set incorrectly, however their `pciserial_board' entries have been
reused for Titan serial port devices. Define own entries for the latter
devices then, carrying over the settings, so that Oxford entries can be
fixed.
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
drivers/tty/serial/8250/8250_pci.c | 44 +++++++++++++++++++++++++++++++------
1 file changed, 38 insertions(+), 6 deletions(-)
linux-serial-8250-titan-driver-data.diff
Index: linux-malta-cbus-uart/drivers/tty/serial/8250/8250_pci.c
===================================================================
--- linux-malta-cbus-uart.orig/drivers/tty/serial/8250/8250_pci.c
+++ linux-malta-cbus-uart/drivers/tty/serial/8250/8250_pci.c
@@ -2972,6 +2972,10 @@ enum pci_board_num_t {
pbn_sunix_pci_4s,
pbn_sunix_pci_8s,
pbn_sunix_pci_16s,
+ pbn_titan_1_4000000,
+ pbn_titan_2_4000000,
+ pbn_titan_4_4000000,
+ pbn_titan_8_4000000,
pbn_moxa8250_2p,
pbn_moxa8250_4p,
pbn_moxa8250_8p,
@@ -3759,6 +3763,34 @@ static struct pciserial_board pci_boards
.base_baud = 921600,
.uart_offset = 0x8,
},
+ [pbn_titan_1_4000000] = {
+ .flags = FL_BASE0,
+ .num_ports = 1,
+ .base_baud = 4000000,
+ .uart_offset = 0x200,
+ .first_offset = 0x1000,
+ },
+ [pbn_titan_2_4000000] = {
+ .flags = FL_BASE0,
+ .num_ports = 2,
+ .base_baud = 4000000,
+ .uart_offset = 0x200,
+ .first_offset = 0x1000,
+ },
+ [pbn_titan_4_4000000] = {
+ .flags = FL_BASE0,
+ .num_ports = 4,
+ .base_baud = 4000000,
+ .uart_offset = 0x200,
+ .first_offset = 0x1000,
+ },
+ [pbn_titan_8_4000000] = {
+ .flags = FL_BASE0,
+ .num_ports = 8,
+ .base_baud = 4000000,
+ .uart_offset = 0x200,
+ .first_offset = 0x1000,
+ },
[pbn_moxa8250_2p] = {
.flags = FL_BASE1,
.num_ports = 2,
@@ -4703,22 +4735,22 @@ static const struct pci_device_id serial
pbn_b0_4_921600 },
{ PCI_VENDOR_ID_TITAN, PCI_DEVICE_ID_TITAN_100E,
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_titan_1_4000000 },
{ PCI_VENDOR_ID_TITAN, PCI_DEVICE_ID_TITAN_200E,
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_2_4000000 },
+ pbn_titan_2_4000000 },
{ PCI_VENDOR_ID_TITAN, PCI_DEVICE_ID_TITAN_400E,
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_4_4000000 },
+ pbn_titan_4_4000000 },
{ PCI_VENDOR_ID_TITAN, PCI_DEVICE_ID_TITAN_800E,
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_8_4000000 },
+ pbn_titan_8_4000000 },
{ PCI_VENDOR_ID_TITAN, PCI_DEVICE_ID_TITAN_200EI,
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_2_4000000 },
+ pbn_titan_2_4000000 },
{ PCI_VENDOR_ID_TITAN, PCI_DEVICE_ID_TITAN_200EISI,
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_2_4000000 },
+ pbn_titan_2_4000000 },
{ PCI_VENDOR_ID_TITAN, PCI_DEVICE_ID_TITAN_200V3,
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
pbn_b0_bt_2_921600 },
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/4] serial: 8250: Correct the clock for OxSemi PCIe devices
2021-06-10 18:38 [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Maciej W. Rozycki
2021-06-10 18:38 ` [PATCH 1/4] serial: 8250: Dissociate 4MHz Titan ports from Oxford ports Maciej W. Rozycki
@ 2021-06-10 18:39 ` Maciej W. Rozycki
2021-06-10 18:39 ` [PATCH 3/4] serial: 8250: Add proper clock handling " Maciej W. Rozycki
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2021-06-10 18:39 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-serial, linux-kernel
Oxford Semiconductor PCIe (Tornado) serial port devices are driven by a
fixed 62.5MHz clock input derived from the 100MHz PCI Express clock.
In the enhanced (650) mode, which we select in `autoconfig_has_efr' by
setting the ECB bit in the EFR register, and in the absence of clock
reconfiguration, which we currently don't do, the clock rate is divided
only by the oversampling rate of 16 as it is supplied to the baud rate
generator, yielding the baud base of 3906250. This comes from the reset
values of the TCR and MCR[7] registers which are both zero[1][2][3][4],
choosing the oversampling rate of 16 and the normal (divide by 1) baud
rate generator prescaler respectively. This is the rate that is divided
by the value held in the divisor latch to determine the baud rate used.
Replace the incorrect baud base of 4000000 with the right value of
3906250 then.
References:
[1] "OXPCIe200 PCI Express Multi-Port Bridge", Oxford Semiconductor,
Inc., DS-0045, 10 Nov 2008, Section "Reset Configuration", p. 72
[2] "OXPCIe952 PCI Express Bridge to Dual Serial & Parallel Port",
Oxford Semiconductor, Inc., DS-0046, Mar 06 08, Section "Reset
Configuration", p. 27
[3] "OXPCIe954 PCI Express Bridge to Quad Serial Port", Oxford
Semiconductor, Inc., DS-0047, Feb 08, Section "Reset Configuration",
p. 28
[4] "OXPCIe958 PCI Express Bridge to Octal Serial Port", Oxford
Semiconductor, Inc., DS-0048, Feb 08, Section "Reset Configuration",
p. 28
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Fixes: 7106b4e333bae ("8250: Oxford Semiconductor Devices")
---
drivers/tty/serial/8250/8250_pci.c | 128 ++++++++++++++++++-------------------
1 file changed, 64 insertions(+), 64 deletions(-)
linux-serial-8250-oxsemi-pcie-clock.diff
Index: linux-malta-cbus-uart/drivers/tty/serial/8250/8250_pci.c
===================================================================
--- linux-malta-cbus-uart.orig/drivers/tty/serial/8250/8250_pci.c
+++ linux-malta-cbus-uart/drivers/tty/serial/8250/8250_pci.c
@@ -2851,7 +2851,7 @@ enum pci_board_num_t {
pbn_b0_2_1843200,
pbn_b0_4_1843200,
- pbn_b0_1_4000000,
+ pbn_b0_1_3906250,
pbn_b0_bt_1_115200,
pbn_b0_bt_2_115200,
@@ -2931,10 +2931,10 @@ enum pci_board_num_t {
pbn_plx_romulus,
pbn_endrun_2_4000000,
pbn_oxsemi,
- pbn_oxsemi_1_4000000,
- pbn_oxsemi_2_4000000,
- pbn_oxsemi_4_4000000,
- pbn_oxsemi_8_4000000,
+ pbn_oxsemi_1_3906250,
+ pbn_oxsemi_2_3906250,
+ pbn_oxsemi_4_3906250,
+ pbn_oxsemi_8_3906250,
pbn_intel_i960,
pbn_sgi_ioc3,
pbn_computone_4,
@@ -3081,10 +3081,10 @@ static struct pciserial_board pci_boards
.uart_offset = 8,
},
- [pbn_b0_1_4000000] = {
+ [pbn_b0_1_3906250] = {
.flags = FL_BASE0,
.num_ports = 1,
- .base_baud = 4000000,
+ .base_baud = 3906250,
.uart_offset = 8,
},
@@ -3479,31 +3479,31 @@ static struct pciserial_board pci_boards
.base_baud = 115200,
.uart_offset = 8,
},
- [pbn_oxsemi_1_4000000] = {
+ [pbn_oxsemi_1_3906250] = {
.flags = FL_BASE0,
.num_ports = 1,
- .base_baud = 4000000,
+ .base_baud = 3906250,
.uart_offset = 0x200,
.first_offset = 0x1000,
},
- [pbn_oxsemi_2_4000000] = {
+ [pbn_oxsemi_2_3906250] = {
.flags = FL_BASE0,
.num_ports = 2,
- .base_baud = 4000000,
+ .base_baud = 3906250,
.uart_offset = 0x200,
.first_offset = 0x1000,
},
- [pbn_oxsemi_4_4000000] = {
+ [pbn_oxsemi_4_3906250] = {
.flags = FL_BASE0,
.num_ports = 4,
- .base_baud = 4000000,
+ .base_baud = 3906250,
.uart_offset = 0x200,
.first_offset = 0x1000,
},
- [pbn_oxsemi_8_4000000] = {
+ [pbn_oxsemi_8_3906250] = {
.flags = FL_BASE0,
.num_ports = 8,
- .base_baud = 4000000,
+ .base_baud = 3906250,
.uart_offset = 0x200,
.first_offset = 0x1000,
},
@@ -4510,158 +4510,158 @@ static const struct pci_device_id serial
*/
{ PCI_VENDOR_ID_OXSEMI, 0xc101, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_4000000 },
+ pbn_b0_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc105, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_4000000 },
+ pbn_b0_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc11b, /* OXPCIe952 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc11f, /* OXPCIe952 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc120, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_4000000 },
+ pbn_b0_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc124, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_4000000 },
+ pbn_b0_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc138, /* OXPCIe952 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc13d, /* OXPCIe952 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc140, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_4000000 },
+ pbn_b0_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc141, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_4000000 },
+ pbn_b0_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc144, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_4000000 },
+ pbn_b0_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc145, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_4000000 },
+ pbn_b0_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc158, /* OXPCIe952 2 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_2_4000000 },
+ pbn_oxsemi_2_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc15d, /* OXPCIe952 2 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_2_4000000 },
+ pbn_oxsemi_2_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc208, /* OXPCIe954 4 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_4_4000000 },
+ pbn_oxsemi_4_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc20d, /* OXPCIe954 4 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_4_4000000 },
+ pbn_oxsemi_4_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc308, /* OXPCIe958 8 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_8_4000000 },
+ pbn_oxsemi_8_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc30d, /* OXPCIe958 8 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_8_4000000 },
+ pbn_oxsemi_8_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc40b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc40f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc41b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc41f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc42b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc42f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc43b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc43f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc44b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc44f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc45b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc45f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc46b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc46f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc47b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc47f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc48b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc48f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc49b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc49f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4ab, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4af, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4bb, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4bf, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4cb, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4cf, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
/*
* Mainpine Inc. IQ Express "Rev3" utilizing OxSemi Tornado
*/
{ PCI_VENDOR_ID_MAINPINE, 0x4000, /* IQ Express 1 Port V.34 Super-G3 Fax */
PCI_VENDOR_ID_MAINPINE, 0x4001, 0, 0,
- pbn_oxsemi_1_4000000 },
+ pbn_oxsemi_1_3906250 },
{ PCI_VENDOR_ID_MAINPINE, 0x4000, /* IQ Express 2 Port V.34 Super-G3 Fax */
PCI_VENDOR_ID_MAINPINE, 0x4002, 0, 0,
- pbn_oxsemi_2_4000000 },
+ pbn_oxsemi_2_3906250 },
{ PCI_VENDOR_ID_MAINPINE, 0x4000, /* IQ Express 4 Port V.34 Super-G3 Fax */
PCI_VENDOR_ID_MAINPINE, 0x4004, 0, 0,
- pbn_oxsemi_4_4000000 },
+ pbn_oxsemi_4_3906250 },
{ PCI_VENDOR_ID_MAINPINE, 0x4000, /* IQ Express 8 Port V.34 Super-G3 Fax */
PCI_VENDOR_ID_MAINPINE, 0x4008, 0, 0,
- pbn_oxsemi_8_4000000 },
+ pbn_oxsemi_8_3906250 },
/*
* Digi/IBM PCIe 2-port Async EIA-232 Adapter utilizing OxSemi Tornado
*/
{ PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_2_OX_IBM,
PCI_SUBVENDOR_ID_IBM, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_2_4000000 },
+ pbn_oxsemi_2_3906250 },
/*
* SBS Technologies, Inc. P-Octal and PMC-OCTPRO cards,
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] serial: 8250: Add proper clock handling for OxSemi PCIe devices
2021-06-10 18:38 [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Maciej W. Rozycki
2021-06-10 18:38 ` [PATCH 1/4] serial: 8250: Dissociate 4MHz Titan ports from Oxford ports Maciej W. Rozycki
2021-06-10 18:39 ` [PATCH 2/4] serial: 8250: Correct the clock for OxSemi PCIe devices Maciej W. Rozycki
@ 2021-06-10 18:39 ` Maciej W. Rozycki
2021-06-16 16:58 ` kernel test robot
` (2 more replies)
2021-06-10 18:39 ` [PATCH 4/4] serial: 8250: Define RX trigger levels for OxSemi 950 devices Maciej W. Rozycki
2021-06-15 12:07 ` [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Greg Kroah-Hartman
4 siblings, 3 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2021-06-10 18:39 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-serial, linux-kernel
Oxford Semiconductor PCIe (Tornado) 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[1][2][3][4]. The prescaler has to be explicitly
enabled though by setting bit 7 in the MCR or otherwise it is bypassed
as if the value of 1 was used.
By using these parameters rates from 15625000bps down to 1bps can be
obtained, with either exact or highly-accurate actual bit rates for
standard and many non-standard rates.
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.
- Set the MCR mask and force bits in the UART template so as to have
MCR[7] always set and then have 8250 core propagate those settings, if
supplied as non-zero, overriding the ALPHA_KLUDGE_MCR default.
- 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 1.000.
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 1 will be
used.
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 usually.
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.
Here are the figures for the standard and some non-standard baud rates
(including those quoted in Oxford Semiconductor documentation), giving
the requested rate (r), the actual rate yielded (a) and its deviation
from the requested rate (d), and the values of the oversampling rate
(tcr), the clock prescaler (cpr) and the divisor (div) produced by the
new `get_divisor' handler:
r: 15625000, a: 15625000.00, d: 0.0000%, tcr: 4, cpr: 1.000, div: 1
r: 12500000, a: 12500000.00, d: 0.0000%, tcr: 5, cpr: 1.000, div: 1
r: 10416666, a: 10416666.67, d: 0.0000%, tcr: 6, cpr: 1.000, div: 1
r: 8928571, a: 8928571.43, d: 0.0000%, tcr: 7, cpr: 1.000, div: 1
r: 7812500, a: 7812500.00, d: 0.0000%, tcr: 8, cpr: 1.000, div: 1
r: 4000000, a: 4000000.00, d: 0.0000%, tcr: 5, cpr: 3.125, div: 1
r: 3686400, a: 3676470.59, d: -0.2694%, tcr: 8, cpr: 2.125, div: 1
r: 3500000, a: 3496503.50, d: -0.0999%, tcr: 13, cpr: 1.375, div: 1
r: 3000000, a: 2976190.48, d: -0.7937%, tcr: 14, cpr: 1.500, div: 1
r: 2500000, a: 2500000.00, d: 0.0000%, tcr: 10, cpr: 2.500, div: 1
r: 2000000, a: 2000000.00, d: 0.0000%, tcr: 10, cpr: 3.125, div: 1
r: 1843200, a: 1838235.29, d: -0.2694%, tcr: 16, cpr: 2.125, div: 1
r: 1500000, a: 1492537.31, d: -0.4975%, tcr: 5, cpr: 8.375, div: 1
r: 1152000, a: 1152073.73, d: 0.0064%, tcr: 14, cpr: 3.875, div: 1
r: 921600, a: 919117.65, d: -0.2694%, tcr: 16, cpr: 2.125, div: 2
r: 576000, a: 576036.87, d: 0.0064%, tcr: 14, cpr: 3.875, div: 2
r: 460800, a: 460829.49, d: 0.0064%, tcr: 7, cpr: 3.875, div: 5
r: 230400, a: 230414.75, d: 0.0064%, tcr: 14, cpr: 3.875, div: 5
r: 115200, a: 115207.37, d: 0.0064%, tcr: 14, cpr: 1.250, div: 31
r: 57600, a: 57603.69, d: 0.0064%, tcr: 8, cpr: 3.875, div: 35
r: 38400, a: 38402.46, d: 0.0064%, tcr: 14, cpr: 3.875, div: 30
r: 19200, a: 19201.23, d: 0.0064%, tcr: 8, cpr: 3.875, div: 105
r: 9600, a: 9600.06, d: 0.0006%, tcr: 9, cpr: 1.125, div: 643
r: 4800, a: 4799.98, d: -0.0004%, tcr: 7, cpr: 2.875, div: 647
r: 2400, a: 2400.02, d: 0.0008%, tcr: 9, cpr: 2.250, div: 1286
r: 1200, a: 1200.00, d: 0.0000%, tcr: 14, cpr: 2.875, div: 1294
r: 300, a: 300.00, d: 0.0000%, tcr: 11, cpr: 2.625, div: 7215
r: 200, a: 200.00, d: 0.0000%, tcr: 16, cpr: 1.250, div: 15625
r: 150, a: 150.00, d: 0.0000%, tcr: 13, cpr: 2.250, div: 14245
r: 134, a: 134.00, d: 0.0000%, tcr: 11, cpr: 2.625, div: 16153
r: 110, a: 110.00, d: 0.0000%, tcr: 12, cpr: 1.000, div: 47348
r: 75, a: 75.00, d: 0.0000%, tcr: 4, cpr: 5.875, div: 35461
r: 50, a: 50.00, d: 0.0000%, tcr: 16, cpr: 1.250, div: 62500
r: 25, a: 25.00, d: 0.0000%, tcr: 16, cpr: 2.500, div: 62500
r: 4, a: 4.00, d: 0.0000%, tcr: 16, cpr: 20.000, div: 48828
r: 2, a: 2.00, d: 0.0000%, tcr: 16, cpr: 40.000, div: 48828
r: 1, a: 1.00, d: 0.0000%, tcr: 16, cpr: 63.875, div: 61154
References:
[1] "OXPCIe200 PCI Express Multi-Port Bridge", Oxford Semiconductor,
Inc., DS-0045, 10 Nov 2008, Section "950 Mode", pp. 64-65
[2] "OXPCIe952 PCI Express Bridge to Dual Serial & Parallel Port",
Oxford Semiconductor, Inc., DS-0046, Mar 06 08, Section "950 Mode",
p. 20
[3] "OXPCIe954 PCI Express Bridge to Quad Serial Port", Oxford
Semiconductor, Inc., DS-0047, Feb 08, Section "950 Mode", p. 20
[4] "OXPCIe958 PCI Express Bridge to Octal Serial Port", Oxford
Semiconductor, Inc., DS-0048, Feb 08, Section "950 Mode", p. 20
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
drivers/tty/serial/8250/8250.h | 23 ++
drivers/tty/serial/8250/8250_core.c | 6
drivers/tty/serial/8250/8250_pci.c | 330 ++++++++++++++++++++++++++++--------
drivers/tty/serial/8250/8250_port.c | 21 --
4 files changed, 292 insertions(+), 88 deletions(-)
linux-serial-8250-oxsemi-pcie-prescaler.diff
Index: linux-malta-cbus-uart/drivers/tty/serial/8250/8250.h
===================================================================
--- linux-malta-cbus-uart.orig/drivers/tty/serial/8250/8250.h
+++ linux-malta-cbus-uart/drivers/tty/serial/8250/8250.h
@@ -120,6 +120,29 @@ static inline void serial_out(struct uar
up->port.serial_out(&up->port, offset, value);
}
+/*
+ * For the 16C950
+ */
+static inline void serial_icr_write(struct uart_8250_port *up,
+ int offset, int value)
+{
+ serial_out(up, UART_SCR, offset);
+ serial_out(up, UART_ICR, value);
+}
+
+static inline unsigned int serial_icr_read(struct uart_8250_port *up,
+ int offset)
+{
+ unsigned int value;
+
+ serial_icr_write(up, UART_ACR, up->acr | UART_ACR_ICRRD);
+ serial_out(up, UART_SCR, offset);
+ value = serial_in(up, UART_ICR);
+ serial_icr_write(up, UART_ACR, up->acr);
+
+ return value;
+}
+
void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p);
static inline int serial_dl_read(struct uart_8250_port *up)
Index: linux-malta-cbus-uart/drivers/tty/serial/8250/8250_core.c
===================================================================
--- linux-malta-cbus-uart.orig/drivers/tty/serial/8250/8250_core.c
+++ linux-malta-cbus-uart/drivers/tty/serial/8250/8250_core.c
@@ -1027,6 +1027,12 @@ int serial8250_register_8250_port(struct
uart->rs485_stop_tx = up->rs485_stop_tx;
uart->dma = up->dma;
+ /* Override ALPHA_KLUDGE_MCR default. */
+ if (up->mcr_mask | up->mcr_force) {
+ uart->mcr_mask = up->mcr_mask;
+ uart->mcr_force = up->mcr_force;
+ }
+
/* Take tx_loadsz from fifosize if it wasn't set separately */
if (uart->port.fifosize && !uart->tx_loadsz)
uart->tx_loadsz = uart->port.fifosize;
Index: linux-malta-cbus-uart/drivers/tty/serial/8250/8250_pci.c
===================================================================
--- linux-malta-cbus-uart.orig/drivers/tty/serial/8250/8250_pci.c
+++ linux-malta-cbus-uart/drivers/tty/serial/8250/8250_pci.c
@@ -1066,6 +1066,202 @@ static int pci_oxsemi_tornado_init(struc
return number_uarts;
}
+/* Tornado-specific constants for the TCR and CPR registers; see below. */
+#define OXSEMI_TORNADO_TCR_MASK 0xf
+#define OXSEMI_TORNADO_CPR_MASK 0x1ff
+#define OXSEMI_TORNADO_CPR_MIN 8
+
+/*
+ * Determine the oversampling rate, the clock prescaler, and the clock
+ * divisor for the requested baud rate. The clock rate is 62.5 MHz,
+ * which is four times the baud base, and the prescaler increments in
+ * steps of 1/8. Therefore to make calculations on integers we need
+ * to use a scaled clock rate, which is the baud base multiplied by 32
+ * (or our assumed UART clock rate multiplied by 2).
+ *
+ * The allowed oversampling rates are from 4 up to 16 inclusive (values
+ * from 0 to 3 inclusive map to 16). Likewise the clock prescaler allows
+ * values between 1.000 and 63.875 inclusive (operation for values from
+ * 0.000 to 0.875 has not been specified). The clock divisor is the usual
+ * unsigned 16-bit integer.
+ *
+ * For the most accurate baud rate we use a table of predetermined
+ * oversampling rates and clock prescalers that records all possible
+ * products of the two parameters in the range from 4 up to 255 inclusive,
+ * and additionally 335 for the 1500000bps rate, with the prescaler scaled
+ * by 8. The table is sorted by the decreasing value of the oversampling
+ * rate and ties are resolved by sorting by the decreasing value of the
+ * product. This way preference is given to higher oversampling rates.
+ *
+ * We iterate over the table and choose the product of an oversampling
+ * rate and a clock prescaler that gives the lowest integer division
+ * result deviation, or if an exact integer divider is found we stop
+ * looking for right away. We do some fixup if the resulting clock
+ * divisor required would be out of its unsigned 16-bit integer range.
+ *
+ * Finally we abuse the supposed fractional part returned to encode the
+ * 4-bit value of the oversampling rate and the 9-bit value of the clock
+ * prescaler which will end up in the TCR and CPR/CPR2 registers.
+ */
+static unsigned int pci_oxsemi_tornado_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
+{
+ static u8 p[][2] = {
+ { 16, 14, }, { 16, 13, }, { 16, 12, }, { 16, 11, },
+ { 16, 10, }, { 16, 9, }, { 16, 8, }, { 15, 17, },
+ { 15, 16, }, { 15, 15, }, { 15, 14, }, { 15, 13, },
+ { 15, 12, }, { 15, 11, }, { 15, 10, }, { 15, 9, },
+ { 15, 8, }, { 14, 18, }, { 14, 17, }, { 14, 14, },
+ { 14, 13, }, { 14, 12, }, { 14, 11, }, { 14, 10, },
+ { 14, 9, }, { 14, 8, }, { 13, 19, }, { 13, 18, },
+ { 13, 17, }, { 13, 13, }, { 13, 12, }, { 13, 11, },
+ { 13, 10, }, { 13, 9, }, { 13, 8, }, { 12, 19, },
+ { 12, 18, }, { 12, 17, }, { 12, 11, }, { 12, 9, },
+ { 12, 8, }, { 11, 23, }, { 11, 22, }, { 11, 21, },
+ { 11, 20, }, { 11, 19, }, { 11, 18, }, { 11, 17, },
+ { 11, 11, }, { 11, 10, }, { 11, 9, }, { 11, 8, },
+ { 10, 25, }, { 10, 23, }, { 10, 20, }, { 10, 19, },
+ { 10, 17, }, { 10, 10, }, { 10, 9, }, { 10, 8, },
+ { 9, 27, }, { 9, 23, }, { 9, 21, }, { 9, 19, },
+ { 9, 18, }, { 9, 17, }, { 9, 9, }, { 9, 8, },
+ { 8, 31, }, { 8, 29, }, { 8, 23, }, { 8, 19, },
+ { 8, 17, }, { 8, 8, }, { 7, 35, }, { 7, 31, },
+ { 7, 29, }, { 7, 25, }, { 7, 23, }, { 7, 21, },
+ { 7, 19, }, { 7, 17, }, { 7, 15, }, { 7, 14, },
+ { 7, 13, }, { 7, 12, }, { 7, 11, }, { 7, 10, },
+ { 7, 9, }, { 7, 8, }, { 6, 41, }, { 6, 37, },
+ { 6, 31, }, { 6, 29, }, { 6, 23, }, { 6, 19, },
+ { 6, 17, }, { 6, 13, }, { 6, 11, }, { 6, 10, },
+ { 6, 9, }, { 6, 8, }, { 5, 67, }, { 5, 47, },
+ { 5, 43, }, { 5, 41, }, { 5, 37, }, { 5, 31, },
+ { 5, 29, }, { 5, 25, }, { 5, 23, }, { 5, 19, },
+ { 5, 17, }, { 5, 15, }, { 5, 13, }, { 5, 11, },
+ { 5, 10, }, { 5, 9, }, { 5, 8, }, { 4, 61, },
+ { 4, 59, }, { 4, 53, }, { 4, 47, }, { 4, 43, },
+ { 4, 41, }, { 4, 37, }, { 4, 31, }, { 4, 29, },
+ { 4, 23, }, { 4, 19, }, { 4, 17, }, { 4, 13, },
+ { 4, 9, }, { 4, 8, },
+ };
+ /* Scale the quotient for comparison to get the fractional part. */
+ const unsigned int quot_scale = 65536;
+ unsigned int sclk = port->uartclk * 2;
+ unsigned int sdiv = (sclk + (baud / 2)) / baud;
+ unsigned int best_squot;
+ unsigned int squot;
+ unsigned int quot;
+ u16 cpr;
+ u8 tcr;
+ int i;
+
+ /* Old custom speed handling. */
+ if (baud == 38400 && (port->flags & UPF_SPD_MASK) == UPF_SPD_CUST) {
+ unsigned int cust_div = port->custom_divisor;
+
+ quot = cust_div & UART_DIV_MAX;
+ tcr = (cust_div >> 16) & OXSEMI_TORNADO_TCR_MASK;
+ cpr = (cust_div >> 20) & OXSEMI_TORNADO_CPR_MASK;
+ if (cpr < OXSEMI_TORNADO_CPR_MIN)
+ cpr = OXSEMI_TORNADO_CPR_MIN;
+ } else {
+ best_squot = quot_scale;
+ for (i = 0; i < ARRAY_SIZE(p); i++) {
+ unsigned int spre;
+ unsigned int srem;
+ u8 cp;
+ u8 tc;
+
+ tc = p[i][0];
+ cp = p[i][1];
+ spre = tc * cp;
+
+ srem = sdiv % spre;
+ if (srem > spre / 2)
+ srem = spre - srem;
+ squot = (srem * quot_scale + spre / 2) / spre;
+
+ if (srem == 0) {
+ tcr = tc;
+ cpr = cp;
+ quot = sdiv / spre;
+ break;
+ } else if (squot < best_squot) {
+ best_squot = squot;
+ tcr = tc;
+ cpr = cp;
+ quot = (sdiv + spre / 2) / spre;
+ }
+ }
+ while (tcr <= (OXSEMI_TORNADO_TCR_MASK + 1) >> 1 &&
+ quot % 2 == 0) {
+ quot >>= 1;
+ tcr <<= 1;
+ }
+ while (quot > UART_DIV_MAX) {
+ if (tcr <= (OXSEMI_TORNADO_TCR_MASK + 1) >> 1) {
+ quot >>= 1;
+ tcr <<= 1;
+ } else if (cpr <= OXSEMI_TORNADO_CPR_MASK >> 1) {
+ quot >>= 1;
+ cpr <<= 1;
+ } else {
+ quot = quot * cpr / OXSEMI_TORNADO_CPR_MASK;
+ cpr = OXSEMI_TORNADO_CPR_MASK;
+ }
+ }
+ }
+
+ *frac = (cpr << 8) | (tcr & OXSEMI_TORNADO_TCR_MASK);
+ return quot;
+}
+
+/*
+ * Set the oversampling rate in the transmitter clock cycle register (TCR),
+ * the clock prescaler in the clock prescaler register (CPR and CPR2), and
+ * the clock divisor in the divisor latch (DLL and DLM). Note that for
+ * backwards compatibility any write to CPR clears CPR2 and therefore CPR
+ * has to be written first, followed by CPR2, which occupies the location
+ * of CKS used with earlier UART designs.
+ */
+static void pci_oxsemi_tornado_set_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int quot,
+ unsigned int quot_frac)
+{
+ struct uart_8250_port *up = up_to_u8250p(port);
+ u8 cpr2 = quot_frac >> 16;
+ u8 cpr = quot_frac >> 8;
+ u8 tcr = quot_frac;
+
+ serial_icr_write(up, UART_TCR, tcr);
+ serial_icr_write(up, UART_CPR, cpr);
+ serial_icr_write(up, UART_CKS, cpr2);
+ serial8250_do_set_divisor(port, baud, quot, 0);
+}
+
+/*
+ * For Tornado devices we force MCR[7] set for the Divide-by-M N/8 baud rate
+ * generator prescaler (CPR and CPR2). Otherwise no prescaler would be used.
+ */
+static int pci_oxsemi_tornado_setup(struct serial_private *priv,
+ const struct pciserial_board *board,
+ struct uart_8250_port *up, int idx)
+{
+ struct pci_dev *dev = priv->dev;
+
+ /* OxSemi Tornado devices are all 0xCxxx */
+ if (dev->vendor == PCI_VENDOR_ID_OXSEMI &&
+ (dev->device & 0xF000) == 0xC000) {
+ up->port.get_divisor = pci_oxsemi_tornado_get_divisor;
+ up->port.set_divisor = pci_oxsemi_tornado_set_divisor;
+
+ up->mcr_mask = ~UART_MCR_CLKSEL;
+ up->mcr_force = UART_MCR_CLKSEL;
+ }
+
+ return pci_default_setup(priv, board, up, idx);
+}
+
static int pci_asix_setup(struct serial_private *priv,
const struct pciserial_board *board,
struct uart_8250_port *port, int idx)
@@ -2518,7 +2714,7 @@ static struct pci_serial_quirk pci_seria
.subvendor = PCI_ANY_ID,
.subdevice = PCI_ANY_ID,
.init = pci_oxsemi_tornado_init,
- .setup = pci_default_setup,
+ .setup = pci_oxsemi_tornado_setup,
},
{
.vendor = PCI_VENDOR_ID_MAINPINE,
@@ -2526,7 +2722,7 @@ static struct pci_serial_quirk pci_seria
.subvendor = PCI_ANY_ID,
.subdevice = PCI_ANY_ID,
.init = pci_oxsemi_tornado_init,
- .setup = pci_default_setup,
+ .setup = pci_oxsemi_tornado_setup,
},
{
.vendor = PCI_VENDOR_ID_DIGI,
@@ -2534,7 +2730,7 @@ static struct pci_serial_quirk pci_seria
.subvendor = PCI_SUBVENDOR_ID_IBM,
.subdevice = PCI_ANY_ID,
.init = pci_oxsemi_tornado_init,
- .setup = pci_default_setup,
+ .setup = pci_oxsemi_tornado_setup,
},
{
.vendor = PCI_VENDOR_ID_INTEL,
@@ -2851,7 +3047,7 @@ enum pci_board_num_t {
pbn_b0_2_1843200,
pbn_b0_4_1843200,
- pbn_b0_1_3906250,
+ pbn_b0_1_15625000,
pbn_b0_bt_1_115200,
pbn_b0_bt_2_115200,
@@ -2931,10 +3127,10 @@ enum pci_board_num_t {
pbn_plx_romulus,
pbn_endrun_2_4000000,
pbn_oxsemi,
- pbn_oxsemi_1_3906250,
- pbn_oxsemi_2_3906250,
- pbn_oxsemi_4_3906250,
- pbn_oxsemi_8_3906250,
+ pbn_oxsemi_1_15625000,
+ pbn_oxsemi_2_15625000,
+ pbn_oxsemi_4_15625000,
+ pbn_oxsemi_8_15625000,
pbn_intel_i960,
pbn_sgi_ioc3,
pbn_computone_4,
@@ -3081,10 +3277,10 @@ static struct pciserial_board pci_boards
.uart_offset = 8,
},
- [pbn_b0_1_3906250] = {
+ [pbn_b0_1_15625000] = {
.flags = FL_BASE0,
.num_ports = 1,
- .base_baud = 3906250,
+ .base_baud = 15625000,
.uart_offset = 8,
},
@@ -3479,31 +3675,31 @@ static struct pciserial_board pci_boards
.base_baud = 115200,
.uart_offset = 8,
},
- [pbn_oxsemi_1_3906250] = {
+ [pbn_oxsemi_1_15625000] = {
.flags = FL_BASE0,
.num_ports = 1,
- .base_baud = 3906250,
+ .base_baud = 15625000,
.uart_offset = 0x200,
.first_offset = 0x1000,
},
- [pbn_oxsemi_2_3906250] = {
+ [pbn_oxsemi_2_15625000] = {
.flags = FL_BASE0,
.num_ports = 2,
- .base_baud = 3906250,
+ .base_baud = 15625000,
.uart_offset = 0x200,
.first_offset = 0x1000,
},
- [pbn_oxsemi_4_3906250] = {
+ [pbn_oxsemi_4_15625000] = {
.flags = FL_BASE0,
.num_ports = 4,
- .base_baud = 3906250,
+ .base_baud = 15625000,
.uart_offset = 0x200,
.first_offset = 0x1000,
},
- [pbn_oxsemi_8_3906250] = {
+ [pbn_oxsemi_8_15625000] = {
.flags = FL_BASE0,
.num_ports = 8,
- .base_baud = 3906250,
+ .base_baud = 15625000,
.uart_offset = 0x200,
.first_offset = 0x1000,
},
@@ -4510,158 +4706,158 @@ static const struct pci_device_id serial
*/
{ PCI_VENDOR_ID_OXSEMI, 0xc101, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_3906250 },
+ pbn_b0_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc105, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_3906250 },
+ pbn_b0_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc11b, /* OXPCIe952 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc11f, /* OXPCIe952 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc120, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_3906250 },
+ pbn_b0_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc124, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_3906250 },
+ pbn_b0_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc138, /* OXPCIe952 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc13d, /* OXPCIe952 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc140, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_3906250 },
+ pbn_b0_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc141, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_3906250 },
+ pbn_b0_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc144, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_3906250 },
+ pbn_b0_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc145, /* OXPCIe952 1 Legacy UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_b0_1_3906250 },
+ pbn_b0_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc158, /* OXPCIe952 2 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_2_3906250 },
+ pbn_oxsemi_2_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc15d, /* OXPCIe952 2 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_2_3906250 },
+ pbn_oxsemi_2_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc208, /* OXPCIe954 4 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_4_3906250 },
+ pbn_oxsemi_4_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc20d, /* OXPCIe954 4 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_4_3906250 },
+ pbn_oxsemi_4_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc308, /* OXPCIe958 8 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_8_3906250 },
+ pbn_oxsemi_8_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc30d, /* OXPCIe958 8 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_8_3906250 },
+ pbn_oxsemi_8_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc40b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc40f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc41b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc41f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc42b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc42f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc43b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc43f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc44b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc44f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc45b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc45f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc46b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc46f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc47b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc47f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc48b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc48f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc49b, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc49f, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4ab, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4af, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4bb, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4bf, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4cb, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_OXSEMI, 0xc4cf, /* OXPCIe200 1 Native UART */
PCI_ANY_ID, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
/*
* Mainpine Inc. IQ Express "Rev3" utilizing OxSemi Tornado
*/
{ PCI_VENDOR_ID_MAINPINE, 0x4000, /* IQ Express 1 Port V.34 Super-G3 Fax */
PCI_VENDOR_ID_MAINPINE, 0x4001, 0, 0,
- pbn_oxsemi_1_3906250 },
+ pbn_oxsemi_1_15625000 },
{ PCI_VENDOR_ID_MAINPINE, 0x4000, /* IQ Express 2 Port V.34 Super-G3 Fax */
PCI_VENDOR_ID_MAINPINE, 0x4002, 0, 0,
- pbn_oxsemi_2_3906250 },
+ pbn_oxsemi_2_15625000 },
{ PCI_VENDOR_ID_MAINPINE, 0x4000, /* IQ Express 4 Port V.34 Super-G3 Fax */
PCI_VENDOR_ID_MAINPINE, 0x4004, 0, 0,
- pbn_oxsemi_4_3906250 },
+ pbn_oxsemi_4_15625000 },
{ PCI_VENDOR_ID_MAINPINE, 0x4000, /* IQ Express 8 Port V.34 Super-G3 Fax */
PCI_VENDOR_ID_MAINPINE, 0x4008, 0, 0,
- pbn_oxsemi_8_3906250 },
+ pbn_oxsemi_8_15625000 },
/*
* Digi/IBM PCIe 2-port Async EIA-232 Adapter utilizing OxSemi Tornado
*/
{ PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_2_OX_IBM,
PCI_SUBVENDOR_ID_IBM, PCI_ANY_ID, 0, 0,
- pbn_oxsemi_2_3906250 },
+ pbn_oxsemi_2_15625000 },
/*
* SBS Technologies, Inc. P-Octal and PMC-OCTPRO cards,
Index: linux-malta-cbus-uart/drivers/tty/serial/8250/8250_port.c
===================================================================
--- linux-malta-cbus-uart.orig/drivers/tty/serial/8250/8250_port.c
+++ linux-malta-cbus-uart/drivers/tty/serial/8250/8250_port.c
@@ -528,27 +528,6 @@ serial_port_out_sync(struct uart_port *p
}
/*
- * For the 16C950
- */
-static void serial_icr_write(struct uart_8250_port *up, int offset, int value)
-{
- serial_out(up, UART_SCR, offset);
- serial_out(up, UART_ICR, value);
-}
-
-static unsigned int serial_icr_read(struct uart_8250_port *up, int offset)
-{
- unsigned int value;
-
- serial_icr_write(up, UART_ACR, up->acr | UART_ACR_ICRRD);
- serial_out(up, UART_SCR, offset);
- value = serial_in(up, UART_ICR);
- serial_icr_write(up, UART_ACR, up->acr);
-
- return value;
-}
-
-/*
* FIFO support.
*/
static void serial8250_clear_fifos(struct uart_8250_port *p)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/4] serial: 8250: Define RX trigger levels for OxSemi 950 devices
2021-06-10 18:38 [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Maciej W. Rozycki
` (2 preceding siblings ...)
2021-06-10 18:39 ` [PATCH 3/4] serial: 8250: Add proper clock handling " Maciej W. Rozycki
@ 2021-06-10 18:39 ` Maciej W. Rozycki
2021-06-15 12:07 ` [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Greg Kroah-Hartman
4 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2021-06-10 18:39 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: linux-serial, linux-kernel
Oxford Semiconductor 950 serial port devices have a 128-byte FIFO and in
the enhanced (650) mode, which we select in `autoconfig_has_efr' with
the ECB bit set in the EFR register, they support the receive interrupt
trigger level selectable with FCR bits 7:6 from the set of 16, 32, 112,
120. This applies to the original OX16C950 discrete UART[1] as well as
950 cores embedded into more complex devices.
For these devices we set the default to 112, which sets an excessively
high level of 112 or 7/8 of the FIFO capacity, unlike with other port
types where we choose at most 1/2 of their respective FIFO capacities.
Additionally we don't make the trigger level configurable. Consequently
frequent input overruns happen with high bit rates where hardware flow
control cannot be used (e.g. terminal applications) even with otherwise
highly-performant systems.
Lower the default receive interrupt trigger level to 32 then, and make
it configurable. Document the trigger levels along with other port
types, including the set of 16, 32, 64, 112 for the transmit interrupt
as well[2].
References:
[1] "OX16C950 rev B High Performance UART with 128 byte FIFOs", Oxford
Semiconductor, Inc., DS-0031, Sep 05, Table 10: "Receiver Trigger
Levels", p. 22
[2] same, Table 9: "Transmit Interrupt Trigger Levels", p. 22
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
drivers/tty/serial/8250/8250_port.c | 3 ++-
include/uapi/linux/serial_reg.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
linux-serial-8250-oxsemi-fifo.diff
Index: linux-malta-cbus-uart/drivers/tty/serial/8250/8250_port.c
===================================================================
--- linux-malta-cbus-uart.orig/drivers/tty/serial/8250/8250_port.c
+++ linux-malta-cbus-uart/drivers/tty/serial/8250/8250_port.c
@@ -122,7 +122,8 @@ static const struct serial8250_config ua
.name = "16C950/954",
.fifo_size = 128,
.tx_loadsz = 128,
- .fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
+ .fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01,
+ .rxtrig_bytes = {16, 32, 112, 120},
/* UART_CAP_EFR breaks billionon CF bluetooth card. */
.flags = UART_CAP_FIFO | UART_CAP_SLEEP,
},
Index: linux-malta-cbus-uart/include/uapi/linux/serial_reg.h
===================================================================
--- linux-malta-cbus-uart.orig/include/uapi/linux/serial_reg.h
+++ linux-malta-cbus-uart/include/uapi/linux/serial_reg.h
@@ -62,6 +62,7 @@
* ST16C654: 8 16 56 60 8 16 32 56 PORT_16654
* TI16C750: 1 16 32 56 xx xx xx xx PORT_16750
* TI16C752: 8 16 56 60 8 16 32 56
+ * OX16C950: 16 32 112 120 16 32 64 112 PORT_16C950
* Tegra: 1 4 8 14 16 8 4 1 PORT_TEGRA
*/
#define UART_FCR_R_TRIG_00 0x00
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs
2021-06-10 18:38 [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Maciej W. Rozycki
` (3 preceding siblings ...)
2021-06-10 18:39 ` [PATCH 4/4] serial: 8250: Define RX trigger levels for OxSemi 950 devices Maciej W. Rozycki
@ 2021-06-15 12:07 ` Greg Kroah-Hartman
2021-06-15 14:19 ` Maciej W. Rozycki
4 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-15 12:07 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Jiri Slaby, linux-serial, linux-kernel
On Thu, Jun 10, 2021 at 08:38:55PM +0200, Maciej W. Rozycki wrote:
> Hi,
>
> In the course of verifying support for SMSC FDC37M817 Super I/O chip's
> data rate of 460800bps with an EXSYS EX-44072 option card based on the
> Oxford Semiconductor OXPCIe952 device I have observed the inability to
> transfer any data correctly at this rate between the two devices. Lower
> rates, including 230400bps, appeared to work correctly, also with other
> kinds of serial ports referred to with my previous patch series, which
> strongly indicated something being wrong with the Oxford device.
>
> In the end I have tracked the issue down to our baud base set to 4000000
> for the device being off by 2.4%. Enough for an incorrect divisor value
> of 9 to be chosen to yield the bit rate of 460800bps being particularly
> inaccurate for the baud base selected and caused the actual rate of
> 434027.78bps of being used, -5.8% off. Consequently the ten bits of data
> sent with every 8-bit character were enough for the drift to accumulate up
> to the two ends to get out of sync, with the stop bit interpreted as bit 7
> of data. Obviously whoever wrote this code never actually tried higher
> data rates, or only connected Oxford devices to each other causing the
> systematic errors at both ends to cancel each other.
>
> With the baud base corrected to the chip's default of 3906250 for the 650
> mode which we use (the reset default for the initial 450 mode is 115314,
> approximated for maximum backwards compatibility with legacy OS drivers by
> dividing the device's 62.5MHz clock by 33.875), the new calculated divisor
> value and the new actual bit rate became 8 and 488281.25bps respectively.
> Now +5.96% off, so the stop bit could be missed causing data corruption
> with continuous data streams, but at least that could be worked around by
> using two stop bits instead. Not a good solution though.
>
> So I chose to implement proper clock handling for the chip. The bit rate
> with this device is worked out from the 62.5MHz clock first by choosing an
> oversampling rate between 4 and 16 inclusive, then by the clock prescaler
> between 1 and 63.875 in increments of 0.125, and finally a 16-bit unsigned
> divisor, all of which divide the input clock by the respective value.
>
> By choosing the right values of these three parameters either exact or
> highly-accurate actual bit rates can be programmed for standard and many
> non-standard rates from 1bps up to 15625000bps, e.g. for the data rate of
> 460800bps concerned here I was able to get the accuracy of 0.0064% by
> choosing the values of 7, 3.875, and 5 respectively for the oversampling
> rate, the clock prescaler, and the clock divisor.
>
> Additionally even with my considerably mighty POWER9 box I have observed
> frequent input overruns with the bit rates of 460800bps and higher, and I
> have noticed we have the receive interrupt trigger level set particularly
> high in terms of FIFO usage percentage for 16C950 UARTs and then we don't
> make the levels configurable. Lowering the default to a saner value made
> the overruns go away altogether for rates below 921600bps. As I've only
> verified these changes in terminal environment rather than with modems I
> could not make use of hardware flow control which this chip supports and
> which I presume would prevent overruns from happening even with higher bit
> rates.
>
> There's more that could be done here, for example we don't make use of
> the 950 mode where FIFO trigger levels can be fine-tuned in increments of
> 1, which, interestingly, could help with the lower rate modes as reception
> is quite choppy with them, owing to the minimum receive interrupt trigger
> level of 16 in the 650 mode. I gave the 950 mode a try, but it made the
> chip freeze frequently until new data was received, so clearly I must have
> missed something in the chip's configuration, which I did not investigate.
> Something for a different time perhaps then.
>
> I have verified these changes for standard termios rates between 300bps
> and 460800bps with my WTI CPM-800 site manager device and my Malta board's
> serial ports, as suitable, all working flawlessly in terminal mode now.
> I have verified standard termios rates between 500000bps and 4000000bps as
> well, however for the lack of other high-speed hardware with a pair of
> Oxford devices only. Except for input overruns noted above and growing in
> numbers as the rate increased rates of up to 3500000bps worked flawlessly.
> In particular the rate of 576000bps, still without input overruns, gave
> this nice feeling as if working with a virtual terminal rather than over a
> serial line!
>
> Conversely the rate of 4000000bps showed significant data corruption,
> happening randomly, i.e. some characters went through just fine, while
> other ones became garbled in no particular pattern, unlike with the rate
> inaccuracy described above. Also with no input overruns whatsoever. I
> have double-checked that all the three parameters making up the bit rate
> from the clock rate have been programmed correctly.
>
> Therefore I have concluded this is not an issue with my change (or indeed
> any other part the driver) and it is simply that the rate has exceeded
> either the maximum frequency the EX-44072 board's serial transceivers
> support (I haven't checked the chip types used and I can't persuade myself
> to disassemble the system just to have a look at the board again), or the
> bandwidth of the transmission line used (a flat 8-core telephone cable of
> a standard Cisco console cable assembly). Not an issue to be addressed in
> software and I find it rather astonishing anyway it worked so well for up
> to 3.5MHz already!
>
> I have no modems, so I couldn't verify DCE interoperation, but I don't
> expect issues with the bit rates being more accurate now, or the default
> FIFO receiver trigger level tweaked to be more conservative.
>
> Finally the 16-bit UART_DIV_MAX limitation of the baud rate requested
> with `serial8250_get_baud_rate' makes the standard rates of 200bps and
> lower inaccessible in the regular way with the baud base of 15625000.
> That could be avoided by tweaking our 8250 driver core appropriately, but
> I have figured out with modern serial port usage that would not be the
> best use of my time. Someone who does have a real need to use an Oxford
> device at these low rates can step in and make the necessary chances.
>
> Meanwhile I took advantage of the ancient spd_cust feature we thankfully
> continue supporting and actually did verify not only the standard rates
> between 50bps and 200bps, but the rates of 4bps and 2bps as well, using my
> old x86 server's serial port with the baud base of 115200. That was,
> ahem, an interesting experience both by itself and also with 2bps, which
> revealed a phenomenon with SMSC Super I/O ports not working as documented
> (already noted in the preceding patch series). Eventually I verified the
> 2bps rate with a plain ISA multi I/O card and its 16450 UART my EISA 486
> box has as the remote console, which does support the divisor value of
> 57600 required.
>
> See individual change descriptions for further details including figures.
>
> Please apply.
This patch series causes the following build warning to be added:
drivers/tty/serial/8250/8250_pci.c: In function ‘pci_oxsemi_tornado_setup’:
drivers/tty/serial/8250/8250_pci.c:1258:32: warning: unsigned conversion from ‘int’ to ‘unsigned char’ changes value from ‘-129’ to ‘127’ [-Woverflow]
1258 | up->mcr_mask = ~UART_MCR_CLKSEL;
| ^
Can you fix this up and resend?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs
2021-06-15 12:07 ` [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Greg Kroah-Hartman
@ 2021-06-15 14:19 ` Maciej W. Rozycki
2021-06-15 15:52 ` Greg Kroah-Hartman
0 siblings, 1 reply; 14+ messages in thread
From: Maciej W. Rozycki @ 2021-06-15 14:19 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel
On Tue, 15 Jun 2021, Greg Kroah-Hartman wrote:
> This patch series causes the following build warning to be added:
>
> drivers/tty/serial/8250/8250_pci.c: In function ‘pci_oxsemi_tornado_setup’:
> drivers/tty/serial/8250/8250_pci.c:1258:32: warning: unsigned conversion from ‘int’ to ‘unsigned char’ changes value from ‘-129’ to ‘127’ [-Woverflow]
> 1258 | up->mcr_mask = ~UART_MCR_CLKSEL;
> | ^
>
>
> Can you fix this up and resend?
I've seen that, but that's not a problem with my change, but rather with
<linux/serial_reg.h> making this macro (and the remaining ones from this
group) expand to a signed constant (0x80 rather than 0x80u).
I can fix the header, but that would be a separate change, and mind too
that this is a user header, so it's not clear to me what the impact might
be on user apps making use of it.
We could use a GCC pragma to suppress the warning temporarily across this
piece of code, but it's not clear to me either what our policy has been on
such approach.
Thoughts?
NB casting UART_MCR_CLKSEL here to an unsigned type does not help as GCC
still sees the original constant through the cast; I've already tried that
of course.
Last but not least: do we need to have this warning enabled in the first
place?
Maciej
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs
2021-06-15 14:19 ` Maciej W. Rozycki
@ 2021-06-15 15:52 ` Greg Kroah-Hartman
2021-06-15 17:12 ` Maciej W. Rozycki
0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-15 15:52 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Jiri Slaby, linux-serial, linux-kernel
On Tue, Jun 15, 2021 at 04:19:03PM +0200, Maciej W. Rozycki wrote:
> On Tue, 15 Jun 2021, Greg Kroah-Hartman wrote:
>
> > This patch series causes the following build warning to be added:
> >
> > drivers/tty/serial/8250/8250_pci.c: In function ‘pci_oxsemi_tornado_setup’:
> > drivers/tty/serial/8250/8250_pci.c:1258:32: warning: unsigned conversion from ‘int’ to ‘unsigned char’ changes value from ‘-129’ to ‘127’ [-Woverflow]
> > 1258 | up->mcr_mask = ~UART_MCR_CLKSEL;
> > | ^
> >
> >
> > Can you fix this up and resend?
>
> I've seen that, but that's not a problem with my change, but rather with
> <linux/serial_reg.h> making this macro (and the remaining ones from this
> group) expand to a signed constant (0x80 rather than 0x80u).
As your change causes it to show up, it must have something to do with
it :)
> I can fix the header, but that would be a separate change, and mind too
> that this is a user header, so it's not clear to me what the impact might
> be on user apps making use of it.
You can not change the uapi header, why would you want to?
> We could use a GCC pragma to suppress the warning temporarily across this
> piece of code, but it's not clear to me either what our policy has been on
> such approach.
What pragma?
> Thoughts?
Why does your change cause this to show up?
> NB casting UART_MCR_CLKSEL here to an unsigned type does not help as GCC
> still sees the original constant through the cast; I've already tried that
> of course.
>
> Last but not least: do we need to have this warning enabled in the first
> place?
No idea, but that's a different discussion, with a different group of
people :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs
2021-06-15 15:52 ` Greg Kroah-Hartman
@ 2021-06-15 17:12 ` Maciej W. Rozycki
2021-06-15 21:45 ` David Laight
0 siblings, 1 reply; 14+ messages in thread
From: Maciej W. Rozycki @ 2021-06-15 17:12 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel
On Tue, 15 Jun 2021, Greg Kroah-Hartman wrote:
> > > This patch series causes the following build warning to be added:
> > >
> > > drivers/tty/serial/8250/8250_pci.c: In function ‘pci_oxsemi_tornado_setup’:
> > > drivers/tty/serial/8250/8250_pci.c:1258:32: warning: unsigned conversion from ‘int’ to ‘unsigned char’ changes value from ‘-129’ to ‘127’ [-Woverflow]
> > > 1258 | up->mcr_mask = ~UART_MCR_CLKSEL;
> > > | ^
> > >
> > >
> > > Can you fix this up and resend?
> >
> > I've seen that, but that's not a problem with my change, but rather with
> > <linux/serial_reg.h> making this macro (and the remaining ones from this
> > group) expand to a signed constant (0x80 rather than 0x80u).
>
> As your change causes it to show up, it must have something to do with
> it :)
Of course it does, but the problem comes from the data type signedness
difference between the `mcr_mask' member of `struct uart_8250_port', the
type of which is (rightfully IMO) `unsigned char' (rather than `char' or
`signed char') and the UART_MCR_CLKSEL macro, which expands to a signed
int. My change does not introduce this data type difference, hence it's
not responsible for the problem, though it does expose it.
> > I can fix the header, but that would be a separate change, and mind too
> > that this is a user header, so it's not clear to me what the impact might
> > be on user apps making use of it.
>
> You can not change the uapi header, why would you want to?
To make the data type of the constants it defines such that they can be
assigned to program entities they are supposed to be used with without
changing the sign at truncation time?
> > We could use a GCC pragma to suppress the warning temporarily across this
> > piece of code, but it's not clear to me either what our policy has been on
> > such approach.
>
> What pragma?
#pragma GCC diagnostic ignored "-Woverflow"
> > Thoughts?
>
> Why does your change cause this to show up?
As I have noted above there is a data type signedness difference between
`mcr_mask' and UART_MCR_CLKSEL. So we have the value of 0x80 (128).
Once bitwise-complemented it becomes 0xffffff7f (-129). Once assigned to
`mcr_mask' however it becomes 0x7f (127), which is considered an unsafe
conversion between signed and unsigned integers by GCC, which is why the
compiler complains about it.
The same difference exists with say UART_MCR_OUT2 used in a similar
manner for ALPHA_KLUDGE_MCR, but GCC does not get noisy about it because
the constant UART_MCR_OUT2 expands to is 0x08 and therefore the position
of the bit set there does not coincide with the sign bit once truncated to
8 bits, so the truncation does not cause a sign change. The same warning
would trigger however if the constant were left-shifted by 4 before the
bitwise complement operation, so all these constants should be unsigned.
It does not make sense IMO to operate on signed values in the context of
bit patterns for peripheral hardware registers.
I'll find a way to paper it over if this is what is desired here, e.g. I
guess this piece:
up->mcr_mask = ~0;
up->mcr_mask ^= UART_MCR_CLKSEL;
will do, although I find it obscurer than my original proposal, and surely
asking for a comment (which I think is a sign of a problem).
Maciej
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs
2021-06-15 17:12 ` Maciej W. Rozycki
@ 2021-06-15 21:45 ` David Laight
2021-06-26 4:13 ` Maciej W. Rozycki
0 siblings, 1 reply; 14+ messages in thread
From: David Laight @ 2021-06-15 21:45 UTC (permalink / raw)
To: 'Maciej W. Rozycki', Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel
From: Maciej W. Rozycki
> Sent: 15 June 2021 18:13
...
> As I have noted above there is a data type signedness difference between
> `mcr_mask' and UART_MCR_CLKSEL. So we have the value of 0x80 (128).
> Once bitwise-complemented it becomes 0xffffff7f (-129). Once assigned to
> `mcr_mask' however it becomes 0x7f (127), which is considered an unsafe
> conversion between signed and unsigned integers by GCC, which is why the
> compiler complains about it.
Blame the iso C standards committee for making integer promotions
'value preserving' instead of 'sign preserving' as they were in K&R C.
Try using ^ 0xffu instead of ~.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] serial: 8250: Add proper clock handling for OxSemi PCIe devices
2021-06-10 18:39 ` [PATCH 3/4] serial: 8250: Add proper clock handling " Maciej W. Rozycki
@ 2021-06-16 16:58 ` kernel test robot
2021-06-16 21:00 ` kernel test robot
2021-06-17 11:23 ` kernel test robot
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-06-16 16:58 UTC (permalink / raw)
To: Maciej W. Rozycki, Greg Kroah-Hartman, Jiri Slaby
Cc: kbuild-all, linux-serial, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2897 bytes --]
Hi "Maciej,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.13-rc6 next-20210616]
[cannot apply to tty/tty-testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Maciej-W-Rozycki/serial-8250-Fixes-for-Oxford-Semiconductor-950-UARTs/20210616-201047
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 94f0b2d4a1d0c52035aef425da5e022bd2cb1c71
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/d3a55e8397b71f343ef931cd098d42a13faf9047
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Maciej-W-Rozycki/serial-8250-Fixes-for-Oxford-Semiconductor-950-UARTs/20210616-201047
git checkout d3a55e8397b71f343ef931cd098d42a13faf9047
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/tty/serial/8250/8250_pci.c: In function 'pci_oxsemi_tornado_setup':
>> drivers/tty/serial/8250/8250_pci.c:1258:18: warning: unsigned conversion from 'int' to 'unsigned char' changes value from '-129' to '127' [-Woverflow]
1258 | up->mcr_mask = ~UART_MCR_CLKSEL;
| ^
vim +1258 drivers/tty/serial/8250/8250_pci.c
1241
1242 /*
1243 * For Tornado devices we force MCR[7] set for the Divide-by-M N/8 baud rate
1244 * generator prescaler (CPR and CPR2). Otherwise no prescaler would be used.
1245 */
1246 static int pci_oxsemi_tornado_setup(struct serial_private *priv,
1247 const struct pciserial_board *board,
1248 struct uart_8250_port *up, int idx)
1249 {
1250 struct pci_dev *dev = priv->dev;
1251
1252 /* OxSemi Tornado devices are all 0xCxxx */
1253 if (dev->vendor == PCI_VENDOR_ID_OXSEMI &&
1254 (dev->device & 0xF000) == 0xC000) {
1255 up->port.get_divisor = pci_oxsemi_tornado_get_divisor;
1256 up->port.set_divisor = pci_oxsemi_tornado_set_divisor;
1257
> 1258 up->mcr_mask = ~UART_MCR_CLKSEL;
1259 up->mcr_force = UART_MCR_CLKSEL;
1260 }
1261
1262 return pci_default_setup(priv, board, up, idx);
1263 }
1264
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67962 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] serial: 8250: Add proper clock handling for OxSemi PCIe devices
2021-06-10 18:39 ` [PATCH 3/4] serial: 8250: Add proper clock handling " Maciej W. Rozycki
2021-06-16 16:58 ` kernel test robot
@ 2021-06-16 21:00 ` kernel test robot
2021-06-17 11:23 ` kernel test robot
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-06-16 21:00 UTC (permalink / raw)
To: Maciej W. Rozycki, Greg Kroah-Hartman, Jiri Slaby
Cc: kbuild-all, clang-built-linux, linux-serial, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3070 bytes --]
Hi "Maciej,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.13-rc6 next-20210616]
[cannot apply to tty/tty-testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Maciej-W-Rozycki/serial-8250-Fixes-for-Oxford-Semiconductor-950-UARTs/20210616-201047
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 94f0b2d4a1d0c52035aef425da5e022bd2cb1c71
config: x86_64-randconfig-b001-20210615 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/d3a55e8397b71f343ef931cd098d42a13faf9047
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Maciej-W-Rozycki/serial-8250-Fixes-for-Oxford-Semiconductor-950-UARTs/20210616-201047
git checkout d3a55e8397b71f343ef931cd098d42a13faf9047
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/tty/serial/8250/8250_pci.c:1258:18: warning: implicit conversion from 'int' to 'unsigned char' changes value from -129 to 127 [-Wconstant-conversion]
up->mcr_mask = ~UART_MCR_CLKSEL;
~ ^~~~~~~~~~~~~~~~
1 warning generated.
vim +1258 drivers/tty/serial/8250/8250_pci.c
1241
1242 /*
1243 * For Tornado devices we force MCR[7] set for the Divide-by-M N/8 baud rate
1244 * generator prescaler (CPR and CPR2). Otherwise no prescaler would be used.
1245 */
1246 static int pci_oxsemi_tornado_setup(struct serial_private *priv,
1247 const struct pciserial_board *board,
1248 struct uart_8250_port *up, int idx)
1249 {
1250 struct pci_dev *dev = priv->dev;
1251
1252 /* OxSemi Tornado devices are all 0xCxxx */
1253 if (dev->vendor == PCI_VENDOR_ID_OXSEMI &&
1254 (dev->device & 0xF000) == 0xC000) {
1255 up->port.get_divisor = pci_oxsemi_tornado_get_divisor;
1256 up->port.set_divisor = pci_oxsemi_tornado_set_divisor;
1257
> 1258 up->mcr_mask = ~UART_MCR_CLKSEL;
1259 up->mcr_force = UART_MCR_CLKSEL;
1260 }
1261
1262 return pci_default_setup(priv, board, up, idx);
1263 }
1264
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30253 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] serial: 8250: Add proper clock handling for OxSemi PCIe devices
2021-06-10 18:39 ` [PATCH 3/4] serial: 8250: Add proper clock handling " Maciej W. Rozycki
2021-06-16 16:58 ` kernel test robot
2021-06-16 21:00 ` kernel test robot
@ 2021-06-17 11:23 ` kernel test robot
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-06-17 11:23 UTC (permalink / raw)
To: Maciej W. Rozycki, Greg Kroah-Hartman, Jiri Slaby
Cc: kbuild-all, linux-serial, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2650 bytes --]
Hi "Maciej,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.13-rc6 next-20210616]
[cannot apply to tty/tty-testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Maciej-W-Rozycki/serial-8250-Fixes-for-Oxford-Semiconductor-950-UARTs/20210616-201047
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 94f0b2d4a1d0c52035aef425da5e022bd2cb1c71
config: x86_64-randconfig-s021-20210617 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/d3a55e8397b71f343ef931cd098d42a13faf9047
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Maciej-W-Rozycki/serial-8250-Fixes-for-Oxford-Semiconductor-950-UARTs/20210616-201047
git checkout d3a55e8397b71f343ef931cd098d42a13faf9047
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/tty/serial/8250/8250_pci.c:1258:32: sparse: sparse: cast truncates bits from constant value (ffffff7f becomes 7f)
vim +1258 drivers/tty/serial/8250/8250_pci.c
1241
1242 /*
1243 * For Tornado devices we force MCR[7] set for the Divide-by-M N/8 baud rate
1244 * generator prescaler (CPR and CPR2). Otherwise no prescaler would be used.
1245 */
1246 static int pci_oxsemi_tornado_setup(struct serial_private *priv,
1247 const struct pciserial_board *board,
1248 struct uart_8250_port *up, int idx)
1249 {
1250 struct pci_dev *dev = priv->dev;
1251
1252 /* OxSemi Tornado devices are all 0xCxxx */
1253 if (dev->vendor == PCI_VENDOR_ID_OXSEMI &&
1254 (dev->device & 0xF000) == 0xC000) {
1255 up->port.get_divisor = pci_oxsemi_tornado_get_divisor;
1256 up->port.set_divisor = pci_oxsemi_tornado_set_divisor;
1257
> 1258 up->mcr_mask = ~UART_MCR_CLKSEL;
1259 up->mcr_force = UART_MCR_CLKSEL;
1260 }
1261
1262 return pci_default_setup(priv, board, up, idx);
1263 }
1264
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32991 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs
2021-06-15 21:45 ` David Laight
@ 2021-06-26 4:13 ` Maciej W. Rozycki
0 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2021-06-26 4:13 UTC (permalink / raw)
To: David Laight; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
On Tue, 15 Jun 2021, David Laight wrote:
> > As I have noted above there is a data type signedness difference between
> > `mcr_mask' and UART_MCR_CLKSEL. So we have the value of 0x80 (128).
> > Once bitwise-complemented it becomes 0xffffff7f (-129). Once assigned to
> > `mcr_mask' however it becomes 0x7f (127), which is considered an unsafe
> > conversion between signed and unsigned integers by GCC, which is why the
> > compiler complains about it.
>
> Blame the iso C standards committee for making integer promotions
> 'value preserving' instead of 'sign preserving' as they were in K&R C.
>
> Try using ^ 0xffu instead of ~.
Hmm, this is probably the least horrible way to paper it over, thanks.
Even using a temporary variable (let alone a cast) does not help as GCC
sees through it, and I've given up on converting <linux/serial_reg.h> to
have e.g.:
#define UART_MCR_CLKSEL _UL(0x80) /* Divide clock by 4 (TI16C752, EFR[4]=1) */
as I find that too messy with many of the comments wrapping up. And using
a GCC pragma would require a messy compiler version check.
I have posted an update with a path-of-least-resistance fix then along
with the 4/4 as v2 of this series.
Maciej
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-06-26 4:13 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-10 18:38 [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Maciej W. Rozycki
2021-06-10 18:38 ` [PATCH 1/4] serial: 8250: Dissociate 4MHz Titan ports from Oxford ports Maciej W. Rozycki
2021-06-10 18:39 ` [PATCH 2/4] serial: 8250: Correct the clock for OxSemi PCIe devices Maciej W. Rozycki
2021-06-10 18:39 ` [PATCH 3/4] serial: 8250: Add proper clock handling " Maciej W. Rozycki
2021-06-16 16:58 ` kernel test robot
2021-06-16 21:00 ` kernel test robot
2021-06-17 11:23 ` kernel test robot
2021-06-10 18:39 ` [PATCH 4/4] serial: 8250: Define RX trigger levels for OxSemi 950 devices Maciej W. Rozycki
2021-06-15 12:07 ` [PATCH 0/4] serial: 8250: Fixes for Oxford Semiconductor 950 UARTs Greg Kroah-Hartman
2021-06-15 14:19 ` Maciej W. Rozycki
2021-06-15 15:52 ` Greg Kroah-Hartman
2021-06-15 17:12 ` Maciej W. Rozycki
2021-06-15 21:45 ` David Laight
2021-06-26 4:13 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).