Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Peter Hung 
Hi

Peter Hung 於 2015/11/3 上午 11:51 寫道:
> This driver is for Fintek F81532/F81534 USB to Serial Ports IC.
> Changelog:
> v6
>  1. Re-implement the write()/resume() function. Due to this device cant be
> suitable with generic write(), we'll do the submit write URB when
> write()/received tx empty/set_termios()/resume()
>  2. Logic/Phy Port mapping rewrite in f81534_port_probe() &
> f81534_phy_to_logic_port().
>  3. Introduced "Port Hide" function. Some customer use F81532 reference
> design for HW layout, but really use F81534 IC. We'll check
> F81534_PORT_CONF_DISABLE_PORT flag with in uart mode field to do
> port hide with port not used. It can be avoid end-user to use not
> layouted port.
>  4. The 4x3 output-only open-drain pins for F81532/534 is designed for
> control outer devices (with our EVB for examples, the 4 sets of pins
> are designed to control transceiver mode). So we decide to implement
> with gpiolib interface.

I need to do some improvements with senior advices, so I'll send
v7 patch when I modify & re-test the code complete.

Could you give me some roughly advices of this patch when you having
time.

Thanks for your help
-- 
With Best Regards,
Peter Hung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Peter Hung 

Hi,

Oliver Neukum 於 2015/11/4 下午 04:38 寫道:

On Wed, 2015-11-04 at 16:19 +0800, Peter Hung wrote:

Hi

Oliver Neukum 於 2015/11/3 下午 06:03 寫道:

On Tue, 2015-11-03 at 11:51 +0800, Peter Hung wrote:

+   for (i = 0; i < F81534_NUM_PORT; ++i)
+   atomic_set(_priv->port_active[i], 0);


Should be ATOMIC_INIT()



ATOMIC_INIT() seems to be used only for variable initializer, It cant be
used for dynamic allocation. Should I change it to a normal boolean
flag protecting with spin_lock ?


No, if it doesn't work, use the current code.


OK, I'll use current code.


+static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,


Is the point of passing a pointer to msr locking?


+   bool is_port_open)


This function is used only with URB callback function. The *msr is
reported by H/W with newest MSR. The USB-Serial generic system will
re-submit read URB when callback complete. So this function should
run once on the same time.


Yes, so why don't you pass an u8 as opposed to a pointer to an u8?


I'll re-write it from u8* to u8.


+static int f81534_tiocmget(struct tty_struct *tty)
+{
+   struct usb_serial_port *port = tty->driver_data;
+   struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
+   unsigned long flags;
+   int r;
+   u8 msr, mcr;
+
+   /*
+* We'll avoid to direct read MSR register. The IC will read the MSR
+* changed and report it f81534_process_per_serial_block() by
+* F81534_TOKEN_MSR_CHANGE.
+*
+* When this device in heavy loading (e.g., BurnInTest Loopback Test)
+* The report of MSR register will delay received a bit. It's due to
+* MSR interrupt is lowest priority in 16550A. So we decide to sleep
+* a little time to pass the test.
+*/
+   if (schedule_timeout_interruptible(
+   msecs_to_jiffies(F81534_DELAY_READ_MSR))) {
+   dev_info(>dev, "%s: breaked !!\n", __func__);
+   }


Is the delay necessary or isn't it?
If it is necessary you should do something about the signal.



We add this delay due to stress test (Loop-back & 921600bps with
BurnInTest). It'll receive MSR with some delay when connecting with
DTR-DSR & RTS/CTS, but the delay smaller than 10ms. So we decided to
delay some time to pass the test.


OK, but how do you guarantee the delay you need if you get a signal,
which would abort the delay?



Hmm, you are right. It seems to replace *_interruptible to
*_killable and return -EINTR to guarantee not abort by normal
signal.


Thanks for your advices
--
With Best Regards,
Peter Hung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Peter Hung 

Hi

Andy Shevchenko 於 2015/11/3 下午 05:45 寫道:

On Tue, Nov 3, 2015 at 5:51 AM, Peter Hung  wrote:

+ *Please reference https://bitbucket.org/hpeter/fintek-general/src/
+ *with f81534/tools to get set_gpio.c & set_mode.c. Please use it
+ *carefully.


Would it be good to have this under Documentation ?


I had some documents in source code. The link is the demo program to
implement switch UART/PIN funcion. I'll try to add for document with the
repository.


+static void f81534_dtr_rts(struct usb_serial_port *port, int on);
+
+static int f81534_set_port_mode(struct usb_serial_port *port,
+   enum uart_mode eMode);
+
+static int f81534_save_configure_data(struct usb_serial_port *port);
+
+static int f81534_switch_gpio_mode(struct usb_serial_port *port, u8 mode);
+
+static void f81534_wakeup_all_port(struct usb_serial *serial);
+
+static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,
+   bool is_port_open);
+
+static int f81534_prepare_write_buffer(struct usb_serial_port *port,
+   void *dest, size_t size);
+
+static int f81534_submit_writer(struct usb_serial_port *port, gfp_t mem_flags);


Would it be possible to reshuffle code to reduce amount of forward declarations?



I'll try to reduce it.




+   if ((size <= F81534_MAX_DATA_BLOCK) &&
+   (read_size == (count + 1))) {


No need to have internal parens.



In my opinion, It makes more readable. Should I remove it?



+   if (baudrate <= 115200) {
+   value = 0x01;   /* 1.846m fixed */
+   divisor = f81534_calc_baud_divisor(baudrate, 115200, NULL);
+   port_priv->current_baud_base = 115200;
+   } else {
+   for (count = 0; count < ARRAY_SIZE(baudrate_table) ; ++count) {
+   baud_base = baudrate_table[count];
+   divisor = f81534_calc_baud_divisor(baudrate, baud_base,
+   );
+   if (!rem) {
+   dev_dbg(>dev, "%s: found clockbase %d\n",
+   __func__,
+   baudrate_table[count]);
+   value = clock_table[count];
+   port_priv->current_baud_base = baud_base;
+   break;
+   }
+   }


Can you calculate baud rates more precisely? Any link to datasheet
where it's described?



I'll try to comment it within source code.


+static int f81534_ioctl_get_rs485(struct usb_serial_port *port,
+   struct serial_rs485 __user *arg)
+{
+   int status;
+   struct serial_rs485 data;
+   struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
+   struct f81534_serial_private *serial_priv =
+   usb_get_serial_data(port->serial);


One line?



It's over 80 character with a tab, It cant be one line.

I'll fix other issue that you mention.

Thanks for your advices.
--
With Best Regards,
Peter Hung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Oliver Neukum 
On Wed, 2015-11-04 at 16:19 +0800, Peter Hung wrote:
> Hi
> 
> Oliver Neukum 於 2015/11/3 下午 06:03 寫道:
> > On Tue, 2015-11-03 at 11:51 +0800, Peter Hung wrote:
> >> +static int f81534_attach(struct usb_serial *serial)
> >> +{
> >> +  struct f81534_serial_private *serial_priv = NULL;
> >> +  int status;
> >> +  int i;
> >> +  int offset;
> >> +  uintptr_t setting_idx = (uintptr_t) usb_get_serial_data(serial);
> >> +
> >> +  serial_priv = kzalloc(sizeof(*serial_priv), GFP_KERNEL);
> >> +  if (!serial_priv)
> >> +  return -ENOMEM;
> >> +
> >> +  usb_set_serial_data(serial, serial_priv);
> >> +  serial_priv->setting_idx = setting_idx;
> >> +
> >> +  for (i = 0; i < F81534_NUM_PORT; ++i) {
> >> +  /* Disable all interrupt before submit URB */
> >> +  status = f81534_setregister(serial->dev, i,
> >> +  INTERRUPT_ENABLE_REGISTER, 0x00);
> >> +  if (status) {
> >> +  dev_err(>dev->dev, "%s: IER disable failed\n",
> >> +  __func__);
> >> +  goto failed;
> >> +  }
> >> +  }
> >> +
> >> +  for (i = 0; i < F81534_NUM_PORT; ++i)
> >> +  atomic_set(_priv->port_active[i], 0);
> >
> > Should be ATOMIC_INIT()
> >
> 
> ATOMIC_INIT() seems to be used only for variable initializer, It cant be
> used for dynamic allocation. Should I change it to a normal boolean
> flag protecting with spin_lock ?

No, if it doesn't work, use the current code.

> >> +static int f81534_port_remove(struct usb_serial_port *port)
> >> +{
> >> +  struct f81534_port_private *port_priv;
> >> +
> >> +  f81534_release_gpio(port);
> >> +  port_priv = usb_get_serial_port_data(port);
> >> +  kfree(port_priv);
> >> +  return 0;
> >> +}
> >> +
> >> +static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,
> >
> > Is the point of passing a pointer to msr locking?
> >
> >> +  bool is_port_open)
> 
> This function is used only with URB callback function. The *msr is
> reported by H/W with newest MSR. The USB-Serial generic system will
> re-submit read URB when callback complete. So this function should
> run once on the same time.

Yes, so why don't you pass an u8 as opposed to a pointer to an u8?

> >> +static int f81534_tiocmget(struct tty_struct *tty)
> >> +{
> >> +  struct usb_serial_port *port = tty->driver_data;
> >> +  struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
> >> +  unsigned long flags;
> >> +  int r;
> >> +  u8 msr, mcr;
> >> +
> >> +  /*
> >> +   * We'll avoid to direct read MSR register. The IC will read the MSR
> >> +   * changed and report it f81534_process_per_serial_block() by
> >> +   * F81534_TOKEN_MSR_CHANGE.
> >> +   *
> >> +   * When this device in heavy loading (e.g., BurnInTest Loopback Test)
> >> +   * The report of MSR register will delay received a bit. It's due to
> >> +   * MSR interrupt is lowest priority in 16550A. So we decide to sleep
> >> +   * a little time to pass the test.
> >> +   */
> >> +  if (schedule_timeout_interruptible(
> >> +  msecs_to_jiffies(F81534_DELAY_READ_MSR))) {
> >> +  dev_info(>dev, "%s: breaked !!\n", __func__);
> >> +  }
> >
> > Is the delay necessary or isn't it?
> > If it is necessary you should do something about the signal.
> >
> 
> We add this delay due to stress test (Loop-back & 921600bps with
> BurnInTest). It'll receive MSR with some delay when connecting with
> DTR-DSR & RTS/CTS, but the delay smaller than 10ms. So we decided to
> delay some time to pass the test.

OK, but how do you guarantee the delay you need if you get a signal,
which would abort the delay?

Regards
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Peter Hung 

Hi

Oliver Neukum 於 2015/11/3 下午 06:03 寫道:

On Tue, 2015-11-03 at 11:51 +0800, Peter Hung wrote:

+static int f81534_attach(struct usb_serial *serial)
+{
+   struct f81534_serial_private *serial_priv = NULL;
+   int status;
+   int i;
+   int offset;
+   uintptr_t setting_idx = (uintptr_t) usb_get_serial_data(serial);
+
+   serial_priv = kzalloc(sizeof(*serial_priv), GFP_KERNEL);
+   if (!serial_priv)
+   return -ENOMEM;
+
+   usb_set_serial_data(serial, serial_priv);
+   serial_priv->setting_idx = setting_idx;
+
+   for (i = 0; i < F81534_NUM_PORT; ++i) {
+   /* Disable all interrupt before submit URB */
+   status = f81534_setregister(serial->dev, i,
+   INTERRUPT_ENABLE_REGISTER, 0x00);
+   if (status) {
+   dev_err(>dev->dev, "%s: IER disable failed\n",
+   __func__);
+   goto failed;
+   }
+   }
+
+   for (i = 0; i < F81534_NUM_PORT; ++i)
+   atomic_set(_priv->port_active[i], 0);


Should be ATOMIC_INIT()



ATOMIC_INIT() seems to be used only for variable initializer, It cant be
used for dynamic allocation. Should I change it to a normal boolean
flag protecting with spin_lock ?



+static int f81534_port_remove(struct usb_serial_port *port)
+{
+   struct f81534_port_private *port_priv;
+
+   f81534_release_gpio(port);
+   port_priv = usb_get_serial_port_data(port);
+   kfree(port_priv);
+   return 0;
+}
+
+static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,


Is the point of passing a pointer to msr locking?


+   bool is_port_open)


This function is used only with URB callback function. The *msr is
reported by H/W with newest MSR. The USB-Serial generic system will
re-submit read URB when callback complete. So this function should
run once on the same time.


+static int f81534_tiocmget(struct tty_struct *tty)
+{
+   struct usb_serial_port *port = tty->driver_data;
+   struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
+   unsigned long flags;
+   int r;
+   u8 msr, mcr;
+
+   /*
+* We'll avoid to direct read MSR register. The IC will read the MSR
+* changed and report it f81534_process_per_serial_block() by
+* F81534_TOKEN_MSR_CHANGE.
+*
+* When this device in heavy loading (e.g., BurnInTest Loopback Test)
+* The report of MSR register will delay received a bit. It's due to
+* MSR interrupt is lowest priority in 16550A. So we decide to sleep
+* a little time to pass the test.
+*/
+   if (schedule_timeout_interruptible(
+   msecs_to_jiffies(F81534_DELAY_READ_MSR))) {
+   dev_info(>dev, "%s: breaked !!\n", __func__);
+   }


Is the delay necessary or isn't it?
If it is necessary you should do something about the signal.



We add this delay due to stress test (Loop-back & 921600bps with
BurnInTest). It'll receive MSR with some delay when connecting with
DTR-DSR & RTS/CTS, but the delay smaller than 10ms. So we decided to
delay some time to pass the test.


+static int f81534_prepare_write_buffer(struct usb_serial_port *port,
+   void *dest, size_t size)
+{
+   unsigned char *ptr = (unsigned char *) dest;
+   struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
+   int port_num = port_priv->phy;
+   struct usb_serial *serial = port->serial;
+
+   WARN_ON(size != serial->port[0]->bulk_out_size);
+
+   if (size != F81534_WRITE_BUFFER_SIZE) {
+   WARN_ON(size != F81534_WRITE_BUFFER_SIZE);


What is the sense of this?



I'll remove the double-check section with next version patch.



+   ptr[F81534_RECEIVE_BLOCK_SIZE * 0] = 0;
+   ptr[F81534_RECEIVE_BLOCK_SIZE * 1] = 1;
+   ptr[F81534_RECEIVE_BLOCK_SIZE * 2] = 2;
+   ptr[F81534_RECEIVE_BLOCK_SIZE * 3] = 3;


Either these ...


+   ptr[F81534_RECEIVE_BLOCK_SIZE * port_num + 0] = port_num;


.. or that is redundant



I'll remove it too.

Thanks for your advice.
--
With Best Regards,
Peter Hung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Peter Hung 
Hi

Peter Hung 於 2015/11/3 上午 11:51 寫道:
> This driver is for Fintek F81532/F81534 USB to Serial Ports IC.
> Changelog:
> v6
>  1. Re-implement the write()/resume() function. Due to this device cant be
> suitable with generic write(), we'll do the submit write URB when
> write()/received tx empty/set_termios()/resume()
>  2. Logic/Phy Port mapping rewrite in f81534_port_probe() &
> f81534_phy_to_logic_port().
>  3. Introduced "Port Hide" function. Some customer use F81532 reference
> design for HW layout, but really use F81534 IC. We'll check
> F81534_PORT_CONF_DISABLE_PORT flag with in uart mode field to do
> port hide with port not used. It can be avoid end-user to use not
> layouted port.
>  4. The 4x3 output-only open-drain pins for F81532/534 is designed for
> control outer devices (with our EVB for examples, the 4 sets of pins
> are designed to control transceiver mode). So we decide to implement
> with gpiolib interface.

I need to do some improvements with senior advices, so I'll send
v7 patch when I modify & re-test the code complete.

Could you give me some roughly advices of this patch when you having
time.

Thanks for your help
-- 
With Best Regards,
Peter Hung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Peter Hung 

Hi,

Oliver Neukum 於 2015/11/4 下午 04:38 寫道:

On Wed, 2015-11-04 at 16:19 +0800, Peter Hung wrote:

Hi

Oliver Neukum 於 2015/11/3 下午 06:03 寫道:

On Tue, 2015-11-03 at 11:51 +0800, Peter Hung wrote:

+   for (i = 0; i < F81534_NUM_PORT; ++i)
+   atomic_set(_priv->port_active[i], 0);


Should be ATOMIC_INIT()



ATOMIC_INIT() seems to be used only for variable initializer, It cant be
used for dynamic allocation. Should I change it to a normal boolean
flag protecting with spin_lock ?


No, if it doesn't work, use the current code.


OK, I'll use current code.


+static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,


Is the point of passing a pointer to msr locking?


+   bool is_port_open)


This function is used only with URB callback function. The *msr is
reported by H/W with newest MSR. The USB-Serial generic system will
re-submit read URB when callback complete. So this function should
run once on the same time.


Yes, so why don't you pass an u8 as opposed to a pointer to an u8?


I'll re-write it from u8* to u8.


+static int f81534_tiocmget(struct tty_struct *tty)
+{
+   struct usb_serial_port *port = tty->driver_data;
+   struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
+   unsigned long flags;
+   int r;
+   u8 msr, mcr;
+
+   /*
+* We'll avoid to direct read MSR register. The IC will read the MSR
+* changed and report it f81534_process_per_serial_block() by
+* F81534_TOKEN_MSR_CHANGE.
+*
+* When this device in heavy loading (e.g., BurnInTest Loopback Test)
+* The report of MSR register will delay received a bit. It's due to
+* MSR interrupt is lowest priority in 16550A. So we decide to sleep
+* a little time to pass the test.
+*/
+   if (schedule_timeout_interruptible(
+   msecs_to_jiffies(F81534_DELAY_READ_MSR))) {
+   dev_info(>dev, "%s: breaked !!\n", __func__);
+   }


Is the delay necessary or isn't it?
If it is necessary you should do something about the signal.



We add this delay due to stress test (Loop-back & 921600bps with
BurnInTest). It'll receive MSR with some delay when connecting with
DTR-DSR & RTS/CTS, but the delay smaller than 10ms. So we decided to
delay some time to pass the test.


OK, but how do you guarantee the delay you need if you get a signal,
which would abort the delay?



Hmm, you are right. It seems to replace *_interruptible to
*_killable and return -EINTR to guarantee not abort by normal
signal.


Thanks for your advices
--
With Best Regards,
Peter Hung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Oliver Neukum 
On Wed, 2015-11-04 at 16:19 +0800, Peter Hung wrote:
> Hi
> 
> Oliver Neukum 於 2015/11/3 下午 06:03 寫道:
> > On Tue, 2015-11-03 at 11:51 +0800, Peter Hung wrote:
> >> +static int f81534_attach(struct usb_serial *serial)
> >> +{
> >> +  struct f81534_serial_private *serial_priv = NULL;
> >> +  int status;
> >> +  int i;
> >> +  int offset;
> >> +  uintptr_t setting_idx = (uintptr_t) usb_get_serial_data(serial);
> >> +
> >> +  serial_priv = kzalloc(sizeof(*serial_priv), GFP_KERNEL);
> >> +  if (!serial_priv)
> >> +  return -ENOMEM;
> >> +
> >> +  usb_set_serial_data(serial, serial_priv);
> >> +  serial_priv->setting_idx = setting_idx;
> >> +
> >> +  for (i = 0; i < F81534_NUM_PORT; ++i) {
> >> +  /* Disable all interrupt before submit URB */
> >> +  status = f81534_setregister(serial->dev, i,
> >> +  INTERRUPT_ENABLE_REGISTER, 0x00);
> >> +  if (status) {
> >> +  dev_err(>dev->dev, "%s: IER disable failed\n",
> >> +  __func__);
> >> +  goto failed;
> >> +  }
> >> +  }
> >> +
> >> +  for (i = 0; i < F81534_NUM_PORT; ++i)
> >> +  atomic_set(_priv->port_active[i], 0);
> >
> > Should be ATOMIC_INIT()
> >
> 
> ATOMIC_INIT() seems to be used only for variable initializer, It cant be
> used for dynamic allocation. Should I change it to a normal boolean
> flag protecting with spin_lock ?

No, if it doesn't work, use the current code.

> >> +static int f81534_port_remove(struct usb_serial_port *port)
> >> +{
> >> +  struct f81534_port_private *port_priv;
> >> +
> >> +  f81534_release_gpio(port);
> >> +  port_priv = usb_get_serial_port_data(port);
> >> +  kfree(port_priv);
> >> +  return 0;
> >> +}
> >> +
> >> +static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,
> >
> > Is the point of passing a pointer to msr locking?
> >
> >> +  bool is_port_open)
> 
> This function is used only with URB callback function. The *msr is
> reported by H/W with newest MSR. The USB-Serial generic system will
> re-submit read URB when callback complete. So this function should
> run once on the same time.

Yes, so why don't you pass an u8 as opposed to a pointer to an u8?

> >> +static int f81534_tiocmget(struct tty_struct *tty)
> >> +{
> >> +  struct usb_serial_port *port = tty->driver_data;
> >> +  struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
> >> +  unsigned long flags;
> >> +  int r;
> >> +  u8 msr, mcr;
> >> +
> >> +  /*
> >> +   * We'll avoid to direct read MSR register. The IC will read the MSR
> >> +   * changed and report it f81534_process_per_serial_block() by
> >> +   * F81534_TOKEN_MSR_CHANGE.
> >> +   *
> >> +   * When this device in heavy loading (e.g., BurnInTest Loopback Test)
> >> +   * The report of MSR register will delay received a bit. It's due to
> >> +   * MSR interrupt is lowest priority in 16550A. So we decide to sleep
> >> +   * a little time to pass the test.
> >> +   */
> >> +  if (schedule_timeout_interruptible(
> >> +  msecs_to_jiffies(F81534_DELAY_READ_MSR))) {
> >> +  dev_info(>dev, "%s: breaked !!\n", __func__);
> >> +  }
> >
> > Is the delay necessary or isn't it?
> > If it is necessary you should do something about the signal.
> >
> 
> We add this delay due to stress test (Loop-back & 921600bps with
> BurnInTest). It'll receive MSR with some delay when connecting with
> DTR-DSR & RTS/CTS, but the delay smaller than 10ms. So we decided to
> delay some time to pass the test.

OK, but how do you guarantee the delay you need if you get a signal,
which would abort the delay?

Regards
Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Peter Hung 

Hi

Andy Shevchenko 於 2015/11/3 下午 05:45 寫道:

On Tue, Nov 3, 2015 at 5:51 AM, Peter Hung  wrote:

+ *Please reference https://bitbucket.org/hpeter/fintek-general/src/
+ *with f81534/tools to get set_gpio.c & set_mode.c. Please use it
+ *carefully.


Would it be good to have this under Documentation ?


I had some documents in source code. The link is the demo program to
implement switch UART/PIN funcion. I'll try to add for document with the
repository.


+static void f81534_dtr_rts(struct usb_serial_port *port, int on);
+
+static int f81534_set_port_mode(struct usb_serial_port *port,
+   enum uart_mode eMode);
+
+static int f81534_save_configure_data(struct usb_serial_port *port);
+
+static int f81534_switch_gpio_mode(struct usb_serial_port *port, u8 mode);
+
+static void f81534_wakeup_all_port(struct usb_serial *serial);
+
+static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,
+   bool is_port_open);
+
+static int f81534_prepare_write_buffer(struct usb_serial_port *port,
+   void *dest, size_t size);
+
+static int f81534_submit_writer(struct usb_serial_port *port, gfp_t mem_flags);


Would it be possible to reshuffle code to reduce amount of forward declarations?



I'll try to reduce it.




+   if ((size <= F81534_MAX_DATA_BLOCK) &&
+   (read_size == (count + 1))) {


No need to have internal parens.



In my opinion, It makes more readable. Should I remove it?



+   if (baudrate <= 115200) {
+   value = 0x01;   /* 1.846m fixed */
+   divisor = f81534_calc_baud_divisor(baudrate, 115200, NULL);
+   port_priv->current_baud_base = 115200;
+   } else {
+   for (count = 0; count < ARRAY_SIZE(baudrate_table) ; ++count) {
+   baud_base = baudrate_table[count];
+   divisor = f81534_calc_baud_divisor(baudrate, baud_base,
+   );
+   if (!rem) {
+   dev_dbg(>dev, "%s: found clockbase %d\n",
+   __func__,
+   baudrate_table[count]);
+   value = clock_table[count];
+   port_priv->current_baud_base = baud_base;
+   break;
+   }
+   }


Can you calculate baud rates more precisely? Any link to datasheet
where it's described?



I'll try to comment it within source code.


+static int f81534_ioctl_get_rs485(struct usb_serial_port *port,
+   struct serial_rs485 __user *arg)
+{
+   int status;
+   struct serial_rs485 data;
+   struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
+   struct f81534_serial_private *serial_priv =
+   usb_get_serial_data(port->serial);


One line?



It's over 80 character with a tab, It cant be one line.

I'll fix other issue that you mention.

Thanks for your advices.
--
With Best Regards,
Peter Hung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-04 Thread Peter Hung 

Hi

Oliver Neukum 於 2015/11/3 下午 06:03 寫道:

On Tue, 2015-11-03 at 11:51 +0800, Peter Hung wrote:

+static int f81534_attach(struct usb_serial *serial)
+{
+   struct f81534_serial_private *serial_priv = NULL;
+   int status;
+   int i;
+   int offset;
+   uintptr_t setting_idx = (uintptr_t) usb_get_serial_data(serial);
+
+   serial_priv = kzalloc(sizeof(*serial_priv), GFP_KERNEL);
+   if (!serial_priv)
+   return -ENOMEM;
+
+   usb_set_serial_data(serial, serial_priv);
+   serial_priv->setting_idx = setting_idx;
+
+   for (i = 0; i < F81534_NUM_PORT; ++i) {
+   /* Disable all interrupt before submit URB */
+   status = f81534_setregister(serial->dev, i,
+   INTERRUPT_ENABLE_REGISTER, 0x00);
+   if (status) {
+   dev_err(>dev->dev, "%s: IER disable failed\n",
+   __func__);
+   goto failed;
+   }
+   }
+
+   for (i = 0; i < F81534_NUM_PORT; ++i)
+   atomic_set(_priv->port_active[i], 0);


Should be ATOMIC_INIT()



ATOMIC_INIT() seems to be used only for variable initializer, It cant be
used for dynamic allocation. Should I change it to a normal boolean
flag protecting with spin_lock ?



+static int f81534_port_remove(struct usb_serial_port *port)
+{
+   struct f81534_port_private *port_priv;
+
+   f81534_release_gpio(port);
+   port_priv = usb_get_serial_port_data(port);
+   kfree(port_priv);
+   return 0;
+}
+
+static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,


Is the point of passing a pointer to msr locking?


+   bool is_port_open)


This function is used only with URB callback function. The *msr is
reported by H/W with newest MSR. The USB-Serial generic system will
re-submit read URB when callback complete. So this function should
run once on the same time.


+static int f81534_tiocmget(struct tty_struct *tty)
+{
+   struct usb_serial_port *port = tty->driver_data;
+   struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
+   unsigned long flags;
+   int r;
+   u8 msr, mcr;
+
+   /*
+* We'll avoid to direct read MSR register. The IC will read the MSR
+* changed and report it f81534_process_per_serial_block() by
+* F81534_TOKEN_MSR_CHANGE.
+*
+* When this device in heavy loading (e.g., BurnInTest Loopback Test)
+* The report of MSR register will delay received a bit. It's due to
+* MSR interrupt is lowest priority in 16550A. So we decide to sleep
+* a little time to pass the test.
+*/
+   if (schedule_timeout_interruptible(
+   msecs_to_jiffies(F81534_DELAY_READ_MSR))) {
+   dev_info(>dev, "%s: breaked !!\n", __func__);
+   }


Is the delay necessary or isn't it?
If it is necessary you should do something about the signal.



We add this delay due to stress test (Loop-back & 921600bps with
BurnInTest). It'll receive MSR with some delay when connecting with
DTR-DSR & RTS/CTS, but the delay smaller than 10ms. So we decided to
delay some time to pass the test.


+static int f81534_prepare_write_buffer(struct usb_serial_port *port,
+   void *dest, size_t size)
+{
+   unsigned char *ptr = (unsigned char *) dest;
+   struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
+   int port_num = port_priv->phy;
+   struct usb_serial *serial = port->serial;
+
+   WARN_ON(size != serial->port[0]->bulk_out_size);
+
+   if (size != F81534_WRITE_BUFFER_SIZE) {
+   WARN_ON(size != F81534_WRITE_BUFFER_SIZE);


What is the sense of this?



I'll remove the double-check section with next version patch.



+   ptr[F81534_RECEIVE_BLOCK_SIZE * 0] = 0;
+   ptr[F81534_RECEIVE_BLOCK_SIZE * 1] = 1;
+   ptr[F81534_RECEIVE_BLOCK_SIZE * 2] = 2;
+   ptr[F81534_RECEIVE_BLOCK_SIZE * 3] = 3;


Either these ...


+   ptr[F81534_RECEIVE_BLOCK_SIZE * port_num + 0] = port_num;


.. or that is redundant



I'll remove it too.

Thanks for your advice.
--
With Best Regards,
Peter Hung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-03 Thread Oliver Neukum 
On Tue, 2015-11-03 at 11:51 +0800, Peter Hung wrote:
> This driver is for Fintek F81532/F81534 USB to Serial Ports IC.
> 
> Features:
> 1. F81534 is 1-to-4 & F81532 is 1-to-2 serial ports IC
> 2. Support Baudrate from B50 to B150 (excluding B100).
> 3. The RTS signal can be transformed their behavior with
>configuration by ioctl TIOCGRS485/TIOCSRS485
> 
>If the driver setting with SER_RS485_ENABLED, the RTS signal will
>high with not in TX and low with in TX.
> 
>If the driver setting with SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
>the RTS signal will low with not in TX and high with in TX.
> 
> 4. The 4x3 output-only open-drain pins for F81532/534 is designed for
>control outer devices (with our EVB for examples, the 4 sets of pins
>are designed to control transceiver mode). So it implements as
>gpiolib interface with output-only function.
> 5. It'll save the configuration in internal storage when uart/pins mode
>changed.
> 
>Please reference https://bitbucket.org/hpeter/fintek-general/src/
>with f81534/tools to get set_gpio.c & set_mode.c to change F81532/534
>setting. Please use it carefully.

Hi,

I added remarks among the code they pertain to.

Regards
Oliver
> 
> Signed-off-by: Peter Hung 
> ---
> Changelog:
> v6
> 1. Re-implement the write()/resume() function. Due to this device cant be
>suitable with generic write(), we'll do the submit write URB when
>write()/received tx empty/set_termios()/resume()
> 2. Logic/Phy Port mapping rewrite in f81534_port_probe() &
>f81534_phy_to_logic_port().
> 3. Introduced "Port Hide" function. Some customer use F81532 reference
>design for HW layout, but really use F81534 IC. We'll check
>F81534_PORT_CONF_DISABLE_PORT flag with in uart mode field to do
>port hide with port not used. It can be avoid end-user to use not
>layouted port.
> 4. The 4x3 output-only open-drain pins for F81532/534 is designed for
>control outer devices (with our EVB for examples, the 4 sets of pins
>are designed to control transceiver mode). So we decide to implement
>with gpiolib interface.
>   
> v5
> 1. Change the configure data address F81534_CUSTOM_ADDRESS_START
>from 0x4000 to 0x2f00 (for MP F/W ver:AA66)
> 2. Change f81534_port_disable/enable() from H/W mode to S/W mode
>It'll skip all rx data when port is not opened.
> 3. Some function modifier add with static (Thanks for Paul Bolle)
> 4. It's will direct return when count=0 in f81534_write() to
>reduce spin_lock usage.
> 
> v4
> 1. clearify f81534_process_read_urb() with
>f81534_process_per_serial_block(). (referenced from mxuport.c)
> 2. We limited f81534_write() max tx kfifo with 124Bytes.
>Original subsystem is designed for auto tranmiting fifo data
>if available. But we must wait for tx_empty for next tx data
>(H/W design).
> 
>With this kfifo size limit, we can use generic subsystem api with
>f81534_write(). When usb_serial_generic_write_start() called after
>first write URB complete, the fifo will no data. The generic
>subsystem of write will go to idle state. Until we received
>TX_EMPTY and release write spinlock, the fifo will fill max
>124Bytes by following f81534_write().
> 
> v3
> 1. Migrate read, write and some routine from custom code to usbserial
>subsystem callback function.
> 2. Use more defines to replece magic numbers to make it meaningful
> 3. Make more comments as document in source code.
> 
> v2
> 1. v1 version submit to staging tree, but Greg KH advised me to
>cleanup source code & re-submit it to correct subsystem
> 2. Remove all custom ioctl commands
> 
>  drivers/usb/serial/Kconfig  |   10 +
>  drivers/usb/serial/Makefile |1 +
>  drivers/usb/serial/f81534.c | 2991 
> +++
>  3 files changed, 3002 insertions(+)
>  create mode 100644 drivers/usb/serial/f81534.c
> 
> diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
> index 56ecb8b..0642864 100644
> --- a/drivers/usb/serial/Kconfig
> +++ b/drivers/usb/serial/Kconfig
> @@ -255,6 +255,16 @@ config USB_SERIAL_F81232
> To compile this driver as a module, choose M here: the
> module will be called f81232.
>  
> +config USB_SERIAL_F8153X
> + tristate "USB Fintek F81532/534 Multi-Ports Serial Driver"
> + help
> +   Say Y here if you want to use the Fintek F81532/534 Multi-Ports
> +   usb to serial adapter.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called f81534.
> +
> +
>  config USB_SERIAL_GARMIN
> tristate "USB Garmin GPS driver"
> help
> diff --git a/drivers/usb/serial/Makefile b/drivers/usb/serial/Makefile
> index 349d9df..9e43b7b 100644
> ---

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-03 Thread Andy Shevchenko 
On Tue, Nov 3, 2015 at 5:51 AM, Peter Hung  wrote:
> This driver is for Fintek F81532/F81534 USB to Serial Ports IC.
>
> Features:
> 1. F81534 is 1-to-4 & F81532 is 1-to-2 serial ports IC
> 2. Support Baudrate from B50 to B150 (excluding B100).
> 3. The RTS signal can be transformed their behavior with
>configuration by ioctl TIOCGRS485/TIOCSRS485
>
>If the driver setting with SER_RS485_ENABLED, the RTS signal will
>high with not in TX and low with in TX.
>
>If the driver setting with SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
>the RTS signal will low with not in TX and high with in TX.
>
> 4. The 4x3 output-only open-drain pins for F81532/534 is designed for
>control outer devices (with our EVB for examples, the 4 sets of pins
>are designed to control transceiver mode). So it implements as
>gpiolib interface with output-only function.
> 5. It'll save the configuration in internal storage when uart/pins mode
>changed.
>
>Please reference https://bitbucket.org/hpeter/fintek-general/src/
>with f81534/tools to get set_gpio.c & set_mode.c to change F81532/534
>setting. Please use it carefully.

Few comments below.

> +++ b/drivers/usb/serial/f81534.c
> @@ -0,0 +1,2991 @@
> +/*
> + * F81532/F81534 USB to Serial Ports Bridge
> + *
> + * F81532 => 2 Serial Ports
> + * F81534 => 4 Serial Ports
> + *
> + * Copyright (C) 2014 Tom Tsai (tom_t...@fintek.com.tw)
> + *
> + * The F81532/F81534 had 1 control endpoint for setting,
> + * 1 endpoint bulk-out for all serial port write out and
> + * 1 endpoint bulk-in for all serial port read in.
> + *
> + * write URB is fixed with 512bytes, per serial port used 128Bytes.
> + * is can be described by f81534_prepare_write_buffer()
> + *
> + * read URB is 512Bytes max. per serial port used 128Bytes.
> + * is can be described by f81534_process_read_urb(), it' maybe
> + * received with 128x1,2,3,4 bytes.
> + *
> + * We can control M0(SD)/M1/M2 per ports by gpiolib, This IC contains a
> + * internal flash to save configuration. Due to reduce erase/write operation,
> + * it's recommend sequence to request 3 pins, change value and release 3 gpio
> + * pins. We'll really save configurations when M0(SD)/M1/M2 pin all released
> + * for a port.
> + *
> + * Features:
> + * 1. F81534 is 1-to-4 & F81532 is 1-to-2 serial ports IC
> + * 2. Support Baudrate from B50 to B150 (excluding B100).
> + * 3. The RTS signal can be transformed their behavior with
> + *configuration by default ioctl TIOCGRS485/TIOCSRS485
> + *(for RS232/RS485/RS422 with transceiver)
> + *
> + *If the driver setting with SER_RS485_ENABLED, the RTS signal will
> + *high with not in TX and low with in TX.
> + *
> + *If the driver setting with SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
> + *the RTS signal will low with not in TX and high with in TX.
> + *
> + * 4. There are 4x3 output-only ic pins to control transceiver mode with our
> + *EVB Board. It's can be controlled via gpiolib. We could found the gpio
> + *number from /sys/class/tty/ttyUSB[x]/device/gpiochip[yyy] where
> + *x is F81532/534 serial port and yyy is gpiochip number.
> + *
> + *After we found chip number, we can export 3 gpios(M0(SD)/M1/M2) per
> + *serial port by
> + *   echo yyy > /sys/class/gpio/export
> + *   echo yyy+1 > /sys/class/gpio/export
> + *   echo yyy+2 > /sys/class/gpio/export
> + *
> + *then we can control it with
> + *   echo [M2 value] > /sys/class/gpio/gpio[yyy]/value
> + *   echo [M1 value] > /sys/class/gpio/gpio[yyy+1]/value
> + *   echo [M0(SD) value] > /sys/class/gpio/gpio[yyy+2]/value
> + *which M0(SD)/M1/M2 as your desired value, value is only 0 or 1.
> + *
> + *When configure complete, It's a must to free all gpio by
> + *   echo yyy > /sys/class/gpio/unexport
> + *   echo yyy+1 > /sys/class/gpio/unexport
> + *   echo yyy+2 > /sys/class/gpio/unexport
> + *
> + *The driver will "save" gpio configure after we release
> + *all gpio of a serial port.
> + *
> + *For examples to change mode & gpio with F81532/534
> + *Evalaution Board.
> + *
> + *F81532 EVB
> + *   port0: F81437 (RS232 only)
> + *   port1: F81439 (RS232/RS485/RS422 ... etc.)
> + *F81534 EVB
> + *   port0/1: F81437 (RS232 only)
> + *   port2/3: F81439 (RS232/RS485/RS422 ... etc.)
> + *
> + *   1. RS232 Mode (Default IC Mode)
> + *  1. Set struct serial_rs485 flags "without" SER_RS485_ENABLED
> + * (control F81532/534 RTS control)
> + *  2. Set M0(SD)/M1/M2 as 0/0/1
> + * (control F81532/534 output pin to control transceiver mode)
> + *
> + *   2. RS485 Mode (RTS Low when TX Mode)
> + *  1. Set struct serial_rs485 flags with SER_RS485_ENABLED
> + *  2. Set M0(SD)/M1/M2 as 0/1/0
> + *
> + *   3. RS485 Mode (RTS High when TX Mode)
> + *  1. Set struct serial_rs485 flags with SER_RS485_ENABLED and
> + *

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-03 Thread Oliver Neukum 
On Tue, 2015-11-03 at 11:51 +0800, Peter Hung wrote:
> This driver is for Fintek F81532/F81534 USB to Serial Ports IC.
> 
> Features:
> 1. F81534 is 1-to-4 & F81532 is 1-to-2 serial ports IC
> 2. Support Baudrate from B50 to B150 (excluding B100).
> 3. The RTS signal can be transformed their behavior with
>configuration by ioctl TIOCGRS485/TIOCSRS485
> 
>If the driver setting with SER_RS485_ENABLED, the RTS signal will
>high with not in TX and low with in TX.
> 
>If the driver setting with SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
>the RTS signal will low with not in TX and high with in TX.
> 
> 4. The 4x3 output-only open-drain pins for F81532/534 is designed for
>control outer devices (with our EVB for examples, the 4 sets of pins
>are designed to control transceiver mode). So it implements as
>gpiolib interface with output-only function.
> 5. It'll save the configuration in internal storage when uart/pins mode
>changed.
> 
>Please reference https://bitbucket.org/hpeter/fintek-general/src/
>with f81534/tools to get set_gpio.c & set_mode.c to change F81532/534
>setting. Please use it carefully.

Hi,

I added remarks among the code they pertain to.

Regards
Oliver
> 
> Signed-off-by: Peter Hung 
> ---
> Changelog:
> v6
> 1. Re-implement the write()/resume() function. Due to this device cant be
>suitable with generic write(), we'll do the submit write URB when
>write()/received tx empty/set_termios()/resume()
> 2. Logic/Phy Port mapping rewrite in f81534_port_probe() &
>f81534_phy_to_logic_port().
> 3. Introduced "Port Hide" function. Some customer use F81532 reference
>design for HW layout, but really use F81534 IC. We'll check
>F81534_PORT_CONF_DISABLE_PORT flag with in uart mode field to do
>port hide with port not used. It can be avoid end-user to use not
>layouted port.
> 4. The 4x3 output-only open-drain pins for F81532/534 is designed for
>control outer devices (with our EVB for examples, the 4 sets of pins
>are designed to control transceiver mode). So we decide to implement
>with gpiolib interface.
>   
> v5
> 1. Change the configure data address F81534_CUSTOM_ADDRESS_START
>from 0x4000 to 0x2f00 (for MP F/W ver:AA66)
> 2. Change f81534_port_disable/enable() from H/W mode to S/W mode
>It'll skip all rx data when port is not opened.
> 3. Some function modifier add with static (Thanks for Paul Bolle)
> 4. It's will direct return when count=0 in f81534_write() to
>reduce spin_lock usage.
> 
> v4
> 1. clearify f81534_process_read_urb() with
>f81534_process_per_serial_block(). (referenced from mxuport.c)
> 2. We limited f81534_write() max tx kfifo with 124Bytes.
>Original subsystem is designed for auto tranmiting fifo data
>if available. But we must wait for tx_empty for next tx data
>(H/W design).
> 
>With this kfifo size limit, we can use generic subsystem api with
>f81534_write(). When usb_serial_generic_write_start() called after
>first write URB complete, the fifo will no data. The generic
>subsystem of write will go to idle state. Until we received
>TX_EMPTY and release write spinlock, the fifo will fill max
>124Bytes by following f81534_write().
> 
> v3
> 1. Migrate read, write and some routine from custom code to usbserial
>subsystem callback function.
> 2. Use more defines to replece magic numbers to make it meaningful
> 3. Make more comments as document in source code.
> 
> v2
> 1. v1 version submit to staging tree, but Greg KH advised me to
>cleanup source code & re-submit it to correct subsystem
> 2. Remove all custom ioctl commands
> 
>  drivers/usb/serial/Kconfig  |   10 +
>  drivers/usb/serial/Makefile |1 +
>  drivers/usb/serial/f81534.c | 2991 
> +++
>  3 files changed, 3002 insertions(+)
>  create mode 100644 drivers/usb/serial/f81534.c
> 
> diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
> index 56ecb8b..0642864 100644
> --- a/drivers/usb/serial/Kconfig
> +++ b/drivers/usb/serial/Kconfig
> @@ -255,6 +255,16 @@ config USB_SERIAL_F81232
> To compile this driver as a module, choose M here: the
> module will be called f81232.
>  
> +config USB_SERIAL_F8153X
> + tristate "USB Fintek F81532/534 Multi-Ports Serial Driver"
> + help
> +   Say Y here if you want to use the Fintek F81532/534 Multi-Ports
> +   usb to serial adapter.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called f81534.
> +
> +
>  config USB_SERIAL_GARMIN
> tristate "USB Garmin GPS driver"
> help
> diff --git a/drivers/usb/serial/Makefile b/drivers/usb/serial/Makefile
> index 349d9df..9e43b7b 100644
>

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-03 Thread Andy Shevchenko 
On Tue, Nov 3, 2015 at 5:51 AM, Peter Hung  wrote:
> This driver is for Fintek F81532/F81534 USB to Serial Ports IC.
>
> Features:
> 1. F81534 is 1-to-4 & F81532 is 1-to-2 serial ports IC
> 2. Support Baudrate from B50 to B150 (excluding B100).
> 3. The RTS signal can be transformed their behavior with
>configuration by ioctl TIOCGRS485/TIOCSRS485
>
>If the driver setting with SER_RS485_ENABLED, the RTS signal will
>high with not in TX and low with in TX.
>
>If the driver setting with SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
>the RTS signal will low with not in TX and high with in TX.
>
> 4. The 4x3 output-only open-drain pins for F81532/534 is designed for
>control outer devices (with our EVB for examples, the 4 sets of pins
>are designed to control transceiver mode). So it implements as
>gpiolib interface with output-only function.
> 5. It'll save the configuration in internal storage when uart/pins mode
>changed.
>
>Please reference https://bitbucket.org/hpeter/fintek-general/src/
>with f81534/tools to get set_gpio.c & set_mode.c to change F81532/534
>setting. Please use it carefully.

Few comments below.

> +++ b/drivers/usb/serial/f81534.c
> @@ -0,0 +1,2991 @@
> +/*
> + * F81532/F81534 USB to Serial Ports Bridge
> + *
> + * F81532 => 2 Serial Ports
> + * F81534 => 4 Serial Ports
> + *
> + * Copyright (C) 2014 Tom Tsai (tom_t...@fintek.com.tw)
> + *
> + * The F81532/F81534 had 1 control endpoint for setting,
> + * 1 endpoint bulk-out for all serial port write out and
> + * 1 endpoint bulk-in for all serial port read in.
> + *
> + * write URB is fixed with 512bytes, per serial port used 128Bytes.
> + * is can be described by f81534_prepare_write_buffer()
> + *
> + * read URB is 512Bytes max. per serial port used 128Bytes.
> + * is can be described by f81534_process_read_urb(), it' maybe
> + * received with 128x1,2,3,4 bytes.
> + *
> + * We can control M0(SD)/M1/M2 per ports by gpiolib, This IC contains a
> + * internal flash to save configuration. Due to reduce erase/write operation,
> + * it's recommend sequence to request 3 pins, change value and release 3 gpio
> + * pins. We'll really save configurations when M0(SD)/M1/M2 pin all released
> + * for a port.
> + *
> + * Features:
> + * 1. F81534 is 1-to-4 & F81532 is 1-to-2 serial ports IC
> + * 2. Support Baudrate from B50 to B150 (excluding B100).
> + * 3. The RTS signal can be transformed their behavior with
> + *configuration by default ioctl TIOCGRS485/TIOCSRS485
> + *(for RS232/RS485/RS422 with transceiver)
> + *
> + *If the driver setting with SER_RS485_ENABLED, the RTS signal will
> + *high with not in TX and low with in TX.
> + *
> + *If the driver setting with SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
> + *the RTS signal will low with not in TX and high with in TX.
> + *
> + * 4. There are 4x3 output-only ic pins to control transceiver mode with our
> + *EVB Board. It's can be controlled via gpiolib. We could found the gpio
> + *number from /sys/class/tty/ttyUSB[x]/device/gpiochip[yyy] where
> + *x is F81532/534 serial port and yyy is gpiochip number.
> + *
> + *After we found chip number, we can export 3 gpios(M0(SD)/M1/M2) per
> + *serial port by
> + *   echo yyy > /sys/class/gpio/export
> + *   echo yyy+1 > /sys/class/gpio/export
> + *   echo yyy+2 > /sys/class/gpio/export
> + *
> + *then we can control it with
> + *   echo [M2 value] > /sys/class/gpio/gpio[yyy]/value
> + *   echo [M1 value] > /sys/class/gpio/gpio[yyy+1]/value
> + *   echo [M0(SD) value] > /sys/class/gpio/gpio[yyy+2]/value
> + *which M0(SD)/M1/M2 as your desired value, value is only 0 or 1.
> + *
> + *When configure complete, It's a must to free all gpio by
> + *   echo yyy > /sys/class/gpio/unexport
> + *   echo yyy+1 > /sys/class/gpio/unexport
> + *   echo yyy+2 > /sys/class/gpio/unexport
> + *
> + *The driver will "save" gpio configure after we release
> + *all gpio of a serial port.
> + *
> + *For examples to change mode & gpio with F81532/534
> + *Evalaution Board.
> + *
> + *F81532 EVB
> + *   port0: F81437 (RS232 only)
> + *   port1: F81439 (RS232/RS485/RS422 ... etc.)
> + *F81534 EVB
> + *   port0/1: F81437 (RS232 only)
> + *   port2/3: F81439 (RS232/RS485/RS422 ... etc.)
> + *
> + *   1. RS232 Mode (Default IC Mode)
> + *  1. Set struct serial_rs485 flags "without" SER_RS485_ENABLED
> + * (control F81532/534 RTS control)
> + *  2. Set M0(SD)/M1/M2 as 0/0/1
> + * (control F81532/534 output pin to control transceiver mode)
> + *
> + *   2. RS485 Mode (RTS Low when TX Mode)
> + *  1. Set struct serial_rs485 flags with SER_RS485_ENABLED
> + *  2. Set M0(SD)/M1/M2 as 0/1/0
> + *
> + *   3. RS485 Mode (RTS High when TX Mode)
> + *  1. Set struct serial_rs485 flags with

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-02 Thread kbuild test robot 
Hi Peter,

[auto build test ERROR on usb/usb-next -- if it's inappropriate base, please 
suggest rules for selecting the more suitable base]

url:
https://github.com/0day-ci/linux/commits/Peter-Hung/usb-serial-Add-Fintek-F81532-534-driver/20151103-115336
config: tile-allyesconfig (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=tile 

All errors (new ones prefixed by >>):

   drivers/usb/serial/f81534.c:504:1: warning: data definition has no type or 
storage class [enabled by default]
   drivers/usb/serial/f81534.c:504:1: error: type defaults to 'int' in 
declaration of 'MODULE_DEVICE_TABLE'
   drivers/usb/serial/f81534.c:504:1: warning: parameter names (without types) 
in function declaration [enabled by default]
   drivers/usb/serial/f81534.c:522:19: error: field 'f81534_gpio_chip' has 
incomplete type
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_get':
   drivers/usb/serial/f81534.c:849:4: error: dereferencing pointer to 
incomplete type
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_set':
   drivers/usb/serial/f81534.c:886:33: error: dereferencing pointer to 
incomplete type
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_request':
   drivers/usb/serial/f81534.c:926:33: error: dereferencing pointer to 
incomplete type
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_free':
   drivers/usb/serial/f81534.c:937:33: error: dereferencing pointer to 
incomplete type
   drivers/usb/serial/f81534.c: At top level:
   drivers/usb/serial/f81534.c:960:15: error: variable 
'f81534_gpio_chip_templete' has initializer but incomplete type
   drivers/usb/serial/f81534.c:961:2: error: unknown field 'owner' specified in 
initializer
   drivers/usb/serial/f81534.c:961:11: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:961:11: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:962:2: error: unknown field 'get_direction' 
specified in initializer
   drivers/usb/serial/f81534.c:962:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:962:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:963:2: error: unknown field 'get' specified in 
initializer
   drivers/usb/serial/f81534.c:963:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:963:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:964:2: error: unknown field 'direction_input' 
specified in initializer
   drivers/usb/serial/f81534.c:964:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:964:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:965:2: error: unknown field 'set' specified in 
initializer
   drivers/usb/serial/f81534.c:965:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:965:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:966:2: error: unknown field 'direction_output' 
specified in initializer
   drivers/usb/serial/f81534.c:966:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:966:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:967:2: error: unknown field 'request' specified 
in initializer
   drivers/usb/serial/f81534.c:967:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:967:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:968:2: error: unknown field 'free' specified in 
initializer
   drivers/usb/serial/f81534.c:968:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:968:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:969:2: error: unknown field 'ngpio' specified in 
initializer
   drivers/usb/serial/f81534.c:969:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:969:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:970:2: error: unknown field 'base' specified in 
initializer
   drivers/usb/serial/f81534.c:970:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:970:2: warning: (near

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-02 Thread kbuild test robot 
Hi Peter,

[auto build test ERROR on usb/usb-next -- if it's inappropriate base, please 
suggest rules for selecting the more suitable base]

url:
https://github.com/0day-ci/linux/commits/Peter-Hung/usb-serial-Add-Fintek-F81532-534-driver/20151103-115336
config: m68k-allyesconfig (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=m68k 

All error/warnings (new ones prefixed by >>):

>> drivers/usb/serial/f81534.c:504:1: warning: data definition has no type or 
>> storage class
MODULE_DEVICE_TABLE(usb, id_table);
^
>> drivers/usb/serial/f81534.c:504:1: error: type defaults to 'int' in 
>> declaration of 'MODULE_DEVICE_TABLE' [-Werror=implicit-int]
>> drivers/usb/serial/f81534.c:504:1: warning: parameter names (without types) 
>> in function declaration
>> drivers/usb/serial/f81534.c:522:19: error: field 'f81534_gpio_chip' has 
>> incomplete type
 struct gpio_chip f81534_gpio_chip;
  ^
   In file included from include/linux/list.h:8:0,
from include/linux/preempt.h:10,
from include/linux/spinlock.h:50,
from include/linux/mmzone.h:7,
from include/linux/gfp.h:5,
from include/linux/slab.h:14,
from drivers/usb/serial/f81534.c:99:
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_get':
>> drivers/usb/serial/f81534.c:849:21: error: dereferencing pointer to 
>> incomplete type
   container_of(chip->dev, struct usb_serial_port, dev);
^
   include/linux/kernel.h:811:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_set':
   drivers/usb/serial/f81534.c:887:8: error: dereferencing pointer to 
incomplete type
   chip->dev, struct usb_serial_port, dev);
   ^
   include/linux/kernel.h:811:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_request':
   drivers/usb/serial/f81534.c:927:8: error: dereferencing pointer to 
incomplete type
   chip->dev, struct usb_serial_port, dev);
   ^
   include/linux/kernel.h:811:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_free':
   drivers/usb/serial/f81534.c:938:8: error: dereferencing pointer to 
incomplete type
   chip->dev, struct usb_serial_port, dev);
   ^
   include/linux/kernel.h:811:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
   drivers/usb/serial/f81534.c: At top level:
>> drivers/usb/serial/f81534.c:960:15: error: variable 
>> 'f81534_gpio_chip_templete' has initializer but incomplete type
static struct gpio_chip f81534_gpio_chip_templete = {
  ^
>> drivers/usb/serial/f81534.c:961:2: error: unknown field 'owner' specified in 
>> initializer
 .owner = THIS_MODULE,
 ^
   In file included from include/linux/linkage.h:6:0,
from include/linux/preempt.h:9,
from include/linux/spinlock.h:50,
from include/linux/mmzone.h:7,
from include/linux/gfp.h:5,
from include/linux/slab.h:14,
from drivers/usb/serial/f81534.c:99:
   include/linux/export.h:36:30: warning: excess elements in struct initializer
#define THIS_MODULE ((struct module *)0)
 ^
>> drivers/usb/serial/f81534.c:961:11: note: in expansion of macro 'THIS_MODULE'
 .owner = THIS_MODULE,
  ^
   include/linux/export.h:36:30: warning: (near initialization for 
'f81534_gpio_chip_templete')
#define THIS_MODULE ((struct module *)0)
 ^
>> drivers/usb/serial/f81534.c:961:11: note: in expansion of macro 'THIS_MODULE'
 .owner = THIS_MODULE,
  ^
>> drivers/usb/serial/f81534.c:962:2: error: unknown field 'get_direction' 
>> specified in initializer
 .get_direction = f81534_gpio_get_direction,
 ^
>> drivers/usb/serial/f81534.c:962:2: warning: excess elements in struct 
>> initializer
   drivers/usb/serial/f81534.c:962:2: warning: (near initialization for 
'f81534_gpio_chip_templete')
>> drivers/usb/serial/f81534.c:963:2: error: unknown field 'get' specified in 
>> initializer
 .get = f81534_gpio_get,
 ^

[PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-02 Thread Peter Hung 
This driver is for Fintek F81532/F81534 USB to Serial Ports IC.

Features:
1. F81534 is 1-to-4 & F81532 is 1-to-2 serial ports IC
2. Support Baudrate from B50 to B150 (excluding B100).
3. The RTS signal can be transformed their behavior with
   configuration by ioctl TIOCGRS485/TIOCSRS485

   If the driver setting with SER_RS485_ENABLED, the RTS signal will
   high with not in TX and low with in TX.

   If the driver setting with SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
   the RTS signal will low with not in TX and high with in TX.

4. The 4x3 output-only open-drain pins for F81532/534 is designed for
   control outer devices (with our EVB for examples, the 4 sets of pins
   are designed to control transceiver mode). So it implements as
   gpiolib interface with output-only function.
5. It'll save the configuration in internal storage when uart/pins mode
   changed.

   Please reference https://bitbucket.org/hpeter/fintek-general/src/
   with f81534/tools to get set_gpio.c & set_mode.c to change F81532/534
   setting. Please use it carefully.

Signed-off-by: Peter Hung 
---
Changelog:
v6
1. Re-implement the write()/resume() function. Due to this device cant be
   suitable with generic write(), we'll do the submit write URB when
   write()/received tx empty/set_termios()/resume()
2. Logic/Phy Port mapping rewrite in f81534_port_probe() &
   f81534_phy_to_logic_port().
3. Introduced "Port Hide" function. Some customer use F81532 reference
   design for HW layout, but really use F81534 IC. We'll check
   F81534_PORT_CONF_DISABLE_PORT flag with in uart mode field to do
   port hide with port not used. It can be avoid end-user to use not
   layouted port.
4. The 4x3 output-only open-drain pins for F81532/534 is designed for
   control outer devices (with our EVB for examples, the 4 sets of pins
   are designed to control transceiver mode). So we decide to implement
   with gpiolib interface.
  
v5
1. Change the configure data address F81534_CUSTOM_ADDRESS_START
   from 0x4000 to 0x2f00 (for MP F/W ver:AA66)
2. Change f81534_port_disable/enable() from H/W mode to S/W mode
   It'll skip all rx data when port is not opened.
3. Some function modifier add with static (Thanks for Paul Bolle)
4. It's will direct return when count=0 in f81534_write() to
   reduce spin_lock usage.

v4
1. clearify f81534_process_read_urb() with
   f81534_process_per_serial_block(). (referenced from mxuport.c)
2. We limited f81534_write() max tx kfifo with 124Bytes.
   Original subsystem is designed for auto tranmiting fifo data
   if available. But we must wait for tx_empty for next tx data
   (H/W design).

   With this kfifo size limit, we can use generic subsystem api with
   f81534_write(). When usb_serial_generic_write_start() called after
   first write URB complete, the fifo will no data. The generic
   subsystem of write will go to idle state. Until we received
   TX_EMPTY and release write spinlock, the fifo will fill max
   124Bytes by following f81534_write().

v3
1. Migrate read, write and some routine from custom code to usbserial
   subsystem callback function.
2. Use more defines to replece magic numbers to make it meaningful
3. Make more comments as document in source code.

v2
1. v1 version submit to staging tree, but Greg KH advised me to
   cleanup source code & re-submit it to correct subsystem
2. Remove all custom ioctl commands

 drivers/usb/serial/Kconfig  |   10 +
 drivers/usb/serial/Makefile |1 +
 drivers/usb/serial/f81534.c | 2991 +++
 3 files changed, 3002 insertions(+)
 create mode 100644 drivers/usb/serial/f81534.c

diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 56ecb8b..0642864 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -255,6 +255,16 @@ config USB_SERIAL_F81232
  To compile this driver as a module, choose M here: the
  module will be called f81232.
 
+config USB_SERIAL_F8153X
+   tristate "USB Fintek F81532/534 Multi-Ports Serial Driver"
+   help
+ Say Y here if you want to use the Fintek F81532/534 Multi-Ports
+ usb to serial adapter.
+
+ To compile this driver as a module, choose M here: the
+ module will be called f81534.
+
+
 config USB_SERIAL_GARMIN
tristate "USB Garmin GPS driver"
help
diff --git a/drivers/usb/serial/Makefile b/drivers/usb/serial/Makefile
index 349d9df..9e43b7b 100644
--- a/drivers/usb/serial/Makefile
+++ b/drivers/usb/serial/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_USB_SERIAL_EDGEPORT) += io_edgeport.o
 obj-$(CONFIG_USB_SERIAL_EDGEPORT_TI)   += io_ti.o
 obj-$(CONFIG_USB_SERIAL_EMPEG) += empeg.o
 obj-$(CONFIG_USB_SERIAL_F81232)+= f81232.o
+obj-$(CONFIG_USB_SERIAL_F8153X)

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-02 Thread kbuild test robot 
Hi Peter,

[auto build test ERROR on usb/usb-next -- if it's inappropriate base, please 
suggest rules for selecting the more suitable base]

url:
https://github.com/0day-ci/linux/commits/Peter-Hung/usb-serial-Add-Fintek-F81532-534-driver/20151103-115336
config: tile-allyesconfig (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=tile 

All errors (new ones prefixed by >>):

   drivers/usb/serial/f81534.c:504:1: warning: data definition has no type or 
storage class [enabled by default]
   drivers/usb/serial/f81534.c:504:1: error: type defaults to 'int' in 
declaration of 'MODULE_DEVICE_TABLE'
   drivers/usb/serial/f81534.c:504:1: warning: parameter names (without types) 
in function declaration [enabled by default]
   drivers/usb/serial/f81534.c:522:19: error: field 'f81534_gpio_chip' has 
incomplete type
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_get':
   drivers/usb/serial/f81534.c:849:4: error: dereferencing pointer to 
incomplete type
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_set':
   drivers/usb/serial/f81534.c:886:33: error: dereferencing pointer to 
incomplete type
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_request':
   drivers/usb/serial/f81534.c:926:33: error: dereferencing pointer to 
incomplete type
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_free':
   drivers/usb/serial/f81534.c:937:33: error: dereferencing pointer to 
incomplete type
   drivers/usb/serial/f81534.c: At top level:
   drivers/usb/serial/f81534.c:960:15: error: variable 
'f81534_gpio_chip_templete' has initializer but incomplete type
   drivers/usb/serial/f81534.c:961:2: error: unknown field 'owner' specified in 
initializer
   drivers/usb/serial/f81534.c:961:11: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:961:11: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:962:2: error: unknown field 'get_direction' 
specified in initializer
   drivers/usb/serial/f81534.c:962:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:962:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:963:2: error: unknown field 'get' specified in 
initializer
   drivers/usb/serial/f81534.c:963:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:963:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:964:2: error: unknown field 'direction_input' 
specified in initializer
   drivers/usb/serial/f81534.c:964:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:964:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:965:2: error: unknown field 'set' specified in 
initializer
   drivers/usb/serial/f81534.c:965:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:965:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:966:2: error: unknown field 'direction_output' 
specified in initializer
   drivers/usb/serial/f81534.c:966:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:966:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:967:2: error: unknown field 'request' specified 
in initializer
   drivers/usb/serial/f81534.c:967:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:967:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:968:2: error: unknown field 'free' specified in 
initializer
   drivers/usb/serial/f81534.c:968:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:968:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:969:2: error: unknown field 'ngpio' specified in 
initializer
   drivers/usb/serial/f81534.c:969:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:969:2: warning: (near initialization for 
'f81534_gpio_chip_templete') [enabled by default]
   drivers/usb/serial/f81534.c:970:2: error: unknown field 'base' specified in 
initializer
   drivers/usb/serial/f81534.c:970:2: warning: excess elements in struct 
initializer [enabled by default]
   drivers/usb/serial/f81534.c:970:2: warning: (near

Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-02 Thread kbuild test robot 
Hi Peter,

[auto build test ERROR on usb/usb-next -- if it's inappropriate base, please 
suggest rules for selecting the more suitable base]

url:
https://github.com/0day-ci/linux/commits/Peter-Hung/usb-serial-Add-Fintek-F81532-534-driver/20151103-115336
config: m68k-allyesconfig (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=m68k 

All error/warnings (new ones prefixed by >>):

>> drivers/usb/serial/f81534.c:504:1: warning: data definition has no type or 
>> storage class
MODULE_DEVICE_TABLE(usb, id_table);
^
>> drivers/usb/serial/f81534.c:504:1: error: type defaults to 'int' in 
>> declaration of 'MODULE_DEVICE_TABLE' [-Werror=implicit-int]
>> drivers/usb/serial/f81534.c:504:1: warning: parameter names (without types) 
>> in function declaration
>> drivers/usb/serial/f81534.c:522:19: error: field 'f81534_gpio_chip' has 
>> incomplete type
 struct gpio_chip f81534_gpio_chip;
  ^
   In file included from include/linux/list.h:8:0,
from include/linux/preempt.h:10,
from include/linux/spinlock.h:50,
from include/linux/mmzone.h:7,
from include/linux/gfp.h:5,
from include/linux/slab.h:14,
from drivers/usb/serial/f81534.c:99:
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_get':
>> drivers/usb/serial/f81534.c:849:21: error: dereferencing pointer to 
>> incomplete type
   container_of(chip->dev, struct usb_serial_port, dev);
^
   include/linux/kernel.h:811:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_set':
   drivers/usb/serial/f81534.c:887:8: error: dereferencing pointer to 
incomplete type
   chip->dev, struct usb_serial_port, dev);
   ^
   include/linux/kernel.h:811:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_request':
   drivers/usb/serial/f81534.c:927:8: error: dereferencing pointer to 
incomplete type
   chip->dev, struct usb_serial_port, dev);
   ^
   include/linux/kernel.h:811:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
   drivers/usb/serial/f81534.c: In function 'f81534_gpio_free':
   drivers/usb/serial/f81534.c:938:8: error: dereferencing pointer to 
incomplete type
   chip->dev, struct usb_serial_port, dev);
   ^
   include/linux/kernel.h:811:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
   drivers/usb/serial/f81534.c: At top level:
>> drivers/usb/serial/f81534.c:960:15: error: variable 
>> 'f81534_gpio_chip_templete' has initializer but incomplete type
static struct gpio_chip f81534_gpio_chip_templete = {
  ^
>> drivers/usb/serial/f81534.c:961:2: error: unknown field 'owner' specified in 
>> initializer
 .owner = THIS_MODULE,
 ^
   In file included from include/linux/linkage.h:6:0,
from include/linux/preempt.h:9,
from include/linux/spinlock.h:50,
from include/linux/mmzone.h:7,
from include/linux/gfp.h:5,
from include/linux/slab.h:14,
from drivers/usb/serial/f81534.c:99:
   include/linux/export.h:36:30: warning: excess elements in struct initializer
#define THIS_MODULE ((struct module *)0)
 ^
>> drivers/usb/serial/f81534.c:961:11: note: in expansion of macro 'THIS_MODULE'
 .owner = THIS_MODULE,
  ^
   include/linux/export.h:36:30: warning: (near initialization for 
'f81534_gpio_chip_templete')
#define THIS_MODULE ((struct module *)0)
 ^
>> drivers/usb/serial/f81534.c:961:11: note: in expansion of macro 'THIS_MODULE'
 .owner = THIS_MODULE,
  ^
>> drivers/usb/serial/f81534.c:962:2: error: unknown field 'get_direction' 
>> specified in initializer
 .get_direction = f81534_gpio_get_direction,
 ^
>> drivers/usb/serial/f81534.c:962:2: warning: excess elements in struct 
>> initializer
   drivers/usb/serial/f81534.c:962:2: warning: (near initialization for 
'f81534_gpio_chip_templete')
>> drivers/usb/serial/f81534.c:963:2: error: unknown field 'get' specified in 
>> initializer
 .get = f81534_gpio_get,
 ^

[PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

2015-11-02 Thread Peter Hung 
This driver is for Fintek F81532/F81534 USB to Serial Ports IC.

Features:
1. F81534 is 1-to-4 & F81532 is 1-to-2 serial ports IC
2. Support Baudrate from B50 to B150 (excluding B100).
3. The RTS signal can be transformed their behavior with
   configuration by ioctl TIOCGRS485/TIOCSRS485

   If the driver setting with SER_RS485_ENABLED, the RTS signal will
   high with not in TX and low with in TX.

   If the driver setting with SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
   the RTS signal will low with not in TX and high with in TX.

4. The 4x3 output-only open-drain pins for F81532/534 is designed for
   control outer devices (with our EVB for examples, the 4 sets of pins
   are designed to control transceiver mode). So it implements as
   gpiolib interface with output-only function.
5. It'll save the configuration in internal storage when uart/pins mode
   changed.

   Please reference https://bitbucket.org/hpeter/fintek-general/src/
   with f81534/tools to get set_gpio.c & set_mode.c to change F81532/534
   setting. Please use it carefully.

Signed-off-by: Peter Hung 
---
Changelog:
v6
1. Re-implement the write()/resume() function. Due to this device cant be
   suitable with generic write(), we'll do the submit write URB when
   write()/received tx empty/set_termios()/resume()
2. Logic/Phy Port mapping rewrite in f81534_port_probe() &
   f81534_phy_to_logic_port().
3. Introduced "Port Hide" function. Some customer use F81532 reference
   design for HW layout, but really use F81534 IC. We'll check
   F81534_PORT_CONF_DISABLE_PORT flag with in uart mode field to do
   port hide with port not used. It can be avoid end-user to use not
   layouted port.
4. The 4x3 output-only open-drain pins for F81532/534 is designed for
   control outer devices (with our EVB for examples, the 4 sets of pins
   are designed to control transceiver mode). So we decide to implement
   with gpiolib interface.
  
v5
1. Change the configure data address F81534_CUSTOM_ADDRESS_START
   from 0x4000 to 0x2f00 (for MP F/W ver:AA66)
2. Change f81534_port_disable/enable() from H/W mode to S/W mode
   It'll skip all rx data when port is not opened.
3. Some function modifier add with static (Thanks for Paul Bolle)
4. It's will direct return when count=0 in f81534_write() to
   reduce spin_lock usage.

v4
1. clearify f81534_process_read_urb() with
   f81534_process_per_serial_block(). (referenced from mxuport.c)
2. We limited f81534_write() max tx kfifo with 124Bytes.
   Original subsystem is designed for auto tranmiting fifo data
   if available. But we must wait for tx_empty for next tx data
   (H/W design).

   With this kfifo size limit, we can use generic subsystem api with
   f81534_write(). When usb_serial_generic_write_start() called after
   first write URB complete, the fifo will no data. The generic
   subsystem of write will go to idle state. Until we received
   TX_EMPTY and release write spinlock, the fifo will fill max
   124Bytes by following f81534_write().

v3
1. Migrate read, write and some routine from custom code to usbserial
   subsystem callback function.
2. Use more defines to replece magic numbers to make it meaningful
3. Make more comments as document in source code.

v2
1. v1 version submit to staging tree, but Greg KH advised me to
   cleanup source code & re-submit it to correct subsystem
2. Remove all custom ioctl commands

 drivers/usb/serial/Kconfig  |   10 +
 drivers/usb/serial/Makefile |1 +
 drivers/usb/serial/f81534.c | 2991 +++
 3 files changed, 3002 insertions(+)
 create mode 100644 drivers/usb/serial/f81534.c

diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 56ecb8b..0642864 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -255,6 +255,16 @@ config USB_SERIAL_F81232
  To compile this driver as a module, choose M here: the
  module will be called f81232.
 
+config USB_SERIAL_F8153X
+   tristate "USB Fintek F81532/534 Multi-Ports Serial Driver"
+   help
+ Say Y here if you want to use the Fintek F81532/534 Multi-Ports
+ usb to serial adapter.
+
+ To compile this driver as a module, choose M here: the
+ module will be called f81534.
+
+
 config USB_SERIAL_GARMIN
tristate "USB Garmin GPS driver"
help
diff --git a/drivers/usb/serial/Makefile b/drivers/usb/serial/Makefile
index 349d9df..9e43b7b 100644
--- a/drivers/usb/serial/Makefile
+++ b/drivers/usb/serial/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_USB_SERIAL_EDGEPORT) += io_edgeport.o
 obj-$(CONFIG_USB_SERIAL_EDGEPORT_TI)   += io_ti.o
 obj-$(CONFIG_USB_SERIAL_EMPEG) += empeg.o
 obj-$(CONFIG_USB_SERIAL_F81232)+= f81232.o