Skip to contentSkip to conversation

serial: 8250_pci patches to address issues with pericom_do_set_divisor()

PATCH New Andy Shevchenko 1 participant 3 messages Nov 22, 2021 TTY layer and serial drivers

lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

v3 cover serial: 8250_pci patches to address issues with pericom_do_set_divisor()

A series patches to address three issues one customer managed to hit all at once.

1) Fourth port not being setup correctly on some Pericom chips. Fix the entries in pci_serial_quirks array.

2) Rewrite pericom_do_set_divisor() to always calc divisor and to use the uartclk instead of a hard coded value. Always calculate divisor without passing control to serial8250_do_set_divisor().

Jay, can you test this again, please?

Changelog v3:
- fix possible division by 0 in patch 2 (Jay)

Changelog v2:
- dropped no-fixes patch, left only fixes here
- amended refactoring of the ->set_divisor()
- fixed Fixes tags

Jay Dolan (2):
serial: 8250_pci: Fix ACCES entries in pci_serial_quirks array
serial: 8250_pci: rewrite pericom_do_set_divisor()

 drivers/tty/serial/8250/8250_pci.c | 39 +++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 14 deletions(-)
Signature · 1 line
2.33.0
author
#1 lore raw

PATCH v3 1/2 serial: 8250_pci: Fix ACCES entries in pci_serial_quirks array

From: Jay Dolan <[email protected]>

Fix error in table for PCI_DEVICE_ID_ACCESIO_PCIE_ICM_4S that caused it and PCI_DEVICE_ID_ACCESIO_PCIE_ICM232_4 to be missing their fourth port.

Fixes 78d3820b9bd3 ("serial: 8250_pci: Have ACCES cards that use the four port Pericom PI7C9X7954 chip use the pci_pericom_setup()")
Signed-off-by Jay Dolan <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/tty/serial/8250/8250_pci.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/tty/serial/8250/8250_pci.c +8 -1
index 5d43de143f33..b793d848aeb6 100644
@@ -2291,12 +2291,19 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
 		.setup      = pci_pericom_setup_four_at_eight,
 	},
 	{
-		.vendor     = PCI_DEVICE_ID_ACCESIO_PCIE_ICM_4S,
+		.vendor     = PCI_VENDOR_ID_ACCESIO,
 		.device     = PCI_DEVICE_ID_ACCESIO_PCIE_ICM232_4,
 		.subvendor  = PCI_ANY_ID,
 		.subdevice  = PCI_ANY_ID,
 		.setup      = pci_pericom_setup_four_at_eight,
 	},
+	{
+		.vendor     = PCI_VENDOR_ID_ACCESIO,
+		.device     = PCI_DEVICE_ID_ACCESIO_PCIE_ICM_4S,
+		.subvendor  = PCI_ANY_ID,
+		.subdevice  = PCI_ANY_ID,
+		.setup      = pci_pericom_setup_four_at_eight,
+	},
 	{
 		.vendor     = PCI_VENDOR_ID_ACCESIO,
 		.device     = PCI_DEVICE_ID_ACCESIO_MPCIE_ICM232_4,
Signature · 1 line
2.33.0
author
#2 lore raw

PATCH v3 2/2 serial: 8250_pci: rewrite pericom_do_set_divisor()

In reply to Andy Shevchenko

From: Jay Dolan <[email protected]>

Have pericom_do_set_divisor() use the uartclk instead of a hard coded value to work with different speed crystals. Tested with 14.7456 and 24 MHz crystals.

Have pericom_do_set_divisor() always calculate the divisor rather than call serial8250_do_set_divisor() for rates below baud_base.

Do not write registers or call serial8250_do_set_divisor() if valid divisors could not be found.

Fixes 6bf4e42f1d19 ("serial: 8250: Add support for higher baud rates to Pericom chips")
Signed-off-by Jay Dolan <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/tty/serial/8250/8250_pci.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)
drivers/tty/serial/8250/8250_pci.c +17 -13
index b793d848aeb6..60f8fffdfd77 100644
@@ -1324,29 +1324,33 @@ pericom_do_set_divisor(struct uart_port *port, unsigned int baud,
 {
 	int scr;
 	int lcr;
-	int actual_baud;
-	int tolerance;
 
-	for (scr = 5 ; scr <= 15 ; scr++) {
-		actual_baud = 921600 * 16 / scr;
-		tolerance = actual_baud / 50;
+	for (scr = 16; scr > 4; scr--) {
+		unsigned int maxrate = port->uartclk / scr;
+		unsigned int divisor = max(maxrate / baud, 1U);
+		int delta = maxrate / divisor - baud;
 
-		if ((baud < actual_baud + tolerance) &&
-			(baud > actual_baud - tolerance)) {
+		if (baud > maxrate + baud / 50)
+			continue;
 
+		if (delta > baud / 50)
+			divisor++;
+
+		if (divisor > 0xffff)
+			continue;
+
+		/* Update delta due to possible divisor change */
+		delta = maxrate / divisor - baud;
+		if (abs(delta) < baud / 50) {
 			lcr = serial_port_in(port, UART_LCR);
 			serial_port_out(port, UART_LCR, lcr | 0x80);
-
-			serial_port_out(port, UART_DLL, 1);
-			serial_port_out(port, UART_DLM, 0);
+			serial_port_out(port, UART_DLL, divisor & 0xff);
+			serial_port_out(port, UART_DLM, divisor >> 8 & 0xff);
 			serial_port_out(port, 2, 16 - scr);
 			serial_port_out(port, UART_LCR, lcr);
 			return;
-		} else if (baud > actual_baud) {
-			break;
 		}
 	}
-	serial8250_do_set_divisor(port, baud, quot, quot_frac);
 }
 static int pci_pericom_setup(struct serial_private *priv,
 		  const struct pciserial_board *board,
Signature · 1 line
2.33.0