Date: Thu, 11 Jan 2018 11:05:56 +0100
From: Johan Hovold <johan@kernel.org>
To: "Ji-Ze Hong (Peter Hong)" <hpeter@gmail.com>
Cc: johan@kernel.org, gregkh@linuxfoundation.org,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        peter_hong@fintek.com.tw,
        "Ji-Ze Hong (Peter Hong)" <hpeter+linux_kernel@gmail.com>
Subject: Re: [PATCH V3 2/6] usb: serial: f81534: add auto RTS direction
 support
Message-ID: <20180111100556.GA11344@localhost>
References: <1515653240-5420-1-git-send-email-hpeter+linux_kernel@gmail.com>
 <1515653240-5420-2-git-send-email-hpeter+linux_kernel@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1515653240-5420-2-git-send-email-hpeter+linux_kernel@gmail.com>
User-Agent: Mutt/1.7.2 (2016-11-26)
Sender: linux-kernel-owner@vger.kernel.org

On Thu, Jan 11, 2018 at 02:47:16PM +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>
> ---
> V3:
> 	1: change some BIT() operation to GENMASK().
> 	2: change some dev_info() to dev_dbg().
> 
> V2:
> 	1: Read the configure data from flash and save it to shadow clock
> 	   register.

Series now applied with a few minor tweaks.

> +#define F81534_PORT_CONF_RS232		0
> +#define F81534_PORT_CONF_RS485		BIT(0)
> +#define F81534_PORT_CONF_RS485_INVERT	GENMASK(1, 0)

I replaced GENMASK() with your original (BIT(x) | BIT(y)) for register
values like this one.

>  #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		GENMASK(1, 0)

And renamed this mask to F81534_PORT_CONF_MODE_MASK so its more
obvious to what it applies.

Thanks,
Johan