After a rework for CONNTECH was done, the driver may need a bit of
love in order to become less verbose (in terms of indentation and
code duplication) and hence easier to read.
This clean up series fixes a couple of (not so critical) issues and
cleans up the recently added code. No functional change indented by
the cleaning up part.
Parker, please test this and give your formal Tested-by tag
(you may do it by replying to this message if all patches are
successfully tested; more details about tags are available in
the Submitting Patches documentation).
In v2:
- fixed the EEPROM reading data loop (Ilpo, Parker)
Andy Shevchenko (13):
serial: 8250_exar: Don't return positive values as error codes
serial: 8250_exar: Describe all parameters in kernel doc
serial: 8250_exar: Kill CTI_PCI_DEVICE()
serial: 8250_exar: Use PCI_SUBVENDOR_ID_IBM for subvendor ID
serial: 8250_exar: Trivia typo fixes
serial: 8250_exar: Extract cti_board_init_osc_freq() helper
serial: 8250_exar: Kill unneeded ->board_init()
serial: 8250_exar: Decrease indentation level
serial: 8250_exar: Return directly from switch-cases
serial: 8250_exar: Switch to use dev_err_probe()
serial: 8250_exar: Use BIT() in exar_ee_read()
serial: 8250_exar: Make type of bit the same in exar_ee_*_bit()
serial: 8250_exar: Keep the includes sorted
drivers/tty/serial/8250/8250_exar.c | 459 ++++++++++++----------------
1 file changed, 203 insertions(+), 256 deletions(-)
--
2.43.0.rc1.1336.g36b5255a03ac
Extract cti_board_init_osc_freq() helper to avoid code duplication.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_exar.c | 48 ++++++++++++-----------------
1 file changed, 19 insertions(+), 29 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index ac373cde7e39..956323e2de4a 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -695,7 +695,6 @@ static int cti_read_osc_freq(struct exar8250 *priv, u8 eeprom_offset)
{
u16 lower_word;
u16 upper_word;
- int osc_freq;
lower_word = exar_ee_read(priv, eeprom_offset);
// Check if EEPROM word was blank
@@ -706,10 +705,8 @@ static int cti_read_osc_freq(struct exar8250 *priv, u8 eeprom_offset)
if (upper_word == 0xFFFF)
return -EIO;
- osc_freq = FIELD_PREP(CTI_EE_MASK_OSC_FREQ_LOWER, lower_word) |
- FIELD_PREP(CTI_EE_MASK_OSC_FREQ_UPPER, upper_word);
-
- return osc_freq;
+ return FIELD_PREP(CTI_EE_MASK_OSC_FREQ_LOWER, lower_word) |
+ FIELD_PREP(CTI_EE_MASK_OSC_FREQ_UPPER, upper_word);
}
/**
@@ -890,6 +887,19 @@ static int cti_rs485_config_mpio_tristate(struct uart_port *port,
return cti_tristate_disable(priv, port->port_id);
}
+static void cti_board_init_osc_freq(struct exar8250 *priv, struct pci_dev *pcidev, u8 eeprom_offset)
+{
+ int osc_freq;
+
+ osc_freq = cti_read_osc_freq(priv, eeprom_offset);
+ if (osc_freq <= 0) {
+ dev_warn(&pcidev->dev, "failed to read OSC freq from EEPROM, using default\n");
+ osc_freq = CTI_DEFAULT_PCI_OSC_FREQ;
+ }
+
+ priv->osc_freq = osc_freq;
+}
+
static int cti_port_setup_common(struct exar8250 *priv,
struct pci_dev *pcidev,
int idx, unsigned int offset,
@@ -1095,19 +1105,9 @@ static int cti_board_init_xr17v35x(struct exar8250 *priv,
return 0;
}
-static int cti_board_init_xr17v25x(struct exar8250 *priv,
- struct pci_dev *pcidev)
+static int cti_board_init_xr17v25x(struct exar8250 *priv, struct pci_dev *pcidev)
{
- int osc_freq;
-
- osc_freq = cti_read_osc_freq(priv, CTI_EE_OFF_XR17V25X_OSC_FREQ);
- if (osc_freq < 0) {
- dev_warn(&pcidev->dev,
- "failed to read osc freq from EEPROM, using default\n");
- osc_freq = CTI_DEFAULT_PCI_OSC_FREQ;
- }
-
- priv->osc_freq = osc_freq;
+ cti_board_init_osc_freq(priv, pcidev, CTI_EE_OFF_XR17V25X_OSC_FREQ);
/* enable interrupts on cards that need the "PLX fix" */
switch (pcidev->subsystem_device) {
@@ -1123,19 +1123,9 @@ static int cti_board_init_xr17v25x(struct exar8250 *priv,
return 0;
}
-static int cti_board_init_xr17c15x(struct exar8250 *priv,
- struct pci_dev *pcidev)
+static int cti_board_init_xr17c15x(struct exar8250 *priv, struct pci_dev *pcidev)
{
- int osc_freq;
-
- osc_freq = cti_read_osc_freq(priv, CTI_EE_OFF_XR17C15X_OSC_FREQ);
- if (osc_freq <= 0) {
- dev_warn(&pcidev->dev,
- "failed to read osc freq from EEPROM, using default\n");
- osc_freq = CTI_DEFAULT_PCI_OSC_FREQ;
- }
-
- priv->osc_freq = osc_freq;
+ cti_board_init_osc_freq(priv, pcidev, CTI_EE_OFF_XR17C15X_OSC_FREQ);
/* enable interrupts on cards that need the "PLX fix" */
switch (pcidev->subsystem_device) {
--
2.43.0.rc1.1336.g36b5255a03ac
Trivia typo fixes: interupts --> interrupts and others.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_exar.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 5405a1f463f1..ac373cde7e39 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -641,7 +641,7 @@ pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev,
* @port_num: Port number to set tristate off
*
* Most RS485 capable cards have a power on tristate jumper/switch that ensures
- * the RS422/RS485 transciever does not drive a multi-drop RS485 bus when it is
+ * the RS422/RS485 transceiver does not drive a multi-drop RS485 bus when it is
* not the master. When this jumper is installed the user must set the RS485
* mode to Full or Half duplex to disable tristate prior to using the port.
*
@@ -666,7 +666,7 @@ static int cti_tristate_disable(struct exar8250 *priv, unsigned int port_num)
* @priv: Device's private structure
*
* Some older CTI cards require MPIO_0 to be set low to enable the
- * interupts from the UART to the PLX PCI->PCIe bridge.
+ * interrupts from the UART to the PLX PCI->PCIe bridge.
*
* Return: 0 on success, negative error code on failure
*/
@@ -927,7 +927,7 @@ static int cti_port_setup_fpga(struct exar8250 *priv,
port_type = cti_get_port_type_fpga(priv, pcidev, idx);
- // FPGA shares port offests with XR17C15X
+ // FPGA shares port offsets with XR17C15X
offset = idx * UART_EXAR_XR17C15X_PORT_OFFSET;
port->port.type = PORT_XR17D15X;
@@ -1109,7 +1109,7 @@ static int cti_board_init_xr17v25x(struct exar8250 *priv,
priv->osc_freq = osc_freq;
- /* enable interupts on cards that need the "PLX fix" */
+ /* enable interrupts on cards that need the "PLX fix" */
switch (pcidev->subsystem_device) {
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_A:
--
2.43.0.rc1.1336.g36b5255a03ac
There are two similar switch-cases which use different style.
Unify them and one more to return from the cases directly.
While at it, add missing default in another switch-case.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_exar.c | 80 +++++++++++------------------
1 file changed, 29 insertions(+), 51 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 1f04d4562878..51f6af16c557 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -627,6 +627,8 @@ pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev,
writeb(0xc0, p + UART_EXAR_MPIOINV_7_0);
writeb(0xc0, p + UART_EXAR_MPIOSEL_7_0);
break;
+ default:
+ break;
}
writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
@@ -723,8 +725,6 @@ static enum cti_port_type cti_get_port_type_xr17c15x_xr17v25x(struct exar8250 *p
struct pci_dev *pcidev,
unsigned int port_num)
{
- enum cti_port_type port_type;
-
switch (pcidev->subsystem_device) {
// RS232 only cards
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_232:
@@ -734,24 +734,17 @@ static enum cti_port_type cti_get_port_type_xr17c15x_xr17v25x(struct exar8250 *p
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_232_NS:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_232:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_232_NS:
- port_type = CTI_PORT_TYPE_RS232;
- break;
+ return CTI_PORT_TYPE_RS232;
// 1x RS232, 1x RS422/RS485
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_1_1:
- port_type = (port_num == 0) ?
- CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
- break;
+ return (port_num == 0) ? CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
// 2x RS232, 2x RS422/RS485
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_2:
- port_type = (port_num < 2) ?
- CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
- break;
+ return (port_num < 2) ? CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
// 4x RS232, 4x RS422/RS485
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_SP:
- port_type = (port_num < 4) ?
- CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
- break;
+ return (port_num < 4) ? CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
// RS232/RS422/RS485 HW (jumper) selectable
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4:
@@ -774,32 +767,24 @@ static enum cti_port_type cti_get_port_type_xr17c15x_xr17v25x(struct exar8250 *p
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XP_OPTO:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_XPRS_OPTO:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP:
- port_type = CTI_PORT_TYPE_RS232_422_485_HW;
- break;
+ return CTI_PORT_TYPE_RS232_422_485_HW;
// RS422/RS485 HW (jumper) selectable
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_485:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_485:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_485:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_485:
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_485:
- port_type = CTI_PORT_TYPE_RS422_485;
- break;
+ return CTI_PORT_TYPE_RS422_485;
// 6x RS232, 2x RS422/RS485
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_6_2_SP:
- port_type = (port_num < 6) ?
- CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
- break;
+ return (port_num < 6) ? CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
// 2x RS232, 6x RS422/RS485
case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_6_SP:
- port_type = (port_num < 2) ?
- CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
- break;
+ return (port_num < 2) ? CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
default:
dev_err(&pcidev->dev, "unknown/unsupported device\n");
- port_type = CTI_PORT_TYPE_NONE;
+ return CTI_PORT_TYPE_NONE;
}
-
- return port_type;
}
/**
@@ -816,20 +801,15 @@ static enum cti_port_type cti_get_port_type_fpga(struct exar8250 *priv,
struct pci_dev *pcidev,
unsigned int port_num)
{
- enum cti_port_type port_type;
-
switch (pcidev->device) {
case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG00X:
case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG01X:
case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_16:
- port_type = CTI_PORT_TYPE_RS232_422_485_HW;
- break;
+ return CTI_PORT_TYPE_RS232_422_485_HW;
default:
dev_err(&pcidev->dev, "unknown/unsupported device\n");
return CTI_PORT_TYPE_NONE;
}
-
- return port_type;
}
/**
@@ -1493,35 +1473,33 @@ static irqreturn_t exar_misc_handler(int irq, void *data)
return IRQ_HANDLED;
}
-static unsigned int exar_get_nr_ports(struct exar8250_board *board,
- struct pci_dev *pcidev)
+static unsigned int exar_get_nr_ports(struct exar8250_board *board, struct pci_dev *pcidev)
{
- unsigned int nr_ports = 0;
+ if (pcidev->vendor == PCI_VENDOR_ID_ACCESSIO)
+ return BIT(((pcidev->device & 0x38) >> 3) - 1);
- if (pcidev->vendor == PCI_VENDOR_ID_ACCESSIO) {
- nr_ports = BIT(((pcidev->device & 0x38) >> 3) - 1);
- } else if (board->num_ports > 0) {
- // Check if board struct overrides number of ports
- nr_ports = board->num_ports;
- } else if (pcidev->vendor == PCI_VENDOR_ID_EXAR) {
- // Exar encodes # ports in last nibble of PCI Device ID ex. 0358
- nr_ports = pcidev->device & 0x0f;
- } else if (pcidev->vendor == PCI_VENDOR_ID_CONNECT_TECH) {
- // Handle CTI FPGA cards
+ // Check if board struct overrides number of ports
+ if (board->num_ports > 0)
+ return board->num_ports;
+
+ // Exar encodes # ports in last nibble of PCI Device ID ex. 0358
+ if (pcidev->vendor == PCI_VENDOR_ID_EXAR)
+ return pcidev->device & 0x0f;
+
+ // Handle CTI FPGA cards
+ if (pcidev->vendor == PCI_VENDOR_ID_CONNECT_TECH) {
switch (pcidev->device) {
case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG00X:
case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG01X:
- nr_ports = 12;
- break;
+ return 12;
case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_16:
- nr_ports = 16;
- break;
+ return 16;
default:
- break;
+ return 0;
}
}
- return nr_ports;
+ return 0;
}
static int
--
2.43.0.rc1.1336.g36b5255a03ac
Switch to use dev_err_probe() to simplify the error path and
unify a message template.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_exar.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 51f6af16c557..306bc6d7c141 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -889,11 +889,8 @@ static int cti_port_setup_common(struct exar8250 *priv,
port->port.uartclk = priv->osc_freq;
ret = serial8250_pci_setup_port(pcidev, port, 0, offset, 0);
- if (ret) {
- dev_err(&pcidev->dev,
- "failed to setup pci for port %d err: %d\n", idx, ret);
+ if (ret)
return ret;
- }
port->port.private_data = (void *)priv;
port->port.pm = exar_pm;
@@ -1522,11 +1519,8 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
maxnr = pci_resource_len(pcidev, bar) >> (board->reg_shift + 3);
nr_ports = exar_get_nr_ports(board, pcidev);
- if (nr_ports == 0) {
- dev_err_probe(&pcidev->dev, -ENODEV,
- "failed to get number of ports\n");
- return -ENODEV;
- }
+ if (nr_ports == 0)
+ return dev_err_probe(&pcidev->dev, -ENODEV, "failed to get number of ports\n");
priv = devm_kzalloc(&pcidev->dev, struct_size(priv, line, nr_ports), GFP_KERNEL);
if (!priv)
@@ -1559,7 +1553,7 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
for (i = 0; i < nr_ports && i < maxnr; i++) {
rc = board->setup(priv, pcidev, &uart, i);
if (rc) {
- dev_err(&pcidev->dev, "Failed to setup port %u\n", i);
+ dev_err_probe(&pcidev->dev, rc, "Failed to setup port %u\n", i);
break;
}
@@ -1568,10 +1562,9 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
priv->line[i] = serial8250_register_8250_port(&uart);
if (priv->line[i] < 0) {
- dev_err(&pcidev->dev,
- "Couldn't register serial port %lx, irq %d, type %d, error %d\n",
- uart.port.iobase, uart.port.irq,
- uart.port.iotype, priv->line[i]);
+ dev_err_probe(&pcidev->dev, priv->line[i],
+ "Couldn't register serial port %lx, type %d, irq %d\n",
+ uart.port.iobase, uart.port.iotype, uart.port.irq);
break;
}
}
--
2.43.0.rc1.1336.g36b5255a03ac
Decrease indentation level in some places.
No functional change intended.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_exar.c | 100 ++++++++++++++--------------
1 file changed, 50 insertions(+), 50 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 4e683ca76de0..1f04d4562878 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -607,28 +607,30 @@ pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev,
writeb(32, p + UART_EXAR_TXTRG);
writeb(32, p + UART_EXAR_RXTRG);
+ /* Skip the initial (per device) setup */
+ if (idx)
+ return 0;
+
/*
* Setup Multipurpose Input/Output pins.
*/
- if (idx == 0) {
- switch (pcidev->device) {
- case PCI_DEVICE_ID_COMMTECH_4222PCI335:
- case PCI_DEVICE_ID_COMMTECH_4224PCI335:
- writeb(0x78, p + UART_EXAR_MPIOLVL_7_0);
- writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
- writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
- break;
- case PCI_DEVICE_ID_COMMTECH_2324PCI335:
- case PCI_DEVICE_ID_COMMTECH_2328PCI335:
- writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
- writeb(0xc0, p + UART_EXAR_MPIOINV_7_0);
- writeb(0xc0, p + UART_EXAR_MPIOSEL_7_0);
- break;
- }
- writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
- writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
- writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
+ switch (pcidev->device) {
+ case PCI_DEVICE_ID_COMMTECH_4222PCI335:
+ case PCI_DEVICE_ID_COMMTECH_4224PCI335:
+ writeb(0x78, p + UART_EXAR_MPIOLVL_7_0);
+ writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
+ writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
+ break;
+ case PCI_DEVICE_ID_COMMTECH_2324PCI335:
+ case PCI_DEVICE_ID_COMMTECH_2328PCI335:
+ writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
+ writeb(0xc0, p + UART_EXAR_MPIOINV_7_0);
+ writeb(0xc0, p + UART_EXAR_MPIOSEL_7_0);
+ break;
}
+ writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
+ writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
+ writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
return 0;
}
@@ -853,21 +855,19 @@ static enum cti_port_type cti_get_port_type_xr17v35x(struct exar8250 *priv,
port_flags = exar_ee_read(priv, offset);
port_type = FIELD_GET(CTI_EE_MASK_PORT_FLAGS_TYPE, port_flags);
- if (!CTI_PORT_TYPE_VALID(port_type)) {
- /*
- * If the port type is missing the card assume it is a
- * RS232/RS422/RS485 card to be safe.
- *
- * There is one known board (BEG013) that only has
- * 3 of 4 port types written to the EEPROM so this
- * acts as a work around.
- */
- dev_warn(&pcidev->dev,
- "failed to get port %d type from EEPROM\n", port_num);
- port_type = CTI_PORT_TYPE_RS232_422_485_HW;
- }
+ if (CTI_PORT_TYPE_VALID(port_type))
+ return port_type;
- return port_type;
+ /*
+ * If the port type is missing the card assume it is a
+ * RS232/RS422/RS485 card to be safe.
+ *
+ * There is one known board (BEG013) that only has 3 of 4 port types
+ * written to the EEPROM so this acts as a work around.
+ */
+ dev_warn(&pcidev->dev, "failed to get port %d type from EEPROM\n", port_num);
+
+ return CTI_PORT_TYPE_RS232_422_485_HW;
}
static int cti_rs485_config_mpio_tristate(struct uart_port *port,
@@ -1190,11 +1190,10 @@ static void setup_gpio(struct pci_dev *pcidev, u8 __iomem *p)
* devices will export them as GPIOs, so we pre-configure them safely
* as inputs.
*/
-
u8 dir = 0x00;
if ((pcidev->vendor == PCI_VENDOR_ID_EXAR) &&
- (pcidev->subsystem_vendor != PCI_VENDOR_ID_SEALEVEL)) {
+ (pcidev->subsystem_vendor != PCI_VENDOR_ID_SEALEVEL)) {
// Configure GPIO as inputs for Commtech adapters
dir = 0xff;
} else {
@@ -1284,27 +1283,28 @@ static int sealevel_rs485_config(struct uart_port *port, struct ktermios *termio
if (ret)
return ret;
- if (rs485->flags & SER_RS485_ENABLED) {
- old_lcr = readb(p + UART_LCR);
+ if (!(rs485->flags & SER_RS485_ENABLED))
+ return 0;
- /* Set EFR[4]=1 to enable enhanced feature registers */
- efr = readb(p + UART_XR_EFR);
- efr |= UART_EFR_ECB;
- writeb(efr, p + UART_XR_EFR);
+ old_lcr = readb(p + UART_LCR);
- /* Set MCR to use DTR as Auto-RS485 Enable signal */
- writeb(UART_MCR_OUT1, p + UART_MCR);
+ /* Set EFR[4]=1 to enable enhanced feature registers */
+ efr = readb(p + UART_XR_EFR);
+ efr |= UART_EFR_ECB;
+ writeb(efr, p + UART_XR_EFR);
- /* Set LCR[7]=1 to enable access to DLD register */
- writeb(old_lcr | UART_LCR_DLAB, p + UART_LCR);
+ /* Set MCR to use DTR as Auto-RS485 Enable signal */
+ writeb(UART_MCR_OUT1, p + UART_MCR);
- /* Set DLD[7]=1 for inverted RS485 Enable logic */
- dld = readb(p + UART_EXAR_DLD);
- dld |= UART_EXAR_DLD_485_POLARITY;
- writeb(dld, p + UART_EXAR_DLD);
+ /* Set LCR[7]=1 to enable access to DLD register */
+ writeb(old_lcr | UART_LCR_DLAB, p + UART_LCR);
- writeb(old_lcr, p + UART_LCR);
- }
+ /* Set DLD[7]=1 for inverted RS485 Enable logic */
+ dld = readb(p + UART_EXAR_DLD);
+ dld |= UART_EXAR_DLD_485_POLARITY;
+ writeb(dld, p + UART_EXAR_DLD);
+
+ writeb(old_lcr, p + UART_LCR);
return 0;
}
--
2.43.0.rc1.1336.g36b5255a03ac
We may reuse ->setup() for the same purpose as it was done before.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_exar.c | 171 +++++++++++++---------------
1 file changed, 81 insertions(+), 90 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 956323e2de4a..4e683ca76de0 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -237,14 +237,12 @@ struct exar8250_platform {
* struct exar8250_board - board information
* @num_ports: number of serial ports
* @reg_shift: describes UART register mapping in PCI memory
- * @board_init: quirk run once at ->probe() stage before setting up ports
* @setup: quirk run at ->probe() stage for each port
* @exit: quirk run at ->remove() stage
*/
struct exar8250_board {
unsigned int num_ports;
unsigned int reg_shift;
- int (*board_init)(struct exar8250 *priv, struct pci_dev *pcidev);
int (*setup)(struct exar8250 *priv, struct pci_dev *pcidev,
struct uart_8250_port *port, int idx);
void (*exit)(struct pci_dev *pcidev);
@@ -907,9 +905,6 @@ static int cti_port_setup_common(struct exar8250 *priv,
{
int ret;
- if (priv->osc_freq == 0)
- return -EINVAL;
-
port->port.port_id = idx;
port->port.uartclk = priv->osc_freq;
@@ -927,6 +922,30 @@ static int cti_port_setup_common(struct exar8250 *priv,
return 0;
}
+static int cti_board_init_fpga(struct exar8250 *priv, struct pci_dev *pcidev)
+{
+ int ret;
+ u16 cfg_val;
+
+ // FPGA OSC is fixed to the 33MHz PCI clock
+ priv->osc_freq = CTI_DEFAULT_FPGA_OSC_FREQ;
+
+ // Enable external interrupts in special cfg space register
+ ret = pci_read_config_word(pcidev, CTI_FPGA_CFG_INT_EN_REG, &cfg_val);
+ if (ret)
+ return pcibios_err_to_errno(ret);
+
+ cfg_val |= CTI_FPGA_CFG_INT_EN_EXT_BIT;
+ ret = pci_write_config_word(pcidev, CTI_FPGA_CFG_INT_EN_REG, cfg_val);
+ if (ret)
+ return pcibios_err_to_errno(ret);
+
+ // RS485 gate needs to be enabled; otherwise RTS/CTS will not work
+ exar_write_reg(priv, CTI_FPGA_RS485_IO_REG, 0x01);
+
+ return 0;
+}
+
static int cti_port_setup_fpga(struct exar8250 *priv,
struct pci_dev *pcidev,
struct uart_8250_port *port,
@@ -934,6 +953,13 @@ static int cti_port_setup_fpga(struct exar8250 *priv,
{
enum cti_port_type port_type;
unsigned int offset;
+ int ret;
+
+ if (idx == 0) {
+ ret = cti_board_init_fpga(priv, pcidev);
+ if (ret)
+ return ret;
+ }
port_type = cti_get_port_type_fpga(priv, pcidev, idx);
@@ -953,6 +979,12 @@ static int cti_port_setup_fpga(struct exar8250 *priv,
return cti_port_setup_common(priv, pcidev, idx, offset, port);
}
+static void cti_board_init_xr17v35x(struct exar8250 *priv, struct pci_dev *pcidev)
+{
+ // XR17V35X uses the PCIe clock rather than an oscillator
+ priv->osc_freq = CTI_DEFAULT_PCIE_OSC_FREQ;
+}
+
static int cti_port_setup_xr17v35x(struct exar8250 *priv,
struct pci_dev *pcidev,
struct uart_8250_port *port,
@@ -962,6 +994,9 @@ static int cti_port_setup_xr17v35x(struct exar8250 *priv,
unsigned int offset;
int ret;
+ if (idx == 0)
+ cti_board_init_xr17v35x(priv, pcidev);
+
port_type = cti_get_port_type_xr17v35x(priv, pcidev, idx);
offset = idx * UART_EXAR_XR17V35X_PORT_OFFSET;
@@ -999,6 +1034,22 @@ static int cti_port_setup_xr17v35x(struct exar8250 *priv,
return 0;
}
+static void cti_board_init_xr17v25x(struct exar8250 *priv, struct pci_dev *pcidev)
+{
+ cti_board_init_osc_freq(priv, pcidev, CTI_EE_OFF_XR17V25X_OSC_FREQ);
+
+ /* enable interrupts on cards that need the "PLX fix" */
+ switch (pcidev->subsystem_device) {
+ case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS:
+ case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_A:
+ case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_B:
+ cti_plx_int_enable(priv);
+ break;
+ default:
+ break;
+ }
+}
+
static int cti_port_setup_xr17v25x(struct exar8250 *priv,
struct pci_dev *pcidev,
struct uart_8250_port *port,
@@ -1008,6 +1059,9 @@ static int cti_port_setup_xr17v25x(struct exar8250 *priv,
unsigned int offset;
int ret;
+ if (idx == 0)
+ cti_board_init_xr17v25x(priv, pcidev);
+
port_type = cti_get_port_type_xr17c15x_xr17v25x(priv, pcidev, idx);
offset = idx * UART_EXAR_XR17V25X_PORT_OFFSET;
@@ -1055,6 +1109,25 @@ static int cti_port_setup_xr17v25x(struct exar8250 *priv,
return 0;
}
+static void cti_board_init_xr17c15x(struct exar8250 *priv, struct pci_dev *pcidev)
+{
+ cti_board_init_osc_freq(priv, pcidev, CTI_EE_OFF_XR17C15X_OSC_FREQ);
+
+ /* enable interrupts on cards that need the "PLX fix" */
+ switch (pcidev->subsystem_device) {
+ case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS:
+ case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_A:
+ case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_B:
+ case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS_OPTO:
+ case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_A:
+ case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_B:
+ cti_plx_int_enable(priv);
+ break;
+ default:
+ break;
+ }
+}
+
static int cti_port_setup_xr17c15x(struct exar8250 *priv,
struct pci_dev *pcidev,
struct uart_8250_port *port,
@@ -1063,6 +1136,9 @@ static int cti_port_setup_xr17c15x(struct exar8250 *priv,
enum cti_port_type port_type;
unsigned int offset;
+ if (idx == 0)
+ cti_board_init_xr17c15x(priv, pcidev);
+
port_type = cti_get_port_type_xr17c15x_xr17v25x(priv, pcidev, idx);
offset = idx * UART_EXAR_XR17C15X_PORT_OFFSET;
@@ -1096,78 +1172,6 @@ static int cti_port_setup_xr17c15x(struct exar8250 *priv,
return cti_port_setup_common(priv, pcidev, idx, offset, port);
}
-static int cti_board_init_xr17v35x(struct exar8250 *priv,
- struct pci_dev *pcidev)
-{
- // XR17V35X uses the PCIe clock rather than an oscillator
- priv->osc_freq = CTI_DEFAULT_PCIE_OSC_FREQ;
-
- return 0;
-}
-
-static int cti_board_init_xr17v25x(struct exar8250 *priv, struct pci_dev *pcidev)
-{
- cti_board_init_osc_freq(priv, pcidev, CTI_EE_OFF_XR17V25X_OSC_FREQ);
-
- /* enable interrupts on cards that need the "PLX fix" */
- switch (pcidev->subsystem_device) {
- case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS:
- case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_A:
- case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_B:
- cti_plx_int_enable(priv);
- break;
- default:
- break;
- }
-
- return 0;
-}
-
-static int cti_board_init_xr17c15x(struct exar8250 *priv, struct pci_dev *pcidev)
-{
- cti_board_init_osc_freq(priv, pcidev, CTI_EE_OFF_XR17C15X_OSC_FREQ);
-
- /* enable interrupts on cards that need the "PLX fix" */
- switch (pcidev->subsystem_device) {
- case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS:
- case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_A:
- case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_B:
- case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS_OPTO:
- case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_A:
- case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_B:
- cti_plx_int_enable(priv);
- break;
- default:
- break;
- }
-
- return 0;
-}
-
-static int cti_board_init_fpga(struct exar8250 *priv, struct pci_dev *pcidev)
-{
- int ret;
- u16 cfg_val;
-
- // FPGA OSC is fixed to the 33MHz PCI clock
- priv->osc_freq = CTI_DEFAULT_FPGA_OSC_FREQ;
-
- // Enable external interrupts in special cfg space register
- ret = pci_read_config_word(pcidev, CTI_FPGA_CFG_INT_EN_REG, &cfg_val);
- if (ret)
- return pcibios_err_to_errno(ret);
-
- cfg_val |= CTI_FPGA_CFG_INT_EN_EXT_BIT;
- ret = pci_write_config_word(pcidev, CTI_FPGA_CFG_INT_EN_REG, cfg_val);
- if (ret)
- return pcibios_err_to_errno(ret);
-
- // RS485 gate needs to be enabled; otherwise RTS/CTS will not work
- exar_write_reg(priv, CTI_FPGA_RS485_IO_REG, 0x01);
-
- return 0;
-}
-
static int
pci_xr17c154_setup(struct exar8250 *priv, struct pci_dev *pcidev,
struct uart_8250_port *port, int idx)
@@ -1574,15 +1578,6 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
if (rc)
return rc;
- if (board->board_init) {
- rc = board->board_init(priv, pcidev);
- if (rc) {
- dev_err_probe(&pcidev->dev, rc,
- "failed to init serial board\n");
- return rc;
- }
- }
-
for (i = 0; i < nr_ports && i < maxnr; i++) {
rc = board->setup(priv, pcidev, &uart, i);
if (rc) {
@@ -1664,22 +1659,18 @@ static const struct exar8250_board pbn_fastcom335_8 = {
};
static const struct exar8250_board pbn_cti_xr17c15x = {
- .board_init = cti_board_init_xr17c15x,
.setup = cti_port_setup_xr17c15x,
};
static const struct exar8250_board pbn_cti_xr17v25x = {
- .board_init = cti_board_init_xr17v25x,
.setup = cti_port_setup_xr17v25x,
};
static const struct exar8250_board pbn_cti_xr17v35x = {
- .board_init = cti_board_init_xr17v35x,
.setup = cti_port_setup_xr17v35x,
};
static const struct exar8250_board pbn_cti_fpga = {
- .board_init = cti_board_init_fpga,
.setup = cti_port_setup_fpga,
};
--
2.43.0.rc1.1336.g36b5255a03ac
Use BIT() in exar_ee_read() like other functions do.
While at it, explain the EEPROM type and the dummy bit requirement.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_exar.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 306bc6d7c141..93d2c8dd3cd8 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -324,6 +324,7 @@ static inline u8 exar_ee_read_bit(struct exar8250 *priv)
* @ee_addr: Offset of EEPROM to read word from
*
* Read a single 16bit word from an Exar UART's EEPROM.
+ * The type of the EEPROM is AT93C46D.
*
* Return: EEPROM word
*/
@@ -340,13 +341,13 @@ static u16 exar_ee_read(struct exar8250 *priv, u8 ee_addr)
exar_ee_write_bit(priv, 0);
// Send address to read from
- for (i = 1 << (UART_EXAR_REGB_EE_ADDR_SIZE - 1); i; i >>= 1)
- exar_ee_write_bit(priv, (ee_addr & i));
+ for (i = UART_EXAR_REGB_EE_ADDR_SIZE - 1; i >= 0; i--)
+ exar_ee_write_bit(priv, ee_addr & BIT(i));
- // Read data 1 bit at a time
- for (i = 0; i <= UART_EXAR_REGB_EE_DATA_SIZE; i++) {
- data <<= 1;
- data |= exar_ee_read_bit(priv);
+ // Read data 1 bit at a time starting with a dummy bit
+ for (i = UART_EXAR_REGB_EE_DATA_SIZE; i >= 0; i--) {
+ if (exar_ee_read_bit(priv))
+ data |= BIT(i);
}
exar_ee_deselect(priv);
--
2.43.0.rc1.1336.g36b5255a03ac
Keep the includes sorted.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_exar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 85085b73706c..616128254bbd 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -6,6 +6,7 @@
*
* Copyright (C) 2017 Sudip Mukherjee, All Rights Reserved.
*/
+#include <linux/bitfield.h>
#include <linux/bits.h>
#include <linux/delay.h>
#include <linux/device.h>
@@ -20,7 +21,6 @@
#include <linux/property.h>
#include <linux/string.h>
#include <linux/types.h>
-#include <linux/bitfield.h>
#include <linux/serial_8250.h>
#include <linux/serial_core.h>
--
2.43.0.rc1.1336.g36b5255a03ac
Make type of bit parameter and returned one the same in exar_ee_*_bit().
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_exar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 93d2c8dd3cd8..85085b73706c 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -280,7 +280,7 @@ static inline void exar_ee_deselect(struct exar8250 *priv)
exar_write_reg(priv, UART_EXAR_REGB, 0x00);
}
-static inline void exar_ee_write_bit(struct exar8250 *priv, int bit)
+static inline void exar_ee_write_bit(struct exar8250 *priv, u8 bit)
{
u8 value = UART_EXAR_REGB_EECS;
--
2.43.0.rc1.1336.g36b5255a03ac
On Fri, 3 May 2024 20:15:52 +0300
Andy Shevchenko <[email protected]> wrote:
> After a rework for CONNTECH was done, the driver may need a bit of
> love in order to become less verbose (in terms of indentation and
> code duplication) and hence easier to read.
>
> This clean up series fixes a couple of (not so critical) issues and
> cleans up the recently added code. No functional change indented by
> the cleaning up part.
>
> Parker, please test this and give your formal Tested-by tag
> (you may do it by replying to this message if all patches are
> successfully tested; more details about tags are available in
> the Submitting Patches documentation).
>
I was able to test the Connect Tech related code and everything is
work as expected. I can't test the non-CTI related changes but they
are pretty minor.
Tested-by: Parker Newman <[email protected]>
> In v2:
> - fixed the EEPROM reading data loop (Ilpo, Parker)
>
> Andy Shevchenko (13):
> serial: 8250_exar: Don't return positive values as error codes
> serial: 8250_exar: Describe all parameters in kernel doc
> serial: 8250_exar: Kill CTI_PCI_DEVICE()
> serial: 8250_exar: Use PCI_SUBVENDOR_ID_IBM for subvendor ID
> serial: 8250_exar: Trivia typo fixes
> serial: 8250_exar: Extract cti_board_init_osc_freq() helper
> serial: 8250_exar: Kill unneeded ->board_init()
> serial: 8250_exar: Decrease indentation level
> serial: 8250_exar: Return directly from switch-cases
> serial: 8250_exar: Switch to use dev_err_probe()
> serial: 8250_exar: Use BIT() in exar_ee_read()
> serial: 8250_exar: Make type of bit the same in exar_ee_*_bit()
> serial: 8250_exar: Keep the includes sorted
>
> drivers/tty/serial/8250/8250_exar.c | 459 ++++++++++++----------------
> 1 file changed, 203 insertions(+), 256 deletions(-)
>