USB: serial: mos7840: Add a product ID for the new product

[Johan Hovold <johan@kernel.org>]

On Mon, Nov 12, 2018 at 03:16:05PM +0800, Jackychou wrote:
> From: JackyChou <jackychou@asix.com.tw>
> 
> Add a new PID 0x7843 to the driver.
> Let the new products be able to set up 3 serial ports with the driver.
> 
> Signed-off-by: JackyChou <jackychou@asix.com.tw>
> ---
>  drivers/usb/serial/mos7840.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index b42bad85097a..7667b22fa2f7 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -94,6 +94,7 @@
>  /* The native mos7840/7820 component */
>  #define USB_VENDOR_ID_MOSCHIP           0x9710
>  #define MOSCHIP_DEVICE_ID_7840          0x7840
> +#define MOSCHIP_DEVICE_ID_7843          0x7843
>  #define MOSCHIP_DEVICE_ID_7820          0x7820
>  #define MOSCHIP_DEVICE_ID_7810          0x7810
>  /* The native component can have its vendor/device id's overridden
> @@ -176,6 +177,7 @@ enum mos7840_flag {
>  
>  static const struct usb_device_id id_table[] = {
>  	{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7840)},
> +	{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7843)},
>  	{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7820)},
>  	{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7810)},
>  	{USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2)},
> @@ -298,7 +300,7 @@ static int mos7840_set_uart_reg(struct usb_serial_port *port, __u16 reg,
>  	val = val & 0x00ff;
>  	/* For the UART control registers, the application number need
>  	   to be Or'ed */
> -	if (port->serial->num_ports == 4) {
> +	if (port->serial->num_ports == 4 || port->serial->num_ports == 3) {

Please use num_ports > 2 here.

>  		val |= ((__u16)port->port_number + 1) << 8;
>  	} else {
>  		if (port->port_number == 0) {
> @@ -332,7 +334,7 @@ static int mos7840_get_uart_reg(struct usb_serial_port *port, __u16 reg,
>  		return -ENOMEM;
>  
>  	/* Wval  is same as application number */
> -	if (port->serial->num_ports == 4) {
> +	if (port->serial->num_ports == 4 || port->serial->num_ports == 3) {

Same here.

>  		Wval = ((__u16)port->port_number + 1) << 8;
>  	} else {
>  		if (port->port_number == 0) {
> @@ -2071,9 +2073,12 @@ static int mos7840_probe(struct usb_serial *serial,
>  			VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
>  
>  	/* For a MCS7840 device GPIO0 must be set to 1 */
> -	if (buf[0] & 0x01)
> -		device_type = MOSCHIP_DEVICE_ID_7840;
> -	else if (mos7810_check(serial))
> +	if (buf[0] & 0x01) {
> +		if (product == MOSCHIP_DEVICE_ID_7843)
> +			device_type = MOSCHIP_DEVICE_ID_7843;

What about 7843 devices that use a different PID? Is there a reliable
way to detect the type without relying on PID?

Otherwise, there's no point in verifying GPIO0 for these ones.

> +		else
> +			device_type = MOSCHIP_DEVICE_ID_7840;
> +	} else if (mos7810_check(serial))

Nit: if you add braces to one of the branches you need to add it to all
of them.

>  		device_type = MOSCHIP_DEVICE_ID_7810;
>  	else
>  		device_type = MOSCHIP_DEVICE_ID_7820;
> @@ -2091,7 +2096,10 @@ static int mos7840_calc_num_ports(struct usb_serial *serial,
>  	int device_type = (unsigned long)usb_get_serial_data(serial);
>  	int num_ports;
>  
> -	num_ports = (device_type >> 4) & 0x000F;
> +	if (device_type == MOSCHIP_DEVICE_ID_7843)
> +		num_ports = 3;
> +	else
> +		num_ports = (device_type >> 4) & 0x000F;
>  
>  	/*
>  	 * num_ports is currently never zero as device_type is one of
> @@ -2146,7 +2154,8 @@ static int mos7840_port_probe(struct usb_serial_port *port)
>  		mos7840_port->SpRegOffset = 0x0;
>  		mos7840_port->ControlRegOffset = 0x1;
>  		mos7840_port->DcrRegOffset = 0x4;
> -	} else if ((mos7840_port->port_num == 2) && (serial->num_ports == 4)) {
> +	} else if ((mos7840_port->port_num == 2) &&
> +		   ((serial->num_ports == 4) || (serial->num_ports == 3))) {
>  		mos7840_port->SpRegOffset = 0x8;
>  		mos7840_port->ControlRegOffset = 0x9;
>  		mos7840_port->DcrRegOffset = 0x16;
> @@ -2154,7 +2163,8 @@ static int mos7840_port_probe(struct usb_serial_port *port)
>  		mos7840_port->SpRegOffset = 0xa;
>  		mos7840_port->ControlRegOffset = 0xb;
>  		mos7840_port->DcrRegOffset = 0x19;
> -	} else if ((mos7840_port->port_num == 3) && (serial->num_ports == 4)) {
> +	} else if ((mos7840_port->port_num == 3) &&
> +		   ((serial->num_ports == 4) || (serial->num_ports == 3))) {
>  		mos7840_port->SpRegOffset = 0xa;
>  		mos7840_port->ControlRegOffset = 0xb;
>  		mos7840_port->DcrRegOffset = 0x19;

This is getting rather ugly. Can't you clean up the current code so that
it covers your device without the need of such changes first? 

From the looks of it, the only reason for the above is to map the
offsets for port 2 in the 2-port case to the offsets of port 3. It would
be more straight-forward to handle that particular case separately.

This also relates to the set/get_uart_reg functions above, which also
should handle the 2-port case specifically instead.

Thanks,
Johan