usb:serial: Add Fintek F81532/534 driver
Subsystems (2)
PATCH v11 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:
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 | 1380 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1391 insertions(+) create mode 100644 drivers/usb/serial/f81534.c
drivers/usb/serial/Kconfig +10 -0
@@ -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
@@ -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 +1380 -0
1,380 lines not on this page · raw message
Signature · 1 line
1.9.1
Johan Hovold
maintainerQuoted 10 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=
sharingF81534 spec:
https://drive.google.com/file/d/0B8vRwwYO7aMFV29pQWJqbVBNc00/view?usp=
sharing
Quoted 5 lines
Features:
1. F81532 is 1-to-2 & F81534 is 1-to-4 serial ports IC
2. Support Baudrate from B50 to B115200.Reviewed-by Johan Hovold <[email protected]>
You must never add other peoples' Reviewed-by tags unless you've explicitly been given permission to do so (e.g. "fix this minor thing up and then you can add...").
Please make sure to read the section about Reviewed-by tags in Documentation/SubmittingPatches.
Quoted 8 lines
Signed-off-by Ji-Ze Hong (Peter Hong) <[email protected]>---
Changelog:
V11
1. ReduceF81534_MAX_BUS_RETRYfrom 2000 to 20. We are only using internal SPI bus to read flash whenattach()&calc_num_ports()and never read flash when the F81532/534 in full loading, so we can reduce retry count.
Does this mean you can remove that retry mechanism completely?
drivers/usb/serial/f81534.c line 73
Quoted 12 lines
2. Modify
attach()&calc_num_ports()read default value only when can't find the custom setting.
3. Changetx_emptyprotect method from spinlock to set_bit()/ clear_bit()/test_and_clear_bit().
4. Move calculate tty_idx[i] fromport_probe()toattach().
5. Addf81534_tx_empty()drivers/usb/serial/f81534.c new file +5
-0+#define DRIVER_DESC "Fintek F81532/F81534" +#define FINTEK_VENDOR_ID_1 0x1934 +#define FINTEK_VENDOR_ID_2 0x2C42 +#define FINTEK_DEVICE_ID 0x1202 +#define F81534_MAX_TX_SIZE 100
You never replies to my question about why this is not 124 as for rx.
Quoted 29 lines
drivers/usb/serial/f81534.c new file +29
-0+#define F81534_MAX_RX_SIZE 124 +#define F81534_RECEIVE_BLOCK_SIZE 128 + +#define F81534_TOKEN_RECEIVE 0x01 +#define F81534_TOKEN_WRITE 0x02 +#define F81534_TOKEN_TX_EMPTY 0x03 +#define F81534_TOKEN_MSR_CHANGE 0x04 + +/* +* We used interal SPI bus to access FLASH section. We must wait the SPI bus to +* idle if we performed any command. +* +* SPI Bus status register: F81534_BUS_REG_STATUS +* Bit 0/1 : BUSY +* Bit 2 : IDLE +*/ +#define F81534_BUS_BUSY (BIT(0) | BIT(1)) +#define F81534_BUS_IDLE BIT(2) +#define F81534_BUS_READ_DATA 0x1004 +#define F81534_BUS_REG_STATUS 0x1003 +#define F81534_BUS_REG_START 0x1002 +#define F81534_BUS_REG_END 0x1001 + +#define F81534_CMD_READ 0x03 + +#define F81534_DEFAULT_BAUD_RATE 9600 +#define F81534_MAX_BAUDRATE 115200 + +#define F81534_PORT_CONF_DISABLE_PORT BIT(3)
drivers/usb/serial/f81534.c line 112
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+#define F81534_PORT_CONF_NOT_EXIST_PORT BIT(7) +#define F81534_PORT_UNAVAILABLE \ + (F81534_PORT_CONF_DISABLE_PORT | F81534_PORT_CONF_NOT_EXIST_PORT) + +#define F81534_1X_RXTRIGGER 0xc3 +#define F81534_8X_RXTRIGGER 0xcf + +static const struct usb_device_id f81534_id_table[] = { + {USB_DEVICE (FINTEK_VENDOR_ID_1 ,FINTEK_DEVICE_ID )}, + {USB_DEVICE (FINTEK_VENDOR_ID_2 ,FINTEK_DEVICE_ID )},
Add a space after { and before }.
Quoted 14 lines
drivers/usb/serial/f81534.c new file +14
-0+ {}/* Terminating entry */ +}; + +#define F81534_TX_EMPTY_BIT 0 + +struct f81534_serial_private { +u8 conf_data[F81534_DEF_CONF_SIZE ]; +int tty_idx[F81534_NUM_PORT ]; +u8 setting_idx; +int opened_port; +struct mutex urb_mutex; +}; + +struct f81534_port_private {
drivers/usb/serial/f81534.c line 156
Quoted 10 lines
drivers/usb/serial/f81534.c new file +8
-0+struct mutex mcr_mutex; +unsigned long tx_empty; +spinlock_t msr_lock; +u8 shadow_mcr; +u8 shadow_msr; +u8 phy_num; +}; +static int f81534_set_normal_register(struct usb_serial *serial, u16 reg,
What do mean by "normal" here? Could you give this a more descriptive name?
Perhaps just call this one f81534_set_register and add a "port" or "uart" infix to the current f81534_set_register below (e.g. rename it f81534_set_uart_register, and similar for get).
Or simply replace "normal" with "generic" above.
Quoted 52 lines
drivers/usb/serial/f81534.c new file +51
-0+ u8 data) +{ +struct usb_interface *interface = serial->interface; +struct usb_device *dev = serial->dev; +size_t count =F81534_USB_MAX_RETRY ; +int status; +u8 *tmp; + + tmp =kmalloc (sizeof (u8),GFP_KERNEL ); +if (!tmp) +return -ENOMEM ; + + *tmp = data; + +/* +* Our device maybe not reply when heavily loading, We'll retry for +* F81534_USB_MAX_RETRY times. +*/ +while (count--) { + status =usb_control_msg (dev,usb_sndctrlpipe (dev,0 ), +F81534_SET_GET_REGISTER , +USB_TYPE_VENDOR |USB_DIR_OUT , + reg,0 , tmp,sizeof (u8), +F81534_USB_TIMEOUT ); +if (status >0 ) { + status =0 ; +break ; + }else if (status ==0 ) { + status = -EIO ; + } + } + +if (status <0 ) { +dev_err (&interface->dev,"%s: reg: %x data: %x failed: %d\n" , + __func__, reg, data, status); + } + +kfree (tmp); +return status; +} +static int f81534_set_register (struct usb_serial_port *port,u16 reg,u8 data) +{ +struct f81534_port_private *port_priv =usb_get_serial_port_data (port); + +return f81534_set_normal_register (port->serial, + reg + port_priv->phy_num *F81534_UART_OFFSET , data); +} + +static int f81534_get_register (struct usb_serial_port *port,u16 reg,u8 *data) +{ +struct f81534_port_private *port_priv =usb_get_serial_port_data (port);
drivers/usb/serial/f81534.c line 262
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+ +return f81534_get_normal_register (port->serial, + reg + port_priv->phy_num *F81534_UART_OFFSET , data); +} + +/* +* If we try to access the internal flash via SPI bus, we should check the bus +* status for every command. e.g., F81534_BUS_REG_START/F81534_BUS_REG_END +*/ +static int f81534_command_delay (struct usb_serial *serial)
Can you give this function a more descriptive name as well, perhaps f81534_wait_for_spi_idle()?
Quoted 12 lines
drivers/usb/serial/f81534.c new file +12
-0+{ +size_t count =F81534_MAX_BUS_RETRY ; +u8 tmp; +int status; + + do { + status = f81534_get_normal_register(serial, + F81534_BUS_REG_STATUS, + &tmp); + if (status) + return status; +
drivers/usb/serial/f81534.c line 284
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+if (tmp &F81534_BUS_BUSY ) +continue ; + +if (tmp &F81534_BUS_IDLE ) +break ; + + }while (--count); + +if (!count) { + dev_err(&serial->interface->dev, "%s: retry exceed than %d\n",
Please rewrite this error message. Perhaps "timed out waiting for idle bus" or something similar.
Quoted 4 lines
drivers/usb/serial/f81534.c new file +4
-0+ __func__, F81534_MAX_BUS_RETRY); +return -EIO ; + } +
drivers/usb/serial/f81534.c line 298
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+ status =f81534_set_normal_register (serial,F81534_BUS_REG_STATUS , + tmp & ~F81534_BUS_IDLE ); +if (status) +return status; + +return 0 ; +} + +static int f81534_get_normal_register_with_delay (struct usb_serial *serial, +u16 reg,u8 *data)
And then call these f81534_get_spi_register?
drivers/usb/serial/f81534.c line 308
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+{ +int status; + + status =f81534_get_normal_register (serial, reg, data); +if (status) +return status; + + status =f81534_command_delay (serial); +if (status) +return status;
Why do you need a delay after reading?
Quoted 12 lines
drivers/usb/serial/f81534.c new file +12
-0+ +return 0 ; +} + +static int f81534_set_normal_register_with_delay (struct usb_serial *serial, +u16 reg,u8 data) +{ +int status; + + status =f81534_set_normal_register (serial, reg, data); +if (status) +return status;
drivers/usb/serial/f81534.c line 330
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+ + status =f81534_command_delay (serial); +if (status) +return status; + +return 0 ; +} + +static int f81534_read_data (struct usb_serial *serial,u32 address, +size_t size,u8 *buf)
And call this f81534_read_flash (or read_spi) which is more descriptive.
Quoted 59 lines
drivers/usb/serial/f81534.c new file +59
-0+{ +u8 tmp_buf[F81534_MAX_DATA_BLOCK ]; +size_t block =0 ; +size_t read_size; +size_t count; +int status; +int offset; +u16 reg_tmp; + + status =f81534_set_normal_register_with_delay (serial, +F81534_BUS_REG_START ,F81534_CMD_READ ); +if (status) +return status; + + status =f81534_set_normal_register_with_delay (serial, +F81534_BUS_REG_START , (address >>16 ) &0xff ); +if (status) +return status; + + status =f81534_set_normal_register_with_delay (serial, +F81534_BUS_REG_START , (address >>8 ) &0xff ); +if (status) +return status; + + status =f81534_set_normal_register_with_delay (serial, +F81534_BUS_REG_START , (address >>0 ) &0xff ); +if (status) +return status; + +/* Continuous read mode */ + do { + read_size = min_t(size_t, F81534_MAX_DATA_BLOCK, size); + + for (count = 0; count < read_size; ++count) { + /* To write F81534_BUS_REG_END when final byte */ + if (size <= F81534_MAX_DATA_BLOCK && + read_size == count + 1) + reg_tmp = F81534_BUS_REG_END; + else + reg_tmp = F81534_BUS_REG_START; + + /* + * Dummy code, force IC to generate a read pulse, the + * set of value 0xf1 is dont care (any value is ok) + */ + status = f81534_set_normal_register_with_delay(serial, + reg_tmp, 0xf1); + if (status) + return status; + + status = f81534_get_normal_register_with_delay(serial, + F81534_BUS_READ_DATA, + &tmp_buf[count]); + if (status) + return status; + + offset = count + block * F81534_MAX_DATA_BLOCK; + buf[offset] = tmp_buf[count]; + }
drivers/usb/serial/f81534.c line 399
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+ + size -= read_size; + ++block; + }while (size); + +return 0 ; +} + +static void f81534_prepare_write_buffer (struct usb_serial_port *port,u8 *buf, +size_t size)
You never use size in this function. You need to make sure you never overwrite the provided buffer using some sanity checks.
Quoted 43 lines
drivers/usb/serial/f81534.c new file +43
-0+{ +struct f81534_port_private *port_priv =usb_get_serial_port_data (port); +int phy_num = port_priv->phy_num; +u8 tx_len; +int i; + +/* +* The block layout is fixed with 4x128 Bytes, per 128 Bytes a port. +* index 0: port phy idx (e.g., 0,1,2,3) +* index 1: only F81534_TOKEN_WRITE +* index 2: serial TX out length +* index 3: fix to 0 +* index 4~127: serial out data block +*/ +for (i =0 ; i <F81534_NUM_PORT ; ++i) { + buf[i *F81534_RECEIVE_BLOCK_SIZE ] = i; + buf[i *F81534_RECEIVE_BLOCK_SIZE +1 ] =F81534_TOKEN_WRITE ; + buf[i *F81534_RECEIVE_BLOCK_SIZE +2 ] =0 ; + buf[i *F81534_RECEIVE_BLOCK_SIZE +3 ] =0 ; + } + + tx_len =kfifo_out_locked (&port->write_fifo, + &buf[phy_num *F81534_RECEIVE_BLOCK_SIZE +4 ], +F81534_MAX_TX_SIZE , &port->lock); + + buf[phy_num *F81534_RECEIVE_BLOCK_SIZE +2 ] = tx_len; +} + +static int f81534_submit_writer (struct usb_serial_port *port,gfp_t mem_flags) +{ +struct f81534_port_private *port_priv =usb_get_serial_port_data (port); +struct urb *urb; +unsigned long flags; +int result; + +/* Check is any data in write_fifo */ +spin_lock_irqsave (&port->lock, flags); + +if (kfifo_is_empty (&port->write_fifo)) { +spin_unlock_irqrestore (&port->lock, flags); +return 0 ; + } +
drivers/usb/serial/f81534.c line 452
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+spin_unlock_irqrestore (&port->lock, flags); + +/* Check H/W is TXEMPTY */ +if (!test_and_clear_bit (F81534_TX_EMPTY_BIT , &port_priv->tx_empty)) +return 0 ; + + urb = port->write_urbs[0 ]; +f81534_prepare_write_buffer (port, port->bulk_out_buffers[0 ], + port->bulk_out_size); + urb->transfer_buffer_length =F81534_WRITE_BUFFER_SIZE ;
You need to make sure the buffers have the expected size. They are currently set to the endpoint size, but you can you can make sure they are never smaller than F81534_WRITE_BUFFER_SIZE by setting bulk_out_size in the usb_serial_driver struct.
Quoted 16 lines
drivers/usb/serial/f81534.c new file +15
-0+ + result =usb_submit_urb (urb, mem_flags); +if (result) { +set_bit (F81534_TX_EMPTY_BIT , &port_priv->tx_empty); +dev_err (&port->dev,"%s: submit failed: %d\n" , __func__, + result); +return result; + } + +usb_serial_port_softint (port); +return 0 ; +} +static int f81534_update_mctrl (struct usb_serial_port *port,unsigned int set, +unsigned int clear) +{
drivers/usb/serial/f81534.c line 547
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+struct f81534_port_private *port_priv =usb_get_serial_port_data (port); +int status; +u8 tmp; + +mutex_lock (&port_priv->mcr_mutex); + +if (((set | clear) & (TIOCM_DTR |TIOCM_RTS )) ==0 ) { +mutex_unlock (&port_priv->mcr_mutex); +return 0 ;/* no change */ + }
No need to hold the mutex for the above check.
Quoted 47 lines
drivers/usb/serial/f81534.c new file +46
-0+ +/* 'Set' takes precedence over 'Clear' */ + clear &= ~set; + +/* Always enable UART_MCR_OUT2 */ + tmp =UART_MCR_OUT2 | port_priv->shadow_mcr; + +if (clear &TIOCM_DTR ) + tmp &= ~UART_MCR_DTR ; + +if (clear &TIOCM_RTS ) + tmp &= ~UART_MCR_RTS ; + +if (set &TIOCM_DTR ) + tmp |=UART_MCR_DTR ; + +if (set &TIOCM_RTS ) + tmp |=UART_MCR_RTS ; + + status =f81534_set_register (port,F81534_MODEM_CONTROL_REG , tmp); +if (status <0 ) { +dev_err (&port->dev,"%s: MCR write failed\n" , __func__); +mutex_unlock (&port_priv->mcr_mutex); +return status; + } + + port_priv->shadow_mcr = tmp; +mutex_unlock (&port_priv->mcr_mutex); +return 0 ; +} +static int f81534_open (struct tty_struct *tty,struct usb_serial_port *port) +{ +struct f81534_serial_private *serial_priv = +usb_get_serial_data (port->serial); +struct f81534_port_private *port_priv =usb_get_serial_port_data (port); +int status; + + status =f81534_set_register (port, +F81534_FIFO_CONTROL_REG ,UART_FCR_ENABLE_FIFO | +UART_FCR_CLEAR_RCVR |UART_FCR_CLEAR_XMIT ); +if (status) { +dev_err (&port->dev,"%s: Clear FIFO failed: %d\n" , __func__, + status); +return status; + } +
drivers/usb/serial/f81534.c
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+if (tty) +f81534_set_termios (tty, port,NULL ); + + status =f81534_read_msr (port); +if (status) +return status; + +mutex_lock (&serial_priv->urb_mutex); + +set_bit (F81534_TX_EMPTY_BIT , &port_priv->tx_empty);
Move this outside of the locked section.
Quoted 22 lines
drivers/usb/serial/f81534.c new file +21
-0+ +/* Submit Read URBs for first port opened */ +if (!serial_priv->opened_port) { + status =f81534_submit_read_urb (port->serial,GFP_KERNEL ); +if (status) + goto exit; + } + + serial_priv->opened_port++; + +exit: +mutex_unlock (&serial_priv->urb_mutex); + +return status; +} +static void f81534_process_per_serial_block (struct usb_serial_port *port, +u8 *data) +{ +struct f81534_port_private *port_priv =usb_get_serial_port_data (port); +int phy_num = data[0 ]; +size_t read_size =0 ;
drivers/usb/serial/f81534.c line 948
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+size_t i; +char tty_flag; +int status; +u8 lsr; + +if (phy_num >=F81534_NUM_PORT ) { +dev_err (&port->dev,"%s: phy_num: %d larger than: %d\n" , + __func__, phy_num,F81534_NUM_PORT ); +return ; + }
You already verified this in f81534_process_read_urb() below.
Quoted 112 lines
drivers/usb/serial/f81534.c new file +112
-0+ +/* +* The block layout is 128 Bytes +* index 0: port phy idx (e.g., 0,1,2,3), +* index 1: It's could be +* F81534_TOKEN_RECEIVE +* F81534_TOKEN_TX_EMPTY +* F81534_TOKEN_MSR_CHANGE +* index 2: serial in size (data+lsr, must be even) +* meaningful for F81534_TOKEN_RECEIVE only +* index 3: current MSR with this device +* index 4~127: serial in data block (data+lsr, must be even) +*/ +switch (data[1 ]) { +case F81534_TOKEN_TX_EMPTY : +set_bit (F81534_TX_EMPTY_BIT , &port_priv->tx_empty); + +/* Try to submit writer */ + status =f81534_submit_writer (port,GFP_ATOMIC ); +if (status) +dev_err (&port->dev,"%s: submit failed\n" , __func__); +return ; + +case F81534_TOKEN_MSR_CHANGE : +f81534_msr_changed (port, data[3 ]); +return ; + +case F81534_TOKEN_RECEIVE : + read_size = data[2 ]; +if (read_size >F81534_MAX_RX_SIZE ) { +dev_err (&port->dev, +"%s: phy: %d read_size: %zu larger than: %d\n" , + __func__, phy_num, read_size, +F81534_MAX_RX_SIZE ); +return ; + } + +break ; + +default : +dev_warn (&port->dev,"%s: unknown token: %02x\n" , __func__, + data[1 ]); +return ; + } + +for (i =4 ; i <4 + read_size; i +=2 ) { + tty_flag =TTY_NORMAL ; + lsr = data[i +1 ]; + +if (lsr &UART_LSR_BRK_ERROR_BITS ) { +if (lsr &UART_LSR_BI ) { + tty_flag =TTY_BREAK ; + port->icount.brk++; +usb_serial_handle_break (port); + }else if (lsr &UART_LSR_PE ) { + tty_flag =TTY_PARITY ; + port->icount.parity++; + }else if (lsr &UART_LSR_FE ) { + tty_flag =TTY_FRAME ; + port->icount.frame++; + } + +if (lsr &UART_LSR_OE ) { + port->icount.overrun++; +tty_insert_flip_char (&port->port,0 , +TTY_OVERRUN ); + } + } + +if (port->port.console && port->sysrq) { +if (usb_serial_handle_sysrq_char (port, data[i])) +continue ; + } + +tty_insert_flip_char (&port->port, data[i], tty_flag); + } + +tty_flip_buffer_push (&port->port); +} + +static void f81534_process_read_urb (struct urb *urb) +{ +struct f81534_serial_private *serial_priv; +struct usb_serial_port *port; +struct usb_serial *serial; +u8 *buf; +int phy_port_num; +int tty_port_num; +size_t i; + +if (!urb->actual_length || + urb->actual_length %F81534_RECEIVE_BLOCK_SIZE ) { +return ; + } + + port = urb->context; + serial = port->serial; + buf = urb->transfer_buffer; + serial_priv =usb_get_serial_data (serial); + +for (i =0 ; i < urb->actual_length; i +=F81534_RECEIVE_BLOCK_SIZE ) { + phy_port_num = buf[i]; +if (phy_port_num >=F81534_NUM_PORT ) { +dev_err (&port->dev, +"%s: phy_port_num: %d larger than: %d\n" , + __func__, phy_port_num,F81534_NUM_PORT ); +continue ; + } + + tty_port_num = serial_priv->tty_idx[phy_port_num]; + port = serial->port[tty_port_num]; +
drivers/usb/serial/f81534.c
Quoted 10 lines
drivers/usb/serial/f81534.c new file +9
-0+if (tty_port_initialized (&port->port)) +f81534_process_per_serial_block (port, &buf[i]); + } +} +static bool f81534_tx_empty (struct usb_serial_port *port) +{ +struct f81534_port_private *port_priv =usb_get_serial_port_data (port); + +return !!test_bit (F81534_TX_EMPTY_BIT , &port_priv->tx_empty);
!! not needed.
Quoted 25 lines
drivers/usb/serial/f81534.c new file +25
-0+} + +static int f81534_resume (struct usb_serial *serial) +{ +struct f81534_serial_private *serial_priv = +usb_get_serial_data (serial); +struct usb_serial_port *port; +int status; +size_t i; + +/* +* We'll register port 0 bulkin when port had opened, It'll take all +* port received data, MSR register change and TX_EMPTY information. +*/ +mutex_lock (&serial_priv->urb_mutex); + +if (serial_priv->opened_port) { + status =f81534_submit_read_urb (serial,GFP_NOIO ); +if (status) { +mutex_unlock (&serial_priv->urb_mutex); +return status; + } + } + +mutex_unlock (&serial_priv->urb_mutex);
drivers/usb/serial/f81534.c line 1339
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+ +for (i =0 ; i < serial->num_ports; i++) { + port = serial->port[i]; +if (!tty_port_initialized (&port->port)) +continue ; + + status =f81534_submit_writer (port,GFP_NOIO ); +if (status) { +dev_err (&port->dev,"%s: submit failed\n" , __func__); +return status;
You may still want to continue with the remaining ports as you did in v10 (but return an error in the end).
Quoted 9 lines
drivers/usb/serial/f81534.c new file +9
-0+ } + } + +return 0 ; +} + +static struct usb_serial_driver f81534_device = { + .driver = { + .owner =THIS_MODULE ,
drivers/usb/serial/f81534.c line 1358
Quoted 10 lines
drivers/usb/serial/f81534.c new file +10
-0+ .name = "f81534", + }, + .description = DRIVER_DESC, + .id_table = f81534_id_table, + .open = f81534_open, + .close = f81534_close, + .write = f81534_write, + .tx_empty = f81534_tx_empty, + .calc_num_ports = f81534_calc_num_ports, + .attach = f81534_attach,
You need to add a .probe function in which you make sure that you have the expected bulk-in and out endpoints (or you could crash later on).
You could also verify the endpoint size in this function instead of setting the .bulk_out_size field mentioned above.
Thanks,
Johan
Hi Johan,
Johan Hovold 於 2016/11/2 下午 08:37 寫道:
Quoted 10 lines · Ji-Ze Hong (Peter Hong)
Quoted 2 lines
Reviewed-by Johan Hovold <[email protected]>You must never add other peoples' Reviewed-by tags unless you've explicitly been given permission to do so (e.g. "fix this minor thing up and then you can add...").
Please make sure to read the section about Reviewed-by tags in
Documentation/SubmittingPatches.
Sorry for misusing "Reviewed-by" tags.
Quoted 8 lines
Quoted 6 lines
Changelog:
V11
1. ReduceF81534_MAX_BUS_RETRYfrom 2000 to 20. We are only using internal SPI bus to read flash whenattach()&calc_num_ports()and never read flash when the F81532/534 in full loading, so we can reduce retry count.Does this mean you can remove that retry mechanism completely?
The mechanism is for checking SPI Bus busy status. We need to perform read/write operation only when the bus idle. So we need remain the check mechanism.
Quoted 8 lines
Quoted 7 lines
2. Modify
attach()&calc_num_ports()read default value only when can't find the custom setting.
3. Changetx_emptyprotect method from spinlock to set_bit()/ clear_bit()/test_and_clear_bit().
4. Move calculate tty_idx[i] fromport_probe()toattach().
5. Addf81534_tx_empty()
Quoted 7 lines
drivers/usb/serial/f81534.c line 73
Quoted 5 lines
drivers/usb/serial/f81534.c new file +5
-0+#define DRIVER_DESC "Fintek F81532/F81534" +#define FINTEK_VENDOR_ID_1 0x1934 +#define FINTEK_VENDOR_ID_2 0x2C42 +#define FINTEK_DEVICE_ID 0x1202 +#define F81534_MAX_TX_SIZE 100You never replies to my question about why this is not 124 as for rx.
The limit for TX with 100 bytes is tuned for high-speed TX performance. But this patch is not contained for high-baudrate, so we'll change it for 124 byte with next patch.
Quoted 11 lines
Quoted 3 lines
+static int f81534_set_normal_register(struct
usb_serial*serial, u16 reg,What do mean by "normal" here? Could you give this a more descriptive name?
Perhaps just call this one
f81534_set_registerand add a "port" or "uart" infix to the currentf81534_set_registerbelow (e.g. rename itf81534_set_uart_register, and similar for get).
Or simply replace "normal" with "generic" above.
OK, I'll rename such functions with following:
Generic read/write USB:
f81534_set/get_normal_register -> f81534_set/get_register
UART port read/write:
f81534_set/get_register -> f81534_set/get_port_register
SPI bus read/write
f81534_set_normal_register_with_delay -> f81534_set/get_spi_register
Quoted 4 lines
Quoted 4 lines
drivers/usb/serial/f81534.c new file +4
-0+{ +int status; + + status =f81534_get_normal_register (serial, reg, data);
Quoted 8 lines
drivers/usb/serial/f81534.c line 308
Quoted 6 lines
drivers/usb/serial/f81534.c new file +6
-0+if (status) +return status; + + status =f81534_command_delay (serial); +if (status) +return status;Why do you need a delay after reading?
Sorry for misleading.
I'll rename f81534_command_delay to f81534_wait_for_spi_idle.
Quoted 5 lines
drivers/usb/serial/f81534.c line 399
Quoted 2 lines
drivers/usb/serial/f81534.c new file +2
-0+static void f81534_prepare_write_buffer (struct usb_serial_port *port,u8 *buf, +size_t size)You never use size in this function. You need to make sure you never overwrite the provided buffer using some sanity checks.
OK, I'll add probe() to check bulk_in/out endpoints count & packet size.
Quoted 5 lines
Quoted 5 lines
drivers/usb/serial/f81534.c new file +5
-0+/* Check H/W is TXEMPTY */ +if (!test_and_clear_bit (F81534_TX_EMPTY_BIT , &port_priv->tx_empty)) +return 0 ; + + urb = port->write_urbs[0 ];
Quoted 8 lines
drivers/usb/serial/f81534.c line 452
Quoted 3 lines
drivers/usb/serial/f81534.c new file +3
-0+f81534_prepare_write_buffer (port, port->bulk_out_buffers[0 ], + port->bulk_out_size); + urb->transfer_buffer_length =F81534_WRITE_BUFFER_SIZE ;You need to make sure the buffers have the expected size. They are currently set to the endpoint size, but you can you can make sure they are never smaller than
F81534_WRITE_BUFFER_SIZEby settingbulk_out_sizein theusb_serial_driverstruct.
Thanks for your notice. We had tested it on USB 2.0 Full-speed, the packet size will reduce to 64, it will cause buffer overflow. So I'll reconfirm the packet size in probe().
Thanks for your comments.
Signature · 2 lines
With Best Regards, Peter Hong