Re: [PATCH] tty: add Moxa Smartio MUE serial driver
On Tue, Feb 9, 2016 at 2:10 PM, Mathieu OTHACEHE wrote: >> I'm sorry, but it looks like 8250 based driver if I'm not mistaken. In >> which case why not to use 8250_core.c / 8250_port.c and entire 8250/ >> infrastructure? > > Well, the vendor is providing two drivers for his serial pci cards : mxser > and mxupcie. > The mxser driver has been cleaned up and integrated to mainline a long time > ago (drivers/tty/mxser.c). > I'm trying to do the same thing with the mxupcie driver. > > mxser and mxupcie are quite similar, maybe both should use 8250 > infrastructure and not > have independant drivers ? Might be. Though for new driver it would be much easier to do. After that someone might move the existing driver. -- With Best Regards, Andy Shevchenko
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
> I'm sorry, but it looks like 8250 based driver if I'm not mistaken. In > which case why not to use 8250_core.c / 8250_port.c and entire 8250/ > infrastructure? Well, the vendor is providing two drivers for his serial pci cards : mxser and mxupcie. The mxser driver has been cleaned up and integrated to mainline a long time ago (drivers/tty/mxser.c). I'm trying to do the same thing with the mxupcie driver. mxser and mxupcie are quite similar, maybe both should use 8250 infrastructure and not have independant drivers ? Mathieu
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
> I'm sorry, but it looks like 8250 based driver if I'm not mistaken. In > which case why not to use 8250_core.c / 8250_port.c and entire 8250/ > infrastructure? Well, the vendor is providing two drivers for his serial pci cards : mxser and mxupcie. The mxser driver has been cleaned up and integrated to mainline a long time ago (drivers/tty/mxser.c). I'm trying to do the same thing with the mxupcie driver. mxser and mxupcie are quite similar, maybe both should use 8250 infrastructure and not have independant drivers ? Mathieu
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
On Tue, Feb 9, 2016 at 2:10 PM, Mathieu OTHACEHEwrote: >> I'm sorry, but it looks like 8250 based driver if I'm not mistaken. In >> which case why not to use 8250_core.c / 8250_port.c and entire 8250/ >> infrastructure? > > Well, the vendor is providing two drivers for his serial pci cards : mxser > and mxupcie. > The mxser driver has been cleaned up and integrated to mainline a long time > ago (drivers/tty/mxser.c). > I'm trying to do the same thing with the mxupcie driver. > > mxser and mxupcie are quite similar, maybe both should use 8250 > infrastructure and not > have independant drivers ? Might be. Though for new driver it would be much easier to do. After that someone might move the existing driver. -- With Best Regards, Andy Shevchenko
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
On Mon, Feb 1, 2016 at 10:34 PM, Mathieu OTHACEHE wrote: > Add a driver which supports: > > - 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 > > This driver is based on 1.16.7 GPL MOXA driver written by Eric Lo > and available on MOXA website. The original driver was based on > Linux serial driver. > > Signed-off-by: Mathieu OTHACEHE > --- > > Hi, > > Here is a new driver for MOXA Smartio MUE cards. It is based > on the vendor driver available on MOXA website and on the > mainline mxser driver. > > I was able to test it on a CP-168EL-A card on PC. Some of the > cards (118E-A, 138E-A, 134EL-A, 116E-A-A et 116E-A-B) have > a CPLD module programmable via GPIO. > For now, I dropped all the code related to CPLD/GPIO because I > can't test it on my card. > > Mathieu I'm sorry, but it looks like 8250 based driver if I'm not mistaken. In which case why not to use 8250_core.c / 8250_port.c and entire 8250/ infrastructure? -- With Best Regards, Andy Shevchenko
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
On Mon, Feb 1, 2016 at 10:34 PM, Mathieu OTHACEHEwrote: > Add a driver which supports: > > - 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 > > This driver is based on 1.16.7 GPL MOXA driver written by Eric Lo > and available on MOXA website. The original driver was based on > Linux serial driver. > > Signed-off-by: Mathieu OTHACEHE > --- > > Hi, > > Here is a new driver for MOXA Smartio MUE cards. It is based > on the vendor driver available on MOXA website and on the > mainline mxser driver. > > I was able to test it on a CP-168EL-A card on PC. Some of the > cards (118E-A, 138E-A, 134EL-A, 116E-A-A et 116E-A-B) have > a CPLD module programmable via GPIO. > For now, I dropped all the code related to CPLD/GPIO because I > can't test it on my card. > > Mathieu I'm sorry, but it looks like 8250 based driver if I'm not mistaken. In which case why not to use 8250_core.c / 8250_port.c and entire 8250/ infrastructure? -- With Best Regards, Andy Shevchenko
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
Hi Mathieu, [auto build test ERROR on tty/tty-testing] [also build test ERROR on v4.5-rc2 next-20160205] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Mathieu-OTHACEHE/tty-add-Moxa-Smartio-MUE-serial-driver/20160202-043752 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing config: ia64-allmodconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All errors (new ones prefixed by >>): drivers/tty/mxupcie.c: In function 'mxupcie_get_serial_info': >> drivers/tty/mxupcie.c:803:2: error: implicit declaration of function >> 'copy_to_user' [-Werror=implicit-function-declaration] if (copy_to_user(retinfo, , sizeof(*retinfo))) ^ drivers/tty/mxupcie.c: In function 'mxupcie_set_serial_info': >> drivers/tty/mxupcie.c:822:2: error: implicit declaration of function >> 'copy_from_user' [-Werror=implicit-function-declaration] if (copy_from_user(_serial, new_info, sizeof(new_serial))) ^ drivers/tty/mxupcie.c: In function 'mxupcie_get_lsr_info': >> drivers/tty/mxupcie.c:897:2: error: implicit declaration of function >> 'put_user' [-Werror=implicit-function-declaration] put_user(result, value); ^ cc1: some warnings being treated as errors vim +/copy_to_user +803 drivers/tty/mxupcie.c 797 tmp.baud_base = info->baud_base; 798 tmp.close_delay = info->close_delay; 799 tmp.closing_wait = info->closing_wait; 800 tmp.custom_divisor = info->custom_divisor; 801 tmp.hub6 = 0; 802 > 803 if (copy_to_user(retinfo, , sizeof(*retinfo))) 804 return -EFAULT; 805 806 return 0; 807 } 808 809 static int mxupcie_set_serial_info(struct tty_struct *tty, 810 struct serial_struct *new_info) 811 { 812 struct mxupcie_port *info = tty->driver_data; 813 struct tty_port *port = >port; 814 struct serial_struct new_serial; 815 unsigned int flags; 816 int retval = 0; 817 unsigned long sp_flags; 818 819 if (!new_info) 820 return -EFAULT; 821 > 822 if (copy_from_user(_serial, new_info, sizeof(new_serial))) 823 return -EFAULT; 824 825 if ((new_serial.irq != info->board->irq) || 826 (new_serial.port != *info->ioaddr)) 827 return -EINVAL; 828 829 flags = port->flags & ASYNC_SPD_MASK; 830 831 if (!capable(CAP_SYS_ADMIN)) { 832 if ((new_serial.baud_base != info->baud_base) || 833 (new_serial.close_delay != info->close_delay) || 834 ((new_serial.flags & ~ASYNC_USR_MASK) != 835 (port->flags & ~ASYNC_USR_MASK))) 836 return -EPERM; 837 838 port->flags = ((port->flags & ~ASYNC_USR_MASK) | 839 (new_serial.flags & ASYNC_USR_MASK)); 840 } else { 841 /* 842 * OK, past this point, all the error checking has been done. 843 * At this point, we start making changes. 844 */ 845 port->flags = ((port->flags & ~ASYNC_FLAGS) | 846 (new_serial.flags & ASYNC_FLAGS)); 847 info->close_delay = new_serial.close_delay * HZ / 100; 848 info->closing_wait = new_serial.closing_wait * HZ / 100; 849 850 if ((new_serial.baud_base != info->baud_base) || 851 (new_serial.custom_divisor != info->custom_divisor)) { 852 853 if (new_serial.custom_divisor == 0) 854 return -EINVAL; 855 856 info->custom_baud_rate = new_serial.baud_base / 857 new_serial.custom_divisor; 858 } 859 } 860 861 if (test_bit(ASYNCB_INITIALIZED, >flags)) { 862 if (flags != (port->flags & ASYNC_SPD_MASK)) { 863 spin_lock_irqsave(>slock, sp_flags); 864 mxupcie_change_speed(tty, NULL); 865 spin_unlock_irqrestore(>slock, sp_flags); 866 } 867 } else { 868 retval = mxupcie_activate(port, tty); 869 if (retval == 0) 870
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
Hi Mathieu, [auto build test ERROR on tty/tty-testing] [also build test ERROR on v4.5-rc2 next-20160205] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Mathieu-OTHACEHE/tty-add-Moxa-Smartio-MUE-serial-driver/20160202-043752 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing config: ia64-allmodconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All errors (new ones prefixed by >>): drivers/tty/mxupcie.c: In function 'mxupcie_get_serial_info': >> drivers/tty/mxupcie.c:803:2: error: implicit declaration of function >> 'copy_to_user' [-Werror=implicit-function-declaration] if (copy_to_user(retinfo, , sizeof(*retinfo))) ^ drivers/tty/mxupcie.c: In function 'mxupcie_set_serial_info': >> drivers/tty/mxupcie.c:822:2: error: implicit declaration of function >> 'copy_from_user' [-Werror=implicit-function-declaration] if (copy_from_user(_serial, new_info, sizeof(new_serial))) ^ drivers/tty/mxupcie.c: In function 'mxupcie_get_lsr_info': >> drivers/tty/mxupcie.c:897:2: error: implicit declaration of function >> 'put_user' [-Werror=implicit-function-declaration] put_user(result, value); ^ cc1: some warnings being treated as errors vim +/copy_to_user +803 drivers/tty/mxupcie.c 797 tmp.baud_base = info->baud_base; 798 tmp.close_delay = info->close_delay; 799 tmp.closing_wait = info->closing_wait; 800 tmp.custom_divisor = info->custom_divisor; 801 tmp.hub6 = 0; 802 > 803 if (copy_to_user(retinfo, , sizeof(*retinfo))) 804 return -EFAULT; 805 806 return 0; 807 } 808 809 static int mxupcie_set_serial_info(struct tty_struct *tty, 810 struct serial_struct *new_info) 811 { 812 struct mxupcie_port *info = tty->driver_data; 813 struct tty_port *port = >port; 814 struct serial_struct new_serial; 815 unsigned int flags; 816 int retval = 0; 817 unsigned long sp_flags; 818 819 if (!new_info) 820 return -EFAULT; 821 > 822 if (copy_from_user(_serial, new_info, sizeof(new_serial))) 823 return -EFAULT; 824 825 if ((new_serial.irq != info->board->irq) || 826 (new_serial.port != *info->ioaddr)) 827 return -EINVAL; 828 829 flags = port->flags & ASYNC_SPD_MASK; 830 831 if (!capable(CAP_SYS_ADMIN)) { 832 if ((new_serial.baud_base != info->baud_base) || 833 (new_serial.close_delay != info->close_delay) || 834 ((new_serial.flags & ~ASYNC_USR_MASK) != 835 (port->flags & ~ASYNC_USR_MASK))) 836 return -EPERM; 837 838 port->flags = ((port->flags & ~ASYNC_USR_MASK) | 839 (new_serial.flags & ASYNC_USR_MASK)); 840 } else { 841 /* 842 * OK, past this point, all the error checking has been done. 843 * At this point, we start making changes. 844 */ 845 port->flags = ((port->flags & ~ASYNC_FLAGS) | 846 (new_serial.flags & ASYNC_FLAGS)); 847 info->close_delay = new_serial.close_delay * HZ / 100; 848 info->closing_wait = new_serial.closing_wait * HZ / 100; 849 850 if ((new_serial.baud_base != info->baud_base) || 851 (new_serial.custom_divisor != info->custom_divisor)) { 852 853 if (new_serial.custom_divisor == 0) 854 return -EINVAL; 855 856 info->custom_baud_rate = new_serial.baud_base / 857 new_serial.custom_divisor; 858 } 859 } 860 861 if (test_bit(ASYNCB_INITIALIZED, >flags)) { 862 if (flags != (port->flags & ASYNC_SPD_MASK)) { 863 spin_lock_irqsave(>slock, sp_flags); 864 mxupcie_change_speed(tty, NULL); 865 spin_unlock_irqrestore(>slock, sp_flags); 866 } 867 } else { 868 retval = mxupcie_activate(port, tty); 869 if (retval == 0) 870
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
Thank you for your comments. I'll come up with v2 soon but, I have a question about this point : > > + /* clear Rx/Tx FIFO's */ > > + for (i = 0; i < reset_cnt; i++) { > > + iowrite8((UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT), > > +info->ioaddr + UART_FCR); > > + msleep(sleep_interval); > > No can do - you have a spinlock held while you are tring to sleep. I'm > not btw clear that you actually need the lock. The tty_port layer ensures > activate/shutdown don't cross or get duplicated. The only protection you > might need is versus interrupts, and in that case you could free the IRQ > up and claim it in activate/shutdown. So is it possible to replace spin_lock_irqsave/restore by local_irq_save/restore in activate/shutdown to protect versus interrupts ? And is it allowed to call msleep while holding local_irq_save ? Thank you, Mathieu
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
Thank you for your comments. I'll come up with v2 soon but, I have a question about this point : > > + /* clear Rx/Tx FIFO's */ > > + for (i = 0; i < reset_cnt; i++) { > > + iowrite8((UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT), > > +info->ioaddr + UART_FCR); > > + msleep(sleep_interval); > > No can do - you have a spinlock held while you are tring to sleep. I'm > not btw clear that you actually need the lock. The tty_port layer ensures > activate/shutdown don't cross or get duplicated. The only protection you > might need is versus interrupts, and in that case you could free the IRQ > up and claim it in activate/shutdown. So is it possible to replace spin_lock_irqsave/restore by local_irq_save/restore in activate/shutdown to protect versus interrupts ? And is it allowed to call msleep while holding local_irq_save ? Thank you, Mathieu
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
> Here is a new driver for MOXA Smartio MUE cards. It is based > on the vendor driver available on MOXA website and on the > mainline mxser driver. > > I was able to test it on a CP-168EL-A card on PC. Some of the > cards (118E-A, 138E-A, 134EL-A, 116E-A-A et 116E-A-B) have > a CPLD module programmable via GPIO. > For now, I dropped all the code related to CPLD/GPIO because I > can't test it on my card. Thanks Some comments (and I realise these are probably not bugs you added!) > +static int mxupcie_set_baud(struct tty_struct *tty, long newspd) > +{ > + int quot = 0; > + unsigned char cval; > + int custom = 0; > + struct mxupcie_port *info = tty->driver_data; > + > + if (newspd > info->max_baud) > + return 0; This doesn't look quite the behavioru you want. > + > + if (newspd == 38400 && > + (info->port.flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST) { > + info->speed = info->custom_baud_rate; > + custom = 1; > + > + quot = info->baud_base / info->speed; > + if (info->speed <= 0 || info->speed > info->max_baud) > + quot = 0; > + else > + set_linear_baud(info->ioaddr, info->speed); The custom speed stuff can go really - it's handled by termios directly these days. > > +static int mxupcie_change_speed(struct tty_struct *tty, > + struct ktermios *old_termios) > +{ Be careful with this - if CMSPAR is set the these indicate mark or space (and if you don't support that then clear the CMSPAR bit in tty->termios) Likewise you should write the actual baud rate into the tty->termios using the helper functions (see how 8250.c does it - it's a nice example) > +static void mxupcie_shutdown(struct tty_port *port) > +{ > + struct mxupcie_port *info; > + unsigned char reg_flag; > + int i; > + unsigned long sp_flags; > + > + info = container_of(port, struct mxupcie_port, port); > + > + spin_lock_irqsave(>slock, sp_flags); > + > + wake_up_interruptible(>port.delta_msr_wait); > + > + if (info->port.xmit_buf) { > + free_page((unsigned long)info->port.xmit_buf); > + info->port.xmit_buf = NULL; > + } > + > + reg_flag = 0; > + iowrite8(reg_flag, info->ioaddr + MOXA_PUART_EFR); > + iowrite8(reg_flag, info->ioaddr + MOXA_PUART_SFR); > + > + info->ier = 0; > + iowrite8(0x00, info->ioaddr + UART_IER); > + > + if (info->speed < 9600) { > + int sleep_interval = 0; > + int reset_cnt = 0; > + > + if (info->speed <= 600) { > + sleep_interval = 10; > + reset_cnt = MX_FIFO_RESET_CNT; > + } else { > + sleep_interval = 1; > + reset_cnt = MX_FIFO_RESET_CNT / 10; > + } > + > + /* Workaround for clear FIFO in low baudrate */ > + iowrite8(0x0f, info->ioaddr + MOXA_PUART_ADJ_CLK); > + iowrite8(0x03, info->ioaddr + MOXA_PUART_ADJ_ENABLE); > + > + /* clear Rx/Tx FIFO's */ > + for (i = 0; i < reset_cnt; i++) { > + iowrite8((UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT), > + info->ioaddr + UART_FCR); > + msleep(sleep_interval); No can do - you have a spinlock held while you are tring to sleep. I'm not btw clear that you actually need the lock. The tty_port layer ensures activate/shutdown don't cross or get duplicated. The only protection you might need is versus interrupts, and in that case you could free the IRQ up and claim it in activate/shutdown. > +static int mxupcie_open(struct tty_struct *tty, struct file *filp) > +{ > + struct mxupcie_port *info; > + struct mxupcie_board *board; > + int line; > + > + line = tty->index; > + if (line == MXUPCIE_PORTS) > + return 0; What is this for ? > + > + if ((line < 0) || (line > MXUPCIE_PORTS)) > + return -ENODEV; How can this happen ? > + /* > + * Before we drop DTR, make sure the UART transmitter > + * has completely drained; this is especially > + * important if there is a transmit FIFO! > + */ > + timeout = jiffies + HZ; > + while (!(ioread8(info->ioaddr + UART_LSR) & UART_LSR_TEMT)) { > + schedule_timeout_interruptible(5); Always 5 - doesn't it depend on HZ ? > + if (time_after(jiffies, timeout)) > + break; > + } > +} > + > +static void mxupcie_close(struct tty_struct *tty, struct file *filp) > +{ > + struct mxupcie_port *info = tty->driver_data; > + struct tty_port *port = >port; > + > + if (tty->index == MXUPCIE_PORTS || info == NULL) > + return; Can info be null, given the tty_port code does it matter ? > + if (tty_port_close_start(port, tty, filp) == 0) > + return; > + > + mutex_lock(>mutex); > +
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
On Mon, Feb 01, 2016 at 09:34:19PM +0100, Mathieu OTHACEHE wrote: > Add a driver which supports: > > - 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 > > This driver is based on 1.16.7 GPL MOXA driver written by Eric Lo > and available on MOXA website. The original driver was based on > Linux serial driver. > > Signed-off-by: Mathieu OTHACEHE > --- > > Hi, > > Here is a new driver for MOXA Smartio MUE cards. It is based > on the vendor driver available on MOXA website and on the > mainline mxser driver. > > I was able to test it on a CP-168EL-A card on PC. Some of the > cards (118E-A, 138E-A, 134EL-A, 116E-A-A et 116E-A-B) have > a CPLD module programmable via GPIO. > For now, I dropped all the code related to CPLD/GPIO because I > can't test it on my card. > > Mathieu > > MAINTAINERS |5 + > drivers/tty/Kconfig | 10 + > drivers/tty/Makefile |1 + > drivers/tty/mxupcie.c | 1874 > + > drivers/tty/mxupcie.h | 129 Minor nit, but why do you need a .h file here when no one else includes it? Why not just put it into the .c file to keep things nice and self-contained? thanks, greg k-h
[PATCH] tty: add Moxa Smartio MUE serial driver
Add a driver which supports: - 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 This driver is based on 1.16.7 GPL MOXA driver written by Eric Lo and available on MOXA website. The original driver was based on Linux serial driver. Signed-off-by: Mathieu OTHACEHE --- Hi, Here is a new driver for MOXA Smartio MUE cards. It is based on the vendor driver available on MOXA website and on the mainline mxser driver. I was able to test it on a CP-168EL-A card on PC. Some of the cards (118E-A, 138E-A, 134EL-A, 116E-A-A et 116E-A-B) have a CPLD module programmable via GPIO. For now, I dropped all the code related to CPLD/GPIO because I can't test it on my card. Mathieu MAINTAINERS |5 + drivers/tty/Kconfig | 10 + drivers/tty/Makefile |1 + drivers/tty/mxupcie.c | 1874 + drivers/tty/mxupcie.h | 129 5 files changed, 2019 insertions(+) create mode 100644 drivers/tty/mxupcie.c create mode 100644 drivers/tty/mxupcie.h diff --git a/MAINTAINERS b/MAINTAINERS index 2b4eb31..3e6c950 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7274,6 +7274,11 @@ S: Maintained F: Documentation/serial/moxa-smartio F: drivers/tty/mxser.* +MOXA SMARTIO/INDUSTIO MUE SERIAL CARD +M: Mathieu Othacehe +S: Maintained +F: drivers/tty/mxupcie.* + MR800 AVERMEDIA USB FM RADIO DRIVER M: Alexey Klimov L: linux-me...@vger.kernel.org diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig index c01f450..8a14bd075 100644 --- a/drivers/tty/Kconfig +++ b/drivers/tty/Kconfig @@ -259,6 +259,16 @@ config MOXA_SMARTIO This driver can also be built as a module. The module will be called mxser. If you want to do that, say M here. +config MOXA_SMARTIO_MUE + tristate "Moxa SmartIO MUE support" + depends on SERIAL_NONSTANDARD && PCI + help + Say Y here if you have a Moxa SmartIO MUE multiport serial card + and/or want to help develop a new version of this driver. + + This driver can also be built as a module. The module will be called + mxupcie. If you want to do that, say M here. + config SYNCLINK tristate "Microgate SyncLink card support" depends on SERIAL_NONSTANDARD && PCI && ISA_DMA_API diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile index 5817e23..0e37ff2 100644 --- a/drivers/tty/Makefile +++ b/drivers/tty/Makefile @@ -21,6 +21,7 @@ obj-$(CONFIG_CYCLADES)+= cyclades.o obj-$(CONFIG_ISI) += isicom.o obj-$(CONFIG_MOXA_INTELLIO)+= moxa.o obj-$(CONFIG_MOXA_SMARTIO) += mxser.o +obj-$(CONFIG_MOXA_SMARTIO_MUE) += mxupcie.o obj-$(CONFIG_NOZOMI) += nozomi.o obj-$(CONFIG_ROCKETPORT) += rocket.o obj-$(CONFIG_SYNCLINK_GT) += synclink_gt.o diff --git a/drivers/tty/mxupcie.c b/drivers/tty/mxupcie.c new file mode 100644 index 000..ffdcb5b --- /dev/null +++ b/drivers/tty/mxupcie.c @@ -0,0 +1,1874 @@ +/* + * mxupcie.c -- MOXA Smartio/Industio MUE multiport serial driver. + * + * Copyright (C) 1999-2015 Moxa Technologies (supp...@moxa.com). + * Copyright (C) 2016 Mathieu OTHACEHE + * + * This code is based on the 1.16.7 moxa driver which is based on + * Linux serial driver. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "mxupcie.h" + +enum { + MXUPCIE_BOARD_CP102E = 0, + MXUPCIE_BOARD_CP102EL, + MXUPCIE_BOARD_CP132EL, + MXUPCIE_BOARD_CP114EL, + MXUPCIE_BOARD_CP104EL_A, + MXUPCIE_BOARD_CP168EL_A, + MXUPCIE_BOARD_CP118EL_A, + MXUPCIE_BOARD_CP118E_A_I, + MXUPCIE_BOARD_CP138E_A, + MXUPCIE_BOARD_CP134EL_A, + MXUPCIE_BOARD_CP116E_A_A, + MXUPCIE_BOARD_CP116E_A_B +}; + +struct mxupcie_card_info { + char *name; + unsigned int nports; + int flags; +}; + +static const struct mxupcie_card_info mxupcie_cards[] = { + {"CP-102E series", 2, MX_FLAG_232}, + {"CP-102EL series", 2, MX_FLAG_232}, + {"CP-132EL series", 2, MX_FLAG_422 | MX_FLAG_485}, + {"CP-114EL
[PATCH] tty: add Moxa Smartio MUE serial driver
Add a driver which supports: - 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 This driver is based on 1.16.7 GPL MOXA driver written by Eric Lo and available on MOXA website. The original driver was based on Linux serial driver. Signed-off-by: Mathieu OTHACEHE--- Hi, Here is a new driver for MOXA Smartio MUE cards. It is based on the vendor driver available on MOXA website and on the mainline mxser driver. I was able to test it on a CP-168EL-A card on PC. Some of the cards (118E-A, 138E-A, 134EL-A, 116E-A-A et 116E-A-B) have a CPLD module programmable via GPIO. For now, I dropped all the code related to CPLD/GPIO because I can't test it on my card. Mathieu MAINTAINERS |5 + drivers/tty/Kconfig | 10 + drivers/tty/Makefile |1 + drivers/tty/mxupcie.c | 1874 + drivers/tty/mxupcie.h | 129 5 files changed, 2019 insertions(+) create mode 100644 drivers/tty/mxupcie.c create mode 100644 drivers/tty/mxupcie.h diff --git a/MAINTAINERS b/MAINTAINERS index 2b4eb31..3e6c950 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7274,6 +7274,11 @@ S: Maintained F: Documentation/serial/moxa-smartio F: drivers/tty/mxser.* +MOXA SMARTIO/INDUSTIO MUE SERIAL CARD +M: Mathieu Othacehe +S: Maintained +F: drivers/tty/mxupcie.* + MR800 AVERMEDIA USB FM RADIO DRIVER M: Alexey Klimov L: linux-me...@vger.kernel.org diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig index c01f450..8a14bd075 100644 --- a/drivers/tty/Kconfig +++ b/drivers/tty/Kconfig @@ -259,6 +259,16 @@ config MOXA_SMARTIO This driver can also be built as a module. The module will be called mxser. If you want to do that, say M here. +config MOXA_SMARTIO_MUE + tristate "Moxa SmartIO MUE support" + depends on SERIAL_NONSTANDARD && PCI + help + Say Y here if you have a Moxa SmartIO MUE multiport serial card + and/or want to help develop a new version of this driver. + + This driver can also be built as a module. The module will be called + mxupcie. If you want to do that, say M here. + config SYNCLINK tristate "Microgate SyncLink card support" depends on SERIAL_NONSTANDARD && PCI && ISA_DMA_API diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile index 5817e23..0e37ff2 100644 --- a/drivers/tty/Makefile +++ b/drivers/tty/Makefile @@ -21,6 +21,7 @@ obj-$(CONFIG_CYCLADES)+= cyclades.o obj-$(CONFIG_ISI) += isicom.o obj-$(CONFIG_MOXA_INTELLIO)+= moxa.o obj-$(CONFIG_MOXA_SMARTIO) += mxser.o +obj-$(CONFIG_MOXA_SMARTIO_MUE) += mxupcie.o obj-$(CONFIG_NOZOMI) += nozomi.o obj-$(CONFIG_ROCKETPORT) += rocket.o obj-$(CONFIG_SYNCLINK_GT) += synclink_gt.o diff --git a/drivers/tty/mxupcie.c b/drivers/tty/mxupcie.c new file mode 100644 index 000..ffdcb5b --- /dev/null +++ b/drivers/tty/mxupcie.c @@ -0,0 +1,1874 @@ +/* + * mxupcie.c -- MOXA Smartio/Industio MUE multiport serial driver. + * + * Copyright (C) 1999-2015 Moxa Technologies (supp...@moxa.com). + * Copyright (C) 2016 Mathieu OTHACEHE + * + * This code is based on the 1.16.7 moxa driver which is based on + * Linux serial driver. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "mxupcie.h" + +enum { + MXUPCIE_BOARD_CP102E = 0, + MXUPCIE_BOARD_CP102EL, + MXUPCIE_BOARD_CP132EL, + MXUPCIE_BOARD_CP114EL, + MXUPCIE_BOARD_CP104EL_A, + MXUPCIE_BOARD_CP168EL_A, + MXUPCIE_BOARD_CP118EL_A, + MXUPCIE_BOARD_CP118E_A_I, + MXUPCIE_BOARD_CP138E_A, + MXUPCIE_BOARD_CP134EL_A, + MXUPCIE_BOARD_CP116E_A_A, + MXUPCIE_BOARD_CP116E_A_B +}; + +struct mxupcie_card_info { + char *name; + unsigned int nports; + int flags; +}; + +static const struct mxupcie_card_info mxupcie_cards[] = { + {"CP-102E series", 2, MX_FLAG_232}, + {"CP-102EL series", 2,
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
On Mon, Feb 01, 2016 at 09:34:19PM +0100, Mathieu OTHACEHE wrote: > Add a driver which supports: > > - 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 > > This driver is based on 1.16.7 GPL MOXA driver written by Eric Lo > and available on MOXA website. The original driver was based on > Linux serial driver. > > Signed-off-by: Mathieu OTHACEHE> --- > > Hi, > > Here is a new driver for MOXA Smartio MUE cards. It is based > on the vendor driver available on MOXA website and on the > mainline mxser driver. > > I was able to test it on a CP-168EL-A card on PC. Some of the > cards (118E-A, 138E-A, 134EL-A, 116E-A-A et 116E-A-B) have > a CPLD module programmable via GPIO. > For now, I dropped all the code related to CPLD/GPIO because I > can't test it on my card. > > Mathieu > > MAINTAINERS |5 + > drivers/tty/Kconfig | 10 + > drivers/tty/Makefile |1 + > drivers/tty/mxupcie.c | 1874 > + > drivers/tty/mxupcie.h | 129 Minor nit, but why do you need a .h file here when no one else includes it? Why not just put it into the .c file to keep things nice and self-contained? thanks, greg k-h
Re: [PATCH] tty: add Moxa Smartio MUE serial driver
> Here is a new driver for MOXA Smartio MUE cards. It is based > on the vendor driver available on MOXA website and on the > mainline mxser driver. > > I was able to test it on a CP-168EL-A card on PC. Some of the > cards (118E-A, 138E-A, 134EL-A, 116E-A-A et 116E-A-B) have > a CPLD module programmable via GPIO. > For now, I dropped all the code related to CPLD/GPIO because I > can't test it on my card. Thanks Some comments (and I realise these are probably not bugs you added!) > +static int mxupcie_set_baud(struct tty_struct *tty, long newspd) > +{ > + int quot = 0; > + unsigned char cval; > + int custom = 0; > + struct mxupcie_port *info = tty->driver_data; > + > + if (newspd > info->max_baud) > + return 0; This doesn't look quite the behavioru you want. > + > + if (newspd == 38400 && > + (info->port.flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST) { > + info->speed = info->custom_baud_rate; > + custom = 1; > + > + quot = info->baud_base / info->speed; > + if (info->speed <= 0 || info->speed > info->max_baud) > + quot = 0; > + else > + set_linear_baud(info->ioaddr, info->speed); The custom speed stuff can go really - it's handled by termios directly these days. > > +static int mxupcie_change_speed(struct tty_struct *tty, > + struct ktermios *old_termios) > +{ Be careful with this - if CMSPAR is set the these indicate mark or space (and if you don't support that then clear the CMSPAR bit in tty->termios) Likewise you should write the actual baud rate into the tty->termios using the helper functions (see how 8250.c does it - it's a nice example) > +static void mxupcie_shutdown(struct tty_port *port) > +{ > + struct mxupcie_port *info; > + unsigned char reg_flag; > + int i; > + unsigned long sp_flags; > + > + info = container_of(port, struct mxupcie_port, port); > + > + spin_lock_irqsave(>slock, sp_flags); > + > + wake_up_interruptible(>port.delta_msr_wait); > + > + if (info->port.xmit_buf) { > + free_page((unsigned long)info->port.xmit_buf); > + info->port.xmit_buf = NULL; > + } > + > + reg_flag = 0; > + iowrite8(reg_flag, info->ioaddr + MOXA_PUART_EFR); > + iowrite8(reg_flag, info->ioaddr + MOXA_PUART_SFR); > + > + info->ier = 0; > + iowrite8(0x00, info->ioaddr + UART_IER); > + > + if (info->speed < 9600) { > + int sleep_interval = 0; > + int reset_cnt = 0; > + > + if (info->speed <= 600) { > + sleep_interval = 10; > + reset_cnt = MX_FIFO_RESET_CNT; > + } else { > + sleep_interval = 1; > + reset_cnt = MX_FIFO_RESET_CNT / 10; > + } > + > + /* Workaround for clear FIFO in low baudrate */ > + iowrite8(0x0f, info->ioaddr + MOXA_PUART_ADJ_CLK); > + iowrite8(0x03, info->ioaddr + MOXA_PUART_ADJ_ENABLE); > + > + /* clear Rx/Tx FIFO's */ > + for (i = 0; i < reset_cnt; i++) { > + iowrite8((UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT), > + info->ioaddr + UART_FCR); > + msleep(sleep_interval); No can do - you have a spinlock held while you are tring to sleep. I'm not btw clear that you actually need the lock. The tty_port layer ensures activate/shutdown don't cross or get duplicated. The only protection you might need is versus interrupts, and in that case you could free the IRQ up and claim it in activate/shutdown. > +static int mxupcie_open(struct tty_struct *tty, struct file *filp) > +{ > + struct mxupcie_port *info; > + struct mxupcie_board *board; > + int line; > + > + line = tty->index; > + if (line == MXUPCIE_PORTS) > + return 0; What is this for ? > + > + if ((line < 0) || (line > MXUPCIE_PORTS)) > + return -ENODEV; How can this happen ? > + /* > + * Before we drop DTR, make sure the UART transmitter > + * has completely drained; this is especially > + * important if there is a transmit FIFO! > + */ > + timeout = jiffies + HZ; > + while (!(ioread8(info->ioaddr + UART_LSR) & UART_LSR_TEMT)) { > + schedule_timeout_interruptible(5); Always 5 - doesn't it depend on HZ ? > + if (time_after(jiffies, timeout)) > + break; > + } > +} > + > +static void mxupcie_close(struct tty_struct *tty, struct file *filp) > +{ > + struct mxupcie_port *info = tty->driver_data; > + struct tty_port *port = >port; > + > + if (tty->index == MXUPCIE_PORTS || info == NULL) > + return; Can info be null, given the tty_port code does it matter ? > + if (tty_port_close_start(port, tty, filp) == 0) > + return; > + > + mutex_lock(>mutex); > +