Re: [PATCH V1 2/4] usb: serial: f81534: add auto RTS direction support

[Johan Hovold <johan@kernel.org>]

On Thu, Nov 16, 2017 at 03:46:07PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The F81532/534 had auto RTS direction support for RS485 mode.
> We'll read it from internal Flash with address 0x2f01~0x2f04 for 4 ports.
> There are 4 conditions below:
> 	0: F81534_PORT_CONF_RS232.
> 	1: F81534_PORT_CONF_RS485.
> 	2: value error, default to F81534_PORT_CONF_RS232.
> 	3: F81534_PORT_CONF_RS485_INVERT.
> 
> F81532/534 Clock register (offset +08h)
> 
> Bit0:	UART Enable (always on)
> Bit2-1:	Clock source selector
> 			00: 1.846MHz.
> 			01: 18.46MHz.
> 			10: 24MHz.
> 			11: 14.77MHz.
> Bit4:	Auto direction(RTS) control (RTS pin Low when TX)
> Bit5:	Invert direction(RTS) when Bit4 enabled (RTS pin high when TX)
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81534.c | 54 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c
> index 76c676ef5f0d..b2d10309c335 100644
> --- a/drivers/usb/serial/f81534.c
> +++ b/drivers/usb/serial/f81534.c
> @@ -102,11 +102,16 @@
>  
>  #define F81534_DEFAULT_BAUD_RATE	9600
>  
> +#define F81534_PORT_CONF_RS232		0
> +#define F81534_PORT_CONF_RS485		BIT(0)
> +#define F81534_PORT_CONF_RS485_INVERT	(BIT(0) | BIT(1))
>  #define F81534_PORT_CONF_DISABLE_PORT	BIT(3)
>  #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_UART_MODE_MASK		(BIT(0) | BIT(1))
> +
>  #define F81534_1X_RXTRIGGER		0xc3
>  #define F81534_8X_RXTRIGGER		0xcf
>  
> @@ -119,6 +124,8 @@
>   *			01: 18.46MHz.
>   *			10: 24MHz.
>   *			11: 14.77MHz.
> + * Bit4:	Auto direction(RTS) control (RTS pin Low when TX)
> + * Bit5:	Invert direction(RTS) when Bit4 enabled (RTS pin high when TX)
>   */
>  
>  #define F81534_CLK_1_846_MHZ		BIT(0)
> @@ -126,6 +133,9 @@
>  #define F81534_CLK_24_MHZ		(BIT(0) | BIT(2))
>  #define F81534_CLK_14_77_MHZ		(BIT(0) | BIT(1) | BIT(2))
>  
> +#define F81534_CLK_RS485_MODE		BIT(4)
> +#define F81534_CLK_RS485_INVERT		BIT(5)
> +
>  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) },
> @@ -485,16 +495,20 @@ static int f81534_set_port_config(struct usb_serial_port *port,
>  		struct tty_struct *tty, u32 baudrate, u32 old_baudrate, u8 lcr)
>  {
>  	struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
> +	struct f81534_serial_private *serial_priv;
>  	u32 divisor;
>  	int status;
>  	int idx;
>  	u8 value;
> +	u8 tmp;
>  	static u32 const baudrate_table[] = {115200, 921600, 1152000,
>  			1500000};
>  	static u8 const clock_table[] = {F81534_CLK_1_846_MHZ,
>  			F81534_CLK_14_77_MHZ, F81534_CLK_18_46_MHZ,
>  			F81534_CLK_24_MHZ};
>  
> +	serial_priv = usb_get_serial_data(port->serial);
> +
>  	do {
>  		for (idx = 0; idx < ARRAY_SIZE(baudrate_table); ++idx) {
>  			if (baudrate <= baudrate_table[idx] &&
> @@ -520,8 +534,25 @@ static int f81534_set_port_config(struct usb_serial_port *port,
>  	} while (1);
>  
>  	port_priv->baud_base = baudrate_table[idx];
> -	status = f81534_set_port_register(port, F81534_CLOCK_REG,
> -			clock_table[idx]);
> +	tmp = serial_priv->conf_data[port_priv->phy_num];
> +
> +	switch (tmp & F81534_UART_MODE_MASK) {
> +	case F81534_PORT_CONF_RS485_INVERT:
> +		value = F81534_CLK_RS485_MODE | F81534_CLK_RS485_INVERT;
> +		break;
> +	case F81534_PORT_CONF_RS485:
> +		value = F81534_CLK_RS485_MODE;
> +		break;
> +
> +	default:
> +		/* fall through, default RS232 Mode */
> +	case F81534_PORT_CONF_RS232:
> +		value = 0;
> +		break;
> +	}
> +
> +	value |= clock_table[idx];

With the shadow clock register I mentioned earlier, this can be
determined once at probe instead of at every termios change.

> +	status = f81534_set_port_register(port, F81534_CLOCK_REG, value);
>  	if (status) {
>  		dev_err(&port->dev, "CLOCK_REG setting failed\n");
>  		return status;
> @@ -1270,9 +1301,12 @@ static void f81534_lsr_worker(struct work_struct *work)
>  
>  static int f81534_port_probe(struct usb_serial_port *port)
>  {
> +	struct f81534_serial_private *serial_priv;
>  	struct f81534_port_private *port_priv;
>  	int ret;
> +	u8 value;
>  
> +	serial_priv = usb_get_serial_data(port->serial);
>  	port_priv = devm_kzalloc(&port->dev, sizeof(*port_priv), GFP_KERNEL);
>  	if (!port_priv)
>  		return -ENOMEM;
> @@ -1304,6 +1338,22 @@ static int f81534_port_probe(struct usb_serial_port *port)
>  	if (ret)
>  		return ret;
>  
> +	value = serial_priv->conf_data[port_priv->phy_num];
> +	switch (value & F81534_UART_MODE_MASK) {
> +	case F81534_PORT_CONF_RS485_INVERT:
> +		dev_info(&port->dev, "RS485 invert mode.\n");
> +		break;
> +	case F81534_PORT_CONF_RS485:
> +		dev_info(&port->dev, "RS485 mode.\n");
> +		break;
> +
> +	default:
> +		/* fall through, default RS232 Mode */

No need for fallthrough comment here.

> +	case F81534_PORT_CONF_RS232:
> +		dev_info(&port->dev, "RS232 mode.\n");
> +		break;
> +	}
> +
>  	return 0;
>  }

Johan