Re: [PATCH] serial: 8250_pci: add MOXA Smartio MUE boards support
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2016-02-15 13:53:40
Also in:
lkml
On Fri, 2016-02-12 at 19:28 +0100, Mathieu OTHACEHE wrote:
Add support for : - CP-102E: 2 ports RS232 PCIE card - CP-102EL: 2 ports RS232 PCIE card - CP-132EL: 2 ports RS422/485 PCIE card - CP-114EL: 4 ports RS232/422/485 PCIE card - CP-104EL-A: 4 ports RS232 PCIE card - CP-168EL-A: 8 ports RS232 PCIE card - CP-118EL-A: 8 ports RS232/422/485 PCIE card - CP-118E-A: 8 ports RS422/485 PCIE card - CP-138E-A: 8 ports RS422/485 PCIE card - CP-134EL-A: 4 ports RS422/485 PCIE card - CP-116E-A (A): 8 ports RS232/422/485 PCIE card - CP-116E-A (B): 8 ports RS232/422/485 PCIE card Signed-off-by: Mathieu OTHACEHE <redacted> --- Hi, This patch add support for MOXA Smartio MUE boards to 8250 driver. I already submitted an independant driver based on the vendor one : https://lkml.org/lkml/2016/2/1/720 But as Andy pointed out, it seems to be a better idea to add this support directly in 8250 driver.
Yes, something like this. It would be even better to have something like 8250_moxa.c at some point. I don't know if it worth to do right now. See my comments below.
quoted hunk ↗ jump to hunk
I was able to test this patch on a CP-168EL-A on PC. Mathieu drivers/tty/serial/8250/8250_pci.c | 193 +++++++++++++++++++++++++++++++++++++ 1 file changed, 193 insertions(+)diff --git a/drivers/tty/serial/8250/8250_pci.cb/drivers/tty/serial/8250/8250_pci.c index 8f8d5c5..67c1028 100644--- a/drivers/tty/serial/8250/8250_pci.c +++ b/drivers/tty/serial/8250/8250_pci.c@@ -1862,6 +1862,25 @@ pci_wch_ch38x_setup(struct serial_private*priv, return pci_default_setup(priv, board, port, idx); } +static int pci_mxupcie_setup(struct serial_private *priv, + const struct pciserial_board *board, + struct uart_8250_port *port, int idx) +{ + unsigned int bar, offset; + + /* + * MOXA Smartio MUE boards with 4 ports have + * a different offset for port #3 + */ + if (idx == 3) { + bar = FL_GET_BASE(board->flags); + offset = 7 * board->uart_offset; + return setup_port(priv, port, bar, offset, board-quoted
reg_shift);+ } else { + return pci_default_setup(priv, board, port, idx); + } +}
So, looks like it would be pretty simple to have it in a separate file.
quoted hunk ↗ jump to hunk
+ #define PCI_VENDOR_ID_SBSMODULARIO 0x124B #define PCI_SUBVENDOR_ID_SBSMODULARIO 0x124B #define PCI_DEVICE_ID_OCTPRO 0x0001@@ -1927,6 +1946,19 @@ pci_wch_ch38x_setup(struct serial_private*priv, #define PCI_DEVICE_ID_PERICOM_PI7C9X7954 0x7954 #define PCI_DEVICE_ID_PERICOM_PI7C9X7958 0x7958 +#define PCI_DEVICE_ID_MOXA_CP102E 0x1024 +#define PCI_DEVICE_ID_MOXA_CP102EL 0x1025 +#define PCI_DEVICE_ID_MOXA_CP132EL 0x1322 +#define PCI_DEVICE_ID_MOXA_CP114EL 0x1144 +#define PCI_DEVICE_ID_MOXA_CP104EL_A 0x1045 +#define PCI_DEVICE_ID_MOXA_CP168EL_A 0x1683 +#define PCI_DEVICE_ID_MOXA_CP118EL_A 0x1182 +#define PCI_DEVICE_ID_MOXA_CP118E_A_I 0x1183 +#define PCI_DEVICE_ID_MOXA_CP138E_A 0x1381 +#define PCI_DEVICE_ID_MOXA_CP134EL_A 0x1342 +#define PCI_DEVICE_ID_MOXA_CP116E_A_A 0x1160 +#define PCI_DEVICE_ID_MOXA_CP116E_A_B 0x1161
But you can keep this sorted by IDs, right?
quoted hunk ↗ jump to hunk
@@ -2938,6 +2994,19 @@ enum pci_board_num_t {pbn_pericom_PI7C9X7952, pbn_pericom_PI7C9X7954, pbn_pericom_PI7C9X7958,
+ pbn_moxa_CP102E, + pbn_moxa_CP102EL, + pbn_moxa_CP132EL, + pbn_moxa_CP114EL, + pbn_moxa_CP104EL_A, + pbn_moxa_CP168EL_A, + pbn_moxa_CP118EL_A, + pbn_moxa_CP118E_A_I, + pbn_moxa_CP138E_A, + pbn_moxa_CP134EL_A, + pbn_moxa_CP116E_A_A, + pbn_moxa_CP116E_A_B
This part seems too verbose. You have similarities in the bunch of boards. +
quoted hunk ↗ jump to hunk
}; /*@@ -3794,6 +3863,81 @@ static struct pciserial_board pci_boards[] = {.base_baud = 921600, .uart_offset = 0x8, }, + /* + * MOXA Smartio MUE boards + */ + [pbn_moxa_CP102E] = { + .flags = FL_BASE1, + .num_ports = 2, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP102EL] = { + .flags = FL_BASE1, + .num_ports = 2, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP132EL] = { + .flags = FL_BASE1, + .num_ports = 2, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP114EL] = { + .flags = FL_BASE1, + .num_ports = 4, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP104EL_A] = { + .flags = FL_BASE1, + .num_ports = 4, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP168EL_A] = { + .flags = FL_BASE1, + .num_ports = 8, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP118EL_A] = { + .flags = FL_BASE1, + .num_ports = 8, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP118E_A_I] = { + .flags = FL_BASE1, + .num_ports = 8, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP138E_A] = { + .flags = FL_BASE1, + .num_ports = 8, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP134EL_A] = { + .flags = FL_BASE1, + .num_ports = 4, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP116E_A_A] = { + .flags = FL_BASE1, + .num_ports = 8, + .base_baud = 921600, + .uart_offset = 0x200, + }, + [pbn_moxa_CP116E_A_B] = { + .flags = FL_BASE1, + .num_ports = 8, + .base_baud = 921600, + .uart_offset = 0x200,
So, I see only 3. Only difference is a number of ports. So, if you go with 8250_moxa.c you may use your private structure to keep only what is needed. -- Andy Shevchenko [off-list ref] Intel Finland Oy