Skip to contentSkip to conversation

usb:serial: Add Fintek F81532/534 driver

PATCH In review Ji-Ze Hong (Peter Hong) 2 participants 4 messages Nov 14, 2016 – Nov 29, 2016

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v12 1/1 usb:serial: Add Fintek F81532/534 driver

This driver is for Fintek F81532/F81534 USB to Serial Ports IC.

F81532 spec:
https://drive.google.com/file/d/0B8vRwwYO7aMFOTRRMmhWQVNvajQ/view?usp=
sharing

F81534 spec:
https://drive.google.com/file/d/0B8vRwwYO7aMFV29pQWJqbVBNc00/view?usp=
sharing

Features:
1. F81532 is 1-to-2 & F81534 is 1-to-4 serial ports IC
2. Support Baudrate from B50 to B115200.

---
Changelog:
V12

    1. Max TX change from 100 to 124 bytes.
    2. Add probe() to verify endpoints & packet size.
    3. Rename function names.
         set/get_normal_register() -> set/get_register()
         set/get_register() -> set/get_port_register()
         set/get_register_delay() -> set/get_spi_register()
         read_data() -> read_flash()
         command_delay() -> wait_for_spi()

V11
1. Reduce F81534_MAX_BUS_RETRY from 2000 to 20. We are only using internal SPI bus to read flash when attach() & calc_num_ports() and never read flash when the F81532/534 in full loading, so we can reduce retry count.
2. Modify attach() & calc_num_ports() read default value only when can't find the custom setting.
3. Change tx_empty protect method from spinlock to set_bit()/ clear_bit()/test_and_clear_bit().
4. Move calculate tty_idx[i] from port_probe() to attach().
5. Add f81534_tx_empty()

V10
1. Change the submit/kill timming for read URBs, submit when first serial port open and kill when final port close.
2. Remove all source code about controlling GPIOs.
3. Change the f81534_tiocmget() from reading shadow MSR with delay 20ms to read MSR register directly.
4. Using tty_port_initialized() to check port opened/closed.
5. Add sanity check for variables.

v9
1. Remove lots of code to make more generic for F81532/534. e.g., high baud rate support, RS485/422 mode switch, most of GPIO control and internal storage write functional.
2. Change f81534_tiocmget() MSR delay from schedule_timeout_killable() to wait_for_completion_killable_timeout(). This IC will delayed receiving MSR change when doing loop-back test e.g., BurnInTest. We'll reset completion "msr_done" in f81534_update_mctrl(). If we changed MCR, the next f81534_tiocmget() will delay for 20ms or continue with new MSR arrived.
3. Fix for non-zero buffer allocated in f81534_setup_ports(). It'll make device malfunctional with incorrect tx length for other ports.

v8
1. Remove driver mode GPIOLIB & RS485 control support, the driver will only load GPIO/UART Mode when driver attach() & port_probe().
2. Add more documents for 3 generation IC with f81534_calc_num_ports().
3. Simplify the GPIO register structure "f81534_pin_control".
4. Change all counter type from int to size_t.
5. Change some failed message with failed: "status code" and remove all exclamation mark in messages.
6. Change all save blocks to block0 due to the driver is only used 1 block (block0) to save data.
7. Change read MSR in open() instead of port_probe().
8. use GFP_ATOMIC kmalloc mode in write().
9. Maintain old style with 1 read URBs and 4 write URBs like mxuports.c I had tested with submit 4 read URBs, but it'll make some port freeze when doing BurnInTest Port test.

v7
1. Make all gpiolib function with #ifdef CONFIG_GPIOLIB marco block. Due to F81532/534 could run without gpiolib, we implements f81534_prepare_gpio()/f81534_release_gpio() always success without CONFIG_GPIOLIB.
2. Fix crash when receiving MSR change on driver load/unload. It's cause by f81534_process_read_urb() get read URB callback data, but port private data is not init complete or released. We solve with 2 modifications.

1. add null pointer check with f81534_process_read_urb(). We'll skip this report when port_priv = NULL.
2. when "one" port f81534_port_remove() is called, kill the port-0 read URB before kfree port_priv.

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.
5. Add device vendor id with 0x2c42

v5
1. Change f81534_port_disable/enable() from H/W mode to S/W mode It'll skip all rx data when port is not opened.
2. Some function modifier add with static (Thanks for Paul Bolle)
3. 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 | 1410 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1421 insertions(+)
 create mode 100644 drivers/usb/serial/f81534.c
drivers/usb/serial/Kconfig +10 -0
index 56ecb8b..d9bc8da 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 +1410 -0

1,410 lines not on this page · raw message

Signature · 1 line
1.9.1
maintainer
#1 lore raw
Quoted 48 lines · Ji-Ze Hong (Peter Hong)

This driver is for Fintek F81532/F81534 USB to Serial Ports IC.

F81532 spec:
https://drive.google.com/file/d/0B8vRwwYO7aMFOTRRMmhWQVNvajQ/view?usp=
sharing

F81534 spec:
https://drive.google.com/file/d/0B8vRwwYO7aMFV29pQWJqbVBNc00/view?usp=
sharing

Features:
1. F81532 is 1-to-2 & F81534 is 1-to-4 serial ports IC
2. Support Baudrate from B50 to B115200.

---
Changelog:
V12

    1. Max TX change from 100 to 124 bytes.
    2. Add probe() to verify endpoints & packet size.
    3. Rename function names.
         set/get_normal_register() -> set/get_register()
         set/get_register() -> set/get_port_register()
         set/get_register_delay() -> set/get_spi_register()
         read_data() -> read_flash()
         command_delay() -> wait_for_spi()
drivers/usb/serial/f81534.c new file +21 -0
+static int f81534_probe(struct usb_serial *serial,
+					const struct usb_device_id *id)
+{
+	struct usb_endpoint_descriptor *endpoint;
+	struct usb_host_interface *iface_desc;
+	struct device *dev;
+	int num_bulk_in = 0;
+	int num_bulk_out = 0;
+	int size_bulk_in = 0;
+	int size_bulk_out = 0;
+	int i;
+
+	dev = &serial->interface->dev;
+	iface_desc = serial->interface->cur_altsetting;
+
+	for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
+		endpoint = &iface_desc->endpoint[i].desc;
+
+		if (usb_endpoint_is_bulk_in(endpoint)) {
+			++num_bulk_in;
+			size_bulk_in = usb_endpoint_maxp(endpoint);

drivers/usb/serial/f81534.c line 1147

Quoted 10 lines
drivers/usb/serial/f81534.c new file +10 -0
+		}
+
+		if (usb_endpoint_is_bulk_out(endpoint)) {
+			++num_bulk_out;
+			size_bulk_out = usb_endpoint_maxp(endpoint);
+		}
+	}
+
+	if (num_bulk_in != 1 || num_bulk_out != 1) {
+		dev_err(dev, "%s: endpoints not matched\n", __func__);

Better to spell out: "expected endpoints not found\n".

Review

Quoted 1 line
(quoted diff) +1 -0
+		return -EINVAL;

And this should be -ENODEV;

drivers/usb/serial/f81534.c line 1154

Quoted 6 lines
drivers/usb/serial/f81534.c new file +6 -0
+	}
+
+	if (size_bulk_out != F81534_WRITE_BUFFER_SIZE ||
+			size_bulk_in != F81534_MAX_RECEIVE_BLOCK_SIZE) {
+		dev_err(dev, "%s: endpoints packet size not matched\n",
+				__func__);

Similarly: "unsupported endpoint max packet size\n".

But just to be clear: You do want to bail out if connected at full speed? You could also ask usb-serial core to allocate large enough buffers (e.g. by setting the bulk_out_size driver field) and the host controller will handle partitioning.

Review

Quoted 1 line
(quoted diff) +1 -0
+		return -EINVAL;

And -ENODEV.

drivers/usb/serial/f81534.c line 384

Quoted 4 lines
drivers/usb/serial/f81534.c new file +4 -0
+	}
+
+	return 0;
+}

Thanks,
Johan

#2 lore raw

Hi Johan,

Johan Hovold 於 2016/11/24 下午 11:04 寫道:

Quoted 12 lines · Ji-Ze Hong (Peter Hong)

drivers/usb/serial/f81534.c line 1154

Quoted 8 lines

This driver is for Fintek F81532/F81534 USB to Serial Ports IC.

drivers/usb/serial/f81534.c new file +6 -0
+	}
+
+	if (size_bulk_out != F81534_WRITE_BUFFER_SIZE ||
+			size_bulk_in != F81534_MAX_RECEIVE_BLOCK_SIZE) {
+		dev_err(dev, "%s: endpoints packet size not matched\n",
+				__func__);

Similarly: "unsupported endpoint max packet size\n".

Quoted 4 lines

But just to be clear: You do want to bail out if connected at full speed? You could also ask usb-serial core to allocate large enough buffers (e.g. by setting the bulk_out_size driver field) and the host controller will handle partitioning.

Yes, this product cannot run on USB full-speed currently due to the bulk-out endpoint design. It need use 512 bytes packet size to transmit 4 ports TX data, but USB Full-speed only provide 64 byte. The F81534 will treat the all bulk-out data as port 0 although the host will spilt 512byte to 8 x 64bytes packets.

Signature · 2 lines
With Best Regards,
Peter Hong
maintainer
#3 lore raw
Quoted 20 lines · Ji-Ze Hong (Peter Hong)

Hi Johan,

Johan Hovold 於 2016/11/24 下午 11:04 寫道:

Quoted 12 lines · Ji-Ze Hong (Peter Hong)

drivers/usb/serial/f81534.c line 1154

Quoted 8 lines

This driver is for Fintek F81532/F81534 USB to Serial Ports IC.

drivers/usb/serial/f81534.c new file +6 -0
+	}
+
+	if (size_bulk_out != F81534_WRITE_BUFFER_SIZE ||
+			size_bulk_in != F81534_MAX_RECEIVE_BLOCK_SIZE) {
+		dev_err(dev, "%s: endpoints packet size not matched\n",
+				__func__);

Similarly: "unsupported endpoint max packet size\n".

Quoted 4 lines

But just to be clear: You do want to bail out if connected at full speed? You could also ask usb-serial core to allocate large enough buffers (e.g. by setting the bulk_out_size driver field) and the host controller will handle partitioning.

Quoted 5 lines

Yes, this product cannot run on USB full-speed currently due to the bulk-out endpoint design. It need use 512 bytes packet size to transmit 4 ports TX data, but USB Full-speed only provide 64 byte. The F81534 will treat the all bulk-out data as port 0 although the host will spilt 512byte to 8 x 64bytes packets.

Ok, thanks for confirming. I'll just fix up the minor comments I had on the new probe function before applying for 4.10 (so no need to resend).

Thanks for sticking with it.

Johan