Skip to contentSkip to conversation

usb:serial:f81534 Add F81532/534 Driver

PATCH Superseded Peter Hung 2 participants 6 messages Jun 15, 2015 – Jul 6, 2015

Subsystems (2)

v2 of 12 Superseded Latest is v12 →

lore
Share & tools
lore

Peter Hung

author
#0 lore raw

PATCH v2 1/1 usb:serial:f81534 Add F81532/534 Driver

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 B1500000 (excluding B1000000).
3. The RTS signal can be transformed their behavior with configuration for transceiver (for RS232/RS485/RS422) (/sys/class/ttyUSBx/uart_mode)
4. There are 4x3 output-only GPIOs to control transceiver mode. It's can be controlled via sysfs (/sys/class/ttyUSBx/gpio)

Changelog from v1:
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

If had any question, Please send email to
[email protected]
[email protected]

---
 drivers/usb/serial/Kconfig  |   10 +
 drivers/usb/serial/Makefile |    1 +
 drivers/usb/serial/f81534.c | 3162 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 3173 insertions(+)
 create mode 100644 drivers/usb/serial/f81534.c
drivers/usb/serial/Kconfig +10 -0
index b7cf198..4bf3011 100644
@@ -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
drivers/usb/serial/Makefile +1 -0
index 349d9df..9e43b7b 100644
@@ -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)			+= f81534.o
 obj-$(CONFIG_USB_SERIAL_FTDI_SIO)		+= ftdi_sio.o
 obj-$(CONFIG_USB_SERIAL_GARMIN)			+= garmin_gps.o
 obj-$(CONFIG_USB_SERIAL_IPAQ)			+= ipaq.o
drivers/usb/serial/f81534.c new file +3162 -0

2,977 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line
1.9.1

Part of the diff is not shown here. View the raw message.

author
#1 lore raw

Hello Johan,

Peter Hung 於 2015/6/15 上午 09:54 寫道:

Quoted 10 lines

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 B1500000 (excluding B1000000).
3. The RTS signal can be transformed their behavior with configuration

    for transceiver (for RS232/RS485/RS422) (/sys/class/ttyUSBx/uart_mode)

4. There are 4x3 output-only GPIOs to control transceiver mode. It's can be controlled via sysfs (/sys/class/ttyUSBx/gpio)

Do you receive my patch?
Are there anything should I do to improve it ?

Signature · 2 lines
With Best Regards,
Peter Hung
maintainer
#2 lore raw

Hi Peter,

Quoted 14 lines · Peter Hung

Hello Johan,

Peter Hung 於 2015/6/15 上午 09:54 寫道:

Quoted 10 lines

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 B1500000 (excluding B1000000).
3. The RTS signal can be transformed their behavior with configuration

    for transceiver (for RS232/RS485/RS422) (/sys/class/ttyUSBx/uart_mode)

4. There are 4x3 output-only GPIOs to control transceiver mode. It's can be controlled via sysfs (/sys/class/ttyUSBx/gpio)

Quoted 1 line

Do you receive my patch?

Yes, I did. I just haven't had time to review it yet.

Quoted 1 line

Are there anything should I do to improve it ?

There are, including

- your custom read and write implementations look odd, you should be able to reuse a lot more of the generic framework
- you'll also need to implement open/and close in some way (enable/disable in hardware or flag in software) as you should not push data to a closed tty
- stack allocated buffers used for DMA (register accessors)
- DMA buffers allocated as part of larger struct (read/write buffers)
- lots of magic constants (e.g. use defines for the registers)
- As Greg already mentioned, you need to implement gpio support using gpiolib, not a custom sysfs interface

I don't have time to look closer at the architectural bits until next week I'm afraid, but perhaps you could start with the above.

Thanks,
Johan

author
#3 lore raw

Hi Johan,

Johan Hovold 於 2015/6/25 下午 04:06 寫道:

Quoted 5 lines

- As Greg already mentioned, you need to implement gpio support using gpiolib, not a custom sysfs interface

I don't have time to look closer at the architectural bits until next week I'm afraid, but perhaps you could start with the above.

Thanks for your advices, I'll try to improve it again. But I had something need to clarify.

1. The sysfs interface of the driver provide "limited output pin" control. only support output 0/1, not support input mode. Should I implement it as gpiolib?

2. If it still recommends to implement with gpiolib, Could I implement it and preserve the sysfs interface for legacy?

Signature · 2 lines
With Best Regards,
Peter Hung
maintainer
#4 lore raw
Quoted 9 lines · Peter Hung

Hi Johan,

Johan Hovold 於 2015/6/25 下午 04:06 寫道:

Quoted 5 lines

- As Greg already mentioned, you need to implement gpio support using gpiolib, not a custom sysfs interface

I don't have time to look closer at the architectural bits until next week I'm afraid, but perhaps you could start with the above.

Quoted 6 lines

Thanks for your advices, I'll try to improve it again. But I had something need to clarify.

1. The sysfs interface of the driver provide "limited output pin" control. only support output 0/1, not support input mode. Should I implement it as gpiolib?

Yes.

Quoted 2 lines

2. If it still recommends to implement with gpiolib, Could I implement it and preserve the sysfs interface for legacy?

No, I'm afraid not. We don't want to have to maintain out-of-tree legacy interfaces.

Johan

maintainer
#5 lore raw
In reply to Johan Hovold
Quoted 36 lines · Johan Hovold

Hi Peter,

Quoted 14 lines · Peter Hung

Hello Johan,

Peter Hung 於 2015/6/15 上午 09:54 寫道:

Quoted 10 lines

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 B1500000 (excluding B1000000).
3. The RTS signal can be transformed their behavior with configuration

    for transceiver (for RS232/RS485/RS422) (/sys/class/ttyUSBx/uart_mode)

4. There are 4x3 output-only GPIOs to control transceiver mode. It's can be controlled via sysfs (/sys/class/ttyUSBx/gpio)

Quoted 1 line

Do you receive my patch?

Yes, I did. I just haven't had time to review it yet.

Quoted 1 line

Are there anything should I do to improve it ?

There are, including

- your custom read and write implementations look odd, you should be able to reuse a lot more of the generic framework
- you'll also need to implement open/and close in some way (enable/disable in hardware or flag in software) as you should not push data to a closed tty
- stack allocated buffers used for DMA (register accessors)
- DMA buffers allocated as part of larger struct (read/write buffers)
- lots of magic constants (e.g. use defines for the registers)
- As Greg already mentioned, you need to implement gpio support using gpiolib, not a custom sysfs interface

Quoted 2 lines

I don't have time to look closer at the architectural bits until next week I'm afraid, but perhaps you could start with the above.

I took a closer look at the protocol, and you should definitely be able to reuse more of the generic implementation, at least for the read path.

You should also make sure to document the bulk-message layout somewhere in the driver. Do you always have to send full 512-byte messages?

Use the read-urbs already allocated by usb-serial core, and take a look at how this is handled in mxuport (e.g. submit port0 read urbs at attach, and demultiplex in the process_read_urb callback). Your message-parsing code needs to be cleaned up.

For the write-path, you at least need to use a fifo per port (usb-serial core will have allocated one for the first port, again see mxuport). Depending on the firmware implementation you may be able to reuse the generic implementation as mxuport does, or you need to keep a custom implementation. Could you elaborate on what buffers you have in the firmware? What happens if you submit data for a port before receiving a "tx empty" notification over the bulk-in endpoints?

Also make sure to remove all unused code (e.g. the READ_AND_SET macro) before resubmitting.

Thanks,
Johan