Re: [PATCH] tty: add Moxa Smartio MUE serial driver

2016-02-09 Thread Andy Shevchenko
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

2016-02-09 Thread Mathieu OTHACEHE
> 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

2016-02-09 Thread Mathieu OTHACEHE
> 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

2016-02-09 Thread Andy Shevchenko
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

2016-02-08 Thread Andy Shevchenko
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

2016-02-08 Thread Andy Shevchenko
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

2016-02-07 Thread kbuild test robot
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

2016-02-07 Thread kbuild test robot
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

2016-02-03 Thread Mathieu OTHACEHE
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

2016-02-03 Thread Mathieu OTHACEHE
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

2016-02-01 Thread One Thousand Gnomes
> 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

2016-02-01 Thread Greg KH
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

2016-02-01 Thread Mathieu OTHACEHE
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

2016-02-01 Thread Mathieu OTHACEHE
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

2016-02-01 Thread Greg KH
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

2016-02-01 Thread One Thousand Gnomes
> 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);
> +