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 <m.othacehe@xxxxxxxxx> > --- > > 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. > 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.c > b/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- > >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. > + > #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? > @@ -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. + > }; > > /* > @@ -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 <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
