Re: [PATCH] USB: serial: cp210x: Improve baudrate support for CP2102N

[Johan Hovold]

On Fri, Jun 15, 2018 at 11:29:57PM +0200, Karoly Pados wrote:
> The CP2102N supports more baudrates than earlier chips by SiLabs.
> This patch adds support for all rates documented in the datasheet
> of this device.
> 
> Signed-off-by: Karoly Pados <pados@xxxxxxxx>
> ---
>  drivers/usb/serial/cp210x.c | 39 ++++++++++++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index b1849f657e01..793b86252c46 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -357,6 +357,9 @@ static struct usb_serial_driver * const serial_drivers[] = {
>  #define CP210X_PARTNUM_CP2104	0x04
>  #define CP210X_PARTNUM_CP2105	0x05
>  #define CP210X_PARTNUM_CP2108	0x08
> +#define CP210X_PARTNUM_CP2102N_QFN28	0x20
> +#define CP210X_PARTNUM_CP2102N_QFN24	0x21
> +#define CP210X_PARTNUM_CP2102N_QFN20	0x22
>  #define CP210X_PARTNUM_UNKNOWN	0xFF
>  
>  /* CP210X_GET_COMM_STATUS returns these 0x13 bytes */
> @@ -758,8 +761,12 @@ static int cp210x_get_line_ctl(struct usb_serial_port *port, u16 *ctl)
>  /*
>   * cp210x_quantise_baudrate
>   * Quantises the baud rate as per AN205 Table 1
> + * The CP2102N is fully (except for baud rate aliasing) software-
> + * compatible, but supports some additional baudrates. However, there is
> + * no quantitisation table available for this model, so in this case we
> + * take the supported baudrate which is closest to the requested one.
>   */
> -static unsigned int cp210x_quantise_baudrate(unsigned int baud)
> +static unsigned int cp210x_quantise_baudrate(unsigned int baud, bool cp2102n)

Pass in a struct usb_serial (or port) as a first argument instead which
allows for more readable code as well as for this to be reused to handle
other device type differences (e.g. only 2108 besides 2102n handles
baudrates over 921.6k).

>  {
>  	if (baud <= 300)
>  		baud = 300;
> @@ -790,10 +797,17 @@ static unsigned int cp210x_quantise_baudrate(unsigned int baud)
>  	else if (baud <= 491520)   baud = 460800;
>  	else if (baud <= 567138)   baud = 500000;
>  	else if (baud <= 670254)   baud = 576000;
> -	else if (baud < 1000000)
> -		baud = 921600;
> -	else if (baud > 2000000)
> -		baud = 2000000;
> +	else if (cp2102n) {

Add a static helper (looks like you add a define in the gpio patch)
cp210x_is_cp2102n(serial) here. You can even test for bit 0x20 in the
helper if you prefer (we can always change that later if needed).

> +		if (baud <= 960800)        baud = 921600;
> +		else if (baud <= 1100000)  baud = 1000000;
> +		else if (baud <= 1350000)  baud = 1200000;
> +		else if (baud <= 1750000)  baud = 1500000;
> +		else if (baud <= 2500000)  baud = 2000000;
> +		else                       baud = 3000000;

And even if the current code uses this odd formatting, your amendments
should not.

> +	} else {
> +		if (baud < 1000000)        baud = 921600;
> +		else if (baud > 2000000)   baud = 2000000;
> +	}
>  	return baud;
>  }
>  
> @@ -1045,16 +1059,19 @@ static void cp210x_get_termios_port(struct usb_serial_port *port,
>  static void cp210x_change_speed(struct tty_struct *tty,
>  		struct usb_serial_port *port, struct ktermios *old_termios)
>  {
> -	u32 baud;
> -
> -	baud = tty->termios.c_ospeed;
> +	bool is_cp2102n;
> +	u32 baud = tty->termios.c_ospeed;
> +	struct cp210x_serial_private *priv = usb_get_serial_data(port->serial);
>  
> -	/* This maps the requested rate to a rate valid on cp2102 or cp2103,
> -	 * or to an arbitrary rate in [1M,2M].
> +	/* This maps the requested rate to a rate valid on cp2102(n) or
> +	 * cp2103 or to an arbitrary rate in [1M,2M].
>  	 *
>  	 * NOTE: B0 is not implemented.
>  	 */
> -	baud = cp210x_quantise_baudrate(baud);
> +	is_cp2102n =	(priv->partnum == CP210X_PARTNUM_CP2102N_QFN28) ||
> +			(priv->partnum == CP210X_PARTNUM_CP2102N_QFN24) ||
> +			(priv->partnum == CP210X_PARTNUM_CP2102N_QFN20);
> +	baud = cp210x_quantise_baudrate(baud, is_cp2102n);

So most of these changes would not be needed. Just pass in port->serial
to cp210x_quantise_baudrate().

Thanks,
Johan